Re: [Qemu-devel] [PATCH v2 1/5] qapi/block: Add "fatal" to BLOCK_IMAGE_CORRUPTED

[Max Reitz <mreitz@redhat.com>]

On 05.09.2014 16:51, Eric Blake wrote:
> On 09/05/2014 08:47 AM, Max Reitz wrote:
>> On 05.09.2014 16:40, Eric Blake wrote:
>>> On 09/05/2014 08:07 AM, Max Reitz wrote:
>>>> Not every BLOCK_IMAGE_CORRUPTED event must be fatal; for example, when
>>>> reading from an image, they should generally not be. Nonetheless, even
>>>> an image only read from may of course be corrupted and this can be
>>>> detected during normal operation. In this case, a non-fatal event should
>>>> be emitted, but the image should not be marked corrupt (in accordance to
>>>> "fatal" set to false).
>>>>
>>> Question - what happens if management misses the signal?  For example,
>>> if libvirt opens qemu on a read-only image, then goes away, then
>>> corruption is detected, then libvirt reconnects.  Does query-block need
>>> to also be updated to report whether a read-only BDS is currently
>>> detected as fatal, but that an event has already been delivered?
>> Well, the obvious problem with that is that corruption currently is a
>> strongly format-specific topic, and only reported for qcow2. So, to do
>> this, we'd have to move the corruption signalling code into the common
>> block layer functions and proceed from there. This actually probably
>> isn't too bad of an idea, anyway. But then we'll need a global
>> bdrv_mark_corrupt() function (which we probably don't want in case we
>> get more flags in the future, so we'll rather want bdrv_set_flag() or
>> something) and I don't really want to make these changes now... We could
>> postpone this, though. Making this change later shouldn't break anything.
>>
>> The other solution would be simply to suppress the stderr message but to
>> always deliver the QMP event.
> That feels like it would flood the channel, if the image is being
> repeatedly read.  The approach of warning exactly once is nice because
> it prevents flooding.  We already have a way to report per-format
> details, is it sufficient to modify ImageInfoSpecificQCow2 to add a bool
> field that reports true if the image is currently known to be corrupted?

Actually, I'm asking myself right now why we don't already have such a 
field. :-)

I'll add it in an independent patch.

Max