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
