Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qemu-img: Resolve relative backing paths in rebase

[Max Reitz]

On 2018-05-09 19:20, Eric Blake wrote:
> On 05/09/2018 10:49 AM, Max Reitz wrote:
>> Currently, rebase interprets a relative path for the new backing image
>> as follows:
>> (1) Open the new backing image with the given relative path (thus
>> relative to
>>      qemu-img's working directory).
>> (2) Write it directly into the overlay's backing path field (thus
>>      relative to the overlay).
>>
>> If the overlay is not in qemu-img's working directory, both will be
>> different interpretations, which may either lead to an error somewhere
>> (either rebase fails because it cannot open the new backing image, or
>>   your overlay becomes unusable because its backing path does not point
>>   to a file), or, even worse, it may result in your rebase being
>> performed for a different backing file than what your overlay will point
>> to after the rebase.
>>
>> Fix this by interpreting the target backing path as relative to the
>> overlay, like qemu-img does everywhere else.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
>> Cc: address@hidden
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
> 
>> -            blk_new_backing = blk_new_open(out_baseimg, NULL,
>> +            overlay_filename = bs->exact_filename[0] ?
>> bs->exact_filename
>> +                                                     : bs->filename;
>> +            out_real_path = g_malloc(PATH_MAX);
>> +
>> +           
>> bdrv_get_full_backing_filename_from_filename(overlay_filename,
>> +                                                         out_baseimg,
>> +                                                         out_real_path,
>> +                                                         PATH_MAX,
>> +                                                         &local_err);
>> +            if (local_err) {
>> +                error_reportf_err(local_err,
>> +                                  "Could not resolve backing
>> filename: ");
>> +                ret = -1;
>> +                goto out;
> 
> Leaks out_real_path.

Well, “leaks” is relative, considering that out: will effectively exit
the process immediately, but I can’t claim it was on purpose. O:-)

(I should have trusted my gut feeling that the single g_free() can’t
possibly be enough...)

Max

>> +            }
>> +
>> +            blk_new_backing = blk_new_open(out_real_path, NULL,
>>                                              options, src_flags,
>> &local_err);
>> +            g_free(out_real_path);
> 
> Otherwise looks good.
>

signature.asc
Description: OpenPGP digital signature