George Dunlap
2013-Mar-11 13:57 UTC
[PATCH v3 1/3] libxl: Streamline vnc argument generation code
Makes the following changes to the vnc generation code:
* Simplifies and comments it, making it easier to read and grok
* Throws an error if duplicate values of display are set, rather
than the current very un-intuitive behavior.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
tools/libxl/libxl_dm.c | 79 ++++++++++++++++++++++++++++++------------------
1 file changed, 50 insertions(+), 29 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a8a36d7..caca7b5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -118,33 +118,43 @@ static char **
libxl__build_device_model_args_old(libxl__gc *gc,
flexarray_vappend(dm_args, "-domain-name", c_info->name,
NULL);
if (vnc) {
- char *vncarg;
- if (vnc->display) {
- if (vnc->listen && strchr(vnc->listen,
'':'') == NULL) {
- vncarg = libxl__sprintf(gc, "%s:%d",
- vnc->listen,
- vnc->display);
- } else {
- vncarg = libxl__sprintf(gc, "127.0.0.1:%d",
vnc->display);
- }
- } else if (vnc->listen) {
+ char *vncarg = NULL;
+
+ flexarray_append(dm_args, "-vnc");
+
+ /*
+ * If vnc->listen is present and contains a :, and
+ * - vnc->display is 0, use vnc->listen
+ * - vnc->display is non-zero, be confused
+ * If vnc->listen is present but doesn''t, use
vnc->listen:vnc->display.
+ * If vnc->listen is not present, use 127.0.0.1:vnc->display
+ * (Remembering that vnc->display already defaults to 0.)
+ */
+ if (vnc->listen) {
if (strchr(vnc->listen, '':'') != NULL) {
+ if (vnc->display) {
+ LOG(ERROR, "vncdisplay set, vnclisten contains
display");
+ return NULL;
+ }
vncarg = vnc->listen;
} else {
- vncarg = libxl__sprintf(gc, "%s:0", vnc->listen);
+ vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+ vnc->display);
}
- } else {
- vncarg = "127.0.0.1:0";
- }
- if (vnc->passwd && (vnc->passwd[0] !=
''\0''))
+ } else
+ vncarg = libxl__sprintf(gc, "127.0.0.1:%d",
vnc->display);
+
+ if (vnc->passwd && vnc->passwd[0]) {
vncarg = libxl__sprintf(gc, "%s,password", vncarg);
- flexarray_append(dm_args, "-vnc");
+ }
+
flexarray_append(dm_args, vncarg);
if (libxl_defbool_val(vnc->findunused)) {
flexarray_append(dm_args, "-vncunused");
}
}
+
if (sdl) {
flexarray_append(dm_args, "-sdl");
if (!libxl_defbool_val(sdl->opengl)) {
@@ -361,37 +371,48 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
if (c_info->name) {
flexarray_vappend(dm_args, "-name", c_info->name, NULL);
}
+
if (vnc) {
- int display = 0;
- const char *addr = "127.0.0.1";
char *vncarg = NULL;
flexarray_append(dm_args, "-vnc");
- if (vnc->display) {
- display = vnc->display;
- if (vnc->listen && strchr(vnc->listen,
'':'') == NULL) {
- addr = vnc->listen;
+ /*
+ * If vnc->listen is present and contains a :, and
+ * - vnc->display is 0, use vnc->listen
+ * - vnc->display is non-zero, be confused
+ * If vnc->listen is present but doesn''t, use
vnc->listen:vnc->display.
+ * If vnc->listen is not present, use 127.0.0.1:vnc->display
+ * (Remembering that vnc->display already defaults to 0.)
+ */
+ if (vnc->listen) {
+ if (strchr(vnc->listen, '':'') != NULL) {
+ if (vnc->display) {
+ LOG(ERROR, "vncdisplay set, vnclisten contains
display");
+ return NULL;
+ }
+ vncarg = vnc->listen;
+ } else {
+ vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+ vnc->display);
}
- } else if (vnc->listen) {
- addr = vnc->listen;
- }
+ } else
+ vncarg = libxl__sprintf(gc, "127.0.0.1:%d",
vnc->display);
- if (strchr(addr, '':'') != NULL)
- vncarg = libxl__sprintf(gc, "%s", addr);
- else
- vncarg = libxl__sprintf(gc, "%s:%d", addr, display);
if (vnc->passwd && vnc->passwd[0]) {
vncarg = libxl__sprintf(gc, "%s,password", vncarg);
}
+
if (libxl_defbool_val(vnc->findunused)) {
/* This option asks to QEMU to try this number of port before to
* give up. So QEMU will try ports between $display and $display +
* 99. This option needs to be the last one of the vnc options. */
vncarg = libxl__sprintf(gc, "%s,to=99", vncarg);
}
+
flexarray_append(dm_args, vncarg);
}
+
if (sdl) {
flexarray_append(dm_args, "-sdl");
/* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
--
1.7.9.5
George Dunlap
2013-Mar-11 13:57 UTC
[PATCH v3 2/3] libxl: Allow multiple USB devices on HVM domain creation
This patch allows an HVM domain to be created with multiple USB
devices.
Since the previous interface only allowed the passing of a single
device, this requires us to add a new element to the hvm struct of
libxl_domain_build_info -- usbdevice_list. For API compatibility, the
old element, usbdevice, remains.
If hvm.usbdevice_list is set, each device listed will cause an extra
"-usbdevice [foo]" to be appended to the qemu command line.
Callers may set either hvm.usbdevice or hvm.usbdevice_list, but not
both; libxl will throw an error if both are set.
In order to allow users of libxl to write software compatible with
older versions of libxl, also define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST.
If this is defined, callers may use either hvm.usbdevice or
hvm.usbdevice_list; otherwise, only hvm.usbdevice will be available.
v3:
- Duplicate functionality in both "new" and "old", since
we''re not
unifying the two anymore.
v2:
- Throw an error if both usbdevice and usbdevice_list are set
- Update and clarify definition based on feedback
- Previous patches means this works for both traditional and upstream
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
tools/libxl/libxl.h | 16 ++++++++++++++++
tools/libxl/libxl_dm.c | 38 ++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_types.idl | 1 +
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 030aa86..00a68b8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -273,6 +273,22 @@
#endif
#endif
+/*
+ * LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain hvm.usbdevice_list, a libxl_string_list type that contains
+ * a list of USB devices to specify on the qemu command-line.
+ *
+ * If it is set, callers may use either hvm.usbdevice or
+ * hvm.usbdevice_list, but not both; if both are set, libxl will
+ * throw an error.
+ *
+ * If this is not defined, callers can only use hvm.usbdevice. Note
+ * that this means only one device can be added at domain build time.
+ */
+#define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
+
/* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
* called from within libxl itself. Callers outside libxl, who
* do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index caca7b5..79c3879 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -198,11 +198,28 @@ static char **
libxl__build_device_model_args_old(libxl__gc *gc,
if (b_info->u.hvm.boot) {
flexarray_vappend(dm_args, "-boot",
b_info->u.hvm.boot, NULL);
}
- if (libxl_defbool_val(b_info->u.hvm.usb) ||
b_info->u.hvm.usbdevice) {
+ if (libxl_defbool_val(b_info->u.hvm.usb)
+ || b_info->u.hvm.usbdevice
+ || b_info->u.hvm.usbdevice_list) {
+ if ( b_info->u.hvm.usbdevice &&
b_info->u.hvm.usbdevice_list )
+ {
+ LOG(ERROR, "%s: Both usbdevice and usbdevice_list
set",
+ __func__);
+ return NULL;
+ }
flexarray_append(dm_args, "-usb");
if (b_info->u.hvm.usbdevice) {
flexarray_vappend(dm_args,
"-usbdevice",
b_info->u.hvm.usbdevice, NULL);
+ } else if (b_info->u.hvm.usbdevice_list) {
+ char **p;
+ for (p = b_info->u.hvm.usbdevice_list;
+ *p;
+ p++) {
+ flexarray_vappend(dm_args,
+ "-usbdevice",
+ *p, NULL);
+ }
}
}
if (b_info->u.hvm.soundhw) {
@@ -479,11 +496,28 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_vappend(dm_args, "-boot",
libxl__sprintf(gc, "order=%s",
b_info->u.hvm.boot), NULL);
}
- if (libxl_defbool_val(b_info->u.hvm.usb) ||
b_info->u.hvm.usbdevice) {
+ if (libxl_defbool_val(b_info->u.hvm.usb)
+ || b_info->u.hvm.usbdevice
+ || b_info->u.hvm.usbdevice_list) {
+ if ( b_info->u.hvm.usbdevice &&
b_info->u.hvm.usbdevice_list )
+ {
+ LOG(ERROR, "%s: Both usbdevice and usbdevice_list
set",
+ __func__);
+ return NULL;
+ }
flexarray_append(dm_args, "-usb");
if (b_info->u.hvm.usbdevice) {
flexarray_vappend(dm_args,
"-usbdevice",
b_info->u.hvm.usbdevice, NULL);
+ } else if (b_info->u.hvm.usbdevice_list) {
+ char **p;
+ for (p = b_info->u.hvm.usbdevice_list;
+ *p;
+ p++) {
+ flexarray_vappend(dm_args,
+ "-usbdevice",
+ *p, NULL);
+ }
}
}
if (b_info->u.hvm.soundhw) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5b080ed..dfb8658 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -330,6 +330,7 @@ libxl_domain_build_info =
Struct("domain_build_info",[
("usbdevice", string),
("soundhw", string),
("xen_platform_pci",
libxl_defbool),
+ ("usbdevice_list",
libxl_string_list),
])),
("pv", Struct(None, [("kernel", string),
("slack_memkb", MemKB),
--
1.7.9.5
George Dunlap
2013-Mar-11 13:57 UTC
[PATCH v3 3/3] xl: Accept a list for usbdevice in config file
Allow the "usbdevice" key to accept a list of USB devices, and pass
them in using the new usbdevice_list domain build element.
For backwards compatibility, still accept singleton values.
Also update the xl.cfg manpage, adding information about how to pass
through host devices.
v2:
- Add some verbiage to make it clear that "usb" is for emulated
devices
- Reference qemu manual for more usbdevice options
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
docs/man/xl.cfg.pod.5 | 28 +++++++++++++++++++---------
tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++++--
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 25523c9..a90fa11 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1145,17 +1145,27 @@ device.
=item B<usb=BOOLEAN>
-Enables or disables a USB bus in the guest.
+Enables or disables an emulated USB bus in the guest.
-=item B<usbdevice=DEVICE>
+=item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
-Adds B<DEVICE> to the USB bus. The USB bus must also be enabled using
-B<usb=1>. The most common use for this option is
B<usbdevice=tablet>
-which adds pointer device using absolute coordinates. Such devices
-function better than relative coordinate devices (such as a standard
-mouse) since many methods of exporting guest graphics (such as VNC)
-work better in this mode. Note that this is independent of the actual
-pointer device you are using on the host/client side.
+Adds B<DEVICE>s to the emulated USB bus. The USB bus must also be
+enabled using B<usb=1>. The most common use for this option is
+B<usbdevice=[''tablet'']> which adds pointer device using
absolute
+coordinates. Such devices function better than relative coordinate
+devices (such as a standard mouse) since many methods of exporting
+guest graphics (such as VNC) work better in this mode. Note that this
+is independent of the actual pointer device you are using on the
+host/client side.
+
+Host devices can also be passed through in this way, by specifying
+host:USBID, where USBID is of the form xxxx:yyyy. The USBID can
+typically be found by using lsusb or usb-devices.
+
+The form usbdevice=DEVICE is also accepted for backwards compatibility.
+
+More valid options can be found in the "usbdevice" section of the
qemu
+documentation.
=back
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a98705e..7195655 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1520,8 +1520,23 @@ skip_vfb:
xlu_cfg_replace_string (config, "serial",
&b_info->u.hvm.serial, 0);
xlu_cfg_replace_string (config, "boot",
&b_info->u.hvm.boot, 0);
xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb,
0);
- xlu_cfg_replace_string (config, "usbdevice",
- &b_info->u.hvm.usbdevice, 0);
+ switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
+
&b_info->u.hvm.usbdevice_list,
+ 1))
+ {
+
+ case 0: break; /* Success */
+ case ESRCH: break; /* Option not present */
+ case EINVAL:
+ /* If it''s not a valid list, try reading it as an atom,
falling through to
+ * an error if it fails */
+ if (!xlu_cfg_replace_string(config, "usbdevice",
&b_info->u.hvm.usbdevice, 0))
+ break;
+ /* FALLTHRU */
+ default:
+ fprintf(stderr,"xl: Unable to parse usbdevice.\n");
+ exit(-ERROR_FAIL);
+ }
xlu_cfg_replace_string (config, "soundhw",
&b_info->u.hvm.soundhw, 0);
xlu_cfg_get_defbool(config, "xen_platform_pci",
&b_info->u.hvm.xen_platform_pci, 0);
--
1.7.9.5
George Dunlap
2013-Mar-19 16:46 UTC
Re: [PATCH v3 1/3] libxl: Streamline vnc argument generation code
On Mon, Mar 11, 2013 at 1:57 PM, George Dunlap <george.dunlap@eu.citrix.com> wrote:> Makes the following changes to the vnc generation code: > * Simplifies and comments it, making it easier to read and grok > * Throws an error if duplicate values of display are set, rather > than the current very un-intuitive behavior. > > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>Ping? -George
Ian Jackson
2013-Mar-25 12:58 UTC
Re: [PATCH v3 1/3] libxl: Streamline vnc argument generation code [and 1 more messages]
George Dunlap writes ("[PATCH v3 1/3] libxl: Streamline vnc argument
generation code"):> Makes the following changes to the vnc generation code:
> * Simplifies and comments it, making it easier to read and grok
> * Throws an error if duplicate values of display are set, rather
> than the current very un-intuitive behavior.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
George Dunlap
2013-Mar-27 11:36 UTC
Re: [PATCH v3 1/3] libxl: Streamline vnc argument generation code [and 1 more messages]
On Mon, Mar 25, 2013 at 12:58 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> George Dunlap writes ("[PATCH v3 1/3] libxl: Streamline vnc argument generation code"): >> Makes the following changes to the vnc generation code: >> * Simplifies and comments it, making it easier to read and grok >> * Throws an error if duplicate values of display are set, rather >> than the current very un-intuitive behavior. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>Just checking, are the other two patches still in your queue of things to review? :-) -George