[libvirt] [PATCH v2 3/4] device: cleanup input device code

Pavel Hrdina phrdina at redhat.com
Tue Jan 12 11:59:43 UTC 2016


The current code was a little bit odd.  At first we've removed all
possible implicit input devices from domain definition to add them later
back if there was any graphics device defined while parsing XML
description.  That's not all, while formating domain definition to XML
description we at first ignore any input devices with bus different to
USB and VIRTIO and few lines later we add implicit input devices to XML.

This seems to me as a lot of code for nothing.  This patch may look
to be more complicated than original approach, but this is a preferred
way to modify/add driver specific stuff only in those drivers and not
deal with them in common parsing/formating functions.

The update is to add those implicit input devices into config XML to
follow the real HW configuration visible by guest OS.

There was also inconsistence between our behavior and QEMU's in the way,
that in QEMU there is no way how to disable those implicit input devices
for x86 architecture and they are available always, even without graphics
device.  This applies also to XEN hypervisor.  VZ driver already does its
part by putting correct implicit devices into live XML.

Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
---
 src/Makefile.am            |  4 +--
 src/conf/domain_conf.c     | 82 +++++-----------------------------------------
 src/libxl/libxl_domain.c   |  5 +++
 src/qemu/qemu_domain.c     | 23 +++++++++++++
 src/xen/xen_driver.c       |  5 +++
 src/xenapi/xenapi_driver.c |  5 +++
 src/xenconfig/xen_common.c | 22 +++++++++++++
 src/xenconfig/xen_common.h |  2 ++
 8 files changed, 72 insertions(+), 76 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index aa5ab69..522c56d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1211,7 +1211,7 @@ libvirt_driver_xen_impl_la_CFLAGS =				\
 		-I$(srcdir)/xenconfig				\
 		$(AM_CFLAGS)
 libvirt_driver_xen_impl_la_LDFLAGS = $(AM_LDFLAGS)
-libvirt_driver_xen_impl_la_LIBADD = $(XEN_LIBS)
+libvirt_driver_xen_impl_la_LIBADD = $(XEN_LIBS) libvirt_xenconfig.la
 libvirt_driver_xen_impl_la_SOURCES = $(XEN_DRIVER_SOURCES)
 endif WITH_XEN
 
@@ -1272,7 +1272,7 @@ if WITH_XENAPI
 noinst_LTLIBRARIES += libvirt_driver_xenapi.la
 libvirt_la_BUILT_LIBADD += libvirt_driver_xenapi.la
 libvirt_driver_xenapi_la_CFLAGS = $(LIBXENSERVER_CFLAGS) $(CURL_CFLAGS) \
-		-I$(srcdir)/conf $(AM_CFLAGS)
+		-I$(srcdir)/conf -I$(srcdir)/xenconfig $(AM_CFLAGS)
 libvirt_driver_xenapi_la_LDFLAGS = $(AM_LDFLAGS)
 libvirt_driver_xenapi_la_LIBADD = $(LIBXENSERVER_LIBS) $(CURL_LIBS)
 libvirt_driver_xenapi_la_SOURCES = $(XENAPI_DRIVER_SOURCES)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 72018a3..474d2fd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16034,27 +16034,6 @@ virDomainDefParseXML(xmlDocPtr xml,
             goto error;
         }
 
-        /* With QEMU / KVM / Xen graphics, mouse + PS/2 is implicit
-         * with graphics, so don't store it.
-         * XXX will this be true for other virt types ? */
-        if ((def->os.type == VIR_DOMAIN_OSTYPE_HVM &&
-             input->bus == VIR_DOMAIN_INPUT_BUS_PS2 &&
-             (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
-              input->type == VIR_DOMAIN_INPUT_TYPE_KBD)) ||
-            (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
-             input->bus == VIR_DOMAIN_INPUT_BUS_XEN &&
-             (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
-              input->type == VIR_DOMAIN_INPUT_TYPE_KBD)) ||
-            (def->os.type == VIR_DOMAIN_OSTYPE_EXE &&
-             (def->virtType == VIR_DOMAIN_VIRT_VZ ||
-              def->virtType == VIR_DOMAIN_VIRT_PARALLELS)  &&
-             input->bus == VIR_DOMAIN_INPUT_BUS_PARALLELS &&
-             (input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
-              input->type == VIR_DOMAIN_INPUT_TYPE_KBD))) {
-            virDomainInputDefFree(input);
-            continue;
-        }
-
         def->inputs[def->ninputs++] = input;
     }
     VIR_FREE(nodes);
