[PATCH 0/6] add support for pvpanic-pci device
*** BLURB HERE *** Kristina Hanicova (6): qemu: introduce QEMU_CAPS_DEVICE_PANIC_PCI conf: add panic model 'pvpanic' tests: add test cases for device pvpanic-pci qemu: assign PCI address to device pvpanic-pci tests: add case for pvpanic-pci without address docs: document panic device 'pvpanic-pci' docs/formatdomain.rst | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 19 +++++ src/qemu/qemu_domain_address.c | 34 ++++++++- src/qemu/qemu_validate.c | 16 +++++ .../caps_6.0.0.aarch64.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0.ppc64.xml | 1 + .../caps_7.1.0.x86_64.xml | 1 + .../caps_7.2.0.x86_64.xml | 1 + .../caps_8.0.0.riscv64.xml | 1 + .../caps_8.0.0.x86_64.xml | 1 + .../pvpanic-pci-aarch64.aarch64-latest.args | 43 ++++++++++++ .../qemuxml2argvdata/pvpanic-pci-aarch64.xml | 20 ++++++ ...invalid-address-aarch64.aarch64-latest.err | 1 + .../pvpanic-pci-invalid-address-aarch64.xml | 20 ++++++ ...pci-no-address-aarch64.aarch64-latest.args | 41 +++++++++++ .../pvpanic-pci-no-address-aarch64.xml | 18 +++++ .../pvpanic-pci-x86_64.x86_64-latest.args | 43 ++++++++++++ tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml | 31 +++++++++ tests/qemuxml2argvtest.c | 4 ++ .../pvpanic-pci-aarch64.aarch64-latest.xml | 63 +++++++++++++++++ ...-pci-no-address-aarch64.aarch64-latest.xml | 53 ++++++++++++++ .../pvpanic-pci-x86_64.x86_64-latest.xml | 69 +++++++++++++++++++ tests/qemuxml2xmltest.c | 4 ++ 36 files changed, 499 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-no-address-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml -- 2.39.1
This capability detects the availability of the pvpanic-pci device that is required in order to use pvpanic on Arm (original pvpanic is an emulated ISA device, for which Arm does not have support). --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.1.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 + 16 files changed, 17 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9c3650e022..d92d5a62dd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -686,6 +686,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "netdev.stream", /* QEMU_CAPS_NETDEV_STREAM */ "virtio-crypto", /* QEMU_CAPS_DEVICE_VIRTIO_CRYPTO */ "cryptodev-backend-lkcf", /* QEMU_CAPS_OBJECT_CRYPTO_LKCF */ + "pvpanic-pci", /* QEMU_CAPS_DEVICE_PANIC_PCI */ ); @@ -1398,6 +1399,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-crypto-pci", QEMU_CAPS_DEVICE_VIRTIO_CRYPTO }, { "virtio-crypto-device", QEMU_CAPS_DEVICE_VIRTIO_CRYPTO }, { "cryptodev-backend-lkcf", QEMU_CAPS_OBJECT_CRYPTO_LKCF }, + { "pvpanic-pci", QEMU_CAPS_DEVICE_PANIC_PCI }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 5bf87c2f8d..b72348cf88 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -665,6 +665,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_NETDEV_STREAM, /* -netdev stream */ QEMU_CAPS_DEVICE_VIRTIO_CRYPTO, /* virtio-crypto device */ QEMU_CAPS_OBJECT_CRYPTO_LKCF, /* -object cryptodev-backend-lkcf */ + QEMU_CAPS_DEVICE_PANIC_PCI, /* -device pvpanic-pci */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index f09c286054..8238fb5d16 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -144,6 +144,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 4a6b88aa10..7a592ff6aa 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -184,6 +184,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index f303f439d6..4da1d42276 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -189,6 +189,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml index a573d465fb..33c9982bd1 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml @@ -156,6 +156,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml index 56269483c0..c485733d83 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml @@ -150,6 +150,7 @@ <flag name='virtio-net.rss'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index e9f65434ee..1f6f16bd4f 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -191,6 +191,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml index 16c4df09c2..6517cd71f6 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml @@ -164,6 +164,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>6002092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index 302ab8dacb..8dae2495e4 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -168,6 +168,7 @@ <flag name='usb-host.guest-resets-all'/> <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>7000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index 893b5670e5..90b83f2995 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -197,6 +197,7 @@ <flag name='migration.blocked-reasons'/> <flag name='sgx-epc'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>7000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.1.0.ppc64.xml index 5ffc2213c1..e0e9a4693f 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0.ppc64.xml @@ -168,6 +168,7 @@ <flag name='query-stats-schemas'/> <flag name='screenshot-format-png'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>7001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml index 5d7d98bb56..1f05dd4ae3 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml @@ -200,6 +200,7 @@ <flag name='query-stats-schemas'/> <flag name='screenshot-format-png'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>7001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml index fccf9b7923..13f7b66b69 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0.x86_64.xml @@ -204,6 +204,7 @@ <flag name='netdev.stream'/> <flag name='virtio-crypto'/> <flag name='cryptodev-backend-lkcf'/> + <flag name='pvpanic-pci'/> <version>7002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100245</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_8.0.0.riscv64.xml index 0e273f168a..751320433a 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0.riscv64.xml @@ -143,6 +143,7 @@ <flag name='screenshot-format-png'/> <flag name='netdev.stream'/> <flag name='virtio-crypto'/> + <flag name='pvpanic-pci'/> <version>7002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml index c8b7f95c4a..56be34799c 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml @@ -204,6 +204,7 @@ <flag name='netdev.stream'/> <flag name='virtio-crypto'/> <flag name='cryptodev-backend-lkcf'/> + <flag name='pvpanic-pci'/> <version>7002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> -- 2.39.1
This patch introduces optional device pvpanic-pci, validates it's address and generates command line. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_command.c | 19 +++++++++++++++++++ src/qemu/qemu_validate.c | 16 ++++++++++++++++ 5 files changed, 38 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff1c78ecd1..c03ca36ae0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -859,6 +859,7 @@ VIR_ENUM_IMPL(virDomainPanicModel, "pseries", "hyperv", "s390", + "pvpanic", ); VIR_ENUM_IMPL(virDomainVideoBackend, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e684edcf95..d2cbc200e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2677,6 +2677,7 @@ typedef enum { VIR_DOMAIN_PANIC_MODEL_PSERIES, VIR_DOMAIN_PANIC_MODEL_HYPERV, VIR_DOMAIN_PANIC_MODEL_S390, + VIR_DOMAIN_PANIC_MODEL_PVPANIC, VIR_DOMAIN_PANIC_MODEL_LAST } virDomainPanicModel; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a57dd212ab..ab4886b783 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8289,6 +8289,7 @@ <value>pseries</value> <value>hyperv</value> <value>s390</value> + <value>pvpanic</value> </choice> </attribute> </optional> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 90dc6b5434..64fee74671 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9557,6 +9557,25 @@ qemuBuildPanicCommandLine(virCommand *cmd, break; } + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: { + g_autoptr(virJSONValue) props = NULL; + + if (virJSONValueObjectAdd(&props, + "s:driver", "pvpanic-pci", + NULL) < 0) + return -1; + + if (def->panics[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (qemuBuildDeviceAddressProps(props, def, &def->panics[i]->info) < 0) + return -1; + } + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; + + break; + } + case VIR_DOMAIN_PANIC_MODEL_S390: case VIR_DOMAIN_PANIC_MODEL_HYPERV: case VIR_DOMAIN_PANIC_MODEL_PSERIES: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c8c289ebb4..87adaaebdc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1146,6 +1146,22 @@ qemuValidateDomainDefPanic(const virDomainDef *def, } break; + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support the " + "PCI pvpanic device")); + return -1; + } + + if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pvpanic is supported only " + "with PCI address type")); + return -1; + } + break; + /* default model value was changed before in post parse */ case VIR_DOMAIN_PANIC_MODEL_DEFAULT: case VIR_DOMAIN_PANIC_MODEL_LAST: -- 2.39.1
On Wed, Feb 08, 2023 at 12:49:01 +0100, Kristina Hanicova wrote:
This patch introduces optional device pvpanic-pci, validates it's address and generates command line.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_command.c | 19 +++++++++++++++++++ src/qemu/qemu_validate.c | 16 ++++++++++++++++ 5 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 90dc6b5434..64fee74671 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9557,6 +9557,25 @@ qemuBuildPanicCommandLine(virCommand *cmd, break; }
+ case VIR_DOMAIN_PANIC_MODEL_PVPANIC: { + g_autoptr(virJSONValue) props = NULL; + + if (virJSONValueObjectAdd(&props, + "s:driver", "pvpanic-pci", + NULL) < 0) + return -1; + + if (def->panics[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
This check doesn't make much sense ...
+ if (qemuBuildDeviceAddressProps(props, def, &def->panics[i]->info) < 0)
... as this does the correct thing for _NONE ...
+ return -1; + } + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; + + break; + } + case VIR_DOMAIN_PANIC_MODEL_S390: case VIR_DOMAIN_PANIC_MODEL_HYPERV: case VIR_DOMAIN_PANIC_MODEL_PSERIES: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c8c289ebb4..87adaaebdc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1146,6 +1146,22 @@ qemuValidateDomainDefPanic(const virDomainDef *def, } break;
+ case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support the " + "PCI pvpanic device"));
Please no linebreaks in error messages.
+ return -1; + } + + if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
... and you also force the correct type here.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pvpanic is supported only " + "with PCI address type"));
Same as above.
+ return -1; + } + break; + /* default model value was changed before in post parse */ case VIR_DOMAIN_PANIC_MODEL_DEFAULT: case VIR_DOMAIN_PANIC_MODEL_LAST: -- 2.39.1
On Wed, Feb 08, 2023 at 01:05:59PM +0100, Peter Krempa wrote:
On Wed, Feb 08, 2023 at 12:49:01 +0100, Kristina Hanicova wrote:
+++ b/src/qemu/qemu_command.c @@ -9557,6 +9557,25 @@ qemuBuildPanicCommandLine(virCommand *cmd, break; }
+ case VIR_DOMAIN_PANIC_MODEL_PVPANIC: { + g_autoptr(virJSONValue) props = NULL; + + if (virJSONValueObjectAdd(&props, + "s:driver", "pvpanic-pci", + NULL) < 0) + return -1; + + if (def->panics[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
This check doesn't make much sense ...
I imagine it was lifted from the ISA variant, handled just above, where it's necessary because not specifying an address is somehow considered a valid configuration. I agree with you that it's not needed for pvpanic-pci. -- Andrea Bolognani / Red Hat / Virtualization
On a Wednesday in 2023, Kristina Hanicova wrote:
This patch introduces optional device pvpanic-pci, validates it's
*its Jano
address and generates command line.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_command.c | 19 +++++++++++++++++++ src/qemu/qemu_validate.c | 16 ++++++++++++++++ 5 files changed, 38 insertions(+)
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- .../pvpanic-pci-aarch64.aarch64-latest.args | 43 ++++++++++++ .../qemuxml2argvdata/pvpanic-pci-aarch64.xml | 20 ++++++ ...invalid-address-aarch64.aarch64-latest.err | 1 + .../pvpanic-pci-invalid-address-aarch64.xml | 20 ++++++ .../pvpanic-pci-x86_64.x86_64-latest.args | 43 ++++++++++++ tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml | 31 +++++++++ tests/qemuxml2argvtest.c | 4 ++ .../pvpanic-pci-aarch64.aarch64-latest.xml | 63 +++++++++++++++++ .../pvpanic-pci-x86_64.x86_64-latest.xml | 69 +++++++++++++++++++ tests/qemuxml2xmltest.c | 3 + 10 files changed, 297 insertions(+) create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args new file mode 100644 index 0000000000..3199cd0dbc --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine virt-6.0,usb=off,gic-version=2,dump-guest-core=off,memory-backend=mach-virt.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \ +-accel tcg \ +-cpu cortex-a15 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-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":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-pci-bridge","id":"pci.2","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"pci-bridge","chassis_nr":3,"id":"pci.3","bus":"pci.2","addr":"0x1"}' \ +-device '{"driver":"pci-bridge","chassis_nr":4,"id":"pci.4","bus":"pci.2","addr":"0x2"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":5,"id":"pci.5","bus":"pcie.0","addr":"0x1.0x1"}' \ +-device '{"driver":"pcie-root-port","port":10,"chassis":6,"id":"pci.6","bus":"pcie.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.5","addr":"0x0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-device '{"driver":"pvpanic-pci","bus":"pci.4","addr":"0x1"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml new file mode 100644 index 0000000000..7e2954e9cf --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err new file mode 100644 index 0000000000..4e74ba5cc9 --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err @@ -0,0 +1 @@ +unsupported configuration: pvpanic is supported only with PCI address type diff --git a/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml new file mode 100644 index 0000000000..baececb2db --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args new file mode 100644 index 0000000000..b0e173113d --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args @@ -0,0 +1,43 @@ +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-x86_64 \ +-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 pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-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 \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"pci-bridge","chassis_nr":1,"id":"pci.1","bus":"pci.0","addr":"0x2"}' \ +-device '{"driver":"pci-bridge","chassis_nr":2,"id":"pci.2","bus":"pci.0","addr":"0x3"}' \ +-device '{"driver":"pci-bridge","chassis_nr":3,"id":"pci.3","bus":"pci.0","addr":"0x4"}' \ +-device '{"driver":"pci-bridge","chassis_nr":4,"id":"pci.4","bus":"pci.0","addr":"0x5"}' \ +-device '{"driver":"pci-bridge","chassis_nr":5,"id":"pci.5","bus":"pci.0","addr":"0x6"}' \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x7"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-device '{"driver":"pvpanic-pci","bus":"pci.5","addr":"0x1"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml new file mode 100644 index 0000000000..4d8a56cb41 --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ae0afb6f1c..4e3815559c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2615,6 +2615,10 @@ mymain(void) DO_TEST_CAPS_LATEST("panic-double"); DO_TEST_CAPS_LATEST("panic-no-address"); + DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("pvpanic-pci-invalid-address-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_VER_FULL("fips-enabled", "x86_64", "5.1.0", ARG_FLAGS, FLAG_FIPS_HOST); DO_TEST_CAPS_ARCH_LATEST_FULL("fips-enabled", "x86_64", ARG_FLAGS, FLAG_FIPS_HOST); diff --git a/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml new file mode 100644 index 0000000000..bb47c4c2a6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml @@ -0,0 +1,63 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-to-pci-bridge'> + <model name='pcie-pci-bridge'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='4'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='5' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='6' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </memballoon> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml b/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml new file mode 100644 index 0000000000..81a984e793 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml @@ -0,0 +1,69 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='1' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='5'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </memballoon> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ae9716062d..269b2d8016 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -908,6 +908,9 @@ mymain(void) DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC); DO_TEST_CAPS_ARCH_LATEST("panic-pseries", "ppc64"); + DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_NOCAPS("disk-backing-chains-index"); DO_TEST_NOCAPS("disk-backing-chains-noindex"); -- 2.39.1
On Wed, Feb 08, 2023 at 12:49:02PM +0100, Kristina Hanicova wrote:
+++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/>
You can use <memballoon model='none'/> here and in the other input XMLs for slightly smaller output files.
+ <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x01' function='0x0'/> + </panic>
This explicit address could be on pcie.0, e.g. <address type='pci' domain='0x0000' bus='0x0' slot='0x04' function='0x0'/> to prevent the pcie-pci-bridge and pci-bridge controllers from being added and, again, produce slightly smaller output files. More discussion about device placement in the upcoming reply to a later patch :) -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Feb 09, 2023 at 07:47:45AM -0800, Andrea Bolognani wrote:
On Wed, Feb 08, 2023 at 12:49:02PM +0100, Kristina Hanicova wrote:
+ <memballoon model='virtio'/>
You can use
<memballoon model='none'/>
here and in the other input XMLs for slightly smaller output files.
You can similarly drop the disk, as well as the fdc and ide controllers, from the x86_64 input file. In general, the smaller the input and output files are, the more obvious it becomes what scenario the test is supposed to cover and the easier it is to focus on just that. Note that none of this is a requirement, just my own (fairly strong) preference :) So if you don't agree that spending a bit of time minimizing the test files is a worthwhile investment, feel free to ignore these comments. -- Andrea Bolognani / Red Hat / Virtualization
It makes sense to accept pvpanic-pci also without specified PCI address and assign one if possible. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_domain_address.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 20b648354b..e80d543ce8 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1062,10 +1062,24 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, } break; + case VIR_DOMAIN_DEVICE_PANIC: + switch ((virDomainPanicModel) dev->data.panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + return pciFlags; + + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_ISA: + case VIR_DOMAIN_PANIC_MODEL_PSERIES: + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + case VIR_DOMAIN_PANIC_MODEL_S390: + case VIR_DOMAIN_PANIC_MODEL_LAST: + return 0; + } + break; + /* These devices don't ever connect with PCI */ case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_REDIRDEV: case VIR_DOMAIN_DEVICE_SMARTCARD: @@ -2454,6 +2468,24 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, return -1; } + for (i = 0; i < def->npanics; i++) { + virDomainPanicDef *panic = def->panics[i]; + + switch (panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + if (virDeviceInfoPCIAddressIsWanted(&panic->info) && + qemuDomainPCIAddressReserveNextAddr(addrs, &panic->info) < 0) + return -1; + break; + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_ISA: + case VIR_DOMAIN_PANIC_MODEL_PSERIES: + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + case VIR_DOMAIN_PANIC_MODEL_S390: + case VIR_DOMAIN_PANIC_MODEL_LAST: + break; + } + } return 0; } -- 2.39.1
On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
+++ b/src/qemu/qemu_domain_address.c @@ -1062,10 +1062,24 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, } break;
+ case VIR_DOMAIN_DEVICE_PANIC: + switch ((virDomainPanicModel) dev->data.panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + return pciFlags;
So, this is correct, because pvpanic-pci is indeed a conventional PCI device (as opposed to a PCI Express device). However, when used with a PCIe-based machine such as aarch64/virt, these flags result in the device being plugged into a pcie-pci-bridge, which in turn is plugged into a pcie-root-port, itself plugged into pcie.0. While this seems to work fine, it's also kind of a waste of PCI controllers. For a "system" device such as pvpanic-pci, I think plugging directly into pcie.0 might be more appropriate. This is particularly true of x86/q35, where with the current implementation switching from ISA pvpanic to pvpanic-pci would result in adding two extra PCI controllers. So I think this would be a good fit for the VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use with virtio-iommu. While this would technically result in libvirt being more restrictive than QEMU in what PCI addresses it accepts for pvpanic-pci, I don't think this would limit users in practice, and in the default case (libvirt automatically picking the address) the resulting configuration would be preferable. We can always decide to relax this restriction later down the line, if it turns out to really be a limiting factor. Eric, what do you think about this idea? (There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work with pciFlags at the moment O:-) It works fine with pcieFlags and virtioFlags. I'll try to figure out why that's the case.) -- Andrea Bolognani / Red Hat / Virtualization
Hi A?drea, Kristina, On 2/9/23 17:33, Andrea Bolognani wrote:
On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
+++ b/src/qemu/qemu_domain_address.c @@ -1062,10 +1062,24 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, } break;
+ case VIR_DOMAIN_DEVICE_PANIC: + switch ((virDomainPanicModel) dev->data.panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + return pciFlags; So, this is correct, because pvpanic-pci is indeed a conventional PCI device (as opposed to a PCI Express device).
However, when used with a PCIe-based machine such as aarch64/virt, these flags result in the device being plugged into a pcie-pci-bridge, which in turn is plugged into a pcie-root-port, itself plugged into pcie.0.
While this seems to work fine, it's also kind of a waste of PCI controllers. For a "system" device such as pvpanic-pci, I think plugging directly into pcie.0 might be more appropriate. This is particularly true of x86/q35, where with the current implementation switching from ISA pvpanic to pvpanic-pci would result in adding two extra PCI controllers.
So I think this would be a good fit for the VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use with virtio-iommu. While this would technically result in libvirt being more restrictive than QEMU in what PCI addresses it accepts for pvpanic-pci, I don't think this would limit users in practice, and in the default case (libvirt automatically picking the address) the resulting configuration would be preferable.
We can always decide to relax this restriction later down the line, if it turns out to really be a limiting factor.
Eric, what do you think about this idea?
I do agree. If we can avoid putting that device downstream to a pcie-to-pci bridge that's much better. Putting it onto pcie.0 looks more appropriate to me indeed. Thanks Eric
(There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work with pciFlags at the moment O:-) It works fine with pcieFlags and virtioFlags. I'll try to figure out why that's the case.)
On Thu, Feb 09, 2023 at 06:26:14PM +0100, Eric Auger wrote:
On 2/9/23 17:33, Andrea Bolognani wrote:
On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
+++ b/src/qemu/qemu_domain_address.c @@ -1062,10 +1062,24 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, } break;
+ case VIR_DOMAIN_DEVICE_PANIC: + switch ((virDomainPanicModel) dev->data.panic->model) { + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + return pciFlags;
So, this is correct, because pvpanic-pci is indeed a conventional PCI device (as opposed to a PCI Express device).
However, when used with a PCIe-based machine such as aarch64/virt, these flags result in the device being plugged into a pcie-pci-bridge, which in turn is plugged into a pcie-root-port, itself plugged into pcie.0.
While this seems to work fine, it's also kind of a waste of PCI controllers. For a "system" device such as pvpanic-pci, I think plugging directly into pcie.0 might be more appropriate. This is particularly true of x86/q35, where with the current implementation switching from ISA pvpanic to pvpanic-pci would result in adding two extra PCI controllers.
So I think this would be a good fit for the VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use with virtio-iommu. While this would technically result in libvirt being more restrictive than QEMU in what PCI addresses it accepts for pvpanic-pci, I don't think this would limit users in practice, and in the default case (libvirt automatically picking the address) the resulting configuration would be preferable.
We can always decide to relax this restriction later down the line, if it turns out to really be a limiting factor.
Eric, what do you think about this idea?
I do agree. If we can avoid putting that device downstream to a pcie-to-pci bridge that's much better. Putting it onto pcie.0 looks more appropriate to me indeed.
Thanks for the input! Let's go with this approach then, unless someone raises concerns with it.
(There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work with pciFlags at the moment O:-) It works fine with pcieFlags and virtioFlags. I'll try to figure out why that's the case.)
Fixed by this patch: https://listman.redhat.com/archives/libvir-list/2023-February/237688.html -- Andrea Bolognani / Red Hat / Virtualization
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- ...pci-no-address-aarch64.aarch64-latest.args | 41 ++++++++++++++ .../pvpanic-pci-no-address-aarch64.xml | 18 +++++++ ...-pci-no-address-aarch64.aarch64-latest.xml | 53 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 113 insertions(+) create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-no-address-aarch64.aarch64-latest.xml diff --git a/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.aarch64-latest.args b/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.aarch64-latest.args new file mode 100644 index 0000000000..d6a8bccbbd --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.aarch64-latest.args @@ -0,0 +1,41 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine virt-6.0,usb=off,gic-version=2,dump-guest-core=off,memory-backend=mach-virt.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \ +-accel tcg \ +-cpu cortex-a15 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-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":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ +-device '{"driver":"pcie-pci-bridge","id":"pci.3","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"pcie-root-port","port":10,"chassis":4,"id":"pci.4","bus":"pcie.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.2","addr":"0x0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-device '{"driver":"pvpanic-pci","bus":"pci.3","addr":"0x1"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.xml b/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.xml new file mode 100644 index 0000000000..04cf1d6832 --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-no-address-aarch64.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/> + <panic model='pvpanic'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pvpanic-pci-no-address-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/pvpanic-pci-no-address-aarch64.aarch64-latest.xml new file mode 100644 index 0000000000..9000f6e1a0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pvpanic-pci-no-address-aarch64.aarch64-latest.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='3' model='pcie-to-pci-bridge'> + <model name='pcie-pci-bridge'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='4' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </memballoon> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 269b2d8016..3a652dde61 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -910,6 +910,7 @@ mymain(void) DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-no-address-aarch64", "aarch64"); DO_TEST_NOCAPS("disk-backing-chains-index"); DO_TEST_NOCAPS("disk-backing-chains-noindex"); -- 2.39.1
On Wed, Feb 08, 2023 at 12:49:04PM +0100, Kristina Hanicova wrote:
+++ b/tests/qemuxml2xmltest.c @@ -910,6 +910,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-no-address-aarch64", "aarch64");
You have forgotten the corresponding qemuxml2argvtest change. The output file is there, so I know you have made the change locally ;) -- Andrea Bolognani / Red Hat / Virtualization
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- docs/formatdomain.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 36c6d87907..c62e184169 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7940,6 +7940,7 @@ Example: usage of panic configuration - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU and KVM only` - 's390' - default for S390 guests. :since:`Since 1.3.5` + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and KVM only` ``address`` address of panic. The default ioport is 0x505. Most users don't need to -- 2.39.1
On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- docs/formatdomain.rst | 1 + 1 file changed, 1 insertion(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 36c6d87907..c62e184169 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7940,6 +7940,7 @@ Example: usage of panic configuration - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU and KVM only` - 's390' - default for S390 guests. :since:`Since 1.3.5` + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and KVM only`
s/devicen/device/ Also is KVM really required? I'd expect that it will also work with TCG VMs.
On Wed, Feb 08, 2023 at 01:09:05PM +0100, Peter Krempa wrote:
On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote:
+++ b/docs/formatdomain.rst @@ -7940,6 +7940,7 @@ Example: usage of panic configuration - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU and KVM only` - 's390' - default for S390 guests. :since:`Since 1.3.5` + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and KVM only`
s/devicen/device/
Also is KVM really required? I'd expect that it will also work with TCG VMs.
I think so as well. But to be honest I'm unclear on what "QEMU and KVM only", as used extensively throughout the document, is intended to mean. Does it mean "only when using the QEMU driver and its KVM domain type", or rather "only when using the QEMU or KVM domain type"? The latter sounds more likely to me, and it would be accurate for the pvpanic-pci device. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Feb 09, 2023 at 08:41:01 -0800, Andrea Bolognani wrote:
On Wed, Feb 08, 2023 at 01:09:05PM +0100, Peter Krempa wrote:
On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote:
+++ b/docs/formatdomain.rst @@ -7940,6 +7940,7 @@ Example: usage of panic configuration - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU and KVM only` - 's390' - default for S390 guests. :since:`Since 1.3.5` + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and KVM only`
s/devicen/device/
Also is KVM really required? I'd expect that it will also work with TCG VMs.
I think so as well. But to be honest I'm unclear on what "QEMU and KVM only", as used extensively throughout the document, is intended to mean. Does it mean "only when using the QEMU driver and its KVM domain type", or rather "only when using the QEMU or KVM domain type"? The latter sounds more likely to me, and it would be accurate for the pvpanic-pci device.
Generaly in our docs "QEMU only" means that it works only with the qemu driver/hypervisor, thus I don't think the interpretation that <domain type='qemu'> or kvm will work here.
On Thu, Feb 09, 2023 at 06:32:12PM +0100, Peter Krempa wrote:
On Thu, Feb 09, 2023 at 08:41:01 -0800, Andrea Bolognani wrote:
On Wed, Feb 08, 2023 at 01:09:05PM +0100, Peter Krempa wrote:
On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote:
+++ b/docs/formatdomain.rst @@ -7940,6 +7940,7 @@ Example: usage of panic configuration - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU and KVM only` - 's390' - default for S390 guests. :since:`Since 1.3.5` + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and KVM only`
s/devicen/device/
Also is KVM really required? I'd expect that it will also work with TCG VMs.
I think so as well. But to be honest I'm unclear on what "QEMU and KVM only", as used extensively throughout the document, is intended to mean. Does it mean "only when using the QEMU driver and its KVM domain type", or rather "only when using the QEMU or KVM domain type"? The latter sounds more likely to me, and it would be accurate for the pvpanic-pci device.
Generaly in our docs "QEMU only" means that it works only with the qemu driver/hypervisor, thus I don't think the interpretation that <domain type='qemu'> or kvm will work here.
A few counterexamples:
Watchdog devices A virtual hardware watchdog device can be added to the guest via the watchdog element. Since 0.7.3, QEMU and KVM only
Memory balloon device A virtual memory balloon device is added to all Xen and KVM/QEMU guests. It will be seen as memballoon element. It will be automatically added when appropriate, so there is no need to explicitly add this element in the guest XML unless a specific PCI slot needs to be assigned. Since 0.8.3, Xen, QEMU and KVM only
Firmware The firmware attribute allows management applications to automatically fill <loader/> and <nvram/> elements and possibly enable some features required by selected firmware. [...] Since 5.2.0 (QEMU and KVM only)
ROM The optional file attribute contains an absolute path to a binary file to be presented to the guest as the device's ROM BIOS. This can be useful, for example, to provide a PXE boot ROM for a virtual function of an sr-iov capable ethernet device (which has no boot ROMs for the VFs). Since 0.9.10 (QEMU and KVM only)
All of the above work perfectly fine with TCG, at least as far as I know. There are a few instances of "(QEMU/KVM only)" in the document, and even a couple of "(KVM only)". tl;dr We're awfully inconsistent about this. I'm okay with dropping the "and KVM" part from this patch. I'd also be very much okay with someone[1] going through the document and changing it to use a single, unambiguous way to indicate whether a feature works with TCG or is restricted with KVM. [1] No, I'm not volunteering to be that someone :) -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Feb 08, 2023 at 12:48:59 +0100, Kristina Hanicova wrote:
Kristina Hanicova (6): qemu: introduce QEMU_CAPS_DEVICE_PANIC_PCI conf: add panic model 'pvpanic' tests: add test cases for device pvpanic-pci qemu: assign PCI address to device pvpanic-pci tests: add case for pvpanic-pci without address
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
docs: document panic device 'pvpanic-pci'
If you agree that dropping the 'KVM' part of where pvpanic-pci works is fine then you can also use my R-b on this one.
On Wed, Feb 08, 2023 at 12:48:59PM +0100, Kristina Hanicova wrote:
qemu: introduce QEMU_CAPS_DEVICE_PANIC_PCI conf: add panic model 'pvpanic' tests: add test cases for device pvpanic-pci qemu: assign PCI address to device pvpanic-pci tests: add case for pvpanic-pci without address docs: document panic device 'pvpanic-pci'
Stray observations: * an entry in the release notes would certainly be warranted for this nice user-visible improvement; * I noticed that panic devices don't currently have aliases associated with them: this looks like an oversight rather than a conscious design decision, and it would be great if you would consider addressing that in a follow-up series :) -- Andrea Bolognani / Red Hat / Virtualization
participants (5)
-
Andrea Bolognani -
Eric Auger -
Ján Tomko -
Kristina Hanicova -
Peter Krempa