Re: [PATCH v8 4/7] qapi: add x-blockdev-replace command

[Markus Armbruster <armbru@redhat.com>]

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>
>>