[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: x-blockdev-reopen & block-dirty-bitmaps
From: |
John Snow |
Subject: |
Re: x-blockdev-reopen & block-dirty-bitmaps |
Date: |
Fri, 14 Feb 2020 15:32:00 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 2/14/20 3:19 PM, Kevin Wolf wrote:
> Am 14.02.2020 um 19:54 hat John Snow geschrieben:
>> Hi, what work remains to make this a stable interface, is it known?
>>
>> We're having a problem with bitmaps where in some cases libvirt wants to
>> commit an image back down to a base image but -- for various reasons --
>> the bitmap was left enabled in the backing image, so it would accrue new
>> writes during the commit.
>>
>> Normally, when creating a snapshot using blockdev-snapshot, the backing
>> file becomes RO and all of the bitmaps become RO too.
>>
>> The goal is to be able to disable (or enable) bitmaps from a backing
>> file before (or atomically just before) a commit operation to allow
>> libvirt greater control on snapshot commit.
>>
>> Now, in my own testing, we can reopen a backing file just fine, delete
>> or disable a bitmap and be done with it -- but the interface isn't
>> stable, so libvirt will be reluctant to use such tricks.
>>
>> Probably a loaded question, but:
>>
>> - What's needed to make the interface stable?
>> - Are there known problem points?
>> - Any suggestions for workarounds in the meantime?
>
> I think I've asked this before, but I don't remember the answer...
>
> What would be the problem with letting the enable/disable command
> temporarily reopen the backing file read-write, like the commit job [1]
> does?
>
I guess no problem? I wouldn't want it to do this automatically, but
perhaps we could add a "force=True" bool where it tries to do just this.
(And once reopen works properly we could deprecate this workaround again.)
> Kevin
>
> [1] I mean, I know that this code is wrong strictly speaking because we
> really should be counting read-write users [2] rather than blindly
> making the node read-only at the end of the operation - but somehow
> it seems to work in practice for commit jobs.
>
> [2] Counting really means just looking at parent BdrvChild links that
> have WRITE permissions. I guess doing it right would mean getting
> rid of BlockDriverState.read_only (which is redundant) and using
> only permissions.
>
OK, sounds good. I'll make a mockup that tries to accurately detect
read-only-ness and reverts changes only if it made any to begin with.
--js
- x-blockdev-reopen & block-dirty-bitmaps, John Snow, 2020/02/14
- Re: x-blockdev-reopen & block-dirty-bitmaps, Kevin Wolf, 2020/02/14
- Re: x-blockdev-reopen & block-dirty-bitmaps,
John Snow <=
- Re: x-blockdev-reopen & block-dirty-bitmaps, Kevin Wolf, 2020/02/17
- Re: x-blockdev-reopen & block-dirty-bitmaps, John Snow, 2020/02/17
- Re: x-blockdev-reopen & block-dirty-bitmaps, Peter Krempa, 2020/02/18
- Re: x-blockdev-reopen & block-dirty-bitmaps, Kevin Wolf, 2020/02/18
- Re: x-blockdev-reopen & block-dirty-bitmaps, Peter Krempa, 2020/02/18
- Re: x-blockdev-reopen & block-dirty-bitmaps, Kevin Wolf, 2020/02/18
Re: x-blockdev-reopen & block-dirty-bitmaps, Alberto Garcia, 2020/02/17