qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RFC 0/3] block: drop inherits_from


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH RFC 0/3] block: drop inherits_from
Date: Thu, 11 Mar 2021 20:28:39 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

11.03.2021 20:09, Kevin Wolf wrote:
Am 11.03.2021 um 16:15 hat Vladimir Sementsov-Ogievskiy geschrieben:
Hi all!

I now work on v3 for "block: update graph permissions update", and I'm
at "[PATCH v2 28/36] block: add bdrv_set_backing_noperm() transaction
action".

So, the problem is we should handle inherits_from carefully, and most
probably it should be updated in bdrv_replace_child_noperm().. And
then, bdrv_replace_child_noperm will become a transaction action,
which should store old inherits_from to the transaction state for
possible rollback.. Or something like this, I didn't try yet. I just
thought, may be we can just drop inherits_from?

I decided to learn the thing a bit, and found that the only usage of
inherits_from is to limit reopen process. When adding bs to
reopen_queue we do add its children recursively, but only those which
inherits from the bs.

That works so starting from

commit 67251a311371c4d22e803f151f47fe817175b6c3
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Thu Apr 9 18:54:04 2015 +0200

     block: Fix reopen flag inheritance


The commit made two things:

1. reopen recursively all* children, not only .file. That's OK.

2. * : not all, but only that inherits_from bs.

[2] Means that we don't reopen some implicitely created children..
And, I want to ask, why?

The reason is the difference between

     -drive if=none,file=test.qcow2

and something like

     -blockdev file,filename=backing.img,node-name=backing
     -blockdev file,filename=test.qcow2,node-name=file
     -blockdev qcow2,file=file,backing=backing

The former means that bs->file and bs->backing inherit options from the
qcow2 layer. If you reopen the qcow2 layer to set cache.direct=on, both
children inherit the same update and both the file itself and the
backing file will use O_DIRECT - this is the same as would happen if you
had set cache.direct=on in the -drive option from the start.

In the -blockdev case, the nodes were defined explicitly without
inheriting from the qcow2 layer. Setting cache.direct=on on the qcow2
layer (which is actually created last) doesn't influence the two file
layers. So a reopen of the qcow2 layer shouldn't change the two file
nodes either: If they didn't inherit the option during bdrv_open(), they
certainly shouldn't inherit it during bdrv_reopen() either.

Hmm. Understand. Than I was wrong. So modern blockdev behaviour is NOT inherit 
options. That makes sense.


For me it seems that if we have reopen process.. And bs involved. And
it has a child.. And child role defines how that child should inherit
options.. Why not to just inherit them?

The -blockdev behaviour makes things a lot more predictable for a
management tool for which we know that it can handle things on the node
level.

So what we really want is not inheriting at all. But compatibility with
-drive doesn't let us. (And actually -blockdev with inline declaration
of children behaves the same as -drive, which may have been a mistake.)


So, inherits_from will disappear together with -drive some good future day. OK, 
thanks for explanation


--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]