[libvirt] [PATCH 2/9] xenconfig: add support for multiple USB devices syntax

Jim Fehlig jfehlig at suse.com
Fri Mar 13 17:03:33 UTC 2015


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.
>
> Adjust tests for changed options order.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
>  src/xenconfig/xen_common.c                     |  66 -------------
>  src/xenconfig/xen_xl.c                         | 127 +++++++++++++++++++++++++
>  src/xenconfig/xen_xm.c                         |  72 ++++++++++++++
>  tests/xmconfigdata/test-fullvirt-usbmouse.cfg  |   4 +-
>  tests/xmconfigdata/test-fullvirt-usbtablet.cfg |   4 +-
>  5 files changed, 203 insertions(+), 70 deletions(-)
>
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index a2a1474..079f77d 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -972,33 +972,6 @@ xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def)
>          if (str &&
>              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);
> -                return -1;
> -
> -            }
> -            def->inputs[0] = input;
> -            def->ninputs = 1;
> -        }
>      }
>  
>      return 0;
> @@ -1949,42 +1922,6 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def)
>  }
>  
>  
> -static int
> -xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def)
> -{
> -    size_t i;
> -
> -    if (STREQ(def->os.type, "hvm")) {
> -        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;
> -
> -                switch (def->inputs[i]->type) {
> -                    case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> -                        if (xenConfigSetString(conf, "usbdevice", "mouse") < 0)
> -                            return -1;
> -
> -                        break;
> -                    case VIR_DOMAIN_INPUT_TYPE_TABLET:
> -                        if (xenConfigSetString(conf, "usbdevice", "tablet") < 0)
> -                            return -1;
> -
> -                        break;
> -                    case VIR_DOMAIN_INPUT_TYPE_KBD:
> -                        if (xenConfigSetString(conf, "usbdevice", "keyboard") < 0)
> -                            return -1;
> -
> -                        break;
> -                }
> -                break;
> -            }
> -        }
> -    }
> -
> -    return 0;
> -}
> -
>  
>  static int
>  xenFormatVif(virConfPtr conf,
> @@ -2059,9 +1996,6 @@ xenFormatConfigCommon(virConfPtr conf,
>      if (xenFormatEmulator(conf, def) < 0)
>          return -1;
>  
> -    if (xenFormatInputDevs(conf, def) < 0)
> -        return -1;
> -
>      if (xenFormatVfb(conf, def, xendConfigVersion) < 0)
>          return -1;
>  
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 95ef5f4..f127ebe 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -289,6 +289,59 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
>      goto cleanup;
>  }
>  
> +static int
> +xenParseXLInputDevs(virConfPtr conf, virDomainDefPtr def)
> +{
> +    const char *str;
> +    virConfValuePtr val;
> +
> +    if (STREQ(def->os.type, "hvm")) {
> +        val = virConfGetValue(conf, "usbdevice");
> +        /* usbdevice can be defined as either a single string or a list */
> +        if (val && val->type == VIR_CONF_LIST) {
> +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
> +            val = val->list;
> +#else
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("multiple USB devices not supported"));
> +            return -1;
> +#endif
> +        }
> +        /* 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;
> +                }
> +            }
> +            val = val->next;
> +        }
> +    }
> +    return 0;
> +}
>  
>  virDomainDefPtr
>  xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
> @@ -310,6 +363,9 @@ xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
>      if (xenParseXLSpice(conf, def) < 0)
>          goto cleanup;
>  
> +    if (xenParseXLInputDevs(conf, def) < 0)
> +        goto cleanup;
> +
>      return def;
>  
>   cleanup:
> @@ -488,6 +544,74 @@ xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
>      return 0;
>  }
>  
> +static int
> +xenFormatXLInputDevs(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)
> +                    goto error;
> +
> +                switch (def->inputs[i]->type) {
> +                    case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> +                        devtype = "mouse";
> +                        break;
> +                    case VIR_DOMAIN_INPUT_TYPE_TABLET:
> +                        devtype = "tablet";
> +                        break;
> +                    case VIR_DOMAIN_INPUT_TYPE_KBD:
> +                        devtype = "keyboard";
> +                        break;
> +                    default:
> +                        continue;
> +                }
> +
> +                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;
>   

I had to stare at this a bit to convince myself memory was being freed
properly. xenConfigSetString copies the string, so need virConfFreeValue
afterwards - check.  virtConfSetValue takes ownership of value, so no
need to free - check.  Cleanup on error - check.  Caller frees values
when conf is destoryed - check.  Coverity, and subsequently John, will
complain loudly if I've missed a check.

ACK.

Regards,
Jim

> +}
> +
>  
>  virConfPtr
>  xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
> @@ -506,6 +630,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
>      if (xenFormatXLSpice(conf, def) < 0)
>          goto cleanup;
>  
> +    if (xenFormatXLInputDevs(conf, def) < 0)
> +        goto cleanup;
> +
>      return conf;
>  
>   cleanup:
> diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
> index 2f57cd2..5d70af6 100644
> --- a/src/xenconfig/xen_xm.c
> +++ b/src/xenconfig/xen_xm.c
> @@ -365,6 +365,38 @@ xenFormatXMDisks(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion)
>  }
>  
>  
> +static int
> +xenParseXMInputDevs(virConfPtr conf, virDomainDefPtr def)
> +{
> +    const char *str;
> +
> +    if (STREQ(def->os.type, "hvm")) {
> +        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_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
> +                virDomainInputDefFree(input);
> +                return -1;
> +            }
> +        }
> +    }
> +    return 0;
> +}
> +
>  /*
>   * Convert an XM config record into a virDomainDef object.
>   */
> @@ -387,6 +419,9 @@ xenParseXM(virConfPtr conf,
>      if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
>           goto cleanup;
>  
> +    if (xenParseXMInputDevs(conf, def) < 0)
> +         goto cleanup;
> +
>      return def;
>  
>   cleanup:
> @@ -394,6 +429,40 @@ xenParseXM(virConfPtr conf,
>      return NULL;
>  }
>  
> +static int
> +xenFormatXMInputDevs(virConfPtr conf, virDomainDefPtr def)
> +{
> +    size_t i;
> +    const char *devtype;
> +
> +    if (STREQ(def->os.type, "hvm")) {
> +        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;
> +
> +                switch (def->inputs[i]->type) {
> +                    case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> +                        devtype = "mouse";
> +                        break;
> +                    case VIR_DOMAIN_INPUT_TYPE_TABLET:
> +                        devtype = "tablet";
> +                        break;
> +                    case VIR_DOMAIN_INPUT_TYPE_KBD:
> +                        devtype = "keyboard";
> +                        break;
> +                    default:
> +                        continue;
> +                }
> +                if (xenConfigSetString(conf, "usbdevice", devtype) < 0)
> +                    return -1;
> +                break;
> +            }
> +        }
> +    }
> +    return 0;
> +}
> +
>  
>  /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
>     either 32, or 64 on a platform where long is big enough.  */
> @@ -418,6 +487,9 @@ xenFormatXM(virConnectPtr conn,
>      if (xenFormatXMDisks(conf, def, xendConfigVersion) < 0)
>          goto cleanup;
>  
> +    if (xenFormatXMInputDevs(conf, def) < 0)
> +        goto cleanup;
> +
>      return conf;
>  
>   cleanup:
> diff --git a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
> index b24d7a4..7b252cf 100755
> --- a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
> +++ b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
> @@ -14,8 +14,6 @@ on_poweroff = "destroy"
>  on_reboot = "restart"
>  on_crash = "restart"
>  device_model = "/usr/lib/xen/bin/qemu-dm"
> -usb = 1
> -usbdevice = "mouse"
>  sdl = 0
>  vnc = 1
>  vncunused = 1
> @@ -25,3 +23,5 @@ vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
>  parallel = "none"
>  serial = "none"
>  disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso,hdc:cdrom,r" ]
> +usb = 1
> +usbdevice = "mouse"
> diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
> index 189cd15..7e6cd36 100755
> --- a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
> +++ b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
> @@ -14,8 +14,6 @@ on_poweroff = "destroy"
>  on_reboot = "restart"
>  on_crash = "restart"
>  device_model = "/usr/lib/xen/bin/qemu-dm"
> -usb = 1
> -usbdevice = "tablet"
>  sdl = 0
>  vnc = 1
>  vncunused = 1
> @@ -25,3 +23,5 @@ vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
>  parallel = "none"
>  serial = "none"
>  disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso,hdc:cdrom,r" ]
> +usb = 1
> +usbdevice = "tablet"
>   




More information about the libvir-list mailing list