[libvirt] [PATCH 11/30] storagefile: Rename qcow2GetExtensions 'format' argument

Cole Robinson crobinso at redhat.com
Fri Oct 11 17:47:29 UTC 2019


On 10/9/19 5:14 PM, Daniel Henrique Barboza wrote:
> 
> 
> On 10/7/19 6:49 PM, Cole Robinson wrote:
>> To backingFormat, which makes it more clear. Move it to the end of
>> the argument list which will scale nicer with future patches
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
> 
> This really makes it clearer. I was getting confused about whether
> 'format' was referring to the image or its backing file in these
> patches.
> 
> 
> 
>>   src/util/virstoragefile.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index d6bd38777b..8cd576a463 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
>> @@ -427,9 +427,9 @@ cowGetBackingStore(char **res,
>>   static int
>> -qcow2GetExtensions(int *format,
>> -                   const char *buf,
>> -                   size_t buf_size)
>> +qcow2GetExtensions(const char *buf,
>> +                   size_t buf_size,
>> +                   int *backingFormat)
>>   {
>>       size_t offset;
>>       size_t extension_start;
>> @@ -509,8 +509,8 @@ qcow2GetExtensions(int *format,
>>           case QCOW2_HDR_EXTENSION_BACKING_FORMAT:
>>               if (buf[offset+len] != '\0')
>>                   break;
>> -            *format = virStorageFileFormatTypeFromString(buf+offset);
>> -            if (*format <= VIR_STORAGE_FILE_NONE)
>> +            *backingFormat = 
>> virStorageFileFormatTypeFromString(buf+offset);
>> +            if (*backingFormat <= VIR_STORAGE_FILE_NONE)
>>                   return -1;
>>           }
>> @@ -561,7 +561,7 @@ qcowXGetBackingStore(char **res,
>>       memcpy(*res, buf + offset, size);
>>       (*res)[size] = '\0';
>> -    if (qcow2GetExtensions(format, buf, buf_size) < 0)
>> +    if (qcow2GetExtensions(buf, buf_size, format) < 0)
>>           return BACKING_STORE_INVALID;
> 
> Not sure if the next patches are changing it or making it obsolete, but 
> you can
> change 'format' to 'backingFormat' inside qcowXGetBackingStore too.
> 

Later patches don't change it, as you've seen. We could change it, but 
context wise it's less necessary there IMO, and also we should then 
change it for the other GetBackingStore functions as well.

Thanks,
Cole




More information about the libvir-list mailing list