Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On 10/4/21 1:05 PM, Andrea Bolognani wrote: On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: On Mon, 4 Oct 2021, Andrea Bolognani wrote: On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: +++ b/src/qemu/qemu_capabilities.c + /* 410 */ + "piix4-acpi-root-hotplug-en", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ Sorry if this is a silly question, but can you please explain where the "-en" suffix comes from? "en" stands for enable. AFAICT there are no other capabilities that use this convention. Moreover, the option can be used both to enable *and* disable hotplug, so the name doesn't seem accurate either... The string is not exposed to users, so ultimately none of this really matters, but I would still suggest changing it to piix4.acpi-root-pci-hotplug I'd be happy to either review a patch of yours that does that, or preparing such a patch myself. BAH!!! Someone else mentioned that in an earlier iteration of the patches (and may have even suggested the "." after piix4 instead of "-"), and I had meant to check/ask about it in the respin, and then forgot. :-/ (somehow my brain skipped right over those 3 characters) I agree with Andrea's suggested change, and apologize for not catching it in review (fortunately it was pushed just *after* a release instead of before, so we can still make such a change). I'll gladly review and/or push any such patch that arrives.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, 4 Oct 2021, Andrea Bolognani wrote: > On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: > > On Mon, 4 Oct 2021, Andrea Bolognani wrote: > > > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > > > +++ b/src/qemu/qemu_capabilities.c > > > > + /* 410 */ > > > > + "piix4-acpi-root-hotplug-en", /* > > > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > > > > > Sorry if this is a silly question, but can you please explain where > > > the "-en" suffix comes from? > > > > "en" stands for enable. > > AFAICT there are no other capabilities that use this convention. > Moreover, the option can be used both to enable *and* disable > hotplug, so the name doesn't seem accurate either... > > The string is not exposed to users, so ultimately none of this really > matters, but I would still suggest changing it to > > piix4.acpi-root-pci-hotplug > > I'd be happy to either review a patch of yours that does that, or > preparing such a patch myself. Sent a patch to rename it.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: > On Mon, 4 Oct 2021, Andrea Bolognani wrote: > > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > > +++ b/src/qemu/qemu_capabilities.c > > > + /* 410 */ > > > + "piix4-acpi-root-hotplug-en", /* > > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > > > Sorry if this is a silly question, but can you please explain where > > the "-en" suffix comes from? > > "en" stands for enable. AFAICT there are no other capabilities that use this convention. Moreover, the option can be used both to enable *and* disable hotplug, so the name doesn't seem accurate either... The string is not exposed to users, so ultimately none of this really matters, but I would still suggest changing it to piix4.acpi-root-pci-hotplug I'd be happy to either review a patch of yours that does that, or preparing such a patch myself. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, 4 Oct 2021, Andrea Bolognani wrote: > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > +++ b/src/qemu/qemu_capabilities.c > > + /* 410 */ > > + "piix4-acpi-root-hotplug-en", /* > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > Sorry if this is a silly question, but can you please explain where > the "-en" suffix comes from? "en" stands for enable.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > +++ b/src/qemu/qemu_capabilities.c > + /* 410 */ > + "piix4-acpi-root-hotplug-en", /* > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ Sorry if this is a silly question, but can you please explain where the "-en" suffix comes from? -- Andrea Bolognani / Red Hat / Virtualization
[PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
The following change in qemu added support for a global boolean flag specific
to i440fx machines that would turn off or on acpi based hotplug for pci root
bus:
3d7e78aaf ("Introduce a new flag for i440fx to disable PCI hotplug on the
root bus")
The option is passed as "-global PIIX4_PM.acpi-root-pci-hotplug=on" etc in qemu
commandline. It is enabled by default. This patch adds the corresponding qemu
capabilities in libvirt as QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG.
Please note that the test specific qemu capabilities .replies files has already
been updated as a part of regular refreshing them when a new qemu version is
released. Hence, no updates to those files are required.
Signed-off-by: Ani Sinha
Reviewed-by: Laine Stump
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 3 +++
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
5 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a1be0cb74e..9d0c96a20c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -639,6 +639,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */
"set-action", /* QEMU_CAPS_SET_ACTION */
"virtio-blk.queue-size", /* QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE */
+
+ /* 410 */
+ "piix4-acpi-root-hotplug-en", /*
QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */
);
@@ -1465,6 +1468,7 @@ static struct virQEMUCapsDevicePropsFlags
virQEMUCapsDevicePropsIDEDrive[] = {
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPiix4PM[] = {
{ "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3, NULL },
{ "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4, NULL },
+{ "acpi-root-pci-hotplug", QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, NULL },
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b0fa1eec35..3460daac00 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -620,6 +620,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_SET_ACTION, /* 'set-action' QMP command */
QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE, /* virtio-blk-*.queue-size */
+/* 410 */
+QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, /* -M pc
PIIX4_PM.acpi-root-pci-hotplug */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 515a970f28..1e5833a9f0 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -230,6 +230,7 @@
+
5002000
0
43100243
diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
index 02eb9a15bb..b54dd8a22e 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
@@ -238,6 +238,7 @@
+
600
0
43100242
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 678b440d92..0ad493191d 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -240,6 +240,7 @@
+
6001000
0
43100243
--
2.25.1
