[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/6] block: Allow changing bs->file on reopen
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v4 2/6] block: Allow changing bs->file on reopen |
Date: |
Fri, 7 May 2021 10:11:47 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
17.03.2021 20:15, Alberto Garcia wrote:
When the x-blockdev-reopen was added it allowed reconfiguring the
graph by replacing backing files, but changing the 'file' option was
forbidden. Because of this restriction some operations are not
possible, notably inserting and removing block filters.
I now started to work on making backup-top filter public..
And I think, we'll need separate commands to insert/remove filters anyway.. As
blockdev-reopen has the following problems:
1. It can't append filter above top node, connected to block-device. (but
bdrv_append() can do it)
2. It can't simultaneously create new node and append it. This is important for
backup-top filter, which unshares write even when has no writing parent. Now
bdrv_append() works in a smart way for it: it first do both graph modification
(add child to filter, and replace original node by filter) and then update
graph permissions. So, we'll need a command which in one roll create filter
node and inserts it where needed.
3. blockdev-reopen requires to specify all options (otherwise, they will be
changed to default). So it requires passing a lot of information. But we don't
need to touch any option of original bs parent to insert a filter between
parent and bs. In other words, we don't want to reopen something. We want to
insert filter.
===
Hmm, another mentioned use of blockdev-reopen was reopening some RO node to RW,
to modify bitmaps.. And here again, blockdev-reopen is not very convenient:
1. Again, it requires to specify all options (at least, that was not default on
node open).. And this only to change one property: read-only. Seems
overcomplicated.
2. Bitmaps modifications are usually done in transactions. Adding a clean
transaction support for small command that reopens only to RW, and back to RO
on transaction finalization seems simpler, than for entire generic
blockdev-reopen.
===
Hmm, interesting. x-blockdev-reopen says that not specified options are reset to default.
x-blockdev-reopen works through bdrv_reopen_multiple, so I think bdrv_reopen_mutliple
should reset options to default as well. Now look at bdrv_reopen_set_read_only(): it
specifies only one option: "read-only". This means that all other options will
be reset to default. But for sure, callers of bdrv_reopen_set_read_only() want only to
change read-only status of node, not all other options. Do we have a bug here?
--
Best regards,
Vladimir