[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] qemu: forbid user aliases for implicit pci-root controllers



For implicit controllers, we do not format user aliases on the command
line so we provide a translation between the user alias and the qemu
alias.

However for pci-root controllers, the logic determining whether to use
'pci' or 'pci.0' depends on:
* domain architecture
* machine type
* QEMU version (for PPC)

Since we do not store the QEMU version in status XML and as of
commit 5b783379 we no longer have the QEMU_CAPS_PCI_MULTIBUS
capability formatted there either, we can only rely on the alias
of the controller formatted there.

Forbid user aliases for the implicit pci-root controllers so that
we retain this information.

This allows us to drop the call to virQEMUCapsHasPCIMultiBus,
and fixing hotplug after daemon restart, since virQEMUCapsHasPCIMultiBus
relies on qemuCaps->arch which is not filled out by reading
the status XML either.

Partially reverts commit 937f3195 which added the user alias ->
qemu alias mapping for implicit PCI controllers.

https://bugzilla.redhat.com/show_bug.cgi?id=1518148
---

While we cannot reliably map user aliases to qemu aliases in
every corner case, I am not sure where to draw the line.

For x86_64, HasPCIMultiBus could be fixed by using def->os.arch instead
of qemuCaps->arch. So only PPC is unfixable, but this looked like the option
with the least amount of exceptions.

 src/conf/domain_conf.c                             |  3 +--
 src/conf/domain_conf.h                             |  2 ++
 src/qemu/qemu_command.c                            | 16 +++++--------
 src/qemu/qemu_domain.c                             | 26 +++++++++++++++++++++-
 .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml |  4 +---
 5 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 66536653b..d3cd598c8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6623,7 +6623,6 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
 }
 
 
-#define USER_ALIAS_PREFIX "ua-"
 #define USER_ALIAS_CHARS \
     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
 
@@ -6680,7 +6679,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
 
         if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) ||
             (xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_USER_ALIAS &&
-             STRPREFIX(aliasStr, USER_ALIAS_PREFIX) &&
+             STRPREFIX(aliasStr, VIR_DOMAIN_USER_ALIAS_PREFIX) &&
              strspn(aliasStr, USER_ALIAS_CHARS) == strlen(aliasStr)))
             VIR_STEAL_PTR(info->alias, aliasStr);
     }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index dabbff1c2..de34fb163 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -57,6 +57,8 @@
 # include "virtypedparam.h"
 # include "virsavecookie.h"
 
+# define VIR_DOMAIN_USER_ALIAS_PREFIX "ua-"
+
 /* forward declarations of all device types, required by
  * virDomainDeviceDef
  */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0eb591253..89e8b5350 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -321,16 +321,12 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
                 contIsPHB = virDomainControllerIsPSeriesPHB(cont);
                 contTargetIndex = cont->opts.pciopts.targetIndex;
 
-                /* When domain has builtin pci-root controller we don't put it
-                 * onto cmd line. Therefore we can't set its alias. In that
-                 * case, use the default one. */
-                if (!qemuDomainIsPSeries(domainDef) &&
-                    cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
-                    if (virQEMUCapsHasPCIMultiBus(qemuCaps, domainDef))
-                        contAlias = "pci.0";
-                    else
-                        contAlias = "pci";
-                } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
+                /* The builtin pcie-root controller is not put on the command line.
+                 * We cannot set a user alias, but we can reliably predict the one
+                 * qemu will use.
+                 * For builtin pci-root controllers we cannot do that once a domain
+                 * is started, but user aliases are forbidden for those. */
+                if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
                     contAlias = "pcie.0";
                 } else {
                     contAlias = cont->info.alias;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9dab6b24b..ec33f3bad 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3718,6 +3718,27 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
 
 
 static int
+qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
+                                      const virDomainDef *def)
+{
+    /* Forbid user aliases for implicit controllers where
+     * the qemu's device alias depends on the version and therefore
+     * we cannot figure it out for an already running domain. */
+    if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+        !qemuDomainIsPSeries(def) &&
+        controller->info.alias &&
+        STRPREFIX(controller->info.alias, VIR_DOMAIN_USER_ALIAS_PREFIX)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("user aliases are not available for "
+                         "implicit pci-root controller"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
 qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                             const virDomainDef *def,
                             void *opaque ATTRIBUTE_UNUSED)
@@ -3761,11 +3782,14 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
         ret = qemuDomainDeviceDefValidateDisk(dev->data.disk);
         break;
 
+    case VIR_DOMAIN_DEVICE_CONTROLLER:
+        ret = qemuDomainDeviceDefValidateController(dev->data.controller, def);
+        break;
+
     case VIR_DOMAIN_DEVICE_LEASE:
     case VIR_DOMAIN_DEVICE_FS:
     case VIR_DOMAIN_DEVICE_INPUT:
     case VIR_DOMAIN_DEVICE_SOUND:
-    case VIR_DOMAIN_DEVICE_CONTROLLER:
     case VIR_DOMAIN_DEVICE_GRAPHICS:
     case VIR_DOMAIN_DEVICE_HUB:
     case VIR_DOMAIN_DEVICE_MEMBALLOON:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml
index c760098fe..d1cb8fea6 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml
@@ -74,9 +74,7 @@
       <alias name='ua-SomeWeirdController'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
-    <controller type='pci' index='0' model='pci-root'>
-      <alias name='ua-MyPCIRootController'/>
-    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
     <controller type='ide' index='0'>
       <alias name='ua-DoesAnybodyStillUseIDE'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
-- 
2.13.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]