Re: [Qemu-devel] [PATCH 02/11] Add migrate-set-capabilities and query-migrate-capabilities

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3202 bytes --]

On 08/06/2012 10:28 AM, Orit Wasserman wrote:

>> That is, BOTH commands end up iterating over a list of caps, and output
>> identical information in the case where caps exist of 'name: state' for
>> each capability.
>>

> The information is different:
> the first:
> MigrationCapabilityStatusList *
> qmp_query_migrate_supported_capabilities(Error **errp)
> {
>     MigrationCapabilityStatusList *caps_list = g_malloc0(sizeof(*caps_list));
> 
>     caps_list->value = g_malloc(sizeof(*caps_list->value));
>     caps_list->value->capability = MIGRATION_CAPABILITY_XBZRLE;
>     caps_list->value->state = true;
>     caps_list->next = NULL;
> 
>     return caps_list;
> }
> 
> the second:
> MigrationCapabilityStatusList *
> qmp_query_migrate_supported_capabilities(Error **errp)

I think you meant this for the second:

+MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
+{
+    MigrationCapabilityStatusList *head = NULL;
+    MigrationCapabilityStatusList *caps;
+    MigrationState *s = migrate_get_current();
+    int i;
+
+    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+        if (head == NULL) {
+            head = g_malloc0(sizeof(*caps));
+            caps = head;
+        } else {
+            caps->next = g_malloc0(sizeof(*caps));
+            caps = caps->next;
+        }
+        caps->value =
+            g_malloc(sizeof(*caps->value));
+        caps->value->capability = i;
+        caps->value->state = s->enabled_capabilities[i];
+    }

> you can look at it as 64bit support one is to know if the processor supports 64 bit
> the other to know if the OS uses 64 bit

Okay, so the QMP code is currently different (one outputs a list where
every entry in the list is hard-coded to true, the other outputs a list
where each entry reflects the current state).  But that still doesn't
address my concern that you don't need two QMP commands.

Merely listing 'xbzrle' in the list is enough to know that 'this
particular qemu knows how to do xbzrle', and the user is free to ignore
the additional information of whether it is actually in use at the time
if all the application cared about was determining the set of known
capabilities.

Given your argument with 64-bit processing, the same reasoning applies -
ask for a list of capabilities, and the result will either omit '64bit'
(the capability is not possible), include '64bit:false' (the capability
is known by the emulator, but not in use by the guest), or include
'64bit:true' (the capability is both known and in-use).  A user that
only cares about querying supported capabilities will check whether
'64bit' is in the list, and throw away the information about whether it
is on or off (and since the QMP command currently returns a hard-coded
true, that information is already being discarded via your
query-migrate-supported-capabilities command).  That is, your
implementation for query-migrate-capabilities is sufficient to satisfy
both class of users, without needing query-migrate-supported-capabilities.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]