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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

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