Re: [Qemu-devel] Re: [PATCH v3 5/5] qed: Consistency check support

[Stefan Hajnoczi]

On Thu, Oct 28, 2010 at 11:37 AM, Kevin Wolf <address@hidden> wrote:
> Am 28.10.2010 12:15, schrieb Stefan Hajnoczi:
>> On Wed, Oct 27, 2010 at 4:52 PM, Kevin Wolf <address@hidden> wrote:
>>> Am 22.10.2010 16:56, schrieb Stefan Hajnoczi:
>>>> This patch adds support for the qemu-img check command.  It also
>>>> introduces a dirty bit in the qed header to mark modified images as
>>>> needing a check.  This bit is cleared when the image file is closed
>>>> cleanly.
>>>>
>>>> If an image file is opened and it has the dirty bit set, a consistency
>>>> check will run and try to fix corrupted table offsets.  These
>>>> corruptions may occur if there is power loss while an allocating write
>>>> is performed.  Once the image is fixed it opens as normal again.
>>>>
>>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>>
>>> Hm, do I understand right that you fix the image and reset the dirty
>>> flag in the header during bdrv_open? So how does this work with
>>> migration, when the destination host opens the QED file before the
>>> source closes it? Doesn't the destination destroy the image by "fixing" it?
>>>
>>> And even if that wasn't the case, clearing the flag means that the
>>> source might do new writes and thinks that the flag is still set. If the
>>> source crashes now, we may need a consistency check, but the dirty flag
>>> isn't set any more.
>>>
>>> Am I missing some detail?
>>
>> You're right, migration is not supported.  This is also true for the
>> other image formats which cache metadata in memory though.
>
> Is this still broken in upstream? This is a shame.
>
> I think the problem was that Anthony was opposed to reopening the image
> because we might lose it on that occasion? Instead of possibly crashing
> a VM in the worst case we regularly corrupt images now... *sigh*
>
>> I am actually looking into migration next together with Adam Litke who
>> has already started.  We'll have to defer accessing the file until the
>> source has flushed/closed it.
>
> Sounds to me like an ugly workaround for the fundamental problem that
> the image file is opened twice at the same time. Ugly because it's not
> even in a central place, but must be done for each image format separately.
>
> Opening the file read-only first and reopening it read-write when the
> migration has completed would be much better. No image format driver
> would have to be changed for that.

I agree.  I'm not sure whether NFS guarantees we see the latest data
when reopening read-write (cached pages will be left on the
destination host from the initial read-only access).

Anyway the solution shouldn't be a massive patch, I'm not too worried about it.

Stefan