[libvirt] [PATCH] libxl: fix regression introduced by commit 4ab8cd77
Commit 4ab8cd77 added a check requiring input devices to have a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the domain otherwise. But virDomainDefParseXML adds implicit mouse and keyboard if a graphics device is configured. See calls to virDomainDefMaybeAddInput. The regression is fixed by removing the check requiring USB input devices, and skipping non-USB input devices when populating USB 'usbdevice' in libxl_domain_build_info struct. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 50ef9d8..2b57d0b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0); if (def->ninputs) { - for (i = 0; i < def->ninputs; i++) { - if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("libxenlight supports only USB input")); - return -1; - } - } #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < 0) return -1; @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, #endif for (i = 0; i < def->ninputs; i++) { char **usbdevice; + + if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) + continue; + #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST usbdevice = &b_info->u.hvm.usbdevice_list[i]; #else -- 1.8.4.5
On Fri, Mar 13, 2015 at 06:38:24PM -0600, Jim Fehlig wrote:
Commit 4ab8cd77 added a check requiring input devices to have a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the domain otherwise. But virDomainDefParseXML adds implicit mouse and keyboard if a graphics device is configured. See calls to virDomainDefMaybeAddInput.
The regression is fixed by removing the check requiring USB input devices, and skipping non-USB input devices when populating USB 'usbdevice' in libxl_domain_build_info struct.
So IIUC the problem is that we're adding an mouse + keyboard with input bus == Xen (paravirt) or bus == ps2. (HVM). With libxl though, these are implicitly present by default when you have graphics enabled, so you don't want libxlMakeDomBuildInfo to see these input devices. You only want to be looking at the USB input devices, as they're the only ones that neeed extra config settings procssed, is that right ?
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 50ef9d8..2b57d0b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
if (def->ninputs) { - for (i = 0; i < def->ninputs; i++) { - if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("libxenlight supports only USB input")); - return -1; - } - } #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < 0) return -1; @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, #endif for (i = 0; i < def->ninputs; i++) { char **usbdevice; + + if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) + continue; + #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST usbdevice = &b_info->u.hvm.usbdevice_list[i]; #else
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
On Fri, Mar 13, 2015 at 06:38:24PM -0600, Jim Fehlig wrote:
Commit 4ab8cd77 added a check requiring input devices to have a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the domain otherwise. But virDomainDefParseXML adds implicit mouse and keyboard if a graphics device is configured. See calls to virDomainDefMaybeAddInput.
The regression is fixed by removing the check requiring USB input devices, and skipping non-USB input devices when populating USB 'usbdevice' in libxl_domain_build_info struct.
So IIUC the problem is that we're adding an mouse + keyboard with input bus == Xen (paravirt) or bus == ps2. (HVM). With libxl though, these are implicitly present by default when you have graphics enabled, so you don't want libxlMakeDomBuildInfo to see these input devices.
Yes. FWIW, the legacy xen driver behaves similarly. Only USB input devices are processed.
You only want to be looking at the USB input devices, as they're the only ones that neeed extra config settings procssed, is that right ?
Correct.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 50ef9d8..2b57d0b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
if (def->ninputs) { - for (i = 0; i < def->ninputs; i++) { - if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("libxenlight supports only USB input")); - return -1; - } - } #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < 0) return -1; @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, #endif for (i = 0; i < def->ninputs; i++) { char **usbdevice; + + if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) + continue; + #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST usbdevice = &b_info->u.hvm.usbdevice_list[i]; #else
ACK
Thanks! Will push shortly. Regards, Jim
participants (2)
-
Daniel P. Berrange -
Jim Fehlig