[libvirt] [PATCH 0/4] Fix hotplug after daemon restart
The logic in virQEMUCapsHasPCIMultiBus cannot be reasonably fixed for already running domains, since we dropped the MULTIBUS capability, so only resort to calling it if the domain is using user aliases. That way only hotplug on PPC machines other than pseries with QEMU < 2.0 (possibly older) that have a user alias on pci-root controller stays broken. Ján Tomko (4): Move USER_ALIAS_PREFIX to the header file qemu: prefer the PCI bus alias from status XML virQEMUCapsHasPCIMultiBus: use def->os.arch virQEMUCapsHasPCIMultiBus: assume true if we have no version information src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_capabilities.c | 18 +++++----- src/qemu/qemu_capspriv.h | 4 --- src/qemu/qemu_command.c | 42 +++++++++++----------- .../ppc64-modern-bulk-result-live.xml | 2 +- .../ppc64-modern-individual-result-live.xml | 2 +- .../x86-modern-bulk-result-live.xml | 2 +- .../x86-modern-individual-add-result-live.xml | 2 +- .../x86-old-bulk-result-live.xml | 2 +- tests/qemuxml2argvtest.c | 5 --- 11 files changed, 39 insertions(+), 45 deletions(-) -- 2.13.6
Allow other parts of code to decide whether the alias is user-specified or not. --- src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b31917dad..41c94ed18 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6656,7 +6656,6 @@ virDomainDeviceAddressParseXML(xmlNodePtr address, } -#define USER_ALIAS_PREFIX "ua-" #define USER_ALIAS_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-" @@ -6713,7 +6712,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 48b07226e..afba0ede3 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 */ -- 2.13.6
On 11/29/2017 09:58 AM, Ján Tomko wrote:
Allow other parts of code to decide whether the alias is user-specified or not. --- src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-)
Why not a convenience API instead? e.g. bool virDomainDeviceUsingUserAlias(const char *aliasStr) { return STRPREFIX(aliasStr, USER_ALIAS_PREFIX); } Hopefully it never changes or gets added to, but keeping UA- hidden in the .c file perhaps works out better longer term... John BTW: Nit - isn't this series a v2 of the first rejected attempt... https://www.redhat.com/archives/libvir-list/2017-November/msg01139.html
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b31917dad..41c94ed18 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6656,7 +6656,6 @@ virDomainDeviceAddressParseXML(xmlNodePtr address, }
-#define USER_ALIAS_PREFIX "ua-" #define USER_ALIAS_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
@@ -6713,7 +6712,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 48b07226e..afba0ede3 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 */
For some corner cases, virQEMUCapsHasPCIMultiBus depends on the QEMU version, which is by desing not stored in the status XML and therefore it cannot be fixed for all existing running domains. Prefer the controller alias read from the status XML when formatting PCI addresses and only fall back to using virQEMUCapsHasPCIMultiBus if the alias is a user alias. This fixes hotplug after daemon restart for domains not using user aliases. https://bugzilla.redhat.com/show_bug.cgi?id=1518148 --- src/qemu/qemu_command.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c78f1b83b..677cfc995 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -318,29 +318,31 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && cont->idx == info->addr.pci.bus) { + contAlias = cont->info.alias; 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) { - contAlias = "pcie.0"; - } else { - contAlias = cont->info.alias; - if (!contAlias) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Device alias was not set for PCI " - "controller with index %u required " - "for device at address %s"), - info->addr.pci.bus, devStr); - goto cleanup; + if (!contAlias) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Device alias was not set for PCI " + "controller with index %u required " + "for device at address %s"), + info->addr.pci.bus, devStr); + goto cleanup; + } + + if (STRPREFIX(contAlias, VIR_DOMAIN_USER_ALIAS_PREFIX)) { + /* 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) { + contAlias = "pcie.0"; } } break; -- 2.13.6
On 11/29/2017 09:58 AM, Ján Tomko wrote:
For some corner cases, virQEMUCapsHasPCIMultiBus depends on the QEMU version, which is by desing not stored in the status XML and therefore
design
it cannot be fixed for all existing running domains.
Prefer the controller alias read from the status XML when formatting PCI addresses and only fall back to using virQEMUCapsHasPCIMultiBus if the alias is a user alias.
This fixes hotplug after daemon restart for domains not using user aliases.
should this mention a partial revert of 937f3195 ? Or maybe there should be a patch to revert that patch and another to do what's being done here with respect to only taking this "path" when a UA alias is being used. John (Although I see Pavel R-B'd things - I figured I'd drop a couple of thoughts too).
--- src/qemu/qemu_command.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c78f1b83b..677cfc995 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -318,29 +318,31 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && cont->idx == info->addr.pci.bus) { + contAlias = cont->info.alias; 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) { - contAlias = "pcie.0"; - } else { - contAlias = cont->info.alias; - if (!contAlias) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Device alias was not set for PCI " - "controller with index %u required " - "for device at address %s"), - info->addr.pci.bus, devStr); - goto cleanup; + if (!contAlias) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Device alias was not set for PCI " + "controller with index %u required " + "for device at address %s"), + info->addr.pci.bus, devStr); + goto cleanup; + } + + if (STRPREFIX(contAlias, VIR_DOMAIN_USER_ALIAS_PREFIX)) { + /* 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) { + contAlias = "pcie.0"; } } break;
We do not fill out qemuCaps->arch when parsing status XML. Use def->os.arch like we do for PPC. This fixes hotplug after daemon restart for domains that use a user alias for the implicit pci-root on x86. https://bugzilla.redhat.com/show_bug.cgi?id=1518148 --- src/qemu/qemu_capabilities.c | 2 +- tests/qemuhotplugtestcpus/ppc64-modern-bulk-result-live.xml | 2 +- tests/qemuhotplugtestcpus/ppc64-modern-individual-result-live.xml | 2 +- tests/qemuhotplugtestcpus/x86-modern-bulk-result-live.xml | 2 +- tests/qemuhotplugtestcpus/x86-modern-individual-add-result-live.xml | 2 +- tests/qemuhotplugtestcpus/x86-old-bulk-result-live.xml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b5cfa70c5..c9d0a66d8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2416,7 +2416,7 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, { /* x86_64 and i686 support PCI-multibus on all machine types * since forever */ - if (ARCH_IS_X86(qemuCaps->arch)) + if (ARCH_IS_X86(def->os.arch)) return true; if (def->os.arch == VIR_ARCH_PPC || diff --git a/tests/qemuhotplugtestcpus/ppc64-modern-bulk-result-live.xml b/tests/qemuhotplugtestcpus/ppc64-modern-bulk-result-live.xml index 662ed6739..43e626725 100644 --- a/tests/qemuhotplugtestcpus/ppc64-modern-bulk-result-live.xml +++ b/tests/qemuhotplugtestcpus/ppc64-modern-bulk-result-live.xml @@ -56,7 +56,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'> - <alias name='pci'/> + <alias name='pci.0'/> </controller> <input type='mouse' bus='ps2'> <alias name='input0'/> diff --git a/tests/qemuhotplugtestcpus/ppc64-modern-individual-result-live.xml b/tests/qemuhotplugtestcpus/ppc64-modern-individual-result-live.xml index 866a81a2d..0a8d37214 100644 --- a/tests/qemuhotplugtestcpus/ppc64-modern-individual-result-live.xml +++ b/tests/qemuhotplugtestcpus/ppc64-modern-individual-result-live.xml @@ -56,7 +56,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'> - <alias name='pci'/> + <alias name='pci.0'/> </controller> <input type='mouse' bus='ps2'> <alias name='input0'/> diff --git a/tests/qemuhotplugtestcpus/x86-modern-bulk-result-live.xml b/tests/qemuhotplugtestcpus/x86-modern-bulk-result-live.xml index a025033e8..fe9a81a09 100644 --- a/tests/qemuhotplugtestcpus/x86-modern-bulk-result-live.xml +++ b/tests/qemuhotplugtestcpus/x86-modern-bulk-result-live.xml @@ -32,7 +32,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'> - <alias name='pci'/> + <alias name='pci.0'/> </controller> <input type='mouse' bus='ps2'> <alias name='input0'/> diff --git a/tests/qemuhotplugtestcpus/x86-modern-individual-add-result-live.xml b/tests/qemuhotplugtestcpus/x86-modern-individual-add-result-live.xml index 8adcb7104..12b28be5f 100644 --- a/tests/qemuhotplugtestcpus/x86-modern-individual-add-result-live.xml +++ b/tests/qemuhotplugtestcpus/x86-modern-individual-add-result-live.xml @@ -32,7 +32,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'> - <alias name='pci'/> + <alias name='pci.0'/> </controller> <input type='mouse' bus='ps2'> <alias name='input0'/> diff --git a/tests/qemuhotplugtestcpus/x86-old-bulk-result-live.xml b/tests/qemuhotplugtestcpus/x86-old-bulk-result-live.xml index b52f049fb..6f50bb5c1 100644 --- a/tests/qemuhotplugtestcpus/x86-old-bulk-result-live.xml +++ b/tests/qemuhotplugtestcpus/x86-old-bulk-result-live.xml @@ -22,7 +22,7 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'> - <alias name='pci'/> + <alias name='pci.0'/> </controller> <input type='mouse' bus='ps2'> <alias name='input0'/> -- 2.13.6
In status XML, we do not store the QEMU version information, we only format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS in commit 5b783379 which was released in libvirt 3.2.0. Therefore the only way of telling if the already running domain at the time of daemon restart has been started with a QEMU that does use 'pci.0' or not on PPC is to look at the pci-root controller's alias. This is not an option if the domain has a user-specified alias for the pci-root. Instead of reintroducing the capability, assume 'pci.0' when we have no version information. That way the only left broken use case would be the combination of user aliases and very old QEMU. https://bugzilla.redhat.com/show_bug.cgi?id=1518148 --- src/qemu/qemu_capabilities.c | 16 ++++++++-------- src/qemu/qemu_capspriv.h | 4 ---- tests/qemuxml2argvtest.c | 5 ----- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c9d0a66d8..7b52d1a71 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2436,6 +2436,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, * ppce500: 1.6.0 */ + /* We do not store the qemu version in domain status XML. + * Hope the user is using a QEMU new enough to use 'pci.0', + * otherwise the results of this function will be wrong + * for domains already running at the time of daemon + * restart */ + if (qemuCaps->version == 0) + return true; + if (qemuCaps->version >= 2000000) return true; @@ -2487,14 +2495,6 @@ virArch virQEMUCapsGetArch(virQEMUCapsPtr qemuCaps) } -void -virQEMUCapsSetVersion(virQEMUCapsPtr qemuCaps, - unsigned int version) -{ - qemuCaps->version = version; -} - - unsigned int virQEMUCapsGetVersion(virQEMUCapsPtr qemuCaps) { return qemuCaps->version; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index f23995ec6..219daa362 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -56,10 +56,6 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps, virArch arch); void -virQEMUCapsSetVersion(virQEMUCapsPtr qemuCaps, - unsigned int version); - -void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virArch hostArch, virDomainVirtType type); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d3ac569e2..385a54615 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -384,11 +384,6 @@ testUpdateQEMUCaps(const struct testInfo *info, virQEMUCapsInitQMPBasicArch(info->qemuCaps); - /* We need to pretend QEMU 2.0.0 is in use so that pSeries guests - * will get the correct alias assigned to their buses. - * See virQEMUCapsHasPCIMultiBus() */ - virQEMUCapsSetVersion(info->qemuCaps, 2000000); - if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0) goto cleanup; -- 2.13.6
On Wed, Nov 29, 2017 at 03:58:57PM +0100, Ján Tomko wrote:
In status XML, we do not store the QEMU version information, we only format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS in commit 5b783379 which was released in libvirt 3.2.0.
Therefore the only way of telling if the already running domain at the time of daemon restart has been started with a QEMU that does use 'pci.0' or not on PPC is to look at the pci-root controller's alias. This is not an option if the domain has a user-specified alias for the pci-root.
Instead of reintroducing the capability, assume 'pci.0' when we have no version information. That way the only left broken use case would be the combination of user aliases and very old QEMU.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148 --- src/qemu/qemu_capabilities.c | 16 ++++++++-------- src/qemu/qemu_capspriv.h | 4 ---- tests/qemuxml2argvtest.c | 5 ----- 3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c9d0a66d8..7b52d1a71 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2436,6 +2436,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, * ppce500: 1.6.0 */
+ /* We do not store the qemu version in domain status XML. + * Hope the user is using a QEMU new enough to use 'pci.0', + * otherwise the results of this function will be wrong + * for domains already running at the time of daemon + * restart */ + if (qemuCaps->version == 0) + return true; +
I don't like this that we will hope that QEMU is new enough, but I cannot think of a better way how to check it and this definitely improves current code. For the whole series: Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
On 11/29/2017 09:58 AM, Ján Tomko wrote:
In status XML, we do not store the QEMU version information, we only format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS in commit 5b783379 which was released in libvirt 3.2.0.
Therefore the only way of telling if the already running domain at the time of daemon restart has been started with a QEMU that does use 'pci.0' or not on PPC is to look at the pci-root controller's alias. This is not an option if the domain has a user-specified alias for the pci-root.
Instead of reintroducing the capability, assume 'pci.0' when we have no version information. That way the only left broken use case would be the combination of user aliases and very old QEMU.
Does this only matter for user aliases? I'm not totally clear on the "scope" of the duration of the problem... Would this be essentially a partial revert of 3a37af1e4 ? John
https://bugzilla.redhat.com/show_bug.cgi?id=1518148 --- src/qemu/qemu_capabilities.c | 16 ++++++++-------- src/qemu/qemu_capspriv.h | 4 ---- tests/qemuxml2argvtest.c | 5 ----- 3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c9d0a66d8..7b52d1a71 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2436,6 +2436,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, * ppce500: 1.6.0 */
+ /* We do not store the qemu version in domain status XML. + * Hope the user is using a QEMU new enough to use 'pci.0', + * otherwise the results of this function will be wrong + * for domains already running at the time of daemon + * restart */ + if (qemuCaps->version == 0) + return true; + if (qemuCaps->version >= 2000000) return true;
@@ -2487,14 +2495,6 @@ virArch virQEMUCapsGetArch(virQEMUCapsPtr qemuCaps) }
-void -virQEMUCapsSetVersion(virQEMUCapsPtr qemuCaps, - unsigned int version) -{ - qemuCaps->version = version; -} - - unsigned int virQEMUCapsGetVersion(virQEMUCapsPtr qemuCaps) { return qemuCaps->version; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index f23995ec6..219daa362 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -56,10 +56,6 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps, virArch arch);
void -virQEMUCapsSetVersion(virQEMUCapsPtr qemuCaps, - unsigned int version); - -void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virArch hostArch, virDomainVirtType type); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d3ac569e2..385a54615 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -384,11 +384,6 @@ testUpdateQEMUCaps(const struct testInfo *info,
virQEMUCapsInitQMPBasicArch(info->qemuCaps);
- /* We need to pretend QEMU 2.0.0 is in use so that pSeries guests - * will get the correct alias assigned to their buses. - * See virQEMUCapsHasPCIMultiBus() */ - virQEMUCapsSetVersion(info->qemuCaps, 2000000); - if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0) goto cleanup;
On Thu, Nov 30, 2017 at 10:10:26AM -0500, John Ferlan wrote:
On 11/29/2017 09:58 AM, Ján Tomko wrote:
In status XML, we do not store the QEMU version information, we only format all the capabilities. We dropped QEMU_CAPS_PCI_MULTIBUS in commit 5b783379 which was released in libvirt 3.2.0.
Therefore the only way of telling if the already running domain at the time of daemon restart has been started with a QEMU that does use 'pci.0' or not on PPC is to look at the pci-root controller's alias. This is not an option if the domain has a user-specified alias for the pci-root.
Instead of reintroducing the capability, assume 'pci.0' when we have no version information. That way the only left broken use case would be the combination of user aliases and very old QEMU.
Does this only matter for user aliases? I'm not totally clear on the "scope" of the duration of the problem...
After patch 2/4 we only need to use virQEMUCapsHasPCIMultiBus for controllers if the alias we read from XML is a user-specified alias, after patch 3/4 virQEMUCapsHasPCIMultiBus is fixed to work on x86 after daemon restart too and after this patch, hotplug on the implicit pci bus will not work on ppc if these conditions are met: * the pci-root controller is using a user alias * the daemon has been restarted while the domain was running so virQEMUCapsHasPCIMultiBus can no longer access the qemu version * the QEMU is old enough to use 'pci' instead of 'pci.0'
Would this be essentially a partial revert of 3a37af1e4 ?
John
I have added the 'partial revert' comments, a helper as proposed for patch 1/4 and pushed the series Jan
participants (3)
-
John Ferlan -
Ján Tomko -
Pavel Hrdina