Re: [PATCH v3 01/11] qapi: Restrict query-uuid command to block code
Philippe Mathieu-Daudé writes:
> +Igor
>
> On 10/1/20 7:04 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé writes:
>>
>>> In commit f68c01470b we restricted the query-uuid command to
>>> machine code, but it is incorrect, as it is also used by the
>>> tools. Therefore move this command again, but to block.json,
>>> which is shared by machine code and tools.
>>>
>>> Fixes: f68c01470b ("qapi: Restrict query-uuid command to machine code")
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé
>>
>> UUIDs are not really a block-specific thing.
>
> This is the discussion we had in v1 with Igor...
>
> UuidInfo is a iSCSI-specific "thing", the original commit
> is f9dadc9855 ("iSCSI: add configuration variables for iSCSI")
> then Paolo introduced 'UuidInfo' in commit 5accc8408f
> ("scsi: prefer UUID to VM name for the initiator name") but
> is misnamed?
UuidInfo is also used by query-uuid and info uuid. query-uuid returns
whatever was set with option -uuid. Option's help text calls it
"machine UUID".
>> QMP query-uuid and HMP info uuid are about the VM, like query-name.
>> That's why they used to be next to query-name in misc.json.
>
> This is GuidInfo, not UuidInfo...
>
> GuidInfo is correctly in machine.json.
GuidInfo is used by query-vm-generation-id and info vm-generation-id.
query-vm-generation-id returns the value of property "guid" of the
vmgenid device.
I don't know why we have both.
>> There's one additional use in block/iscsi.c's get_initiator_name(). I
>> figure that's what pulls it into tools via qemu-img.
>
> Yes.
>
>>
>> Which other QAPI modules are shared by all the executables that use it?
>
> None?
I'd expect at least all the modules block.json includes:
block-core.json, common.json, crypto.json, job.json, sockets.json.
>> What about reverting the commit? How bad would that be for user mode?
>>
>
> The problem is not user-mode, is linking tools.
Which modules are linked now?
Re: [PATCH v3 01/11] qapi: Restrict query-uuid command to block code
+Igor
On 10/1/20 7:04 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé writes:
>
>> In commit f68c01470b we restricted the query-uuid command to
>> machine code, but it is incorrect, as it is also used by the
>> tools. Therefore move this command again, but to block.json,
>> which is shared by machine code and tools.
>>
>> Fixes: f68c01470b ("qapi: Restrict query-uuid command to machine code")
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>
> UUIDs are not really a block-specific thing.
This is the discussion we had in v1 with Igor...
UuidInfo is a iSCSI-specific "thing", the original commit
is f9dadc9855 ("iSCSI: add configuration variables for iSCSI")
then Paolo introduced 'UuidInfo' in commit 5accc8408f
("scsi: prefer UUID to VM name for the initiator name") but
is misnamed?
>
> QMP query-uuid and HMP info uuid are about the VM, like query-name.
> That's why they used to be next to query-name in misc.json.
This is GuidInfo, not UuidInfo...
GuidInfo is correctly in machine.json.
>
> There's one additional use in block/iscsi.c's get_initiator_name(). I
> figure that's what pulls it into tools via qemu-img.
Yes.
>
> Which other QAPI modules are shared by all the executables that use it?
None?
>
> What about reverting the commit? How bad would that be for user mode?
>
The problem is not user-mode, is linking tools.
Re: [PATCH v3 01/11] qapi: Restrict query-uuid command to block code
Philippe Mathieu-Daudé writes:
> In commit f68c01470b we restricted the query-uuid command to
> machine code, but it is incorrect, as it is also used by the
> tools. Therefore move this command again, but to block.json,
> which is shared by machine code and tools.
>
> Fixes: f68c01470b ("qapi: Restrict query-uuid command to machine code")
>
> Signed-off-by: Philippe Mathieu-Daudé
UUIDs are not really a block-specific thing.
QMP query-uuid and HMP info uuid are about the VM, like query-name.
That's why they used to be next to query-name in misc.json.
There's one additional use in block/iscsi.c's get_initiator_name(). I
figure that's what pulls it into tools via qemu-img.
Which other QAPI modules are shared by all the executables that use it?
What about reverting the commit? How bad would that be for user mode?
[PATCH v3 01/11] qapi: Restrict query-uuid command to block code
In commit f68c01470b we restricted the query-uuid command to
machine code, but it is incorrect, as it is also used by the
tools. Therefore move this command again, but to block.json,
which is shared by machine code and tools.
Fixes: f68c01470b ("qapi: Restrict query-uuid command to machine code")
Signed-off-by: Philippe Mathieu-Daudé
---
qapi/block.json | 30 ++
qapi/machine.json | 30 --
block/iscsi.c | 2 +-
stubs/uuid.c | 2 +-
stubs/meson.build | 4 +++-
5 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/qapi/block.json b/qapi/block.json
index a009f7d3a2..4ae1716b56 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -11,6 +11,36 @@
# == Additional block stuff (VM related)
##
+##
+# @UuidInfo:
+#
+# Guest UUID information (Universally Unique Identifier).
+#
+# @UUID: the UUID of the guest
+#
+# Since: 0.14.0
+#
+# Notes: If no UUID was specified for the guest, a null UUID is returned.
+##
+{ 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
+
+##
+# @query-uuid:
+#
+# Query the guest UUID information.
+#
+# Returns: The @UuidInfo for the guest
+#
+# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "query-uuid" }
+# <- { "return": { "UUID": "550e8400-e29b-41d4-a716-44665544" } }
+#
+##
+{ 'command': 'query-uuid', 'returns': 'UuidInfo', 'allow-preconfig': true }
+
##
# @BiosAtaTranslation:
#
diff --git a/qapi/machine.json b/qapi/machine.json
index 756dacb06f..72f014bb5b 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -402,36 +402,6 @@
##
{ 'command': 'query-target', 'returns': 'TargetInfo' }
-##
-# @UuidInfo:
-#
-# Guest UUID information (Universally Unique Identifier).
-#
-# @UUID: the UUID of the guest
-#
-# Since: 0.14.0
-#
-# Notes: If no UUID was specified for the guest, a null UUID is returned.
-##
-{ 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
-
-##
-# @query-uuid:
-#
-# Query the guest UUID information.
-#
-# Returns: The @UuidInfo for the guest
-#
-# Since: 0.14.0
-#
-# Example:
-#
-# -> { "execute": "query-uuid" }
-# <- { "return": { "UUID": "550e8400-e29b-41d4-a716-44665544" } }
-#
-##
-{ 'command': 'query-uuid', 'returns': 'UuidInfo', 'allow-preconfig': true }
-
##
# @GuidInfo:
#
diff --git a/block/iscsi.c b/block/iscsi.c
index e30a7e3606..1effea25ed 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -42,7 +42,7 @@
#include "qemu/uuid.h"
#include "sysemu/replay.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-machine.h"
+#include "qapi/qapi-commands-block.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
#include "crypto/secret.h"
diff --git a/stubs/uuid.c b/stubs/uuid.c
index e5112eb3f6..d6bfb442e0 100644
--- a/stubs/uuid.c
+++ b/stubs/uuid.c
@@ -1,5 +1,5 @@
#include "qemu/osdep.h"
-#include "qapi/qapi-commands-machine.h"
+#include "qapi/qapi-commands-block.h"
#include "qemu/uuid.h"
UuidInfo *qmp_query_uuid(Error **errp)
diff --git a/stubs/meson.build b/stubs/meson.build
index e0b322bc28..2e231590e1 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -39,7 +39,9 @@ stub_ss.add(files('target-get-monitor-def.c'))
stub_ss.add(files('target-monitor-defs.c'))
stub_ss.add(files('tpm.c'))
stub_ss.add(files('trace-control.c'))
-stub_ss.add(files('uuid.c'))
+if have_block
+ stub_ss.add(files('uuid.c'))
+endif
stub_ss.add(files('vmgenid.c'))
stub_ss.add(files('vmstate.c'))
stub_ss.add(files('vm-stop.c'))
--
2.26.2
