[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command |
Date: |
Wed, 18 Oct 2023 14:50:25 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> On 18.10.23 13:45, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>
>>> Add a command that can replace bs in following BdrvChild structures:
>>>
>>> - qdev blk root child
>>> - block-export blk root child
>>> - any child of BlockDriverState selected by child-name
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
> [..]
>
>>> --- /dev/null
>>> +++ b/stubs/blk-by-qdev-id.c
>>> @@ -0,0 +1,9 @@
>>> +#include "qemu/osdep.h"
>>> +#include "qapi/error.h"
>>> +#include "sysemu/block-backend.h"
>>> +
>>> +BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
>>> +{
>>> + error_setg(errp, "blk '%s' not found", id);
>>
>> Is this expected to happen?
>
> Yes, if call the command from qemu-storage-daemon, where qdev-monitor is not
> linked in.
It happens when you try to x-blockdev-replace with "parent-type":
"qdev". Correct?
> Maybe, better message would be
>
> "devices are not supported"
Best to spell out which argument is the problem.
Stupidest solution that could possibly work:
"Parameter 'parent-type' does not accept value 'qdev'"
This is exactly what we'd get if we compiled out the parts that don't
make sense for qemu-storage-daemon.
> Maybe, that possible to use some 'if': notation in qapi, to not include
> support for qdev into the new command, when it compiled into
> qemu-storage-daemon? Seems that would not be simple, as we also need to split
> compilation of the command somehow, now it compiled once both for qemu and
> qemu tools..
That's precisely the problem.
Our reuse of parts of qemu-system-FOO's QAPI schema for
qemu-storage-daemon isn't pretty, but has worked for us so far.
>>> + return NULL;
>>> +}
>> [...]
>> QAPI schema
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>
- [PATCH v8 0/7] blockdev-replace, Vladimir Sementsov-Ogievskiy, 2023/10/17
- [PATCH v8 7/7] iotests: add filter-insertion, Vladimir Sementsov-Ogievskiy, 2023/10/17
- [PATCH v8 2/7] block/export: add blk_by_export_id(), Vladimir Sementsov-Ogievskiy, 2023/10/17
- [PATCH v8 6/7] iotests.py: introduce VM.assert_edges_list() method, Vladimir Sementsov-Ogievskiy, 2023/10/17
- [PATCH v8 5/7] block: bdrv_get_xdbg_block_graph(): report export ids, Vladimir Sementsov-Ogievskiy, 2023/10/17
- [PATCH v8 3/7] block: make bdrv_find_child() function public, Vladimir Sementsov-Ogievskiy, 2023/10/17