[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC 0/3] block: drop inherits_from
From: |
Kevin Wolf |
Subject: |
Re: [PATCH RFC 0/3] block: drop inherits_from |
Date: |
Thu, 11 Mar 2021 18:09:46 +0100 |
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.
> 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.)
Kevin