@@ -16075,29 +16054,6 @@ virDomainDefParseXML(xmlDocPtr xml,
     }
     VIR_FREE(nodes);
 
-    /* If graphics are enabled, there's an implicit PS2 mouse */
-    if (def->ngraphics > 0 &&
-        (ARCH_IS_X86(def->os.arch) || def->os.arch == VIR_ARCH_NONE)) {
-        int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
-
-        if (def->os.type == VIR_DOMAIN_OSTYPE_HVM)
-            input_bus = VIR_DOMAIN_INPUT_BUS_PS2;
-        if (def->os.type == VIR_DOMAIN_OSTYPE_EXE &&
-            (def->virtType == VIR_DOMAIN_VIRT_VZ ||
-             def->virtType == VIR_DOMAIN_VIRT_PARALLELS))
-            input_bus = VIR_DOMAIN_INPUT_BUS_PARALLELS;
-
-        if (virDomainDefMaybeAddInput(def,
-                                      VIR_DOMAIN_INPUT_TYPE_MOUSE,
-                                      input_bus) < 0)
-            goto error;
-
-        if (virDomainDefMaybeAddInput(def,
-                                      VIR_DOMAIN_INPUT_TYPE_KBD,
-                                      input_bus) < 0)
-            goto error;
-    }
-
     /* analysis of the sound devices */
     if ((n = virXPathNodeSet("./devices/sound", ctxt, &nodes)) < 0)
         goto error;
@@ -20885,6 +20841,11 @@ virDomainInputDefFormat(virBufferPtr buf,
     const char *type = virDomainInputTypeToString(def->type);
     const char *bus = virDomainInputBusTypeToString(def->bus);
 
+    /* don't format keyboard into migratable XML for backward compatibility */
+    if (def->type == VIR_DOMAIN_INPUT_TYPE_KBD &&
+        flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)
+        return 0;
+
     if (!type) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unexpected input type %d"), def->type);
@@ -22339,11 +22300,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0)
             goto error;
 
