Re: [Qemu-devel] [PATCH v2] vmgenid: allow VM Generation ID modification via QMP/HMP

[Eric Blake <eblake@redhat.com>]

On 02/28/2018 07:39 AM, Or Idgar wrote:
> From: Or Idgar <oidgar@redhat.com>
> 
> This patch allow changing the Virtual Machine Generation

s/allow/allows/

> ID through QMP/HMP while the vm guest is running.

That's the description of "what" the patch is doing, but not the "why" - 
you want to give some justification why this is useful.  Especially since...

> As the definition block of VMGENID in ACPI includes the
> "Notify" method, we can use it to notify the guest about
> ID changes.
> 
> QMP command example:
>      { "execute": "set-vm-generation-id",
>            "arguments": {
>                "guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
>            }
>      }
> 
> HMP command example:
>      set-vm-generation-id 324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87
> 
> This patch is based off an earlier version by
> Igor Mammedov (imammedo@redhat.com)
> 
> Signed-off-by: Or Idgar <oidgar@redhat.com>
> Reviewed-by: Gal Hammer <ghammer@redhat.com>
> ---

> +++ b/docs/specs/vmgenid.txt
> @@ -240,6 +240,7 @@ The property may be queried via QMP/HMP:
>     (QEMU) query-vm-generation-id
>     {"return": {"guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"}}
>   
> -Setting of this parameter is intentionally left out from the QMP/HMP
> -interfaces.  There are no known use cases for changing the GUID once QEMU is
> -running, and adding this capability would greatly increase the complexity.

...the old docs said no known use exists.  Obviously, you wouldn't be 
adding this unless you had a use case, so please describe that use case 
as the "why".

> +Also, the property may be set via QMP/HMP:
> +
> +  (QEMU) set-vm-generation-id guid=ee6726ce-73b4-4a8b-863c-708f26515847
> +  {"return": {}}
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 4afd57c..4524669 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1871,5 +1871,18 @@ ETEXI
>       },
>   
>   STEXI
> +@item set-vm-generation-id @var{uuid}

HMP commands are generally named with _ rather than -; and are geared 
for easy human consumption (abbreviation is permitted), so I might have 
named the HMP version set_vm_genid.

> +++ b/qapi-schema.json
> @@ -3193,6 +3193,17 @@
>   { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>   
>   ##
> +# @set-vm-generation-id:
> +#
> +# Set Virtual Machine Generation ID
> +#
> +# @guid: new GUID to set as Virtual Machine Generation ID
> +#
> +# Since 2.12
> +##
> +{ 'command': 'set-vm-generation-id', 'data': { 'guid': 'str' } }
> +

The QMP portion looks reasonable.  However, without a why, I'm reluctant 
to give R-b.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org