[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block/qcow2-bitmap: rewrite bi

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block/qcow2-bitmap: rewrite bitmap reopening logic
Date: Thu, 30 May 2019 11:54:43 +0000

30.05.2019 11:23, Vladimir Sementsov-Ogievskiy wrote:
> 29.05.2019 21:08, John Snow wrote:
>> Max has picked this thread up for block discussion, so I'm going to
>> stick to slightly more bitmap related discussion here; we'll resume
>> block discussion in the other tail of this thread.


>>>> And we mark it as inconsistent because we're not sure how we missed it
>>>> earlier. OK.
>>>>> +                }
>>>>> +            } else if (!bdrv_dirty_bitmap_readonly(bitmap)) {
>>>>> +                corruption =
>>>>> +                    "Corruption: bitmap '%s' is not marked IN_USE in the 
>>>>> "
>>>>> +                    "image '%s' and not marked readonly in RAM. Will try 
>>>>> to "
>>>>> +                    "set IN_USE flag.";
>>>>> +
>>>> And in this case, we find the bitmap but it's not marked readonly for
>>>> some reason.
>>>>> +                bdrv_dirty_bitmap_set_readonly(bitmap, true);
>>>> Why set it readonly again?
>>> It is because inconsistance is not synced to the image. "readonly" exactly
>>> means, that for some reasons we did not marked bitmap IN_USE in the image 
>>> and
>>> therefore must not write to it.
>> That's one way of looking at what readonly means. Another was: "The
>> image this bitmap is attached to is read only, and any writes to this
>> bitmap are a logistical error."
>>> So, yes, here occurs new thing: readonly-inconsistent bitmap. It blocks 
>>> guest
>>> writes until we sync it somehow to the image or remove. And we are going to 
>>> sync
>>> it at the end of this function.
>> Right, we've not really used readonly in this way before. It makes sense
>> to a point, but it's a bit of a semantic overload -- the disk is
>> actually RW but the bitmap is RO; the problem that I have with this is
>> that we guard RO bitmaps with assertions and not errors,
> Oops, you are right. I thought we have errors for guest writes in this case.

Aha, I was (partially) right, we have in bdrv_aligned_pwritev:
     if (bdrv_has_readonly_bitmaps(bs)) {
           return -EPERM;

but what about discard, ...? seems it's not handled.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]