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