Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command
Vladimir Sementsov-Ogievskiy writes:
> On 18.10.23 13:45, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy 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
>
> [..]
>
>>> --- /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
>>
Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command
On 18.10.23 13:45, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy 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
[..]
--- /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.
Maybe, better message would be
"devices are not supported"
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..
+return NULL;
+}
[...]
QAPI schema
Acked-by: Markus Armbruster
--
Best regards,
Vladimir
Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command
Vladimir Sementsov-Ogievskiy 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
> ---
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 89751d81f2..cf92e0df8b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6054,3 +6054,86 @@
> ##
> { 'struct': 'DummyBlockCoreForceArrays',
>'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> +
> +##
> +# @BlockParentType:
> +#
> +# @qdev: block device, such as created by device_add, and denoted by
> +# qdev-id
> +#
> +# @driver: block driver node, such as created by blockdev-add, and
> +# denoted by node-name
> +#
> +# @export: block export, such created by block-export-add, and
> +# denoted by export-id
> +#
> +# Since 8.2
> +##
> +{ 'enum': 'BlockParentType',
> + 'data': ['qdev', 'driver', 'export'] }
> +
> +##
> +# @BdrvChildRefQdev:
> +#
> +# @qdev-id: the device's ID or QOM path
> +#
> +# Since 8.2
> +##
> +{ 'struct': 'BdrvChildRefQdev',
> + 'data': { 'qdev-id': 'str' } }
> +
> +##
> +# @BdrvChildRefExport:
> +#
> +# @export-id: block export identifier
> +#
> +# Since 8.2
> +##
> +{ 'struct': 'BdrvChildRefExport',
> + 'data': { 'export-id': 'str' } }
> +
> +##
> +# @BdrvChildRefDriver:
> +#
> +# @node-name: the node name of the parent block node
> +#
> +# @child: name of the child to be replaced, like "file" or "backing"
> +#
> +# Since 8.2
> +##
> +{ 'struct': 'BdrvChildRefDriver',
> + 'data': { 'node-name': 'str', 'child': 'str' } }
> +
> +##
> +# @BlockdevReplace:
> +#
> +# @parent-type: type of the parent, which child is to be replaced
> +#
> +# @new-child: new child for replacement
> +#
> +# Since 8.2
> +##
> +{ 'union': 'BlockdevReplace',
> + 'base': {
> + 'parent-type': 'BlockParentType',
> + 'new-child': 'str'
> + },
> + 'discriminator': 'parent-type',
> + 'data': {
> + 'qdev': 'BdrvChildRefQdev',
> + 'export': 'BdrvChildRefExport',
> + 'driver': 'BdrvChildRefDriver'
> + } }
> +
> +##
> +# @x-blockdev-replace:
> +#
> +# Replace a block-node associated with device (selected by
> +# @qdev-id) or with block-export (selected by @export-id) or
> +# any child of block-node (selected by @node-name and @child)
> +# with @new-child block-node.
> +#
> +# Since 8.2
> +##
> +{ 'command': 'x-blockdev-replace', 'boxed': true,
> + 'data': 'BlockdevReplace' }
> diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
> new file mode 100644
> index 00..0e751ce4f7
> --- /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?
> +return NULL;
> +}
[...]
QAPI schema
Acked-by: Markus Armbruster
