[PATCH v2 15/29] introduce virDomainControllerIsPowerNVPHB
Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 23 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 803c51d63a..13d5eb5b9d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2442,6 +2442,27 @@ virDomainControllerIsPSeriesPHB(const virDomainControllerDef *cont) } +bool +virDomainControllerIsPowerNVPHB(const virDomainControllerDef *cont) +{ +virDomainControllerPCIModelName name; + +/* PowerNV PHBs are pcie-root controllers */ +if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI || +cont->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { +return false; +} + +name = cont->opts.pciopts.modelName; + +/* The actual device used for PHBs is pnv-phb3 */ +if (name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3) +return false; + +return true; +} + + virDomainFSDef * virDomainFSDefNew(virDomainXMLOption *xmlopt) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 93c5092f35..adeafa83e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -,6 +,7 @@ virDomainControllerDef *virDomainControllerDefNew(virDomainControllerType type); void virDomainControllerDefFree(virDomainControllerDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainControllerDef, virDomainControllerDefFree); bool virDomainControllerIsPSeriesPHB(const virDomainControllerDef *cont); +bool virDomainControllerIsPowerNVPHB(const virDomainControllerDef *cont); virDomainFSDef *virDomainFSDefNew(virDomainXMLOption *xmlopt); void virDomainFSDefFree(virDomainFSDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ba3462d849..a18fbd4bb8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -287,6 +287,7 @@ virDomainControllerFindByType; virDomainControllerFindUnusedIndex; virDomainControllerInsert; virDomainControllerInsertPreAlloced; +virDomainControllerIsPowerNVPHB; virDomainControllerIsPSeriesPHB; virDomainControllerModelIDETypeFromString; virDomainControllerModelIDETypeToString; -- 2.34.1
[PATCH v2 19/29] qemu_domain_address.c: change pnv-phb3 minimal downstream slot
The PowerNV PHB3 bus has minimal slot zero. In fact, at this moment, the root complex accepts only a single device, at slot 0x0, due to firmware limitations. The single device restriction is subject to change in upstream QEMU and it's not worth adding this limitation to Libvirt. However, the minimal slot presents a problem. When setting a pnv-phb3-root-port address with slot=0x0, Libvirt changes it to 0x1. This happens because the pnv-phb3 controller is a PCIE_ROOT model, and this model is being set with 'bus->minSlot=1' in domain_addr.c, virDomainPCIAddressBusSetModel(). This means that the root-port is launched with 'addr=0x1' in the QEMU command line and it's not usable by the domain. It is not worth to create a new controller model, replicating all the already existing logic for PCIE_ROOT controllers, just to have a similar PCIE_ROOT bus with minSlots=0. Changing the existing PCIE_ROOT min slot to 0 doesn't make sense either - we would change existing behavior of existing devices. This patch works around this situation by adding a verification in qemuDomainPCIAddressSetCreate() to change the minBus values of the pnv-phb3 devices right before virDomainDeviceInfoIterate(). This is enough to allow for a root port to be added in slot 0 of a pnv-phb3 bus while not being intrusive with existing devices. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain_address.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e7a9e6adda..7d60d277e8 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1628,6 +1628,17 @@ qemuDomainCollectPCIAddressExtension(virDomainDef *def G_GNUC_UNUSED, return virDomainPCIAddressExtensionReserveAddr(addrs, addr); } +static void +qemuDomainTunePowerNVPhbBuses(virDomainPCIAddressSet *addrs) +{ +size_t i; + +for (i = 0; i < addrs->nbuses; i++) { +if (addrs->buses[i].model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) +addrs->buses[i].minSlot = 0; +} +} + static virDomainPCIAddressSet * qemuDomainPCIAddressSetCreate(virDomainDef *def, virQEMUCaps *qemuCaps, @@ -1720,6 +1731,9 @@ qemuDomainPCIAddressSetCreate(virDomainDef *def, virDomainControllerModelPCITypeToString(defaultModel), i); } +if (qemuDomainIsPowerNV(def)) +qemuDomainTunePowerNVPhbBuses(addrs); + if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) goto error; -- 2.34.1
[PATCH v2 26/29] domain_conf.c: add phb4-root-port to IsPowerNVRootPort()
Make the virDomainControllerIsPowerNVRootPort() helper recognize pnv-phb4-root-port as a PowerNV root port. This will spare us from duplicating checks where the constraints of pnv-phb3-root-port also applies for the pnv-phb4-root-port device. Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2b862ffb86..0305c913d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2476,7 +2476,8 @@ virDomainControllerIsPowerNVRootPort(const virDomainControllerDef *cont) name = cont->opts.pciopts.modelName; -if (name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT) +if ((name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT) && +(name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT)) return false; return true; -- 2.34.1
[PATCH v2 29/29] tests: add PowerNV9 tests
Since the nuances of PowerNV PHBs and root ports were already handled when adding support for pnv-phb3* devices, we're already set to support PowerNV9 PHBs and root ports as well. Signed-off-by: Daniel Henrique Barboza --- .../powernv9-dupPHBs.ppc64-latest.err | 1 + tests/qemuxml2argvdata/powernv9-dupPHBs.xml | 27 + .../powernv9-root-port.ppc64-latest.args | 35 + tests/qemuxml2argvdata/powernv9-root-port.xml | 17 tests/qemuxml2argvtest.c | 2 + .../powernv9-root-port.ppc64-latest.xml | 39 +++ .../qemuxml2xmloutdata/powernv9-root-port.xml | 36 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 158 insertions(+) create mode 100644 tests/qemuxml2argvdata/powernv9-dupPHBs.ppc64-latest.err create mode 100644 tests/qemuxml2argvdata/powernv9-dupPHBs.xml create mode 100644 tests/qemuxml2argvdata/powernv9-root-port.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/powernv9-root-port.xml create mode 100644 tests/qemuxml2xmloutdata/powernv9-root-port.ppc64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/powernv9-root-port.xml diff --git a/tests/qemuxml2argvdata/powernv9-dupPHBs.ppc64-latest.err b/tests/qemuxml2argvdata/powernv9-dupPHBs.ppc64-latest.err new file mode 100644 index 00..bc5879640d --- /dev/null +++ b/tests/qemuxml2argvdata/powernv9-dupPHBs.ppc64-latest.err @@ -0,0 +1 @@ +XML error: Multiple pnv-phb controllers with same chip-id and index diff --git a/tests/qemuxml2argvdata/powernv9-dupPHBs.xml b/tests/qemuxml2argvdata/powernv9-dupPHBs.xml new file mode 100644 index 00..49af69e6c4 --- /dev/null +++ b/tests/qemuxml2argvdata/powernv9-dupPHBs.xml @@ -0,0 +1,27 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 1 + +hvm + + +/usr/bin/qemu-system-ppc64 + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvdata/powernv9-root-port.ppc64-latest.args b/tests/qemuxml2argvdata/powernv9-root-port.ppc64-latest.args new file mode 100644 index 00..007f425273 --- /dev/null +++ b/tests/qemuxml2argvdata/powernv9-root-port.ppc64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine powernv9,usb=off,dump-guest-core=off,memory-backend=pnv.ram \ +-accel tcg \ +-cpu POWER9 \ +-m 2048 \ +-object '{"qom-type":"memory-backend-ram","id":"pnv.ram","size":2147483648}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid b20fcfe3-4a0a-4039-8735-9e024256e0f7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pnv-phb4","index":0,"chip-id":0,"id":"pcie.0"}' \ +-device '{"driver":"pnv-phb4-root-port","port":0,"chassis":1,"id":"pci.1","bus":"pcie.0","addr":"0x0"}' \ +-usb \ +-chardev pty,id=charserial0 \ +-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/powernv9-root-port.xml b/tests/qemuxml2argvdata/powernv9-root-port.xml new file mode 100644 index 00..dfd74446f9 --- /dev/null +++ b/tests/qemuxml2argvdata/powernv9-root-port.xml @@ -0,0 +1,17 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 1 + +hvm + + +/usr/bin/qemu-system-ppc64 + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1ca0fd4633..ee831b81b0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2246,6 +2246,8 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("powernv8-root-port", "ppc64"); DO_TEST_CAPS_ARCH_LATEST("powernv8-two-sockets", "ppc64"); DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("powernv8-dupPHBs", "ppc64"); +DO_TEST_CAPS_ARCH_LATEST("powernv9-root-port", "ppc64"); +DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("powernv9-dupPHBs", "ppc64"); DO_TEST("pseries-basic", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/powernv9-root-port.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv9-root-port.ppc64-latest.xml new file mode 100644 index 00..135489cf6e --- /dev/null +++ b/tests/qemuxml2xmloutdata/powernv9-root-port.ppc64-latest.xml @@ -0,0 +1,39 @@ + + QEMUGuest1 +
[PATCH v2 27/29] conf, qemu: add 'pnv-phb4' controller model name
Similar to the existing pnv-phb3 device, pnv-phb4 is also an implementation of pcie-root. No user doc is needed in this case since the user doesn't ideally set PCI model names manually. Signed-off-by: Daniel Henrique Barboza --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain_address.c | 14 +- src/qemu/qemu_validate.c | 2 ++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 116fedc19e..dce2548825 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2599,6 +2599,7 @@ i82801b11-bridge pnv-phb3 +pnv-phb4 pcie-pci-bridge diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0305c913d9..585e7d9dae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -441,6 +441,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "pnv-phb3-root-port", "pnv-phb3", "pnv-phb4-root-port", + "pnv-phb4", ); VIR_ENUM_IMPL(virDomainControllerModelSCSI, @@ -2456,7 +2457,6 @@ virDomainControllerIsPowerNVPHB(const virDomainControllerDef *cont) name = cont->opts.pciopts.modelName; -/* The actual device used for PHBs is pnv-phb3 */ if (name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3) return false; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6532b78322..2755a19e2d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -649,6 +649,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT, +VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e18cc81f04..615182cb85 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2424,6 +2424,18 @@ virDomainControllerGetPowerNVRootPortName(virDomainDef *def) } +static virDomainControllerPCIModelName +virDomainControllerGetPowerNVPHBName(virDomainDef *def) +{ +if (STREQ(def->os.machine, "powernv8")) +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3; +else if (STREQ(def->os.machine, "powernv9")) +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4; + +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE; +} + + static void qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, virDomainDef *def, @@ -2476,7 +2488,7 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: if (qemuDomainIsPowerNV(def)) -*modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3; +*modelName = virDomainControllerGetPowerNVPHBName(def); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 6f49a5f17b..fb907a3109 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3434,6 +3434,8 @@ virValidateControllerPCIModelNameToQEMUCaps(int modelName) return QEMU_CAPS_DEVICE_PNV_PHB3; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT: return QEMU_CAPS_DEVICE_PNV_PHB4; +case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4: +return QEMU_CAPS_DEVICE_PNV_PHB4; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE: return 0; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST: -- 2.34.1
[PATCH v2 21/29] tests: add pnv-phb3-root-port test
Signed-off-by: Daniel Henrique Barboza --- .../powernv8-root-port.ppc64-latest.args | 35 + tests/qemuxml2argvdata/powernv8-root-port.xml | 17 tests/qemuxml2argvtest.c | 1 + .../powernv8-root-port.ppc64-latest.xml | 39 +++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 93 insertions(+) create mode 100644 tests/qemuxml2argvdata/powernv8-root-port.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/powernv8-root-port.xml create mode 100644 tests/qemuxml2xmloutdata/powernv8-root-port.ppc64-latest.xml diff --git a/tests/qemuxml2argvdata/powernv8-root-port.ppc64-latest.args b/tests/qemuxml2argvdata/powernv8-root-port.ppc64-latest.args new file mode 100644 index 00..b32e49c54e --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-root-port.ppc64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine powernv8,usb=off,dump-guest-core=off,memory-backend=pnv.ram \ +-accel tcg \ +-cpu POWER8 \ +-m 2048 \ +-object '{"qom-type":"memory-backend-ram","id":"pnv.ram","size":2147483648}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid b20fcfe3-4a0a-4039-8735-9e024256e0f7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pnv-phb3","index":0,"chip-id":0,"id":"pcie.0"}' \ +-device '{"driver":"pnv-phb3-root-port","port":0,"chassis":1,"id":"pci.1","bus":"pcie.0","addr":"0x0"}' \ +-usb \ +-chardev pty,id=charserial0 \ +-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/powernv8-root-port.xml b/tests/qemuxml2argvdata/powernv8-root-port.xml new file mode 100644 index 00..1acb37222d --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-root-port.xml @@ -0,0 +1,17 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 1 + +hvm + + +/usr/bin/qemu-system-ppc64 + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index de5582a52c..70b57a239f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2243,6 +2243,7 @@ mymain(void) DO_TEST_PARSE_ERROR_NOCAPS("seclabel-device-duplicates"); DO_TEST_CAPS_ARCH_LATEST("powernv8-basic", "ppc64"); +DO_TEST_CAPS_ARCH_LATEST("powernv8-root-port", "ppc64"); DO_TEST("pseries-basic", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/powernv8-root-port.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv8-root-port.ppc64-latest.xml new file mode 100644 index 00..a24154c47f --- /dev/null +++ b/tests/qemuxml2xmloutdata/powernv8-root-port.ppc64-latest.xml @@ -0,0 +1,39 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 2097152 + 1 + +hvm + + + +POWER8 + + + destroy + restart + destroy + +/usr/bin/qemu-system-ppc64 + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1815d9ac1d..596907a25e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -663,6 +663,7 @@ mymain(void) DO_TEST_CAPS_LATEST("virtio-rng-builtin"); DO_TEST_CAPS_ARCH_LATEST("powernv8-basic", "ppc64"); +DO_TEST_CAPS_ARCH_LATEST("powernv8-root-port", "ppc64"); DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, -- 2.34.1
[PATCH v2 22/29] domain_validate.c: allow targetIndex 0 out of idx 0 for PowerNV PHBs
PowerNV PHBs uses the 'targetIndex' attribute in a different manner than pSeries PHBs, having no restiction of targetIndex = 0 in controllers with non-zero indexes. This can happen when the domain has 2 or more sockets and the pnv-phb3 controller can have targetIndex=0 in a different chip-id. Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_validate.c| 5 ++- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_validate.c | 5 +++ .../powernv8-two-sockets.ppc64-latest.args| 35 + .../qemuxml2argvdata/powernv8-two-sockets.xml | 26 + tests/qemuxml2argvtest.c | 1 + .../powernv8-two-sockets.ppc64-latest.xml | 39 +++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/powernv8-two-sockets.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/powernv8-two-sockets.xml create mode 100644 tests/qemuxml2xmloutdata/powernv8-two-sockets.ppc64-latest.xml diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index e9baf1d41a..5510cb073b 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1055,8 +1055,9 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) return -1; } -if ((controller->idx == 0 && opts->targetIndex != 0) || -(controller->idx != 0 && opts->targetIndex == 0)) { +if (!virDomainControllerIsPowerNVPHB(controller) && +((controller->idx == 0 && opts->targetIndex != 0) || + (controller->idx != 0 && opts->targetIndex == 0))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only the PCI controller with index 0 can " "have target index 0, and vice versa")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 89dbc20eee..b58f35390d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5092,7 +5092,7 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont, /* pSeries guests can have multiple pci-root controllers, * but other machine types only support a single one */ -if (!qemuDomainIsPSeries(def) && +if (!qemuDomainIsPowerPC(def) && (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && cont->idx != 0) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 765a1d5811..bb6ff2e70e 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3696,6 +3696,11 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, break; } +/* PowerNV domains, like pSeries guest, can also have + * multiple PHBs. */ +if (virDomainControllerIsPowerNVPHB(cont)) +break; + /* For all other pci-root and pcie-root controllers, though, * the index must be zero */ if (cont->idx != 0) { diff --git a/tests/qemuxml2argvdata/powernv8-two-sockets.ppc64-latest.args b/tests/qemuxml2argvdata/powernv8-two-sockets.ppc64-latest.args new file mode 100644 index 00..67f0611d79 --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-two-sockets.ppc64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine powernv8,usb=off,dump-guest-core=off,memory-backend=pnv.ram \ +-accel tcg \ +-cpu POWER8 \ +-m 2048 \ +-object '{"qom-type":"memory-backend-ram","id":"pnv.ram","size":2147483648}' \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,dies=1,cores=1,threads=1 \ +-uuid b20fcfe3-4a0a-4039-8735-9e024256e0f7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pnv-phb3","index":0,"chip-id":0,"id":"pcie.0"}' \ +-device '{"driver":"pnv-phb3","index":0,"chip-id":1,"id":"pcie.1"}' \ +-usb \ +-chardev pty,id=charserial0 \ +-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/powernv8-two-sockets.xml b/tests/qemuxml2argvdata/powernv8-two-sockets.xml new file mode 100644 index 00..c6f2024a33
[PATCH v2 23/29] domain_conf.c: reject duplicated pnv-phb3 devices
These devices must have unique targetIndex/chip-id pairs. Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c| 35 +++ tests/qemuxml2argvdata/powernv8-dupPHBs.err | 1 + .../powernv8-dupPHBs.ppc64-latest.err | 1 + tests/qemuxml2argvdata/powernv8-dupPHBs.xml | 27 ++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 65 insertions(+) create mode 100644 tests/qemuxml2argvdata/powernv8-dupPHBs.err create mode 100644 tests/qemuxml2argvdata/powernv8-dupPHBs.ppc64-latest.err create mode 100644 tests/qemuxml2argvdata/powernv8-dupPHBs.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 026d801682..d27aafd771 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2482,6 +2482,33 @@ virDomainControllerIsPowerNVRootPort(const virDomainControllerDef *cont) } +/* Caller must ensure that 'cont' is a PowerNV PHB device */ +static bool +virDomainControllerDuplicatedPHB(virDomainDef *def, + virDomainControllerDef *cont) +{ +int chipId = cont->opts.pciopts.chipId; +int targetIndex = cont->opts.pciopts.targetIndex; +size_t i; + +for (i = 0; i < def->ncontrollers; i++) { +virDomainControllerDef *cont2 = def->controllers[i]; + +if (!virDomainControllerIsPowerNVPHB(cont2)) +continue; + +if (cont2 == cont) +continue; + +if (cont2->opts.pciopts.chipId == chipId && +cont2->opts.pciopts.targetIndex == targetIndex) +return true; +} + +return false; +} + + virDomainFSDef * virDomainFSDefNew(virDomainXMLOption *xmlopt) { @@ -4738,6 +4765,14 @@ virDomainDefRejectDuplicateControllers(virDomainDef *def) cont->idx); goto cleanup; } + +if (virDomainControllerIsPowerNVPHB(cont) && +virDomainControllerDuplicatedPHB(def, cont)) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multiple pnv-phb controllers with same chip-id and index")); +goto cleanup; +} + ignore_value(virBitmapSetBit(bitmaps[cont->type], cont->idx)); } diff --git a/tests/qemuxml2argvdata/powernv8-dupPHBs.err b/tests/qemuxml2argvdata/powernv8-dupPHBs.err new file mode 100644 index 00..bc5879640d --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-dupPHBs.err @@ -0,0 +1 @@ +XML error: Multiple pnv-phb controllers with same chip-id and index diff --git a/tests/qemuxml2argvdata/powernv8-dupPHBs.ppc64-latest.err b/tests/qemuxml2argvdata/powernv8-dupPHBs.ppc64-latest.err new file mode 100644 index 00..bc5879640d --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-dupPHBs.ppc64-latest.err @@ -0,0 +1 @@ +XML error: Multiple pnv-phb controllers with same chip-id and index diff --git a/tests/qemuxml2argvdata/powernv8-dupPHBs.xml b/tests/qemuxml2argvdata/powernv8-dupPHBs.xml new file mode 100644 index 00..43ee3051eb --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-dupPHBs.xml @@ -0,0 +1,27 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 1 + +hvm + + +/usr/bin/qemu-system-ppc64 + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 313d082d46..1ca0fd4633 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2245,6 +2245,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("powernv8-basic", "ppc64"); DO_TEST_CAPS_ARCH_LATEST("powernv8-root-port", "ppc64"); DO_TEST_CAPS_ARCH_LATEST("powernv8-two-sockets", "ppc64"); +DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("powernv8-dupPHBs", "ppc64"); DO_TEST("pseries-basic", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, -- 2.34.1
[PATCH v2 24/29] qemu: introduce QEMU_CAPS_DEVICE_PNV_PHB4
This capability enables two devices: - pnv-phb4 device, the pcie-root controller for PowerNV9 domains - pnv-phb4-root-port, the pcie-root-port model that is used with the pnv-phb4 device. As with the pnv-phb3 devices, these are also user creatable only after QEMU 6.2.0 but the capability is available since 5.0.0, meaning that we need to check QEMU version and arch manually before setting it. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_capabilities.c| 7 ++- src/qemu/qemu_capabilities.h| 1 + tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d60240912c..668579dbcc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -663,6 +663,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "device.json+hotplug", /* QEMU_CAPS_DEVICE_JSON */ "hvf", /* QEMU_CAPS_HVF */ "pnv-phb3", /* QEMU_CAPS_DEVICE_PNV_PHB3 */ + "pnv-phb4", /* QEMU_CAPS_DEVICE_PNV_PHB4 */ ); @@ -1403,6 +1404,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { * * { "pnv-phb3", QEMU_CAPS_DEVICE_PNV_PHB3 }, * { "pnv-phb3-root-port", QEMU_CAPS_DEVICE_PNV_PHB3 }, + * { "pnv-phb4", QEMU_CAPS_DEVICE_PNV_PHB4 }, + * { "pnv-phb4-root-port", QEMU_CAPS_DEVICE_PNV_PHB4 }, * * Because they are present in QEMU binaries since QEMU 5.0.0 * but became user creatable only in the QEMU 7.0.0 development @@ -5248,8 +5251,10 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCaps *qemuCaps) * QEMU 6.2.0. The version value set here was taken from a * binary post 6.2.0 release that has user creatable pnv-phb * support. */ -if (qemuCaps->version >= 6002050 && ARCH_IS_PPC64(qemuCaps->arch)) +if (qemuCaps->version >= 6002050 && ARCH_IS_PPC64(qemuCaps->arch)) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_PNV_PHB3); +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_PNV_PHB4); +} } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ed3973c0ba..f69155f863 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -638,6 +638,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON (and works with hot-unplug) */ QEMU_CAPS_HVF, /* Whether Hypervisor.framework is available */ QEMU_CAPS_DEVICE_PNV_PHB3, /* devices pnv-phb3 and pnv-phb3-root-port */ +QEMU_CAPS_DEVICE_PNV_PHB4, /* devices pnv-phb4 and pnv-phb4-root-port */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index cb6c146a28..9833c5548e 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -201,6 +201,7 @@ + 6002050 0 42900243 -- 2.34.1
[PATCH v2 17/29] conf, qemu: add default 'targetIndex' value for pnv-phb3 devs
As done with the 'chip-id' attribute, use zero as a default targetIndex value for pnv-phb3 devices in case it's absent from the controller definition. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain_address.c| 2 ++ src/qemu/qemu_validate.c | 19 ++- .../powernv8-basic.ppc64-latest.xml | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 17c2649fb1..e7a9e6adda 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2858,6 +2858,8 @@ qemuDomainAssignPCIAddresses(virDomainDef *def, if (options->chipId == -1) options->chipId = 0; +if (options->targetIndex == -1) +options->targetIndex = 0; break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index bce52269a6..765a1d5811 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3732,6 +3732,24 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, } break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: +/* PHBs for PowerNV domains must have a targetIndex */ +if (pciopts->targetIndex == -1 && +virDomainControllerIsPowerNVPHB(cont)) { +virReportControllerMissingOption(cont, model, modelName, "targetIndex"); +return -1; +} + +/* + * targetIndex for pcie-root controllers only applies to + * PowerNV PHBs. + */ +if (pciopts->targetIndex != -1 && +!virDomainControllerIsPowerNVPHB(cont)) { +virReportControllerInvalidOption(cont, model, modelName, "targetIndex"); +return -1; +} +break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: @@ -3739,7 +3757,6 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: -case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE: if (pciopts->targetIndex != -1) { virReportControllerInvalidOption(cont, model, modelName, "targetIndex"); diff --git a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml index 28d86d7d9e..bd22d85f6a 100644 --- a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml +++ b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml @@ -19,7 +19,7 @@ /usr/bin/qemu-system-ppc64 - + -- 2.34.1
[PATCH v2 20/29] domain_conf: format pnv-phb3-root-port empty addr
Hiding the empty (:00:0.0) PCI address in the case of devices that will connect to slot 0x0 can be counterintuitive to the user, which will see something like this: Even if the user deliberately adds the root-port element: We end up removing the element after saving the domain file. This happens because virPCIDeviceAddressIsEmpty() can't distinguish between a zeroed address that was set by the user versus an address that wasn't filled at all. Given that all root-ports of PowerNV domains will connect to slot 0 of the PHB, if the PHB controller has index = 0 this scenario will occur every time. This patch aims to alleaviate this behavior by adding a new virDomainDefFormatFlags that will allow an empty address to be formatted in the XML. This flag is then used only when formatting PowerNV root ports. Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c | 25 - src/conf/domain_conf.h | 2 ++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 13d5eb5b9d..026d801682 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2463,6 +2463,25 @@ virDomainControllerIsPowerNVPHB(const virDomainControllerDef *cont) } +static bool +virDomainControllerIsPowerNVRootPort(const virDomainControllerDef *cont) +{ +virDomainControllerPCIModelName name; + +if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI || +cont->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT) { +return false; +} + +name = cont->opts.pciopts.modelName; + +if (name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT) +return false; + +return true; +} + + virDomainFSDef * virDomainFSDefNew(virDomainXMLOption *xmlopt) { @@ -6490,7 +6509,8 @@ virDomainDeviceInfoFormat(virBuffer *buf, switch ((virDomainDeviceAddressType) info->type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: -if (!virPCIDeviceAddressIsEmpty(>addr.pci)) { +if (!virPCIDeviceAddressIsEmpty(>addr.pci) || +flags & VIR_DOMAIN_DEF_FORMAT_EMPTY_PCI_ADDR) { virBufferAsprintf(, " domain='0x%04x' bus='0x%02x' " "slot='0x%02x' function='0x%d'", info->addr.pci.domain, @@ -23958,6 +23978,9 @@ virDomainControllerDefFormat(virBuffer *buf, } } +if (virDomainControllerIsPowerNVRootPort(def)) +flags |= VIR_DOMAIN_DEF_FORMAT_EMPTY_PCI_ADDR; + virDomainControllerDriverFormat(, def); virDomainDeviceInfoFormat(, >info, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index adeafa83e7..dd3d942a35 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3502,6 +3502,8 @@ typedef enum { VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM = 1 << 6, VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT = 1 << 7, VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST= 1 << 8, +/* format empty PCI addr (:00:0.0) */ +VIR_DOMAIN_DEF_FORMAT_EMPTY_PCI_ADDR = 1 << 9, } virDomainDefFormatFlags; /* Use these flags to skip specific domain ABI consistency checks done -- 2.34.1
[PATCH v2 25/29] conf, qemu: add 'pnv-phb4-root-port' PCI controller model name
This device is an implementation of pcie-root-port, similar to its sibling pnv-phb3-root-port. Since it's a new model name that Libvirt automatically sets, we refrain from documenting it to users. Signed-off-by: Daniel Henrique Barboza --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_domain_address.c | 14 +- src/qemu/qemu_validate.c | 8 ++-- 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 47a3107ea1..116fedc19e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2605,6 +2605,7 @@ ioh3420 pcie-root-port pnv-phb3-root-port +pnv-phb4-root-port x3130-upstream diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d27aafd771..2b862ffb86 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -440,6 +440,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "pcie-pci-bridge", "pnv-phb3-root-port", "pnv-phb3", + "pnv-phb4-root-port", ); VIR_ENUM_IMPL(virDomainControllerModelSCSI, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index dd3d942a35..6532b78322 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -648,6 +648,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3, +VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7d60d277e8..e18cc81f04 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2412,6 +2412,18 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, } +static virDomainControllerPCIModelName +virDomainControllerGetPowerNVRootPortName(virDomainDef *def) +{ +if (STREQ(def->os.machine, "powernv8")) +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT; +else if (STREQ(def->os.machine, "powernv9")) +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT; + +return VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE; +} + + static void qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, virDomainDef *def, @@ -2435,7 +2447,7 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (qemuDomainIsPowerNV(def)) { -*modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT; +*modelName = virDomainControllerGetPowerNVRootPortName(def); break; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index bb6ff2e70e..6f49a5f17b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3432,6 +3432,8 @@ virValidateControllerPCIModelNameToQEMUCaps(int modelName) return QEMU_CAPS_DEVICE_PNV_PHB3; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3: return QEMU_CAPS_DEVICE_PNV_PHB3; +case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT: +return QEMU_CAPS_DEVICE_PNV_PHB4; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE: return 0; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST: @@ -3601,12 +3603,14 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420 && pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT && -pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT) { +pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT && +pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT) { virReportControllerInvalidValue(cont, model, modelName, "modelName"); return -1; } -if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT && +if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT || + pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4_ROOT_PORT) && !qemuDomainIsPowerNV(def)) { virReportControllerInvalidValue(cont, model, modelName, "modelName"); return -1; -- 2.34.1
[PATCH v2 16/29] conf, qemu: add default 'chip-id' value for pnv-phb3 controllers
If ommited from the controller definition, chip-id defaults to zero. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain_address.c | 16 +++- src/qemu/qemu_validate.c | 5 + .../powernv8-basic.ppc64-latest.xml | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index f08fea32f5..17c2649fb1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2844,10 +2844,24 @@ qemuDomainAssignPCIAddresses(virDomainDef *def, goto cleanup; } break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: +if (!qemuDomainIsPowerNV(def)) +break; + +/* + * Default to chip-id = 0 since it's guaranteed that one socket + * will always be present. + * + * chipId validation requires CPU topology information that isn't + * available at this point and will be done later on. + */ +if (options->chipId == -1) +options->chipId = 0; + +break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: -case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e8d86a2280..bce52269a6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3648,6 +3648,11 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, virReportControllerInvalidValue(cont, model, modelName, "modelName"); return -1; } + +if (pciopts->chipId != -1 && !virDomainControllerIsPowerNVPHB(cont)) { +virReportControllerInvalidOption(cont, model, modelName, "chip-id"); +return -1; +} break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE: diff --git a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml index ebbc0653ca..28d86d7d9e 100644 --- a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml +++ b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml @@ -19,6 +19,7 @@ /usr/bin/qemu-system-ppc64 + -- 2.34.1
[PATCH v2 14/29] formatdomain.rst: add 'index' semantics for PowerNV domains
We're going to use the 'targetIndex' element for PowerNV PHBs. Clarify that the same attribute will have a different meaning in this context. Signed-off-by: Daniel Henrique Barboza --- docs/formatdomain.rst | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1e44d9a987..d700049c1c 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3896,8 +3896,10 @@ generated by libvirt. :since:`Since 1.2.19 (QEMU only).` the user of the libvirt API to attach host devices to the correct pci-expander-bus when assigning them to the domain). ``index`` - pci-root controllers for pSeries guests use this attribute to record the - order they will show up in the guest. :since:`Since 3.6.0` + pci-root controllers for ``pSeries`` guests use this attribute to record the + order they will show up in the guest (:since:`Since 3.6.0`). :since:`Since 8.1.0`, + ``powernv`` domains uses this attribute to indicate the chip/socket slot a + pcie-root controller will use. ``chip-id`` pcie-root controllers for ``powernv`` domains use this attribute to indicate the chip that will own the controller. A chip is equivalent to a CPU socket. -- 2.34.1
[PATCH v2 28/29] domain_conf.c: add pnv-phb4 to ControllerIsPowerNVPHB()
Update the virDomainControllerIsPowerNVPHB() helper to make the pnv-phb4 device receive the same handling as the existing pnv-phb3. Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 585e7d9dae..42c0bd29da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2457,7 +2457,8 @@ virDomainControllerIsPowerNVPHB(const virDomainControllerDef *cont) name = cont->opts.pciopts.modelName; -if (name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3) +if ((name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3) && +(name != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB4)) return false; return true; -- 2.34.1
[PATCH v2 13/29] conf: parse and format
The 'chip-id' attribute indicates which chip/socket that owns the PowerNV pcie-root controller. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- docs/formatdomain.rst | 6 ++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c| 15 +++ src/conf/domain_conf.h| 1 + 4 files changed, 27 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index e2f99c60a6..1e44d9a987 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3898,6 +3898,12 @@ generated by libvirt. :since:`Since 1.2.19 (QEMU only).` ``index`` pci-root controllers for pSeries guests use this attribute to record the order they will show up in the guest. :since:`Since 3.6.0` +``chip-id`` + pcie-root controllers for ``powernv`` domains use this attribute to indicate + the chip that will own the controller. A chip is equivalent to a CPU socket. + E.g. a ``powernv`` domain with `` sockets=3`` will have 3 chips. + chip-id=0 refers to the first chip, chip-id=1 refers to the second chip and + so on. :since:`Since 8.1.0` For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. pci-root has no diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4c3a1c28a7..47a3107ea1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2655,6 +2655,11 @@ + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 72ee46026a..803c51d63a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2380,6 +2380,7 @@ virDomainControllerDefNew(virDomainControllerType type) def->opts.pciopts.busNr = -1; def->opts.pciopts.targetIndex = -1; def->opts.pciopts.numaNode = -1; +def->opts.pciopts.chipId = -1; break; case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: def->opts.xenbusopts.maxGrantFrames = -1; @@ -9584,6 +9585,16 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, virReportError(VIR_ERR_XML_ERROR, _("Invalid target index '%i' in PCI controller"), def->opts.pciopts.targetIndex); + +if ((rc = virXMLPropInt(targetNodes[0], "chip-id", 0, VIR_XML_PROP_NONE, +>opts.pciopts.chipId, +def->opts.pciopts.chipId)) < 0) +return NULL; + +if ((rc == 1) && def->opts.pciopts.chipId == -1) +virReportError(VIR_ERR_XML_ERROR, + _("Invalid target chip-id '%i' in PCI controller"), + def->opts.pciopts.chipId); } } else if (ntargetNodes > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -23888,6 +23899,7 @@ virDomainControllerDefFormat(virBuffer *buf, def->opts.pciopts.busNr != -1 || def->opts.pciopts.targetIndex != -1 || def->opts.pciopts.numaNode != -1 || +def->opts.pciopts.chipId != -1 || def->opts.pciopts.hotplug != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAddLit(, "opts.pciopts.chassisNr != -1) @@ -23905,6 +23917,9 @@ virDomainControllerDefFormat(virBuffer *buf, if (def->opts.pciopts.targetIndex != -1) virBufferAsprintf(, " index='%d'", def->opts.pciopts.targetIndex); +if (def->opts.pciopts.chipId != -1) +virBufferAsprintf(, " chip-id='%d'", + def->opts.pciopts.chipId); if (def->opts.pciopts.hotplug != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAsprintf(, " hotplug='%s'", virTristateSwitchTypeToString(def->opts.pciopts.hotplug)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1d9c689e83..93c5092f35 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -751,6 +751,7 @@ struct _virDomainPCIControllerOpts { int port; int busNr; /* used by pci-expander-bus, -1 == unspecified */ int targetIndex; /* used by spapr-pci-host-bridge, -1 == unspecified */ +int chipId; /* used by powernv pcie-root controllers, -1 == unspecified */ /* numaNode is a *subelement* of target (to match existing * item in memory target config) -1 == unspecified */ -- 2.34.1
[PATCH v2 18/29] qemu_command.c: add command line for the pnv-phb3 device
The command line for the pnv-phb3 device is similar to the spapr-pci-host-bridge command line but adding the extra 'chip-id' attribute. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_command.c | 21 +-- .../powernv8-basic.ppc64-latest.args | 1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a1fe27297..afb3bf3612 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3221,6 +3221,22 @@ qemuBuildControllerPCIDevProps(virDomainControllerDef *def, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: +if (!virDomainControllerIsPowerNVPHB(def)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported PCI Express root controller")); +return -1; +} + +if (virJSONValueObjectAdd(, + "s:driver", modelName, + "i:index", pciopts->targetIndex, + "i:chip-id", pciopts->chipId, + "s:id", def->info.alias, + NULL) < 0) +return -1; + +break; + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unsupported PCI Express root controller")); return -1; @@ -3408,8 +3424,9 @@ static bool qemuBuildSkipController(const virDomainControllerDef *controller, const virDomainDef *def) { -/* skip pcie-root */ -if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && +/* skip pcie-root for non PowerVM domains */ +if (!qemuDomainIsPowerNV(def) && +controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) return true; diff --git a/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args b/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args index c9616ded13..224e2adba8 100644 --- a/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args +++ b/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args @@ -26,6 +26,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -rtc base=utc \ -no-shutdown \ -boot strict=on \ +-device '{"driver":"pnv-phb3","index":0,"chip-id":0,"id":"pcie.0"}' \ -usb \ -chardev pty,id=charserial0 \ -device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \ -- 2.34.1
[PATCH v2 09/29] qemu: introduce QEMU_CAPS_DEVICE_PNV_PHB3
QEMU_CAPS_DEVICE_PNV_PHB3 indicates binary support for the pnv-phb3 device, the pcie-root controller for PowerNV8 domains, and also the pnv-phb3-root-port device, its pcie-root-port device. This capability is present in QEMU since 5.0.0 but these devices are user-creatable only after QEMU 6.2.0. This means that probing it as default will be misleading for users. Instead, let's use virQEMUCapsInitQMPVersionCaps() to check for the adequate QEMU version and arch and set it manually. Suggested-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_capabilities.c | 19 +++ src/qemu/qemu_capabilities.h | 1 + .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + 3 files changed, 21 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index bb90715569..d60240912c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -662,6 +662,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 420 */ "device.json+hotplug", /* QEMU_CAPS_DEVICE_JSON */ "hvf", /* QEMU_CAPS_HVF */ + "pnv-phb3", /* QEMU_CAPS_DEVICE_PNV_PHB3 */ ); @@ -1397,6 +1398,17 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-vga-gl", QEMU_CAPS_VIRTIO_VGA_GL }, { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST }, { "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI }, +/* + * We don't probe the following PowerNV devices: + * + * { "pnv-phb3", QEMU_CAPS_DEVICE_PNV_PHB3 }, + * { "pnv-phb3-root-port", QEMU_CAPS_DEVICE_PNV_PHB3 }, + * + * Because they are present in QEMU binaries since QEMU 5.0.0 + * but became user creatable only in the QEMU 7.0.0 development + * cycle. Their respective capabilities are being set in + * virQEMUCapsInitQMPVersionCaps(). + */ }; @@ -5231,6 +5243,13 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCaps *qemuCaps) */ if (qemuCaps->version < 5002000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ENABLE_FIPS); + +/* PowerNV pnv-phb devices weren't user creatable up to + * QEMU 6.2.0. The version value set here was taken from a + * binary post 6.2.0 release that has user creatable pnv-phb + * support. */ +if (qemuCaps->version >= 6002050 && ARCH_IS_PPC64(qemuCaps->arch)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_PNV_PHB3); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c6fb87a73a..ed3973c0ba 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -637,6 +637,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 420 */ QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON (and works with hot-unplug) */ QEMU_CAPS_HVF, /* Whether Hypervisor.framework is available */ +QEMU_CAPS_DEVICE_PNV_PHB3, /* devices pnv-phb3 and pnv-phb3-root-port */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index 88eee87587..cb6c146a28 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -200,6 +200,7 @@ + 6002050 0 42900243 -- 2.34.1
[PATCH v2 10/29] conf, qemu: add 'pnv-phb3-root-port' PCI controller model name
Apart from being usable only with pnv-phb3 PCIE host bridges (to be added soon), this device acts as a regular pcie-root-port but with a specific model name. No doc changes in formatdomain.rst were made because the PCI model name isn't something that users are supposed to be setting or changing. Signed-off-by: Daniel Henrique Barboza --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_domain_address.c | 5 + src/qemu/qemu_validate.c | 12 +++- 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 64a797de46..a467fc1437 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2602,6 +2602,7 @@ ioh3420 pcie-root-port +pnv-phb3-root-port x3130-upstream diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f5b15cff33..8db4d44d28 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -438,6 +438,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "pcie-root-port", "spapr-pci-host-bridge", "pcie-pci-bridge", + "pnv-phb3-root-port", ); VIR_ENUM_IMPL(virDomainControllerModelSCSI, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d7352b60e6..7aef7659e9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -646,6 +646,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_PCI_BRIDGE, +VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 3e6eed6ec9..eeececa936 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2420,6 +2420,11 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_PCI_BRIDGE; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: +if (qemuDomainIsPowerNV(def)) { +*modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT; +break; +} + /* Use generic PCIe Root Ports if available, falling back to * ioh3420 otherwise */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index aa686246f5..ea7861b232 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3428,6 +3428,8 @@ virValidateControllerPCIModelNameToQEMUCaps(int modelName) return QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_PCI_BRIDGE: return QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE; +case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT: +return QEMU_CAPS_DEVICE_PNV_PHB3; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE: return 0; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST: @@ -3595,10 +3597,18 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420 && -pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) { +pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT && +pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT) { virReportControllerInvalidValue(cont, model, modelName, "modelName"); return -1; } + +if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT && +!qemuDomainIsPowerNV(def)) { +virReportControllerInvalidValue(cont, model, modelName, "modelName"); +return -1; +} + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: -- 2.34.1
[PATCH v2 07/29] qemu_domain.c: disable default devices for PowerNV machines
PowerNV domains will support pcie-root devices as PHBs, in a similar fashion as pSeries domains supports the spapr-pci-host-bridge as a pci-root model. Set 'addPCIRoot' to false since we'll not be using this buses in this machine. 'addDefaultMemballoon' is also set to false since the balloon driver wasn't really tested with the PowerNV kernel. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b4b293388..89dbc20eee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3670,6 +3670,15 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, * add the definition if not already present */ if (qemuDomainIsPSeries(def)) addPanicDevice = true; + +if (qemuDomainIsPowerNV(def)) { +addPCIRoot = false; +addDefaultUSB = false; +addDefaultUSBKBD = false; +addDefaultUSBMouse = false; +addDefaultMemballoon = false; +} + break; case VIR_ARCH_ALPHA: -- 2.34.1
[PATCH v2 11/29] conf, qemu: add 'pnv-phb3' PCI controller model name
Add support for the pcie-root implementation that PowerNV8 domains uses, pnv-phb3. It consists of a PCI model name that isn't supposed to be changed by users, so no doc changes in formatdomain.rst were made. Signed-off-by: Daniel Henrique Barboza --- docs/schemas/domaincommon.rng| 2 ++ src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_domain_address.c | 3 +++ src/qemu/qemu_validate.c | 8 ++-- tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml | 4 +++- 6 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a467fc1437..4c3a1c28a7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2597,6 +2597,8 @@ pci-bridge i82801b11-bridge + +pnv-phb3 pcie-pci-bridge diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8db4d44d28..57ab5b19dd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -439,6 +439,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "spapr-pci-host-bridge", "pcie-pci-bridge", "pnv-phb3-root-port", + "pnv-phb3", ); VIR_ENUM_IMPL(virDomainControllerModelSCSI, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7aef7659e9..1d9c689e83 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -647,6 +647,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT, +VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index eeececa936..f08fea32f5 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2449,6 +2449,9 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: +if (qemuDomainIsPowerNV(def)) +*modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3; +break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index ea7861b232..e8d86a2280 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3430,6 +3430,8 @@ virValidateControllerPCIModelNameToQEMUCaps(int modelName) return QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3_ROOT_PORT: return QEMU_CAPS_DEVICE_PNV_PHB3; +case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PNV_PHB3: +return QEMU_CAPS_DEVICE_PNV_PHB3; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE: return 0; case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST: @@ -3558,7 +3560,8 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: -if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { +if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE && +!qemuDomainIsPowerNV(def)) { virReportControllerInvalidOption(cont, model, modelName, "modelName"); return -1; } @@ -3640,7 +3643,8 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: -if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { +if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE && +!qemuDomainIsPowerNV(def)) { virReportControllerInvalidValue(cont, model, modelName, "modelName"); return -1; } diff --git a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml index cb9b3cf86f..ebbc0653ca 100644 --- a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml +++ b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml @@ -17,7 +17,9 @@ destroy /usr/bin/qemu-system-ppc64 - + + + -- 2.34.1
[PATCH v2 04/29] qemu_validate.c: use qemuDomainIsPowerPC() in qemuValidateDomainChrDef()
Both PowerNV and pSeries machines don't support parallel ports. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 0a879f0115..8453413b3c 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2007,7 +2007,7 @@ qemuValidateDomainChrDef(const virDomainChrDef *dev, return -1; if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL && -(ARCH_IS_S390(def->os.arch) || qemuDomainIsPSeries(def))) { +(ARCH_IS_S390(def->os.arch) || qemuDomainIsPowerPC(def))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("parallel ports are not supported")); return -1; -- 2.34.1
[PATCH v2 12/29] domain_conf.c: fix identation in virDomainControllerDefParseXML()
The identation of VIR_DOMAIN_CONTROLLER_TYPE_PCI elements are in the same level as the parent 'if (def->type == ...TYPE_PCI)' clause, and the closing bracket of this 'if' looks like a misplaced bracket of the 'targetIndex' clause that comes right before it. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/conf/domain_conf.c | 56 +- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 57ab5b19dd..72ee46026a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9550,40 +9550,40 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, ntargetNodes = virXPathNodeSet("./target", ctxt, ); if (ntargetNodes == 1) { if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { -if (virXMLPropInt(targetNodes[0], "chassisNr", 0, VIR_XML_PROP_NONE, - >opts.pciopts.chassisNr, - def->opts.pciopts.chassisNr) < 0) -return NULL; +if (virXMLPropInt(targetNodes[0], "chassisNr", 0, VIR_XML_PROP_NONE, + >opts.pciopts.chassisNr, + def->opts.pciopts.chassisNr) < 0) +return NULL; -if (virXMLPropInt(targetNodes[0], "chassis", 0, VIR_XML_PROP_NONE, - >opts.pciopts.chassis, - def->opts.pciopts.chassis) < 0) -return NULL; +if (virXMLPropInt(targetNodes[0], "chassis", 0, VIR_XML_PROP_NONE, + >opts.pciopts.chassis, + def->opts.pciopts.chassis) < 0) +return NULL; -if (virXMLPropInt(targetNodes[0], "port", 0, VIR_XML_PROP_NONE, - >opts.pciopts.port, - def->opts.pciopts.port) < 0) -return NULL; +if (virXMLPropInt(targetNodes[0], "port", 0, VIR_XML_PROP_NONE, + >opts.pciopts.port, + def->opts.pciopts.port) < 0) +return NULL; -if (virXMLPropInt(targetNodes[0], "busNr", 0, VIR_XML_PROP_NONE, - >opts.pciopts.busNr, - def->opts.pciopts.busNr) < 0) -return NULL; +if (virXMLPropInt(targetNodes[0], "busNr", 0, VIR_XML_PROP_NONE, + >opts.pciopts.busNr, + def->opts.pciopts.busNr) < 0) +return NULL; -if (virXMLPropTristateSwitch(targetNodes[0], "hotplug", - VIR_XML_PROP_NONE, - >opts.pciopts.hotplug) < 0) -return NULL; +if (virXMLPropTristateSwitch(targetNodes[0], "hotplug", + VIR_XML_PROP_NONE, + >opts.pciopts.hotplug) < 0) +return NULL; -if ((rc = virXMLPropInt(targetNodes[0], "index", 0, VIR_XML_PROP_NONE, - >opts.pciopts.targetIndex, - def->opts.pciopts.targetIndex)) < 0) -return NULL; +if ((rc = virXMLPropInt(targetNodes[0], "index", 0, VIR_XML_PROP_NONE, +>opts.pciopts.targetIndex, +def->opts.pciopts.targetIndex)) < 0) +return NULL; -if ((rc == 1) && def->opts.pciopts.targetIndex == -1) -virReportError(VIR_ERR_XML_ERROR, - _("Invalid target index '%i' in PCI controller"), - def->opts.pciopts.targetIndex); +if ((rc == 1) && def->opts.pciopts.targetIndex == -1) +virReportError(VIR_ERR_XML_ERROR, + _("Invalid target index '%i' in PCI controller"), + def->opts.pciopts.targetIndex); } } else if (ntargetNodes > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", -- 2.34.1
[PATCH v2 06/29] qemu_validate.c: enhance 'machine type not supported' message
Add 'virt type' to allow for an easier time debugging. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_validate.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 8453413b3c..aa686246f5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1101,8 +1101,9 @@ qemuValidateDomainDef(const virDomainDef *def, if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Emulator '%s' does not support machine type '%s'"), - def->emulator, def->os.machine); + _("Emulator '%s' does not support machine type '%s' for virt type '%s'"), + def->emulator, def->os.machine, + virDomainVirtTypeToString(def->virtType)); return -1; } -- 2.34.1
[PATCH v2 02/29] qemu_capabilities.c: use 'MachineIsPowerPC' in DeviceDiskCaps
Both pSeries and PowerNV machines don't have floppy device support. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_capabilities.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dae9d1163d..bb90715569 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6178,8 +6178,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCaps *qemuCaps, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_DEVICE_LUN); -/* PowerPC pseries based VMs do not support floppy device */ -if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch)) { +/* PowerPC domains do not support floppy device */ +if (!qemuDomainMachineIsPowerPC(machine, qemuCaps->arch)) { VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY); VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC); } -- 2.34.1
[PATCH v2 05/29] qemu_domain.c: define ISA as default PowerNV serial
The PowerNV machines uses ISA as the default serial type. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 98a6a2657d..1b4b293388 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5167,6 +5167,8 @@ qemuDomainChrDefPostParse(virDomainChrDef *chr, chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM; } else if (ARCH_IS_S390(def->os.arch)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP; +} else if (qemuDomainIsPowerNV(def)) { +chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; } } -- 2.34.1
[PATCH v2 08/29] tests: add basic PowerNV8 test
We're now able to boot a simple PowerNV8 domain in Libvirt. Signed-off-by: Daniel Henrique Barboza --- .../powernv8-basic.ppc64-latest.args | 33 +++ tests/qemuxml2argvdata/powernv8-basic.xml | 16 + tests/qemuxml2argvtest.c | 2 ++ .../powernv8-basic.ppc64-latest.xml | 31 + tests/qemuxml2xmltest.c | 2 ++ 5 files changed, 84 insertions(+) create mode 100644 tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml diff --git a/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args b/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args new file mode 100644 index 00..c9616ded13 --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-basic.ppc64-latest.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-ppc64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine powernv8,usb=off,dump-guest-core=off,memory-backend=pnv.ram \ +-accel tcg \ +-cpu POWER8 \ +-m 2048 \ +-object '{"qom-type":"memory-backend-ram","id":"pnv.ram","size":2147483648}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid b20fcfe3-4a0a-4039-8735-9e024256e0f7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-usb \ +-chardev pty,id=charserial0 \ +-device '{"driver":"isa-serial","chardev":"charserial0","id":"serial0","index":0}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml new file mode 100644 index 00..a92fc1282c --- /dev/null +++ b/tests/qemuxml2argvdata/powernv8-basic.xml @@ -0,0 +1,16 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 1 + +hvm + + +/usr/bin/qemu-system-ppc64 + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 96d30f2475..de5582a52c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2242,6 +2242,8 @@ mymain(void) DO_TEST_PARSE_ERROR_NOCAPS("seclabel-multiple"); DO_TEST_PARSE_ERROR_NOCAPS("seclabel-device-duplicates"); +DO_TEST_CAPS_ARCH_LATEST("powernv8-basic", "ppc64"); + DO_TEST("pseries-basic", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_DEVICE_SPAPR_VTY); diff --git a/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml new file mode 100644 index 00..cb9b3cf86f --- /dev/null +++ b/tests/qemuxml2xmloutdata/powernv8-basic.ppc64-latest.xml @@ -0,0 +1,31 @@ + + QEMUGuest1 + b20fcfe3-4a0a-4039-8735-9e024256e0f7 + 2097152 + 2097152 + 1 + +hvm + + + +POWER8 + + + destroy + restart + destroy + +/usr/bin/qemu-system-ppc64 + + + + + + + + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 935fd955f4..1815d9ac1d 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -662,6 +662,8 @@ mymain(void) QEMU_CAPS_OBJECT_RNG_EGD); DO_TEST_CAPS_LATEST("virtio-rng-builtin"); +DO_TEST_CAPS_ARCH_LATEST("powernv8-basic", "ppc64"); + DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_DEVICE_NVRAM); -- 2.34.1
[PATCH v2 03/29] qemu_domain: turn qemuDomainMachineIsPSeries() static
The function is now unused outside of qemu_domain.c. Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2e21a95f38..98a6a2657d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8631,7 +8631,7 @@ qemuDomainMachineIsRISCVVirt(const char *machine, /* You should normally avoid this function and use * qemuDomainIsPSeries() instead. */ -bool +static bool qemuDomainMachineIsPSeries(const char *machine, const virArch arch) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d07a279a1b..fe4420bf83 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -759,8 +759,6 @@ virDomainChrDef *qemuFindAgentConfig(virDomainDef *def); * doesn't have "Machine" in the name instead. */ bool qemuDomainMachineIsARMVirt(const char *machine, const virArch arch); -bool qemuDomainMachineIsPSeries(const char *machine, -const virArch arch); bool qemuDomainMachineIsPowerPC(const char *machine, const virArch arch); bool qemuDomainMachineHasBuiltinIDE(const char *machine, -- 2.34.1
[PATCH v2 01/29] qemu_domain.c: add PowerNV machine helpers
The PowerNV (Power Non-Virtualized) QEMU ppc64 machine is the emulation of a bare-metal IBM Power host. It follows the OPAL (OpenPower Abstration Layer) API/ABI, most specifically Skiboot [1]. For now, Libvirt has support for the pSeries QEMU machine, which is the emulation of a logical partition (guest) that would run on top of a bare-metal system. This patch introduces the helpers that are going to be used to add a basic support for PowerNV domains in Libvirt. Given that there are quite a few similarities in how pSeries and PowerNVv should be handled, we're also adding a 'qemuDomainIsPowerPC' helper that will be used in those instances. [1] https://open-power.github.io/skiboot/doc/overview.html Reviewed-by: Peter Krempa Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_domain.c | 41 + src/qemu/qemu_domain.h | 4 2 files changed, 45 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index aa8f6b8d05..2e21a95f38 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8719,6 +8719,47 @@ qemuDomainIsPSeries(const virDomainDef *def) } +/* + * The PowerNV machine does not support KVM acceleration in Power + * hosts. We can skip the usual ARCH_IS_PPC64() since this machine + * is usable with TCG in all host archs. + */ +bool +qemuDomainIsPowerNV(const virDomainDef *def) +{ +if (STRPREFIX(def->os.machine, "powernv")) +return true; + +return false; +} + + +/* + * PowerNV and pSeries domains shares a lot of common traits. This + * helper avoids repeating "if (pseries || powernv)" everywhere this + * is applicable. + */ +bool +qemuDomainIsPowerPC(const virDomainDef *def) +{ +return qemuDomainIsPSeries(def) || qemuDomainIsPowerNV(def); +} + + +/* + * Similar to qemuDomainIsPowerPC(). Usable when the caller doesn't + * have access to a virDomainDef pointer. + */ +bool +qemuDomainMachineIsPowerPC(const char *machine, const virArch arch) +{ +if (STRPREFIX(machine, "powernv")) +return true; + +return qemuDomainMachineIsPSeries(machine, arch); +} + + bool qemuDomainHasPCIRoot(const virDomainDef *def) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e5046367e3..d07a279a1b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -761,6 +761,8 @@ bool qemuDomainMachineIsARMVirt(const char *machine, const virArch arch); bool qemuDomainMachineIsPSeries(const char *machine, const virArch arch); +bool qemuDomainMachineIsPowerPC(const char *machine, +const virArch arch); bool qemuDomainMachineHasBuiltinIDE(const char *machine, const virArch arch); @@ -770,6 +772,8 @@ bool qemuDomainIsS390CCW(const virDomainDef *def); bool qemuDomainIsARMVirt(const virDomainDef *def); bool qemuDomainIsRISCVVirt(const virDomainDef *def); bool qemuDomainIsPSeries(const virDomainDef *def); +bool qemuDomainIsPowerNV(const virDomainDef *def); +bool qemuDomainIsPowerPC(const virDomainDef *def); bool qemuDomainHasPCIRoot(const virDomainDef *def); bool qemuDomainHasPCIeRoot(const virDomainDef *def); bool qemuDomainHasBuiltinIDE(const virDomainDef *def); -- 2.34.1
[PATCH v2 00/29] ppc64 PowerNV machines support
Hi, This v2 has changes proposed by Peter and Daniel on the v1 review. Peter's reviewed-by tags were kept when applicable. The usability change made is that, now, we'll fail to launch powernv domains that has a pnv-phb* device and it's running a QEMU version that doesn't support these devices to be user creatable. Trying to run the 'powernv8-basic' domain with a QEMU 6.2.0 binary will result in an error: $ sudo ./run tools/virsh define ../tests/qemuxml2argvdata/powernv8-basic.xml error: Failed to define domain from ../tests/qemuxml2argvdata/powernv8-basic.xml error: unsupported configuration: The 'pnv-phb3' device is not supported by this QEMU binary Using the current QEMU upstream will allow the domain to be defined and started. Changes from v1: - all tests are now using CAPS_LATEST; - QEMU_CAPS_DEVICE_PNV_PHB3_ROOT_PORT is no longer being used. Capability for the pnv-phb3-root-port is infered to exist if the capabilitity for its PHB (QEMU_CAPS_DEVICE_PNV_PHB3) is present. Same thing for the case of QEMU_CAPS_DEVICE_PNV_PHB4_ROOT_PORT and QEMU_CAPS_DEVICE_PNV_PHB4; - QEMU_CAPS_DEVICE_PNV_PHB3 and QEMU_CAPS_DEVICE_PNV_PHB4 are no longer being probed. They are being set by hand after checking for QEMU version in virQEMUCapsInitQMPVersionCaps(); - patch 01 (QEMU ppc64 capabilities for qemu 7.0): * dropped since it's already upstream - patch 09 (forbid powernv domains migration): * removed. This will be handled on QEMU side - patch 14 (new): * added documentation of the different semantics 'targetIndex' will have for PowerNV PHBs - several other minor changes suggested by Peter - v1 link: https://listman.redhat.com/archives/libvir-list/2022-January/msg00902.html Daniel Henrique Barboza (29): qemu_domain.c: add PowerNV machine helpers qemu_capabilities.c: use 'MachineIsPowerPC' in DeviceDiskCaps qemu_domain: turn qemuDomainMachineIsPSeries() static qemu_validate.c: use qemuDomainIsPowerPC() in qemuValidateDomainChrDef() qemu_domain.c: define ISA as default PowerNV serial qemu_validate.c: enhance 'machine type not supported' message qemu_domain.c: disable default devices for PowerNV machines tests: add basic PowerNV8 test qemu: introduce QEMU_CAPS_DEVICE_PNV_PHB3 conf, qemu: add 'pnv-phb3-root-port' PCI controller model name conf, qemu: add 'pnv-phb3' PCI controller model name domain_conf.c: fix identation in virDomainControllerDefParseXML() conf: parse and format formatdomain.rst: add 'index' semantics for PowerNV domains introduce virDomainControllerIsPowerNVPHB conf, qemu: add default 'chip-id' value for pnv-phb3 controllers conf, qemu: add default 'targetIndex' value for pnv-phb3 devs qemu_command.c: add command line for the pnv-phb3 device qemu_domain_address.c: change pnv-phb3 minimal downstream slot domain_conf: format pnv-phb3-root-port empty addr tests: add pnv-phb3-root-port test domain_validate.c: allow targetIndex 0 out of idx 0 for PowerNV PHBs domain_conf.c: reject duplicated pnv-phb3 devices qemu: introduce QEMU_CAPS_DEVICE_PNV_PHB4 conf, qemu: add 'pnv-phb4-root-port' PCI controller model name domain_conf.c: add phb4-root-port to IsPowerNVRootPort() conf, qemu: add 'pnv-phb4' controller model name domain_conf.c: add pnv-phb4 to ControllerIsPowerNVPHB() tests: add PowerNV9 tests docs/formatdomain.rst | 12 +- docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c| 157 ++ src/conf/domain_conf.h| 8 + src/conf/domain_validate.c| 5 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 28 +++- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 21 ++- src/qemu/qemu_domain.c| 56 ++- src/qemu/qemu_domain.h| 4 +- src/qemu/qemu_domain_address.c| 64 ++- src/qemu/qemu_validate.c | 62 ++- .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 2 + .../powernv8-basic.ppc64-latest.args | 34 tests/qemuxml2argvdata/powernv8-basic.xml | 16 ++ tests/qemuxml2argvdata/powernv8-dupPHBs.err | 1 + .../powernv8-dupPHBs.ppc64-latest.err | 1 + tests/qemuxml2argvdata/powernv8-dupPHBs.xml | 27 +++ .../powernv8-root-port.ppc64-latest.args | 35 tests/qemuxml2argvdata/powernv8-root-port.xml | 17 ++ .../powernv8-two-sockets.ppc64-latest.args| 35 .../qemuxml2argvdata/powernv8-two-sockets.xml | 26 +++ .../powernv9-dupPHBs.ppc64-latest.err | 1 + tests/qemuxml2argvdata/powernv9-dupPHBs.xml | 27 +++ .../powernv9-root-port.ppc64-latest.args | 35 tests/qemuxml2argvdata/powernv9-root-port.xml | 17 ++ tests/qemuxml2argvtest.c | 7 + .../powernv8-basic.ppc64-latest.xml
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani : > Are you going to take a stab at these patches? No, I will just add sles-release to BuildRequires. Olaf pgpGDf1Va_edK.pgp Description: Digitale Signatur von OpenPGP
The unix domain socket remains even after the VM is destroyed
Hello, I found an issue that libvirt isn't close an unix domain socket to connect to the qemu monitor even after the VM is destroyed. This issue happens since commit 695bdb3841 ("src: ensure GSource background unref happens in correct event loop") on the system whose glib version is 2.56. I would appreciate it if you could give any ideas to solve the issue. The socket is allocated in qemuMonitorOpenUnix(), and used by the monitor->socket and monitor->watch: qemuMonitorOpen qemuMonitorOpenUnix if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { qemuMonitorOpenInternal mon->socket = g_socket_new_from_fd(fd, ); qemuMonitorRegister mon->watch = g_socket_create_source(mon->socket, Usually, the socket is closed when the reference counter of the glib object (mon->socket and mon->watch) gets 0: qemuMonitorClose qemuMonitorUnregister vir_g_source_unref(mon->watch, mon->context); g_source_set_callback(idle, virEventGLibSourceUnrefIdle, src, NULL); virEventGLibSourceUnrefIdle g_source_unref(src); <== unref monitor->watch g_object_unref(mon->socket); <== unref monitor->socket It seems that the callback virEventGLibSourceUnrefIdle() to unref the monitor->watch doesn't work when qemuMonitorUnregister() is called via qemuProcessStop(), so the socket isn't closed. I'm not sure why the callback doesn't work at that time. I suspect that the VM is closing so the main loop of the monitor doesn't work any more. We can close the socket to add g_socket_close() before unref the mon->socket, however, it may remain a memory leak because of mon->watch (GSource object), so probably it isn't a good idea to close the socket. We can unref the mon->watch to set NULL to the second argument of vir_g_source_unref() because the default main loop still works at that time, however, I'm not sure it's an appropriate way to avoid the gobject issue which the commit solves... I found this issue on the qemu monitor, and probably the qemu agent has the same issue because the socket procedure is almost the same as the monitor. I would appreciate it if you could give any ideas to solve this issue. Following is to observe the callback working: --- diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c index eb6dcc0111..b8b1770424 100644 --- a/src/util/glibcompat.c +++ b/src/util/glibcompat.c @@ -24,6 +24,9 @@ #include "glibcompat.h" +#include "virlog.h" + +VIR_LOG_INIT("util.glibcompat"); /* * Note that because of the GLIB_VERSION_MAX_ALLOWED constant in * config-post.h, allowing use of functions from newer GLib via @@ -244,6 +247,7 @@ virEventGLibSourceUnrefIdle(gpointer data) GSource *src = data; g_source_unref(src); +VIR_DEBUG("unrefed: %p", src); return FALSE; } @@ -257,6 +261,7 @@ void vir_g_source_unref(GSource *src, GMainContext *ctx) g_source_attach(idle, ctx); g_source_unref(idle); +VIR_DEBUG("unref registered: %p ctx: %p", src, ctx); } #endif --- Case the mon->watch (0x28008af0) is unreffed correctly (via qemuMonitorUpdateWatch()): 18:54:15.403+: 16845: debug : qemuMonitorEmitResume:1159 : mon=0x683ac020 18:54:15.403+: 16845: debug : qemuProcessHandleResume:713 : Transitioned guest test-vm into running state, reason 'booted', event detail 0 18:54:15.404+: 16845: debug : vir_g_source_unref:264 : unref registered: 0x28008af0 ctx: 0x780169a0 18:54:15.404+: 16845: debug : qemuMonitorJSONIOProcessLine:222 : Line [{"return": {}, "id": "libvirt-10"}] 18:54:15.404+: 16845: info : qemuMonitorJSONIOProcessLine:242 : QEMU_MONITOR_RECV_REPLY: mon=0x683ac020 reply={"return": {}, "id": "libvirt-10"} 18:54:15.404+: 16845: debug : vir_g_source_unref:264 : unref registered: 0x2819a260 ctx: 0x780169a0 18:54:15.404+: 16845: debug : virEventGLibSourceUnrefIdle:250 : unrefed: 0x28008af0 Case the mon->watch (0x7802bb30) isn't unreffed (via qemuProcessStop()): 18:54:15.642+: 16589: debug : qemuProcessStop:8008 : Shutting down vm=0xd40edec0 name=test-vm id=3 pid=16842, reason=destroyed, asyncJob=none, flags=0x0 18:54:15.642+: 16589: debug : qemuDomainLogAppendMessage:6733 : Append log message (vm='test-vm' message='2022-01-25 18:54:15.642+: shutting down, reason=destroyed) stdioLogD=1 18:54:15.643+: 16589: info : qemuMonitorClose:834 : QEMU_MONITOR_CLOSE: mon=0x683ac020 18:54:15.643+: 16589: debug : vir_g_source_unref:264 : unref registered: 0x7802bb30 ctx: 0x780169a0 18:54:15.643+: 16845: debug : qemuMonitorJSONIOProcessEvent:209 : handle SHUTDOWN handler=0x4ef057c0 data=0x28007da0 18:54:15.643+: 16845: debug : qemuMonitorEmitShutdown:1132 : mon=0x683ac020 guest=2 18:54:15.643+: 16845: debug : qemuProcessHandleShutdown:572 : vm=0xd40edec0 18:54:15.643+: 16845: debug : qemuProcessHandleShutdown:592 : Transitioned guest
[libvirt PATCH] spec: Don't clean up *.la and *.a files
autotools used to produce those, but meson doesn't. Signed-off-by: Andrea Bolognani --- libvirt.spec.in | 14 -- 1 file changed, 14 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f0ff4c0ece..0e6cd13bb2 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1195,20 +1195,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec) %meson_install -rm -f $RPM_BUILD_ROOT%{_libdir}/*.la -rm -f $RPM_BUILD_ROOT%{_libdir}/*.a -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.la -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-file/*.la -rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-file/*.a -%if %{with_wireshark} -rm -f $RPM_BUILD_ROOT%{wireshark_plugindir}/libvirt.la -%endif - # We don't want to install /etc/libvirt/qemu/networks in the main %%files list # because if the admin wants to delete the default network completely, we don't # want to end up re-incarnating it on every RPM upgrade. -- 2.34.1
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
On Tue, Jan 25, 2022 at 06:30:43PM +0100, Olaf Hering wrote: > Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani : > > /etc/os-release being absent should not result in a build failure. > > Yes. This should be the first thing to fix. > > Commit 29cd1877acd91883df32bf71ec07fe908e96db32 does not say why ID= > was used. This grep should be relaxed to ID_LIKE=. We have to consider *both*, because only looking at ID_LIKE would not work on some distros: $ grep -E '^ID(_LIKE)*=' /etc/os-release ID=fedora $ > Furthermore, if both qemu_user and qemu_group are known, no grep is required. Do you mean that if the user has passed -Dqemu_user=... -Dqemu_group=... to meson we could skip the detection logic? That'd be nice, but I'm afraid it might make the code less readable for very little gain. Are you going to take a stab at these patches? -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 3/3] mingw: Don't ship virkey* manual pages
They're part of the -daemon package on Linux. Signed-off-by: Andrea Bolognani --- This patch is sort of an RFC actually, because maybe even on Linux it would make more sense for these to be included in the -client package instead? Their usefulness in the mingw package still seems questionable. mingw-libvirt.spec.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 95039e1419..39f8cc0cf3 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -250,7 +250,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_includedir}/libvirt/libvirt-admin.h %{mingw32_mandir}/man1/virsh.1* -%{mingw32_mandir}/man7/virkey*.7* # Mingw64 %files -n mingw64-libvirt @@ -310,6 +309,5 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_includedir}/libvirt/libvirt-admin.h %{mingw64_mandir}/man1/virsh.1* -%{mingw64_mandir}/man7/virkey*.7* %changelog -- 2.34.1
[libvirt PATCH 2/3] mings: Don't ship virt-(xml|pki)-validate
These are shell scripts which are not useful in the context of cross-compilation, so it doesn't make sense to include them. Signed-off-by: Andrea Bolognani --- mingw-libvirt.spec.in | 8 1 file changed, 8 deletions(-) diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index 01beb6ff18..95039e1419 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -200,8 +200,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_bindir}/libvirt-0.dll %{mingw32_bindir}/virsh.exe -%{mingw32_bindir}/virt-xml-validate -%{mingw32_bindir}/virt-pki-validate %{mingw32_bindir}/libvirt-lxc-0.dll %{mingw32_bindir}/libvirt-qemu-0.dll %{mingw32_bindir}/libvirt-admin-0.dll @@ -252,8 +250,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_includedir}/libvirt/libvirt-admin.h %{mingw32_mandir}/man1/virsh.1* -%{mingw32_mandir}/man1/virt-xml-validate.1* -%{mingw32_mandir}/man1/virt-pki-validate.1* %{mingw32_mandir}/man7/virkey*.7* # Mingw64 @@ -264,8 +260,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_bindir}/libvirt-0.dll %{mingw64_bindir}/virsh.exe -%{mingw64_bindir}/virt-xml-validate -%{mingw64_bindir}/virt-pki-validate %{mingw64_bindir}/libvirt-lxc-0.dll %{mingw64_bindir}/libvirt-qemu-0.dll %{mingw64_bindir}/libvirt-admin-0.dll @@ -316,8 +310,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_includedir}/libvirt/libvirt-admin.h %{mingw64_mandir}/man1/virsh.1* -%{mingw64_mandir}/man1/virt-xml-validate.1* -%{mingw64_mandir}/man1/virt-pki-validate.1* %{mingw64_mandir}/man7/virkey*.7* %changelog -- 2.34.1
[libvirt PATCH 0/3] mingw: Misc cleanups
Andrea Bolognani (3): mingw: Don't ship virt-admin mings: Don't ship virt-(xml|pki)-validate mingw: Don't ship virkey* manual pages mingw-libvirt.spec.in | 14 -- 1 file changed, 14 deletions(-) -- 2.34.1
[libvirt PATCH 1/3] mingw: Don't ship virt-admin
virt-admin can only be used to configure the local daemon, and we don't build the daemon for Windows. Signed-off-by: Andrea Bolognani --- mingw-libvirt.spec.in | 4 1 file changed, 4 deletions(-) diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index dbcb6d4b8f..01beb6ff18 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -200,7 +200,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_bindir}/libvirt-0.dll %{mingw32_bindir}/virsh.exe -%{mingw32_bindir}/virt-admin.exe %{mingw32_bindir}/virt-xml-validate %{mingw32_bindir}/virt-pki-validate %{mingw32_bindir}/libvirt-lxc-0.dll @@ -253,7 +252,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_includedir}/libvirt/libvirt-admin.h %{mingw32_mandir}/man1/virsh.1* -%{mingw32_mandir}/man1/virt-admin.1* %{mingw32_mandir}/man1/virt-xml-validate.1* %{mingw32_mandir}/man1/virt-pki-validate.1* %{mingw32_mandir}/man7/virkey*.7* @@ -266,7 +264,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_bindir}/libvirt-0.dll %{mingw64_bindir}/virsh.exe -%{mingw64_bindir}/virt-admin.exe %{mingw64_bindir}/virt-xml-validate %{mingw64_bindir}/virt-pki-validate %{mingw64_bindir}/libvirt-lxc-0.dll @@ -319,7 +316,6 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_includedir}/libvirt/libvirt-admin.h %{mingw64_mandir}/man1/virsh.1* -%{mingw64_mandir}/man1/virt-admin.1* %{mingw64_mandir}/man1/virt-xml-validate.1* %{mingw64_mandir}/man1/virt-pki-validate.1* %{mingw64_mandir}/man7/virkey*.7* -- 2.34.1
Re: [PATCH] qemuDomainSetupDisk: Initialize 'targetPaths'
On Tue, Jan 25, 2022 at 05:59:28PM +0100, Peter Krempa wrote: > Compiler isn't able to see that 'virDevMapperGetTargets' in cases e.g. > when the devmapper isn't available may not initialize the value in the > pointer passed as the second argument. > > The usage 'qemuDomainSetupDisk' lead to an accidental infinite loop as > previous calls apparently doctored the stack to a point where > 'g_slist_concat' would end up in an infinite loop trying to find the end > of the list. > > Fixes: 6c49c2ee9fc Full commit hash here maybe? Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani : > /etc/os-release being absent should not result in a build failure. Yes. This should be the first thing to fix. Commit 29cd1877acd91883df32bf71ec07fe908e96db32 does not say why ID= was used. This grep should be relaxed to ID_LIKE=. Furthermore, if both qemu_user and qemu_group are known, no grep is required. Olaf pgprqjtDYdWpd.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
On Tue, Jan 25, 2022 at 03:38:33PM +0100, Olaf Hering wrote: > Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering : > > > Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, > > /etc/os-release must exist during build. > > Now that a missing ^ID= became fatal, was it actually a good idea to be > explicit? > I think poking around for clues in ^ID_LIKE= will be more robust, appending > 'sles' would be not required. You're right, /etc/os-release being absent should not result in a build failure. And instead of matching on ID specifically, we could probably use something like grep -E '^ID(_LIKE)*=' /etc/os-release instead and check only a subset of the strings we currently do. RHEL and CentOS for example are both ID_LIKE=fedora. We'd have to be careful about the order in which the Debian and Ubuntu branches appear though. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
On Tue, Jan 25, 2022 at 02:17:23PM +0100, Olaf Hering wrote: > NAME="SLES" > VERSION="15-SP3" > VERSION_ID="15.3" > PRETTY_NAME="SUSE Linux Enterprise Server 15 SP3" > ID="sles" > ID_LIKE="suse" > ANSI_COLOR="0;32" > CPE_NAME="cpe:/o:suse:sles:15:sp3" > DOCUMENTATION_URL="https://documentation.suse.com/; > > > Signed-off-by: Olaf Hering > --- > meson.build | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
[PATCH] qemuDomainSetupDisk: Initialize 'targetPaths'
Compiler isn't able to see that 'virDevMapperGetTargets' in cases e.g. when the devmapper isn't available may not initialize the value in the pointer passed as the second argument. The usage 'qemuDomainSetupDisk' lead to an accidental infinite loop as previous calls apparently doctored the stack to a point where 'g_slist_concat' would end up in an infinite loop trying to find the end of the list. Fixes: 6c49c2ee9fc Closes: https://gitlab.com/libvirt/libvirt/-/issues/268 Signed-off-by: Peter Krempa --- src/qemu/qemu_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 23b1160c5e..94453033f5 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -251,7 +251,7 @@ qemuDomainSetupDisk(virStorageSource *src, if (!(tmpPath = virPCIDeviceAddressGetIOMMUGroupDev(>nvme->pciAddr))) return -1; } else { -GSList *targetPaths; +GSList *targetPaths = NULL; if (virStorageSourceIsEmpty(next) || !virStorageSourceIsLocalStorage(next)) { -- 2.34.1
[libvirt PATCH v5 4/7] ch_driver: enable typed param string for numatune
From: Vineeth Pillai Enable support of VIR_DRV_FEATURE_TYPED_PARAM_STRING to enable numatune Signed-off-by: Vineeth Pillai Signed-off-by: Praveen K Paladugu --- src/ch/ch_driver.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 956189e83f..32ae15f940 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -943,6 +943,36 @@ static int chStateInitialize(bool privileged, return ret; } +/* Which features are supported by this driver? */ +static int +chConnectSupportsFeature(virConnectPtr conn, int feature) +{ +if (virConnectSupportsFeatureEnsureACL(conn) < 0) +return -1; + +switch ((virDrvFeature) feature) { +case VIR_DRV_FEATURE_TYPED_PARAM_STRING: +return 1; +case VIR_DRV_FEATURE_MIGRATION_V2: +case VIR_DRV_FEATURE_MIGRATION_V3: +case VIR_DRV_FEATURE_MIGRATION_P2P: +case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: +case VIR_DRV_FEATURE_FD_PASSING: +case VIR_DRV_FEATURE_XML_MIGRATABLE: +case VIR_DRV_FEATURE_MIGRATION_OFFLINE: +case VIR_DRV_FEATURE_MIGRATION_PARAMS: +case VIR_DRV_FEATURE_MIGRATION_DIRECT: +case VIR_DRV_FEATURE_MIGRATION_V1: +case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: +case VIR_DRV_FEATURE_REMOTE: +case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: +case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: +case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: +default: +return 0; +} +} + static int chDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) @@ -1285,6 +1315,7 @@ static virHypervisorDriver chHypervisorDriver = { .connectListAllDomains = chConnectListAllDomains, /* 7.5.0 */ .connectListDomains = chConnectListDomains, /* 7.5.0 */ .connectGetCapabilities = chConnectGetCapabilities, /* 7.5.0 */ +.connectSupportsFeature = chConnectSupportsFeature, /* 8.1.0 */ .domainCreateXML = chDomainCreateXML, /* 7.5.0 */ .domainCreate = chDomainCreate, /* 7.5.0 */ .domainCreateWithFlags = chDomainCreateWithFlags, /* 7.5.0 */ -- 2.27.0
[libvirt PATCH v5 0/7] cgroup and thread management in ch driver
This patchset adds support for cgroup management of ch threads. This version correctly manages cgroups for vcpu and emulator threads created by ch. cgroup management for iothreads is not yet supported. Along with cgroup management, this patchset also enables support for pinning vcpu and emulator threads to selected host cpus. v5: * bumped the verion of callbacks in ch driver to 8.1.0 v4: * addressed all open comments in v3 * dropped all the merged commits v3: * addrressed all the formatting comments in v2 patch set * dropped indentation patches are they do not adhere to libvirt coding style * fixed build issue in qemu driver that was introduced in v2 Praveen K Paladugu (3): qemu,hypervisor: refactor some cgroup mgmt methods ch_process: Setup emulator and iothread settings ch_driver: emulator threadinfo & pinning callbacks Vineeth Pillai (4): ch: methods for cgroup mgmt in ch driver ch_driver,ch_domain: vcpupin callback in ch driver ch_driver: enable typed param string for numatune ch_driver: add numatune callbacks for CH driver src/ch/ch_conf.c | 2 + src/ch/ch_conf.h | 4 +- src/ch/ch_domain.c | 64 src/ch/ch_domain.h | 18 +- src/ch/ch_driver.c | 590 + src/ch/ch_monitor.c| 156 + src/ch/ch_monitor.h| 56 +++- src/ch/ch_process.c| 385 - src/ch/ch_process.h| 3 + src/hypervisor/domain_cgroup.c | 457 - src/hypervisor/domain_cgroup.h | 72 src/libvirt_private.syms | 14 +- src/qemu/qemu_cgroup.c | 413 +-- src/qemu/qemu_cgroup.h | 11 - src/qemu/qemu_driver.c | 14 +- src/qemu/qemu_hotplug.c| 7 +- src/qemu/qemu_process.c| 24 +- 17 files changed, 1835 insertions(+), 455 deletions(-) -- 2.27.0
[libvirt PATCH v5 1/7] qemu, hypervisor: refactor some cgroup mgmt methods
Refactor some cgroup management methods from qemu into hypervisor. These methods will be shared with ch driver for cgroup management. Signed-off-by: Praveen K Paladugu --- src/hypervisor/domain_cgroup.c | 457 - src/hypervisor/domain_cgroup.h | 72 ++ src/libvirt_private.syms | 14 +- src/qemu/qemu_cgroup.c | 413 + src/qemu/qemu_cgroup.h | 11 - src/qemu/qemu_driver.c | 14 +- src/qemu/qemu_hotplug.c| 7 +- src/qemu/qemu_process.c| 24 +- 8 files changed, 580 insertions(+), 432 deletions(-) diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index 61b54f071c..05c53ff7d1 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -22,11 +22,12 @@ #include "domain_cgroup.h" #include "domain_driver.h" - +#include "util/virnuma.h" +#include "virlog.h" #include "virutil.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN - +VIR_LOG_INIT("domain.cgroup"); int virDomainCgroupSetupBlkio(virCgroup *cgroup, virDomainBlkiotune blkio) @@ -269,3 +270,455 @@ virDomainCgroupSetMemoryLimitParameters(virCgroup *cgroup, return 0; } + + +int +virDomainCgroupSetupBlkioCgroup(virDomainObj *vm, +virCgroup *cgroup) +{ + +if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { +if (vm->def->blkio.weight || vm->def->blkio.ndevices) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Block I/O tuning is not available on this host")); +return -1; +} else { +return 0; +} +} + +return virDomainCgroupSetupBlkio(cgroup, vm->def->blkio); +} + + +int +virDomainCgroupSetupMemoryCgroup(virDomainObj *vm, + virCgroup *cgroup) +{ + +if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { +if (virMemoryLimitIsSet(vm->def->mem.hard_limit) || +virMemoryLimitIsSet(vm->def->mem.soft_limit) || +virMemoryLimitIsSet(vm->def->mem.swap_hard_limit)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Memory cgroup is not available on this host")); +return -1; +} else { +return 0; +} +} + +return virDomainCgroupSetupMemtune(cgroup, vm->def->mem); +} + + +int +virDomainCgroupSetupCpusetCgroup(virCgroup *cgroup) +{ + +if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) +return 0; + +if (virCgroupSetCpusetMemoryMigrate(cgroup, true) < 0) +return -1; + +return 0; +} + + +int +virDomainCgroupSetupCpuCgroup(virDomainObj *vm, + virCgroup *cgroup) +{ + +if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPU)) { +if (vm->def->cputune.sharesSpecified) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CPU tuning is not available on this host")); +return -1; +} else { +return 0; +} +} + +if (vm->def->cputune.sharesSpecified) { + +if (virCgroupSetCpuShares(cgroup, vm->def->cputune.shares) < 0) +return -1; + +} + +return 0; +} + + +int +virDomainCgroupInitCgroup(const char *prefix, + virDomainObj * vm, + size_t nnicindexes, + int *nicindexes, + virCgroup * cgroup, + int cgroupControllers, + unsigned int maxThreadsPerProc, + bool privileged, + char *machineName) +{ +if (!privileged) +return 0; + +if (!virCgroupAvailable()) +return 0; + +virCgroupFree(cgroup); +cgroup = NULL; + +if (!vm->def->resource) +vm->def->resource = g_new0(virDomainResourceDef, 1); + +if (!vm->def->resource->partition) +vm->def->resource->partition = g_strdup("/machine"); + +if (!g_path_is_absolute(vm->def->resource->partition)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Resource partition '%s' must start with '/'"), + vm->def->resource->partition); +return -1; +} + +if (virCgroupNewMachine(machineName, +prefix, +vm->def->uuid, +NULL, +vm->pid, +false, +nnicindexes, nicindexes, +vm->def->resource->partition, +cgroupControllers, +maxThreadsPerProc, ) < 0) { +if (virCgroupNewIgnoreError()) +return 0; + +return -1; +} + +return 0; +} + + +void
[libvirt PATCH v5 3/7] ch_driver, ch_domain: vcpupin callback in ch driver
From: Vineeth Pillai Signed-off-by: Vineeth Pillai Signed-off-by: Praveen K Paladugu --- src/ch/ch_domain.c | 30 + src/ch/ch_domain.h | 7 ++- src/ch/ch_driver.c | 145 src/ch/ch_monitor.c | 2 +- 4 files changed, 181 insertions(+), 3 deletions(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index 6f0cec8c6e..6fde7122f7 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -20,6 +20,7 @@ #include +#include "datatypes.h" #include "ch_domain.h" #include "domain_driver.h" #include "viralloc.h" @@ -416,3 +417,32 @@ virCHDomainGetMachineName(virDomainObj *vm) return ret; } + +/** + * virCHDomainObjFromDomain: + * @domain: Domain pointer that has to be looked up + * + * This function looks up @domain and returns the appropriate virDomainObjPtr + * that has to be released by calling virDomainObjEndAPI(). + * + * Returns the domain object with incremented reference counter which is locked + * on success, NULL otherwise. + */ +virDomainObj * +virCHDomainObjFromDomain(virDomainPtr domain) +{ +virDomainObj *vm; +virCHDriver *driver = domain->conn->privateData; +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +vm = virDomainObjListFindByUUID(driver->domains, domain->uuid); +if (!vm) { +virUUIDFormat(domain->uuid, uuidstr); +virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, domain->name); +return NULL; +} + +return vm; +} diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h index cb94905b94..60a761ac84 100644 --- a/src/ch/ch_domain.h +++ b/src/ch/ch_domain.h @@ -97,5 +97,8 @@ virCHDomainGetVcpuPid(virDomainObj *vm, bool virCHDomainHasVcpuPids(virDomainObj *vm); -char * -virCHDomainGetMachineName(virDomainObj *vm); +char +*virCHDomainGetMachineName(virDomainObj *vm); + +virDomainObj +*virCHDomainObjFromDomain(virDomain *domain); diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 53e0872207..956189e83f 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -25,6 +25,7 @@ #include "ch_driver.h" #include "ch_monitor.h" #include "ch_process.h" +#include "domain_cgroup.h" #include "datatypes.h" #include "driver.h" #include "viraccessapicheck.h" @@ -1129,6 +1130,148 @@ chDomainGetVcpus(virDomainPtr dom, return ret; } +static int +chDomainPinVcpuLive(virDomainObj *vm, +virDomainDef *def, +int vcpu, +virCHDriver *driver, +virCHDriverConfig *cfg, +virBitmap *cpumap) +{ +g_autoptr(virBitmap) tmpmap = NULL; +g_autoptr(virCgroup) cgroup_vcpu = NULL; +virDomainVcpuDef *vcpuinfo; +virCHDomainObjPrivate *priv = vm->privateData; + +g_autofree char *str = NULL; + +if (!virCHDomainHasVcpuPids(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not supported")); +return -1; +} + +if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) { +virReportError(VIR_ERR_INVALID_ARG, + _("vcpu %d is out of range of live cpu count %d"), + vcpu, virDomainDefGetVcpusMax(def)); +return -1; +} + +if (!(tmpmap = virBitmapNewCopy(cpumap))) +return -1; + +if (!(str = virBitmapFormat(cpumap))) +return -1; + +if (vcpuinfo->online) { +/* Configure the corresponding cpuset cgroup before set affinity. */ +if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu, + false, _vcpu) < 0) +return -1; +if (virDomainCgroupSetupCpusetCpus(cgroup_vcpu, cpumap) < 0) +return -1; +} + +if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, false) < 0) +return -1; +} + +virBitmapFree(vcpuinfo->cpumask); +vcpuinfo->cpumask = tmpmap; +tmpmap = NULL; + +if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) +return -1; + +return 0; +} + + +static int +chDomainPinVcpuFlags(virDomainPtr dom, + unsigned int vcpu, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ +virCHDriver *driver = dom->conn->privateData; +virDomainObj *vm; +virDomainDef *def; +virDomainDef *persistentDef; +int ret = -1; +virBitmap *pcpumap = NULL; +virDomainVcpuDef *vcpuinfo = NULL; +g_autoptr(virCHDriverConfig) cfg = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +cfg = virCHDriverGetConfig(driver); + +if (!(vm = virCHDomainObjFromDomain(dom))) +goto cleanup; + +if (virDomainPinVcpuFlagsEnsureACL(dom->conn,
[libvirt PATCH v5 6/7] ch_process: Setup emulator and iothread settings
using virCHProcessSetupPid Signed-off-by: Praveen K Paladugu --- src/ch/ch_monitor.c | 60 +++ src/ch/ch_monitor.h | 2 ++ src/ch/ch_process.c | 77 - 3 files changed, 138 insertions(+), 1 deletion(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 6c1d889a82..2a7cffb696 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -930,3 +930,63 @@ virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info) { return virCHMonitorGet(mon, URL_VM_INFO, info); } + +/** + * virCHMonitorGetIOThreads: + * @mon: Pointer to the monitor + * @iothreads: Location to return array of IOThreadInfo data + * + * Retrieve the list of iothreads defined/running for the machine + * + * Returns count of IOThreadInfo structures on success + *-1 on error. + */ +int virCHMonitorGetIOThreads(virCHMonitor *mon, + virDomainIOThreadInfo ***iothreads) +{ +size_t nthreads = 0, niothreads = 0; +int thd_index; +virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL; + +*iothreads = NULL; +nthreads = virCHMonitorRefreshThreadInfo(mon); + +iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads); + +for (thd_index = 0; thd_index < nthreads; thd_index++) { +virBitmap *map = NULL; +if (mon->threads[thd_index].type == virCHThreadTypeIO) { +iothreadinfo = g_new0(virDomainIOThreadInfo, 1); + +iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid; + +if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id))) +goto cleanup; + +if (virBitmapToData(map, &(iothreadinfo->cpumap), +&(iothreadinfo->cpumaplen)) < 0) { +virBitmapFree(map); +goto cleanup; +} +virBitmapFree(map); +/* Append to iothreadinfolist */ +iothreadinfolist[niothreads] = iothreadinfo; +niothreads++; +} +} +VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist, + niothreads, nthreads); +*iothreads = iothreadinfolist; +VIR_DEBUG("niothreads = %ld", niothreads); +return niothreads; + +cleanup: +if (iothreadinfolist) { +for (thd_index = 0; thd_index < niothreads; thd_index++) +VIR_FREE(iothreadinfolist[thd_index]); +VIR_FREE(iothreadinfolist); +} +if (iothreadinfo) +VIR_FREE(iothreadinfo); +return -1; +} diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index 6646316454..ffc80e8910 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -119,3 +119,5 @@ int virCHMonitorGetCPUInfo(virCHMonitor *mon, size_t maxvcpus); size_t virCHMonitorGetThreadInfo(virCHMonitor *mon, bool refresh, virCHMonitorThreadInfo **threads); +int virCHMonitorGetIOThreads(virCHMonitor *mon, + virDomainIOThreadInfo ***iothreads); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 1a0730a4d1..2b81c9f757 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -41,7 +41,6 @@ VIR_LOG_INIT("ch.ch_process"); #define START_VM_POSTFIX ": starting up vm\n" - static virCHMonitor * virCHProcessConnectMonitor(virCHDriver *driver, virDomainObj *vm) @@ -310,6 +309,74 @@ virCHProcessSetupPid(virDomainObj *vm, return ret; } +static int +virCHProcessSetupIOThread(virDomainObj *vm, + virDomainIOThreadInfo *iothread) +{ +virCHDomainObjPrivate *priv = vm->privateData; +return virCHProcessSetupPid(vm, iothread->iothread_id, +VIR_CGROUP_THREAD_IOTHREAD, +iothread->iothread_id, +priv->autoCpuset, /* This should be updated when CLH supports accepting + iothread settings from input domain definition */ +vm->def->cputune.iothread_period, +vm->def->cputune.iothread_quota, +NULL); /* CLH doesn't allow choosing a scheduler for iothreads.*/ +} + +static int +virCHProcessSetupIOThreads(virDomainObj *vm) +{ +virCHDomainObjPrivate *priv = vm->privateData; +virDomainIOThreadInfo **iothreads = NULL; +size_t i; +size_t niothreads; + +niothreads = virCHMonitorGetIOThreads(priv->monitor, ); +for (i = 0; i < niothreads; i++) { +VIR_DEBUG("IOThread index = %ld , tid = %d", i, iothreads[i]->iothread_id); +if (virCHProcessSetupIOThread(vm, iothreads[i]) < 0) +return -1; +} +return 0; +} + + +static int +virCHProcessSetupEmulatorThread(virDomainObj *vm, + virCHMonitorEmuThreadInfo emuthread) +{ +return
[libvirt PATCH v5 2/7] ch: methods for cgroup mgmt in ch driver
From: Vineeth Pillai Signed-off-by: Vineeth Pillai Signed-off-by: Praveen K Paladugu --- src/ch/ch_conf.c| 2 + src/ch/ch_conf.h| 4 +- src/ch/ch_domain.c | 34 + src/ch/ch_domain.h | 11 +- src/ch/ch_monitor.c | 96 ++ src/ch/ch_monitor.h | 54 +++- src/ch/ch_process.c | 308 ++-- src/ch/ch_process.h | 3 + 8 files changed, 492 insertions(+), 20 deletions(-) diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c index 98f1e89003..be12934dcd 100644 --- a/src/ch/ch_conf.c +++ b/src/ch/ch_conf.c @@ -125,6 +125,8 @@ virCHDriverConfigNew(bool privileged) if (!(cfg = virObjectNew(virCHDriverConfigClass))) return NULL; +cfg->cgroupControllers = -1; /* Auto detect */ + if (privileged) { if (virGetUserID(CH_USER, >user) < 0) return NULL; diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h index 8fe69c8545..1790295ede 100644 --- a/src/ch/ch_conf.h +++ b/src/ch/ch_conf.h @@ -35,11 +35,13 @@ struct _virCHDriverConfig { char *stateDir; char *logDir; - +int cgroupControllers; uid_t user; gid_t group; }; +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCHDriverConfig, virObjectUnref); + struct _virCHDriver { virMutex lock; diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index a746d0f5fd..6f0cec8c6e 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -319,6 +319,40 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev, _("Serial can only be enabled for a PTY")); return -1; } +return 0; +} + +int +virCHDomainRefreshThreadInfo(virDomainObj *vm) +{ +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); +virCHMonitorThreadInfo *info = NULL; +size_t nthreads, ncpus = 0; +size_t i; + +nthreads = virCHMonitorGetThreadInfo(virCHDomainGetMonitor(vm), + true, ); + +for (i = 0; i < nthreads; i++) { +virCHDomainVcpuPrivate *vcpupriv; +virDomainVcpuDef *vcpu; +virCHMonitorCPUInfo *vcpuInfo; + +if (info[i].type != virCHThreadTypeVcpu) +continue; + +/* TODO: hotplug support */ +vcpuInfo = [i].vcpuInfo; +vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid); +vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu); +vcpupriv->tid = vcpuInfo->tid; +ncpus++; +} + +/* TODO: Remove the warning when hotplug is implemented.*/ +if (ncpus != maxvcpus) +VIR_WARN("Mismatch in the number of cpus, expected: %ld, actual: %ld", + maxvcpus, ncpus); return 0; } diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h index 4d0b5479b8..cb94905b94 100644 --- a/src/ch/ch_domain.h +++ b/src/ch/ch_domain.h @@ -53,11 +53,13 @@ typedef struct _virCHDomainObjPrivate virCHDomainObjPrivate; struct _virCHDomainObjPrivate { struct virCHDomainJobObj job; -virChrdevs *chrdevs; -virCHDriver *driver; -virCHMonitor *monitor; char *machineName; virBitmap *autoCpuset; +virBitmap *autoNodeset; +virCHDriver *driver; +virCHMonitor *monitor; +virCgroup *cgroup; +virChrdevs *chrdevs; }; #define CH_DOMAIN_PRIVATE(vm) \ @@ -87,7 +89,8 @@ void virCHDomainObjEndJob(virDomainObj *obj); int -virCHDomainRefreshVcpuInfo(virDomainObj *vm); +virCHDomainRefreshThreadInfo(virDomainObj *vm); + pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int vcpuid); diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index a19f0c7e33..d984bf9822 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -41,6 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor"); static virClass *virCHMonitorClass; static void virCHMonitorDispose(void *obj); +static void virCHMonitorThreadInfoFree(virCHMonitor * mon); static int virCHMonitorOnceInit(void) { @@ -578,6 +579,7 @@ static void virCHMonitorDispose(void *opaque) virCHMonitor *mon = opaque; VIR_DEBUG("mon=%p", mon); +virCHMonitorThreadInfoFree(mon); virObjectUnref(mon->vm); } @@ -743,6 +745,100 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue **response return ret; } +static void +virCHMonitorThreadInfoFree(virCHMonitor *mon) +{ +mon->nthreads = 0; +if (mon->threads) +VIR_FREE(mon->threads); +} + +static size_t +virCHMonitorRefreshThreadInfo(virCHMonitor *mon) +{ +virCHMonitorThreadInfo *info = NULL; +g_autofree pid_t *tids = NULL; +virDomainObj *vm = mon->vm; +size_t ntids = 0; +size_t i; + + +virCHMonitorThreadInfoFree(mon); +if (virProcessGetPids(vm->pid, , ) < 0) { +mon->threads = NULL; +return 0; +} + +info = g_new0(virCHMonitorThreadInfo, ntids); +for (i = 0; i < ntids; i++) { +g_autofree char *proc = NULL; +g_autofree char *data = NULL; + +proc = g_strdup_printf("/proc/%d/task/%d/comm", + (int)vm->pid,
[libvirt PATCH v5 7/7] ch_driver: emulator threadinfo & pinning callbacks
Signed-off-by: Praveen K Paladugu --- src/ch/ch_driver.c | 154 + 1 file changed, 154 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index d257c025ef..d60ff468f0 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -1303,6 +1303,158 @@ chDomainPinVcpu(virDomainPtr dom, VIR_DOMAIN_AFFECT_LIVE); } + + +static int +chDomainGetEmulatorPinInfo(virDomainPtr dom, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ +virDomainObj *vm = NULL; +virDomainDef *def; +virCHDomainObjPrivate *priv; +bool live; +int ret = -1; +virBitmap *cpumask = NULL; +g_autoptr(virBitmap) bitmap = NULL; +virBitmap *autoCpuset = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +if (!(vm = chDomObjFromDomain(dom))) +goto cleanup; + +if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0) +goto cleanup; + +if (!(def = virDomainObjGetOneDefState(vm, flags, ))) +goto cleanup; + +if (live) { +priv = vm->privateData; +autoCpuset = priv->autoCpuset; +} +if (def->cputune.emulatorpin) { +cpumask = def->cputune.emulatorpin; +} else if (def->cpumask) { +cpumask = def->cpumask; +} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && + autoCpuset) { +cpumask = autoCpuset; +} else { +if (!(bitmap = virHostCPUGetAvailableCPUsBitmap())) +goto cleanup; +cpumask = bitmap; +} + +virBitmapToDataBuf(cpumask, cpumaps, maplen); + +ret = 1; + + cleanup: +virDomainObjEndAPI(); +return ret; +} + +static int +chDomainPinEmulator(virDomainPtr dom, +unsigned char *cpumap, +int maplen, +unsigned int flags) +{ +virCHDriver *driver = dom->conn->privateData; +virDomainObj *vm; +virCgroup *cgroup_emulator = NULL; +virDomainDef *def; +virDomainDef *persistentDef; +int ret = -1; +virCHDomainObjPrivate *priv; +virBitmap *pcpumap = NULL; +g_autoptr(virCHDriverConfig) cfg = NULL; +g_autofree char *str = NULL; +virTypedParameterPtr eventParams = NULL; +int eventNparams = 0; +int eventMaxparams = 0; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +cfg = virCHDriverGetConfig(driver); + +if (!(vm = chDomObjFromDomain(dom))) +goto cleanup; + +if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0) +goto cleanup; + +if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0) +goto cleanup; + +if (virDomainObjGetDefs(vm, flags, , ) < 0) +goto endjob; + +priv = vm->privateData; + +if (!(pcpumap = virBitmapNewData(cpumap, maplen))) +goto endjob; + +if (virBitmapIsAllClear(pcpumap)) { +virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Empty cpu list for pinning")); +goto endjob; +} + +if (def) { +if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, + 0, false, _emulator) < 0) +goto endjob; + +if (virDomainCgroupSetupCpusetCpus(cgroup_emulator, pcpumap) < 0) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); +goto endjob; +} +} + +if (virProcessSetAffinity(vm->pid, pcpumap, false) < 0) +goto endjob; + +virBitmapFree(def->cputune.emulatorpin); +def->cputune.emulatorpin = NULL; + +if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) +goto endjob; + +if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) +goto endjob; + +str = virBitmapFormat(pcpumap); +if (virTypedParamsAddString(, , +, +VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN, +str) < 0) +goto endjob; + +} + + +ret = 0; + +endjob: +virCHDomainObjEndJob(vm); + +cleanup: +if (cgroup_emulator) +virCgroupFree(cgroup_emulator); +virBitmapFree(pcpumap); +virDomainObjEndAPI(); +return ret; +} + #define CH_NB_NUMA_PARAM 2 static int @@ -1603,6 +1755,8 @@ static virHypervisorDriver chHypervisorDriver = { .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 8.0.0 */ .domainPinVcpu = chDomainPinVcpu, /* 8.1.0 */
[libvirt PATCH v5 5/7] ch_driver: add numatune callbacks for CH driver
From: Vineeth Pillai Signed-off-by: Vineeth Pillai Signed-off-by: Praveen K Paladugu --- src/ch/ch_driver.c | 260 + 1 file changed, 260 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 32ae15f940..d257c025ef 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -43,6 +43,7 @@ #include "viruri.h" #include "virutil.h" #include "viruuid.h" +#include "virnuma.h" #define VIR_FROM_THIS VIR_FROM_CH @@ -1302,6 +1303,263 @@ chDomainPinVcpu(virDomainPtr dom, VIR_DOMAIN_AFFECT_LIVE); } +#define CH_NB_NUMA_PARAM 2 + +static int +chDomainGetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ +size_t i; +virDomainObj *vm = NULL; +virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; +virCHDomainObjPrivate *priv; +g_autofree char *nodeset = NULL; +int ret = -1; +virDomainDef *def = NULL; +bool live = false; +virBitmap *autoNodeset = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +if (!(vm = virCHDomainObjFromDomain(dom))) +return -1; +priv = vm->privateData; + +if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0) +goto cleanup; + +if (!(def = virDomainObjGetOneDefState(vm, flags, ))) +goto cleanup; + +if (live) +autoNodeset = priv->autoNodeset; + +if ((*nparams) == 0) { +*nparams = CH_NB_NUMA_PARAM; +ret = 0; +goto cleanup; +} + +for (i = 0; i < CH_NB_NUMA_PARAM && i < *nparams; i++) { +virMemoryParameterPtr param = [i]; + +switch (i) { +case 0: /* fill numa mode here */ +ignore_value(virDomainNumatuneGetMode(def->numa, -1, )); + +if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE, +VIR_TYPED_PARAM_INT, tmpmode) < 0) +goto cleanup; + +break; + +case 1: /* fill numa nodeset here */ +nodeset = virDomainNumatuneFormatNodeset(def->numa, autoNodeset, -1); + +if (!nodeset || +virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, +VIR_TYPED_PARAM_STRING, nodeset) < 0) +goto cleanup; + +nodeset = NULL; +break; + +/* coverity[dead_error_begin] */ +default: +break; +/* should not hit here */ +} +} + +if (*nparams > CH_NB_NUMA_PARAM) +*nparams = CH_NB_NUMA_PARAM; +ret = 0; + + cleanup: +virDomainObjEndAPI(); +return ret; +} + +static int +chDomainSetNumaParamsLive(virDomainObj *vm, + virBitmap *nodeset) +{ +g_autoptr(virCgroup) cgroup_temp = NULL; +virCHDomainObjPrivate *priv = vm->privateData; +g_autofree char *nodeset_str = NULL; +virDomainNumatuneMemMode mode; +size_t i = 0; + + +if (virDomainNumatuneGetMode(vm->def->numa, -1, ) == 0 && +mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("change of nodeset for running domain requires strict numa mode")); +return -1; +} + +if (!virNumaNodesetIsAvailable(nodeset)) +return -1; + +/* Ensure the cpuset string is formatted before passing to cgroup */ +if (!(nodeset_str = virBitmapFormat(nodeset))) +return -1; + +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, _temp) < 0 || +virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) +return -1; + + +for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { +virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i); + +if (!vcpu->online) +continue; + +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i, + false, _temp) < 0 || +virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) +return -1; + +return 0; +} + +for (i = 0; i < vm->def->niothreadids; i++) { +if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + vm->def->iothreadids[i]->iothread_id, + false, _temp) < 0 || +virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) +return -1; +} + +/* set nodeset for root cgroup */ +if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0) +return -1; + +return 0; +} + +static int +chDomainSetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, +
Re: [libvirt PATCH v7 08/18] qemu: Introduce virQEMUCapsTypeIsAccelerated
On Fri, Jan 21, 2022 at 06:54:48PM +0100, Andrea Bolognani wrote: > From: Roman Bolshakov > > It replaces hardcoded checks for KVM. It'll be cleaner to use > the function once multiple accelerators are supported in the > QEMU driver. > > Explicit KVM domain checks should be done only when a feature is > available only for KVM. > > Signed-off-by: Roman Bolshakov > Signed-off-by: Andrea Bolognani > Tested-by: Brad Laue > Tested-by: Christophe Fergeau > --- > src/qemu/qemu_capabilities.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH v7 07/18] qemu: Introduce virQEMUCapsAccelStr
On Fri, Jan 21, 2022 at 06:54:47PM +0100, Andrea Bolognani wrote: > From: Roman Bolshakov > > This makes possible to add more accelerators by touching less code and > reduces code duplication. > > Signed-off-by: Roman Bolshakov > Signed-off-by: Andrea Bolognani > Tested-by: Brad Laue > Tested-by: Christophe Fergeau > --- > src/qemu/qemu_capabilities.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 09/33] qemu_migration.c: forbid powernv domains migration
On Mon, Jan 24, 2022 at 10:09:30AM -0300, Daniel Henrique Barboza wrote: > > > On 1/21/22 11:33, Daniel P. Berrangé wrote: > > On Thu, Jan 20, 2022 at 10:52:12AM -0300, Daniel Henrique Barboza wrote: > > > The PowerNV machine does not implement any form of migration. > > > > What do you mean by that ? > > > > Migration is a general feature in QEMU, not typically something > > that a machine types opts in/out of. > > What I meant with this patch is that migration, albeit possible, will not > work. > These emulations doesn't implement vmstate() for their internal states or its > specific devices. > > The reason is a more complicated version of "because no one bother to do it". > It's > technically doable of course (as you said, every QEMU machine is migratable > per > default) but not something that we have in our long-term scope. > > > This patch had the intention to tell users to 'don't even bother migrating > these > domains, it doesn't work'. We can live without it and let QEMU error out > normally. > > > > > > It is possible for devices to register migration blockers to > > prevent it, but libvirt shouldn't try to second guess that. > > > I agree that this kind of handling is something that belongs to QEMU and it's > best that Libvirt doesn't speculate about it. > > > > > > Overall I'd like to see a clear justification for why libvirt > > should enforce a policy here, as opposed to letting QEMU > > accept or reject the migration. > > > I'm fine with dropping this patch. Ok, lets drop this one and rely on QEMU reporting the lack of support. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] conf: network: Allow hostnames to start with a number
From: Nicolas Lécureuil RFC952 mandated that hostnames would start with an alpha character. This requirement was later relaxed by RFC1123 which allowed hostnames to start with a number as well. https://datatracker.ietf.org/doc/html/rfc952 https://datatracker.ietf.org/doc/html/rfc1123#page-13 Signed-off-by: Nicolas Lécureuil Reviewed-by: Peter Krempa --- Pushed after review of a trivial merge request. src/conf/network_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 10d3330fdf..c769bbaeb5 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -548,7 +548,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName, } name = virXMLPropString(node, "name"); -if (name && (!g_ascii_isalpha(name[0]))) { +if (name && !(g_ascii_isalpha(name[0]) || g_ascii_isdigit(name[0]))) { virReportError(VIR_ERR_XML_ERROR, _("Cannot use host name '%s' in network '%s'"), name, networkName); -- 2.34.1
Re: [PATCH v1] meson: recognize sles when guessing default_qemu_user
Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering : > Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, > /etc/os-release must exist during build. Now that a missing ^ID= became fatal, was it actually a good idea to be explicit? I think poking around for clues in ^ID_LIKE= will be more robust, appending 'sles' would be not required. Olaf pgp3FI9zmzCv7.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH 3/3] qemuDomainDetachDeviceLive: Handle hostevs with unassigned type of address
On a Tuesday in 2022, Michal Privoznik wrote: A can have which means libvirt manages the device detach from/reattach to the host but the device is never exposed to the guest. This means that we have to take a shortcut during hotunplug (e.g. never ask QEMU on the monitor to detach the device, or never wait for DEVICE_DELETED event). Signed-off-by: Michal Privoznik --- src/qemu/qemu_hotplug.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b998b51f5a..bde5f683d8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6043,6 +6043,7 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, { virDomainDeviceDef detach = { .type = match->type }; virDomainDeviceInfo *info = NULL; +bool unassigned = false; int ret = -1; switch ((virDomainDeviceType)match->type) { @@ -6181,6 +6182,8 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, return -1; } +unassigned = info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED; + if (qemuIsMultiFunctionDevice(vm->def, info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug %s device with multifunction PCI guest address: " @@ -6225,8 +6228,10 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, * Issue the qemu monitor command to delete the device (based on * its alias), and optionally wait a short time in case the * DEVICE_DELETED event arrives from qemu right away. + * Unassigned devices are not exposed to QEMU, so mimic + * !async case. */ -if (!async) +if (!async || unassigned) qemuDomainMarkDeviceForRemoval(vm, info); This does not seem right - we won't be waiting for an event from QEMU so we don't need to mark the alias for unassigned devices. if (qemuDomainDeleteDevice(vm, info->alias) < 0) { qemuDomainDeleteDevice is the one that calls 'device_del' on the monitor, it should not be called for unassigned devices. @@ -6235,11 +6240,13 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, goto cleanup; } -if (async) { +if (async && !unassigned) { ret = 0; } else { -if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) +if (unassigned || +(ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) { ret = qemuDomainRemoveDevice(driver, vm, ); RemoveDevice is the only interesting function. Instead of prefixing each line of code with if (unassigned) or if (!unassigned) depending on whether it makes sense for unassigned hostdevs, it would be more readable to call qemuDomainRemoveDevice earlier and return early. Jano +} } cleanup: -- 2.34.1 signature.asc Description: PGP signature
Re: [PATCH 2/3] qemuDomainAttachHostPCIDevice: Handle hostevs with unassigned type of address
On a Tuesday in 2022, Michal Privoznik wrote: A can have which means libvirt manages the device detach from/reattach to the host but the device is never exposed to the guest. This means that we have to take a shortcut during hotplug, similar to the one we are taking when constructing the command line (see qemuBuildHostdevCommandLine()). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2040548 Signed-off-by: Michal Privoznik --- src/qemu/qemu_hotplug.c | 7 +++ 1 file changed, 7 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 1/3] domain_validate: Refuse VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED
On a Tuesday in 2022, Michal Privoznik wrote: We document that can be used only for -s. However, corresponding validation rule is missing. Let's put the rule into hypervisor agnostic part of validation process so that all drivers can benefit. Signed-off-by: Michal Privoznik --- src/conf/domain_validate.c | 45 ++ 1 file changed, 45 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH v1] meson: recognize sles when guessing default_qemu_user
NAME="SLES" VERSION="15-SP3" VERSION_ID="15.3" PRETTY_NAME="SUSE Linux Enterprise Server 15 SP3" ID="sles" ID_LIKE="suse" ANSI_COLOR="0;32" CPE_NAME="cpe:/o:suse:sles:15:sp3" DOCUMENTATION_URL="https://documentation.suse.com/; Signed-off-by: Olaf Hering --- meson.build | 1 + 1 file changed, 1 insertion(+) Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, /etc/os-release must exist during build. diff --git a/meson.build b/meson.build index b6d1286f3f..b2d9e8ed65 100644 --- a/meson.build +++ b/meson.build @@ -1673,6 +1673,7 @@ if not get_option('driver_qemu').disabled() os_release.contains('fedora') or os_release.contains('gentoo') or os_release.contains('rhel') or +os_release.contains('sles') or os_release.contains('suse')) default_qemu_user = 'qemu' default_qemu_group = 'qemu'
Re: [PATCH 11/33] conf: add 'pnv-phb3-root-port' domain definition
On Tue, Jan 25, 2022 at 09:40:48 -0300, Daniel Henrique Barboza wrote: > > > On 1/21/22 11:17, Peter Krempa wrote: > > On Thu, Jan 20, 2022 at 10:52:14 -0300, Daniel Henrique Barboza wrote: > > > Apart from being usable only with pnv-phb3 PCIE host bridges (to be > > > added soon), this device acts as a regular pcie-root-port but with a > > > specific model name. > > > > > > Signed-off-by: Daniel Henrique Barboza > > > --- > > > docs/schemas/domaincommon.rng | 1 + > > > src/conf/domain_conf.c| 1 + > > > src/conf/domain_conf.h| 1 + > > > src/qemu/qemu_validate.c | 2 ++ > > > 4 files changed, 5 insertions(+) > > > > [...] > > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > > index 393f9d9478..c540b740df 100644 > > > --- a/src/conf/domain_conf.c > > > +++ b/src/conf/domain_conf.c > > > @@ -437,6 +437,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, > > > "pcie-root-port", > > > "spapr-pci-host-bridge", > > > "pcie-pci-bridge", > > > + "pnv-phb3-root-port", > > > ); > > > > Missing corresponding 'docs/formatdomain.rst' change. > > The reason I didn't add a note in that doc is because of this specific > paragraph: > > "PCI controllers also have an optional subelement with an attribute > name. The name > attribute holds the name of the specific device that qemu is emulating (e.g. > "i82801b11-bridge") > rather than simply the class of device ("pcie-to-pci-bridge", "pci-bridge"), > which is set in > the controller element's model attribute. In almost all cases, you should not > manually add a > subelement to a controller, nor should you modify one that is > automatically generated > by libvirt. Since 1.2.19 (QEMU only)." > > This, summed up with the fact that not all PCI model names are documented in > this doc (e.g. > the ioh3420 root port model name), gave me the impression that we don't > want/bother to > specify these details to the user. > > > All this said, I can add a patch that documents all the model names currently > supported, > then I can add the new stuff on top of it. No with the documentation stating that it's not really for users and having an already existing status-quo of not mentioning what we already have it's okay. Just mention in the commit message the reason for not adding docs.
Re: [PATCH 11/33] conf: add 'pnv-phb3-root-port' domain definition
On 1/21/22 11:17, Peter Krempa wrote: On Thu, Jan 20, 2022 at 10:52:14 -0300, Daniel Henrique Barboza wrote: Apart from being usable only with pnv-phb3 PCIE host bridges (to be added soon), this device acts as a regular pcie-root-port but with a specific model name. Signed-off-by: Daniel Henrique Barboza --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 1 + src/conf/domain_conf.h| 1 + src/qemu/qemu_validate.c | 2 ++ 4 files changed, 5 insertions(+) [...] diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 393f9d9478..c540b740df 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -437,6 +437,7 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "pcie-root-port", "spapr-pci-host-bridge", "pcie-pci-bridge", + "pnv-phb3-root-port", ); Missing corresponding 'docs/formatdomain.rst' change. The reason I didn't add a note in that doc is because of this specific paragraph: "PCI controllers also have an optional subelement with an attribute name. The name attribute holds the name of the specific device that qemu is emulating (e.g. "i82801b11-bridge") rather than simply the class of device ("pcie-to-pci-bridge", "pci-bridge"), which is set in the controller element's model attribute. In almost all cases, you should not manually add a subelement to a controller, nor should you modify one that is automatically generated by libvirt. Since 1.2.19 (QEMU only)." This, summed up with the fact that not all PCI model names are documented in this doc (e.g. the ioh3420 root port model name), gave me the impression that we don't want/bother to specify these details to the user. All this said, I can add a patch that documents all the model names currently supported, then I can add the new stuff on top of it. Thanks, Daniel
Re: [PATCH] libxl: Add lock process indicator to saved VM state
On 1/25/22 03:16, Jim Fehlig wrote: > Commit fa58f571ee added a lock processes indicator to the > libxlDomainObjPrivate struct to note that a lock process was > successfully started for the VM. However, the commit neglected to > add the indicator to the VM's saved state file. As a result, the > indicator is lost on libvirtd restart, along with the knowledge of > whether a lock process was started for the VM. > > This change adds support for the indicator in the domainObjPrivate > data parse and format callbacks, ensuring its value survives libvirtd > restarts. > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_domain.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index feca60f7d2..8962adc60f 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -226,6 +226,7 @@ libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, > libxlDomainObjPrivate *priv = vm->privateData; > > priv->lockState = virXPathString("string(./lockstate)", ctxt); > +priv->lockProcessRunning = virXPathBoolean("count(./lockProcessRunning) > > 0", ctxt); Alternatively, virXPathBoolean("boolean(./lockProcessRunning)", ctxt); should do the same. It may be less expensive to evaluate than count(). > > return 0; > } > @@ -239,6 +240,9 @@ libxlDomainObjPrivateXMLFormat(virBuffer *buf, > if (priv->lockState) > virBufferAsprintf(buf, "%s\n", > priv->lockState); > > +if (priv->lockProcessRunning) > +virBufferAddLit(buf, "\n"); > + > return 0; > } > Reviewed-by: Michal Privoznik Michal
[PATCH 3/3] qemuDomainDetachDeviceLive: Handle hostevs with unassigned type of address
A can have which means libvirt manages the device detach from/reattach to the host but the device is never exposed to the guest. This means that we have to take a shortcut during hotunplug (e.g. never ask QEMU on the monitor to detach the device, or never wait for DEVICE_DELETED event). Signed-off-by: Michal Privoznik --- src/qemu/qemu_hotplug.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b998b51f5a..bde5f683d8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6043,6 +6043,7 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, { virDomainDeviceDef detach = { .type = match->type }; virDomainDeviceInfo *info = NULL; +bool unassigned = false; int ret = -1; switch ((virDomainDeviceType)match->type) { @@ -6181,6 +6182,8 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, return -1; } +unassigned = info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED; + if (qemuIsMultiFunctionDevice(vm->def, info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug %s device with multifunction PCI guest address: " @@ -6225,8 +6228,10 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, * Issue the qemu monitor command to delete the device (based on * its alias), and optionally wait a short time in case the * DEVICE_DELETED event arrives from qemu right away. + * Unassigned devices are not exposed to QEMU, so mimic + * !async case. */ -if (!async) +if (!async || unassigned) qemuDomainMarkDeviceForRemoval(vm, info); if (qemuDomainDeleteDevice(vm, info->alias) < 0) { @@ -6235,11 +6240,13 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, goto cleanup; } -if (async) { +if (async && !unassigned) { ret = 0; } else { -if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) +if (unassigned || +(ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) { ret = qemuDomainRemoveDevice(driver, vm, ); +} } cleanup: -- 2.34.1
[PATCH 2/3] qemuDomainAttachHostPCIDevice: Handle hostevs with unassigned type of address
A can have which means libvirt manages the device detach from/reattach to the host but the device is never exposed to the guest. This means that we have to take a shortcut during hotplug, similar to the one we are taking when constructing the command line (see qemuBuildHostdevCommandLine()). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2040548 Signed-off-by: Michal Privoznik --- src/qemu/qemu_hotplug.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f36de2385a..b998b51f5a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1668,6 +1668,12 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, qemuDomainFillDeviceIsolationGroup(vm->def, ); } +if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) { +/* Unassigned devices are not exposed to QEMU. Our job is done here. */ +ret = 0; +goto done; +} + if (qemuDomainEnsurePCIAddress(vm, ) < 0) goto error; releaseaddr = true; @@ -1692,6 +1698,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, exit_monitor: qemuDomainObjExitMonitor(driver, vm); + done: virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) goto error; -- 2.34.1
[PATCH 0/3] Fix hotplug of unassigned hostdevs
Honestly, I don't understand the reasoning behind (esp. when the device is not accessible to the guest anyway). We offer virNodeDeviceDettach() and virNodeDeviceReAttach(). But apparently there are some issues with hotplug/hotunplug of hostdevs with such type of address. Michal Prívozník (3): domain_validate: Refuse VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED qemuDomainAttachHostPCIDevice: Handle hostevs with unassigned type of address qemuDomainDetachDeviceLive: Handle hostevs with unassigned type of address src/conf/domain_validate.c | 45 ++ src/qemu/qemu_hotplug.c| 20 ++--- 2 files changed, 62 insertions(+), 3 deletions(-) -- 2.34.1
[PATCH 1/3] domain_validate: Refuse VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED
We document that can be used only for -s. However, corresponding validation rule is missing. Let's put the rule into hypervisor agnostic part of validation process so that all drivers can benefit. Signed-off-by: Michal Privoznik --- src/conf/domain_validate.c | 45 ++ 1 file changed, 45 insertions(+) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index e9baf1d41a..d5e74db243 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2257,10 +2257,55 @@ virDomainGraphicsDefValidate(const virDomainDef *def, return 0; } +static int +virDomainDeviceInfoValidate(const virDomainDeviceDef *dev) +{ +virDomainDeviceInfo *info; + +if (!(info = virDomainDeviceGetInfo(dev))) +return 0; + +switch ((virDomainDeviceAddressType) info->type) { +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM: +/* No validation for these address types yet */ +break; + +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED: +if (dev->type != VIR_DOMAIN_DEVICE_HOSTDEV) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("address of type '%s' is supported only for hostdevs"), + virDomainDeviceAddressTypeToString(info->type)); +return -1; +} +break; + +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST: +default: +virReportEnumRangeError(virDomainDeviceAddressType, info->type); +return -1; +} + +return 0; +} + static int virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, const virDomainDef *def) { +if (virDomainDeviceInfoValidate(dev) < 0) +return -1; + switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: return virDomainDiskDefValidate(def, dev->data.disk); -- 2.34.1
[PATCH] qemuDomainAttachHostPCIDevice: Fix coding style
Our coding style requires that a body of an if() longer than two lines is wrapped in a curly braces. There's one offender in qemuDomainAttachHostPCIDevice(). Fortunately, there was no functional problem because one of the lines is a comment. Signed-off-by: Michal Privoznik --- Pushed under trivial rule. src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 22acbd0852..f36de2385a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1663,9 +1663,10 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, qemuAssignDeviceHostdevAlias(vm->def, >alias, -1); -if (qemuDomainIsPSeries(vm->def)) +if (qemuDomainIsPSeries(vm->def)) { /* Isolation groups are only relevant for pSeries guests */ qemuDomainFillDeviceIsolationGroup(vm->def, ); +} if (qemuDomainEnsurePCIAddress(vm, ) < 0) goto error; -- 2.34.1
Re: [PATCH v2] virProcessGetStatInfo: add a comment describing why we can not report error
ping ... On Fri, 21 Jan 2022, Ani Sinha wrote: > virProcessGetStatInfo() currently is unable to report error conditions because > that breaks libvirt's public best effort APIs. We add a comment in the > function > to indicate this. Adding comment here prevents others from going down the path > of reporting error conditions in this functions in the future. It also reminds > us that at some point in the future we need to fix the code so that this > limitations no longer exists. > > Please also see commit > 105dace22cc7 ("Revert "report error when virProcessGetStatInfo() is unable to > parse data"") > > Signed-off-by: Ani Sinha > --- > src/util/virprocess.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/src/util/virprocess.c b/src/util/virprocess.c > index b559a4257e..9422829b8b 100644 > --- a/src/util/virprocess.c > +++ b/src/util/virprocess.c > @@ -1784,6 +1784,12 @@ virProcessGetStatInfo(unsigned long long *cpuTime, > virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, > ) < 0 || > virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) < 0 > || > virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, > ) < 0) { > +/* This function can not report error at present. Reporting error > here > + * causes some of libvirt's best effort public APIs to fail. This > + * resuts in external API behavior change. Until we can fix this in > + * a way so that public API behavior remains unchanged, we can only > + * write a warning log here. > + */ > VIR_WARN("cannot parse process status data"); > } > > -- > 2.25.1 > >
Re: [libvirt PATCH 0/2] qemu: support the SeaBIOS/EDK2 debug console
Hi, > > On balance I felt was/is the winner. Essentially every > > impl of can be said to be a general purpose data > > channel for passing arbitrary bytes. We choose to explicitly > > distinguish from based on the intended use > > case of the thing. > > This interpretation directly contradicts our documentation, > specifically > > https://libvirt.org/formatdomain.html#console > > The console element is used to represent interactive serial > consoles. Yep. Something where it make sense to run a getty on. Which surely isn't the case for isa-debugcon ... take care, Gerd