[libvirt] [PATCH 06.1/10] xenconfig: add support for multiple USB devices syntax

[Jim Fehlig]

Marek Marczykowski-Górecki wrote:
> In Xen>=4.3, libxl supports new syntax for USB devices:
> usbdevice=[ "DEVICE", "DEVICE", ... ]
> Add support for that in xenconfig driver. When only one device is
> defined, keep using old syntax for backward compatibility.
>   

Cool, thanks for doing this!  But I have a "small" nit...

> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
>  src/xenconfig/xen_common.c | 108 +++++++++++++++++++++++++++++++--------------
>   

xm only supported the usbdevice=DEVICE syntax, but the code in
xen_common is used to parse/format both xm and xl config.  I think
parsing/formating of these devices should be moved to xen-xm and xen-xl.

Regards,
Jim

>  1 file changed, 76 insertions(+), 32 deletions(-)
>
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index a2a1474..4494f1d 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -964,6 +964,7 @@ static int
>  xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def)
>  {
>      const char *str;
> +    virConfValuePtr val;
>  
>      if (STREQ(def->os.type, "hvm")) {
>          if (xenConfigGetString(conf, "soundhw", &str, NULL) < 0)
> @@ -973,31 +974,42 @@ xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def)
>              xenParseSxprSound(def, str) < 0)
>              return -1;
>  
> -        if (xenConfigGetString(conf, "usbdevice", &str, NULL) < 0)
> -            return -1;
>  
> -        if (str &&
> -            (STREQ(str, "tablet") ||
> -             STREQ(str, "mouse") ||
> -             STREQ(str, "keyboard"))) {
> -            virDomainInputDefPtr input;
> -            if (VIR_ALLOC(input) < 0)
> -                return -1;
> -
> -            input->bus = VIR_DOMAIN_INPUT_BUS_USB;
> -            if (STREQ(str, "mouse"))
> -                input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
> -            else if (STREQ(str, "tablet"))
> -                input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
> -            else if (STREQ(str, "keyboard"))
> -                input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
> -            if (VIR_ALLOC_N(def->inputs, 1) < 0) {
> -                virDomainInputDefFree(input);
> +        val = virConfGetValue(conf, "usbdevice");
> +        /* usbdevice can be defined as either a single string or a list */
> +        if (val && val->type == VIR_CONF_LIST)
> +            val = val->list;
> +        /* otherwise val->next is NULL, so can be handled by the same code */
> +        while (val) {
> +            if (val->type != VIR_CONF_STRING) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("config value %s was malformed"),
> +                               "usbdevice");
>                  return -1;
> +            }
> +            str = val->str;
> +
> +            if (str &&
> +                    (STREQ(str, "tablet") ||
> +                     STREQ(str, "mouse") ||
> +                     STREQ(str, "keyboard"))) {
> +                virDomainInputDefPtr input;
> +                if (VIR_ALLOC(input) < 0)
> +                    return -1;
>  
> +                input->bus = VIR_DOMAIN_INPUT_BUS_USB;
> +                if (STREQ(str, "mouse"))
> +                    input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
> +                else if (STREQ(str, "tablet"))
> +                    input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
> +                else if (STREQ(str, "keyboard"))
> +                    input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
> +                if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
> +                    virDomainInputDefFree(input);
> +                    return -1;
> +                }
>              }
> -            def->inputs[0] = input;
> -            def->ninputs = 1;
> +            val = val->next;
>          }
>      }
>  
> @@ -1953,36 +1965,68 @@ static int
>  xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def)
>  {
>      size_t i;
> +    const char *devtype;
> +    virConfValuePtr usbdevices = NULL, lastdev;
>  
>      if (STREQ(def->os.type, "hvm")) {
> +        if (VIR_ALLOC(usbdevices) < 0)
> +            goto error;
> +
> +        usbdevices->type = VIR_CONF_LIST;
> +        usbdevices->list = NULL;
> +        lastdev = NULL;
>          for (i = 0; i < def->ninputs; i++) {
>              if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
>                  if (xenConfigSetInt(conf, "usb", 1) < 0)
> -                    return -1;
> +                    goto error;
>  
>                  switch (def->inputs[i]->type) {
>                      case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> -                        if (xenConfigSetString(conf, "usbdevice", "mouse") < 0)
> -                            return -1;
> -
> +                        devtype = "mouse";
>                          break;
>                      case VIR_DOMAIN_INPUT_TYPE_TABLET:
> -                        if (xenConfigSetString(conf, "usbdevice", "tablet") < 0)
> -                            return -1;
> -
> +                        devtype = "tablet";
>                          break;
>                      case VIR_DOMAIN_INPUT_TYPE_KBD:
> -                        if (xenConfigSetString(conf, "usbdevice", "keyboard") < 0)
> -                            return -1;
> -
> +                        devtype = "keyboard";
>                          break;
> +                    default:
> +                        continue;
>                  }
> -                break;
> +
> +                if (lastdev == NULL) {
> +                    if (VIR_ALLOC(lastdev) < 0)
> +                        goto error;
> +                    usbdevices->list = lastdev;
> +                } else {
> +                    if (VIR_ALLOC(lastdev->next) < 0)
> +                        goto error;
> +                    lastdev = lastdev->next;
> +                }
> +                lastdev->type = VIR_CONF_STRING;
> +                if (VIR_STRDUP(lastdev->str, devtype) < 0)
> +                    goto error;
>              }
>          }
> +        if (usbdevices->list != NULL) {
> +            if (usbdevices->list->next == NULL) {
> +                /* for compatibility with Xen <= 4.2, use old syntax when
> +                 * only one device present */
> +                if (xenConfigSetString(conf, "usbdevice", usbdevices->list->str) < 0)
> +                    goto error;
> +                virConfFreeValue(usbdevices);
> +            } else {
> +                virConfSetValue(conf, "usbdevice", usbdevices);
> +            }
> +        } else {
> +            VIR_FREE(usbdevices);
> +        }
>      }
>  
>      return 0;
> + error:
> +    virConfFreeValue(usbdevices);
> +    return -1;
>  }
>  
>  
>