8:58 a.m.
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
8:58 a.m.
New subject: [libvirt] [PATCH 1/4] Move USER_ALIAS_PREFIX to the header file
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
9:05 a.m.
New subject: [libvirt] [PATCH 1/4] Move USER_ALIAS_PREFIX to the header file
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
*/
8:58 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: prefer the PCI bus alias from status XML
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
9:06 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: prefer the PCI bus alias from status XML
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.
https://bugzilla.redhat.com/show_bug.cgi?id=1518148
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;
8:58 a.m.
New subject: [libvirt] [PATCH 3/4] virQEMUCapsHasPCIMultiBus: use def->os.arch
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
8:58 a.m.
New subject: [libvirt] [PATCH 4/4] virQEMUCapsHasPCIMultiBus: assume true if we have no version information
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
8:53 a.m.
New subject: [libvirt] [PATCH 4/4] virQEMUCapsHasPCIMultiBus: assume true if we have no version information
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(a)redhat.com>
9:10 a.m.
New subject: [libvirt] [PATCH 4/4] virQEMUCapsHasPCIMultiBus: assume true if we have no version information
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;
9:56 a.m.
New subject: [libvirt] [PATCH 4/4] virQEMUCapsHasPCIMultiBus: assume true if we have no version information
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
2548
days inactive
2549
days old
9 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Pavel Hrdina