[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 4/5] storage: don't follow backing chain symlinks too eagerly



On 02/15/2013 03:08 PM, Peter Krempa wrote:
> On 02/15/13 21:38, Eric Blake wrote:
>> If you have a qcow2 file /path1/to/file pointed to by symlink
>> /path2/symlink, and pass qemu /path2/symlink, then qemu treats
>> a relative backing file in the qcow2 metadata as being relative
>> to /path2, not /path1/to.  Yes, this means that it is possible
>> to create a qcow2 file where the choice of WHICH directory and
>> symlink you access its contents from will then determine WHICH
>> backing file (if any) you actually find; the results can be
>> rather screwy, but we have to match what qemu does.
>>
>> Libvirt and qemu default to creating absolute backing file
>> names, so most users don't hit this.  But at least VDSM uses
>> symlinks and relative backing names alongside the
>> --reuse-external flags to libvirt snapshot operations, with the
>> result that libvirt was failing to follow the intended chain of
>> backing files, and then backing files were not granted the
>> necessary sVirt permissions to be opened by qemu.
>>
>> See https://bugzilla.redhat.com/show_bug.cgi?id=903248 for
>> more gory details.  This fixes a regression introduced in
>> commit 8250783.
>>
>> I tested this patch by creating the following chain:

and that chain is more-or-less in patch 5/5.

>> +        } else if (!d_len) {
> 
> I'd go for (d_len == 0) here so that it will not be confused with pointers.

Done.

> 
>> +            start = ".";
>> +            d_len = 1;
>> +        }
>> +        if (virAsprintf(&combined, "%.*s/%s", (int)d_len, start,
>> path) < 0) {
>> +            virReportOOMError();
>> +            goto cleanup;
>> +        }
>>       }
> 
> Otherwise looks reasonable.
> 
> ACK.

Will push shortly.  Thanks for the review.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]