Re: [Qemu-devel] [PATCH] qemu-img: Add --backing-chain option to info command

[Eric Blake]

On 10/12/2012 08:27 AM, Kevin Wolf wrote:
> Am 12.10.2012 16:24, schrieb Eric Blake:
>> On 10/12/2012 08:09 AM, Stefan Hajnoczi wrote:
>>> The qemu-img info --backing-chain option enumerates the backing file
>>> chain.  For example, for base.qcow2 <- snap1.qcow2 <- snap2.qcow2 the
>>> output becomes:
>>>
>>
>>> +    do {
>>> +        bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
>>> +                           false);
>>> +        if (!bs) {
>>> +            goto err;
>>> +        }
>>
>>> +    } while (filename);
>>
>> Eww - infinite loop if presented with malicious data where someone has
>> used 'qemu-img rebase -u' to create a cycle.  I think you need a
>> followup patch that hashes which files have been opened to date, and
>> abort the loop once a cycle is detected.
> 
> That would already cause problems in bdrv_open(), so I'd consider it a
> separate bug. We should fail gracefully when trying to open such an
> image. Once it's open, other code can trust that the chain makes sense.

Hmm.  For 'qemu-img info', I can see two behaviors, both useful, when
presented with a corrupt image.  One is to error out right away (because
qemu would be unable to use the image).  But the other is for debugging
WHY the image is corrupt, at which point I want qemu-img info to display
as much information as possible, INCLUDING what backing file is recorded
in the header, so that I can follow the loop and decide where to break
the loop.  Sounds like we might need another flag to bdrv_open() on
whether to detect cycles; as well as fixing qemu-img info to check for
cycles on its own when it bypasses normal cycle-checking in bdrv_open.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature