[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/9] block: Remove child references from bs->{op
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH 3/9] block: Remove child references from bs->{options, explicit_options} |
Date: |
Wed, 29 Aug 2018 13:53:57 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 29 Aug 2018 12:43:06 PM CEST, Max Reitz wrote:
> On 2018-08-26 16:09, Alberto Garcia wrote:
>> Block drivers allow opening their children using a reference to an
>> existing BlockDriverState. These references remain stored in the
>> 'options' and 'explicit_options' QDicts, but we don't need to keep
>> them once everything is open.
>>
>> What is more important, these values can become wrong if the children
>> change:
>>
>> $ qemu-img create -f qcow2 hd0.qcow2 10M
>> $ qemu-img create -f qcow2 hd1.qcow2 10M
>> $ qemu-img create -f qcow2 hd2.qcow2 10M
>> $ $QEMU -drive if=none,file=hd0.qcow2,node-name=hd0 \
>> -drive if=none,file=hd1.qcow2,node-name=hd1,backing=hd0 \
>> -drive file=hd2.qcow2,node-name=hd2,backing=hd1
>>
>> After this hd2 has hd1 as its backing file. Now let's remove it using
>> block_stream:
>>
>> (qemu) block_stream hd2 0 hd0.qcow2
>>
>> Now hd0 is the backing file of hd2, but hd2's options QDicts still
>> contain backing=hd1.
>>
>> Signed-off-by: Alberto Garcia <address@hidden>
>> ---
>> block.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> Also, the question is, why would we discard the child options, but
> keep the references (which we do not add, so we only have the ones
> specified by the user).
We discussed this a couple of weeks ago:
https://lists.gnu.org/archive/html/qemu-block/2018-08/msg00478.html
In the end I got convinced that we didn't need to keep the child
references.
>> @@ -3313,6 +3317,12 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
>> bs->open_flags = reopen_state->flags;
>> bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
>>
>> + /* Remove child references from bs->options and bs->explicit_options */
>> + QLIST_FOREACH(child, &bs->children, next) {
>> + qdict_del(bs->explicit_options, child->name);
>> + qdict_del(bs->options, child->name);
>> + }
>> +
>
> Why don't you remove the child options as well?
Because there's no child options here at this point. They are removed
from the parent QDict in bdrv_reopen_queue_child():
QLIST_FOREACH(child, &bs->children, next) {
/* ... */
child_key_dot = g_strdup_printf("%s.", child->name);
qdict_extract_subqdict(explicit_options, NULL, child_key_dot);
qdict_extract_subqdict(options, &new_child_options, child_key_dot);
g_free(child_key_dot);
bdrv_reopen_queue_child(bs_queue, child->bs, new_child_options, 0,
child->role, options, flags);
}
Berto
- [Qemu-devel] [PATCH 5/9] block: Allow child references on reopen, (continued)
[Qemu-devel] [PATCH 8/9] block: Allow changing 'detect-zeroes' on reopen, Alberto Garcia, 2018/08/26
[Qemu-devel] [PATCH 3/9] block: Remove child references from bs->{options, explicit_options}, Alberto Garcia, 2018/08/26
[Qemu-devel] [PATCH 2/9] file-posix: x-check-cache-dropped should default to false on reopen, Alberto Garcia, 2018/08/26
[Qemu-devel] [PATCH 9/9] block: Allow changing 'force-share' on reopen, Alberto Garcia, 2018/08/26
[Qemu-devel] [PATCH 6/9] file-posix: Forbid trying to change unsupported options during reopen, Alberto Garcia, 2018/08/26