Re: [PATCH v4 2/6] block: Allow changing bs->file on reopen

[Vladimir Sementsov-Ogievskiy]

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