-    for (n = 0; n < def->ninputs; n++)
-        if ((def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB ||
-             def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) &&
-            virDomainInputDefFormat(buf, def->inputs[n], flags) < 0)
+    for (n = 0; n < def->ninputs; n++) {
+        if (virDomainInputDefFormat(buf, def->inputs[n], flags) < 0)
             goto error;
+    }
 
     if (def->tpm) {
         if (virDomainTPMDefFormat(buf, def->tpm, flags) < 0)
@@ -22351,32 +22311,6 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     }
 
     if (def->ngraphics > 0) {
-        /* If graphics is enabled, add the implicit mouse/keyboard */
-        if ((ARCH_IS_X86(def->os.arch)) || def->os.arch == VIR_ARCH_NONE) {
-            virDomainInputDef autoInput = {
-                .type = VIR_DOMAIN_INPUT_TYPE_MOUSE,
-                .info = { .alias = NULL },
-            };
-
-            if (def->os.type == VIR_DOMAIN_OSTYPE_HVM)
-                autoInput.bus = VIR_DOMAIN_INPUT_BUS_PS2;
-            else if (def->os.type == VIR_DOMAIN_OSTYPE_EXE &&
-                     (def->virtType == VIR_DOMAIN_VIRT_VZ ||
-                      def->virtType == VIR_DOMAIN_VIRT_PARALLELS))
-                autoInput.bus = VIR_DOMAIN_INPUT_BUS_PARALLELS;
-            else
-               autoInput.bus = VIR_DOMAIN_INPUT_BUS_XEN;
-
-            if (virDomainInputDefFormat(buf, &autoInput, flags) < 0)
-                goto error;
-
-            if (!(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) {
-                autoInput.type = VIR_DOMAIN_INPUT_TYPE_KBD;
-                if (virDomainInputDefFormat(buf, &autoInput, flags) < 0)
-                    goto error;
-            }
-        }
-
         for (n = 0; n < def->ngraphics; n++)
             if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0)
                 goto error;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index cf5c9f6..ee49ba7 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -35,6 +35,7 @@
 #include "virstring.h"
 #include "virtime.h"
 #include "locking/domain_lock.h"
+#include "xenconfig/xen_common.h"
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
@@ -396,6 +397,10 @@ libxlDomainDefPostParse(virDomainDefPtr def,
         def->consoles[0] = chrdef;
     }
 
+    /* add implicit input devices */
+    if (xenDomainDefAddImplicitInputDevice(def) < 0)
+        return -1;
+
     /* memory hotplug tunables are not supported by this driver */
     if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0)
         return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0fa2dbe..1df1b74 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1035,6 +1035,25 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = {
 
 
 static int
+qemuDomainDefAddImplicitInputDevice(virDomainDef *def)
+{
+    if (ARCH_IS_X86(def->os.arch)) {
+        if (virDomainDefMaybeAddInput(def,
+                                      VIR_DOMAIN_INPUT_TYPE_MOUSE,
+                                      VIR_DOMAIN_INPUT_BUS_PS2) < 0)
+            return -1;
+
+        if (virDomainDefMaybeAddInput(def,
+                                      VIR_DOMAIN_INPUT_TYPE_KBD,
+                                      VIR_DOMAIN_INPUT_BUS_PS2) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
+static int
 qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
                                virQEMUCapsPtr qemuCaps)
 {
@@ -1049,6 +1068,10 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
     bool addPanicDevice = false;
     int ret = -1;
 
+    /* add implicit input devices */
+    if (qemuDomainDefAddImplicitInputDevice(def) < 0)
+        goto cleanup;
+
     /* Add implicit PCI root controller if the machine has one */
     switch (def->os.arch) {
     case VIR_ARCH_I686:
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 198390e..4da2d6d 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -46,6 +46,7 @@
 
 #include "xen_sxpr.h"
 #include "xen_xm.h"
+#include "xen_common.h"
 #include "xen_hypervisor.h"
 #include "xend_internal.h"
 #include "xs_internal.h"
@@ -382,6 +383,10 @@ xenDomainDefPostParse(virDomainDefPtr def,
         def->memballoon = memballoon;
     }
 
+    /* add implicit input device */
+    if (xenDomainDefAddImplicitInputDevice(def) <0)
+        return -1;
+
     /* memory hotplug tunables are not supported by this driver */
     if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0)
         return -1;
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 44a00c5..629495c 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -39,6 +39,7 @@
 #include "xenapi_driver_private.h"
 #include "xenapi_utils.h"
 #include "virstring.h"
+#include "xen_common.h"
 
 #define VIR_FROM_THIS VIR_FROM_XENAPI
 
@@ -79,6 +80,10 @@ xenapiDomainDefPostParse(virDomainDefPtr def,
                          unsigned int parseFlags ATTRIBUTE_UNUSED,
                          void *opaque ATTRIBUTE_UNUSED)
 {
+    /* add implicit input device */
+    if (xenDomainDefAddImplicitInputDevice(def) < 0)
+        return -1;
+
     /* memory hotplug tunables are not supported by this driver */
     if (virDomainDefCheckUnsupportedMemoryHotplug(def) < 0)
         return -1;
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 7cc8639..4824536 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -1829,3 +1829,25 @@ xenFormatConfigCommon(virConfPtr conf,
 
     return 0;
 }
+
+
+int
+xenDomainDefAddImplicitInputDevice(virDomainDefPtr def)
+{
+    virDomainInputBus implicitInputBus = VIR_DOMAIN_INPUT_BUS_XEN;
+
+    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM)
+        implicitInputBus = VIR_DOMAIN_INPUT_BUS_PS2;
+
+    if (virDomainDefMaybeAddInput(def,
+                                  VIR_DOMAIN_INPUT_TYPE_MOUSE,
+                                  implicitInputBus) < 0)
+        return -1;
+
+    if (virDomainDefMaybeAddInput(def,
+                                  VIR_DOMAIN_INPUT_TYPE_KBD,
+                                  implicitInputBus) < 0)
+        return -1;
+
+    return 0;
+}
diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h
index 3d9b03c..9ddf210 100644
--- a/src/xenconfig/xen_common.h
+++ b/src/xenconfig/xen_common.h
@@ -61,4 +61,6 @@ int xenFormatConfigCommon(virConfPtr conf,
                           virConnectPtr conn);
 
 
+int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def);
+
 #endif /* __VIR_XEN_COMMON_H__ */
-- 
2.7.0




More information about the libvir-list mailing list