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

[Kevin Wolf <kwolf@redhat.com>]

Am 12.10.2012 16:32, schrieb 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.

Makes sense. Though I think BDRV_O_NO_BACKING is enough to implement
this functionality in qemu-img. We'd just have to have an error code
that allows qemu-img to check if we detected a loop so that it can start
searching the broken image.

Kevin