On 05/20/2013 05:24 PM, Liu Yuan wrote:
> On 05/20/2013 03:50 PM, Kai Zhang wrote:
>> +bool farm_contains_snapshot(uint32_t idx, const char *tag)
>> +{
>> + unsigned char trunk_sha1[SHA1_LEN];
>> + return (get_trunk_sha1(idx, tag, trunk_sha1) == 0);
>> }
>
> farm_contain_snapshot() get a more uniform with other helpers.
>
>> + /* create active vdi based on last vdi snapshot */
>> + struct vdi_entry *vdi, *next;
>> + uint32_t new_vid;
>> + list_for_each_entry(vdi, &last_vdi_list, list) {
>> + if (do_vdi_create(vdi->name,
>> + vdi->vdi_size,
>> + vdi->vdi_id, &new_vid,
>> + false, vdi->nr_copies) < 0)
>> + goto out;
>> + }
>
> Declaration in the middle of the function looks kind of odd to me. I
> think it is nicer to add a helper function to be more self-descriptive
> here since you think this variables aren't relevant to other parts of
> the function.
>
> Kazutaka, since it is quit hard to rebase a large patch set, after next
> series fix this kind of trivial issues, I'd like to merge this patch set
> for better incremental development, any ideas?
>
Cc Kazutaka
|
|