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@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> 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"
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list