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, Vladimir