[Kimchi-devel] [PATCH][Kimchi] Allow vm update graphics type

[Paulo Ricardo Paz Vital]

Few comments below. Also, would be nice to have some new (or updated) testcase.

On Aug 23 10:53AM, Ramon Medeiros wrote:
> Allow vm to choose between spice and vnc
> 
> Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
> ---
>  API.json     |  6 ++++++
>  docs/API.md  |  1 +
>  i18n.py      |  2 +-
>  model/vms.py | 10 ++++++++++
>  4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/API.json b/API.json
> index a3af02d..dca6840 100644
> --- a/API.json
> +++ b/API.json
> @@ -377,6 +377,12 @@
>                              "description": "Life time for the graphics password.",
>                              "type": "number",
>                              "error": "KCHVM0032E"
> +                        },
> +                        "type": {
> +                            "description": "Graphics type",
> +                            "type": "string",
> +                            "enum": ["spice", "vnc"],
> +                            "error": "KCHVM0054E"
>                          }
>                      }
>                  },
> diff --git a/docs/API.md b/docs/API.md
> index 7bd677f..bb8049b 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -166,6 +166,7 @@ server.
>          * passwdValidTo *(optional)*: lifetime for the console password. When
>                                        omitted the password will be valid just
>                                        for 30 seconds.
> +        * type *(optional)*: graphics type. VNC os Spice.

There's a little typo here.

>      * cpu_info *(optional)*: CPU-specific information.
>          * maxvcpus *(optional)*: The maximum number of vCPUs that can be
>                assigned to the VM. If topology is specified, maxvcpus must be a
> diff --git a/i18n.py b/i18n.py
> index e8d9c05..183329e 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -106,7 +106,7 @@ messages = {
>      "KCHVM0051E": _("Cannot reset %(name)s. Virtual machine is already shut off."),
>      "KCHVM0052E": _("Boot order must be a list. Devices accepted: hd, cdrom, fd or network."),
>      "KCHVM0053E": _("Bootmenu must be boolean. Values accepted: true of false."),
> -
> +    "KCHVM0054E": _("Graphics type. Values accepted: vnc or spice."),

IMO, a better message would be "Graphic type not valid. Values accepted:
vnc or spice."

>      "KCHVM0055E": _("Migrate to localhost %(host)s is not allowed."),
>      "KCHVM0056E": _("To migrate a virtual machine to the remote host %(host)s the user %(user)s must have password-less login to the remote host."),
>      "KCHVM0057E": _("Can not migrate virtual machine %(name)s when its in %(state)s state."),
> diff --git a/model/vms.py b/model/vms.py
> index 3380278..cdcb501 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -290,6 +290,12 @@ class VMModel(object):
> 
>              # GRAPHICS can be updated offline or online
>              if 'graphics' in params:
> +
> +                # some parameters cannot change while vm is running
> +                if DOM_STATE_MAP[dom.info()[0]] != 'shutoff':
> +                    if 'type' in params['graphics']:
> +                        raise InvalidParameter('KCHVM0074E',
> +                                               {'params': 'graphics type'})
>                  dom = self._update_graphics(dom, params)
> 
>              # Live updates
> @@ -701,6 +707,10 @@ class VMModel(object):
>              valid_to = time.strftime('%Y-%m-%dT%H:%M:%S', expire_time)
>              graphics.attrib['passwdValidTo'] = valid_to
> 
> +        gtype = params['graphics'].get('type')
> +        if gtype is not None:
> +            graphics.attrib['type'] = gtype
> +
>          conn = self.conn.get()
>          if not dom.isActive():
>              return conn.defineXML(ET.tostring(root, encoding="utf-8"))
> -- 
> 2.5.5
> 
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> 

-- 
Paulo Ricardo Paz Vital
Linux Technology Center, IBM Systems
http://www.ibm.com/linux/ltc/