[libvirt] [PATCH] qemu: add default pci-root device to mips*/malta guests
The MIPS Malta board has a root PCI controller. It is not sent during migration, so it needs to be added by default. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- src/qemu/qemu_domain.c | 15 ++++++++++++++- src/qemu/qemu_domain.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 173f82c..75b0545 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1763,6 +1763,14 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX); break; + case VIR_ARCH_MIPS: + case VIR_ARCH_MIPSEL: + case VIR_ARCH_MIPS64: + case VIR_ARCH_MIPS64EL: + if (qemuDomainMachineIsMalta(def)) + addPCIRoot = true; + break; + case VIR_ARCH_PPC64: case VIR_ARCH_PPC64LE: addPCIRoot = true; @@ -4654,6 +4662,11 @@ qemuDomainMachineIsVirt(const virDomainDef *def) STRPREFIX(def->os.machine, "virt-"); } +bool +qemuDomainMachineIsMalta(const virDomainDef *def) +{ + return STRPREFIX(def->os.machine, "malta"); +} static bool qemuCheckMemoryDimmConflict(const virDomainDef *def, @@ -4830,7 +4843,7 @@ bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def) { return qemuDomainMachineIsI440FX(def) || - STREQ(def->os.machine, "malta") || + qemuDomainMachineIsMalta(def) || STREQ(def->os.machine, "sun4u") || STREQ(def->os.machine, "g3beige"); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 95f821c..adba5fa 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -585,6 +585,7 @@ bool qemuDomainMachineIsI440FX(const virDomainDef *def); bool qemuDomainMachineNeedsFDC(const virDomainDef *def); bool qemuDomainMachineIsS390CCW(const virDomainDef *def); bool qemuDomainMachineIsVirt(const virDomainDef *def); +bool qemuDomainMachineIsMalta(const virDomainDef *def); bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def); int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, -- 2.8.1
Thanks for the patch. Can you provide a working mips XML config? We don't have one in the unit test suite. On 05/03/2016 05:23 PM, Aurelien Jarno wrote:
The MIPS Malta board has a root PCI controller. It is not sent during migration, so it needs to be added by default.
I'm a bit confused by the migration reference... does specifying the PCI controller on the command line somehow convince qemu to send the PCI device state during migration? Or am I
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- src/qemu/qemu_domain.c | 15 ++++++++++++++- src/qemu/qemu_domain.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)
The patch looks fine to me but should have some test suite representation, tests/qemuxml2argvtest at least - Cole
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 173f82c..75b0545 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1763,6 +1763,14 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX); break;
+ case VIR_ARCH_MIPS: + case VIR_ARCH_MIPSEL: + case VIR_ARCH_MIPS64: + case VIR_ARCH_MIPS64EL: + if (qemuDomainMachineIsMalta(def)) + addPCIRoot = true; + break; + case VIR_ARCH_PPC64: case VIR_ARCH_PPC64LE: addPCIRoot = true; @@ -4654,6 +4662,11 @@ qemuDomainMachineIsVirt(const virDomainDef *def) STRPREFIX(def->os.machine, "virt-"); }
+bool +qemuDomainMachineIsMalta(const virDomainDef *def) +{ + return STRPREFIX(def->os.machine, "malta"); +}
static bool qemuCheckMemoryDimmConflict(const virDomainDef *def, @@ -4830,7 +4843,7 @@ bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def) { return qemuDomainMachineIsI440FX(def) || - STREQ(def->os.machine, "malta") || + qemuDomainMachineIsMalta(def) || STREQ(def->os.machine, "sun4u") || STREQ(def->os.machine, "g3beige"); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 95f821c..adba5fa 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -585,6 +585,7 @@ bool qemuDomainMachineIsI440FX(const virDomainDef *def); bool qemuDomainMachineNeedsFDC(const virDomainDef *def); bool qemuDomainMachineIsS390CCW(const virDomainDef *def); bool qemuDomainMachineIsVirt(const virDomainDef *def); +bool qemuDomainMachineIsMalta(const virDomainDef *def); bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
On 2016-05-04 09:54, Cole Robinson wrote:
Thanks for the patch.
Can you provide a working mips XML config? We don't have one in the unit test suite.
On 05/03/2016 05:23 PM, Aurelien Jarno wrote:
The MIPS Malta board has a root PCI controller. It is not sent during migration, so it needs to be added by default.
I'm a bit confused by the migration reference... does specifying the PCI controller on the command line somehow convince qemu to send the PCI device state during migration? Or am I
No it doesn't convince it to send the PCI device state. The problem is that the PCI root devices is not created at all on the target side. For what I understood this is due to the code in qemuDomainDefFormatBuf() dropping pci-root controller "for migration compatibility". This can be seen by using virsh managedsave followed by virsh start. The QEMU command line doesn't include the pci root controller, which causes the following error: "error: XML error: No PCI buses available". Indeed looking at the saved image, it is missing from the XML, and this actually not specific to MIPS.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- src/qemu/qemu_domain.c | 15 ++++++++++++++- src/qemu/qemu_domain.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)
The patch looks fine to me but should have some test suite representation, tests/qemuxml2argvtest at least
Ok. I have tried to define a basic xml file similar to the arm-vexpressa9-basic one. Unfortunately when running the testsuite I get the following error: libvirt: Capabilities Utils error : invalid argument: could not find capabilities for arch=mips I guess the capabilities for mips should be defined somewhere, but I fail to see where. Can you please give me a hint? Thanks, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net
On 05/09/2016 01:55 PM, Aurelien Jarno wrote:
On 2016-05-04 09:54, Cole Robinson wrote:
Thanks for the patch.
Can you provide a working mips XML config? We don't have one in the unit test suite.
On 05/03/2016 05:23 PM, Aurelien Jarno wrote:
The MIPS Malta board has a root PCI controller. It is not sent during migration, so it needs to be added by default.
I'm a bit confused by the migration reference... does specifying the PCI controller on the command line somehow convince qemu to send the PCI device state during migration? Or am I
No it doesn't convince it to send the PCI device state. The problem is that the PCI root devices is not created at all on the target side. For what I understood this is due to the code in qemuDomainDefFormatBuf() dropping pci-root controller "for migration compatibility".
This can be seen by using virsh managedsave followed by virsh start. The QEMU command line doesn't include the pci root controller, which causes the following error: "error: XML error: No PCI buses available". Indeed looking at the saved image, it is missing from the XML, and this actually not specific to MIPS.
Hmm. There was another fix in this area recently, but for USB controllers: commit 192a53e07c5fefd9dad2f310886209b76dcc5d83 Author: Shivaprasad G Bhat <shivaprasadbhat@gmail.com> Date: Fri Apr 29 19:31:51 2016 +0530 send default USB controller in xml to destination during migration Maybe we can skip the migration back compat in certain cases as well. For example if mips PCI requires a pci-root controller to be manually specified in the XML (does it? or does libvirt add it by default?), then we probably want to skip that PCI check for mips, and maybe other archs too. Probably skip it in every case except those where libvirt adds a pci-root controller by default
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- src/qemu/qemu_domain.c | 15 ++++++++++++++- src/qemu/qemu_domain.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)
The patch looks fine to me but should have some test suite representation, tests/qemuxml2argvtest at least
Ok. I have tried to define a basic xml file similar to the arm-vexpressa9-basic one. Unfortunately when running the testsuite I get the following error:
libvirt: Capabilities Utils error : invalid argument: could not find capabilities for arch=mips
I guess the capabilities for mips should be defined somewhere, but I fail to see where. Can you please give me a hint?
See for example testQemuAddArmGuest in tests/testutilsqemu.c where this is set up for armv7l Thanks, Cole
participants (2)
-
Aurelien Jarno -
Cole Robinson