Re: [PATCH] qemu: capabilities: rename piix4-acpi-root-hotplug-en to more appropriate name
On Mon, 4 Oct 2021, Laine Stump wrote: > On 10/4/21 1:26 PM, Ani Sinha wrote: > > The capability name piix4-acpi-root-hotplug-en is not conventional and > > appreared to be confusing to some. "en" suffix is also incorrect as the > > capability in qemu is used to both enable and disable hotplug on the pci > > root > > bus on the i440fx. Hence, rename it to piix4.acpi-root-pci-hotplug so that > > it > > is clearer, less confusing and more accurate. > > > > Signed-off-by: Ani Sinha > > --- > > src/qemu/qemu_capabilities.c | 2 +- > > tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 2 +- > > tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 2 +- > > tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 2 +- > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > > index 9d0c96a20c..d7b79ef7c0 100644 > > --- a/src/qemu/qemu_capabilities.c > > +++ b/src/qemu/qemu_capabilities.c > > @@ -641,7 +641,7 @@ VIR_ENUM_IMPL(virQEMUCaps, > > "virtio-blk.queue-size", /* QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE > > */ > > /* 410 */ > > - "piix4-acpi-root-hotplug-en", /* > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > + "piix4.acpi-root-pci-hotplug", /* > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > Since the enum matched the capability name exactly except that one is "piix4" > and the other is "PIIX", I changed the enum to match this new name before > pushing. > > Reviewed-by: Laine Stump > > Will be pushed in a few minutes as soon as gitlab CI finishes running on my > review branch. Alright. I have made equivalent changes for pci-hotplug-bridge patchset as well. Please take a look. Hopefully we can wrap that patchset within this week before we all start forgetting the context. > > > ); > > diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml > > b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml > > index 1e5833a9f0..834fb86636 100644 > > --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml > > +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml > > @@ -230,7 +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 b54dd8a22e..e9d1a26400 100644 > > --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml > > +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml > > @@ -238,7 +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 0ad493191d..971d55e0cc 100644 > > --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml > > +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml > > @@ -240,7 +240,7 @@ > > > > > > > > - > > + > > 6001000 > > 0 > > 43100243 > > > >
[PATCH v6 3/4] qemu: command: add support for acpi-bridge-hotplug feature
This change adds backend qemu command line support for new libvirt global
feature 'acpi-bridge-hotplug'. This option can be used as following:
The '' sub-element under '' is also newly introduced.
'acpi-bridge-hotplug' turns on the following command line option to qemu for
x86 guests:
(pc machines): -global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=
(q35 machines): -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=
This change also adds the required qemuxml2argv unit tests in order to test
correct qemu arguments. Unit tests have also been added to test qemu capability
validation checks as well as checks for using this option with the right
architecture.
Signed-off-by: Ani Sinha
---
src/qemu/qemu_command.c | 14
.../aarch64-acpi-hotplug-bridge-disable.err | 1 +
...pc-i440fx-acpi-hotplug-bridge-disable.args | 31 +
.../pc-i440fx-acpi-hotplug-bridge-disable.err | 1 +
.../q35-acpi-hotplug-bridge-disable.args | 33 +++
.../q35-acpi-hotplug-bridge-disable.err | 1 +
tests/qemuxml2argvtest.c | 16 +
7 files changed, 97 insertions(+)
create mode 100644
tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
create mode 100644
tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
create mode 100644
tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 08f6d735f8..6c8c99e595 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6069,6 +6069,7 @@ qemuBuildPMCommandLine(virCommand *cmd,
qemuDomainObjPrivate *priv)
{
virQEMUCaps *qemuCaps = priv->qemuCaps;
+int acpihp_br =
def->pci_features[(virDomainPCI)VIR_DOMAIN_PCI_ACPI_BRIDGE_HP];
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) {
/* with new qemu we always want '-no-shutdown' on startup and we set
@@ -6114,6 +6115,19 @@ qemuBuildPMCommandLine(virCommand *cmd,
pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO);
}
+if (acpihp_br) {
+const char *pm_object = "PIIX4_PM";
+
+if (qemuDomainIsQ35(def) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE))
+pm_object = "ICH9-LPC";
+
+virCommandAddArg(cmd, "-global");
+virCommandAddArgFormat(cmd,
"%s.acpi-pci-hotplug-with-bridge-support=%s",
+ pm_object,
+ virTristateSwitchTypeToString(acpihp_br));
+}
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
new file mode 100644
index 00..9f0a88b826
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
@@ -0,0 +1 @@
+unsupported configuration: acpi-bridge-hotplug is not available for
architecture 'aarch64'
diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
new file mode 100644
index 00..a1e5dc85c7
--- /dev/null
+++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-i440fx \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-i440fx/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-i440fx/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-i440fx/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=i440fx,debug-threads=on \
+-S \
+-object
secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-i440fx/master-key.aes \
+-machine pc-i440fx-2.5,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 56f5055c-1b8d-490c-844a-ad646a1c \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-i440fx/monitor.sock,server=on,wait=off
\
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off \
+-boot strict=on \
+-usb \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
new file mode 100644
index 00..8c09a3cd76
--- /dev/null
+++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
@@ -0,0 +1 @@
+unsupported configuration: acpi-bridge-hotplug is not available with this QEMU
binary
diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
[PATCH v6 0/4] introduce support for acpi-bridge-hotplug feature
changelog:
v6: rebased to latest. capabilities have been renamed as per suggestions that
were made here:
https://listman.redhat.com/archives/libvir-list/2021-October/msg00061.html
v5: rebased the patchset with the latest master.
v4: split the original series into two - pci-root controller specific one
(https://www.mail-archive.com/libvir-list@redhat.com/msg221645.html)
and this one specific to pci bridges.
The conf xml has been introduced as per suggestion by Berrange here:
https://patchew.org/Libvirt/20210912032631.2853520-1-...@anisinha.ca
Changes has been introduced to parse and validate the xml accordingly
as well as to add backend qemu commandline option.
v3: reorganized the patches as per Laine's suggestion. Added more
details in commit messages. Added conf description in formatdomain.rst.
Added changelog for next release.
v2: fixed bugs and added additional missing unit tests.
v1: initial implementation. Had some bugs and missed some unit tests
This change introduces a new libvirt sub-element under that
can be used to configure all pci related features.
Currently the only sub-sub element supported by this sub-element is
'acpi-bridge-hotplug' as shown below:
The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.
'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option are required. For pc machine type in x86, this option
is available in qemu for a long time, from version 2.1.
Please see the following changes in qemu repo:
9e047b982452c6 ("piix4: add acpi pci hotplug support")
133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge
hotplug is disabled")
For q35 machine type, this was introduced in qemu 6.1 with the following
changes in qemu repo:
(a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
(b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
The reasons for enabling ACPI based hotplug for PCIe (q35) based machines (as
opposed to native hotplug) are outlined in (b). There are use cases where users
would still want to use native hotplug (see notes). Therefore, this config
option
enables users to choose either ACPI based hotplug or native hotplug for bridges
(for example for pcie root port controller in q35 machines).
Notes:
One concrete example of why one might still want to use native hotplug with
pcie-root-port controller is the fact that we are still discovering issues with
acpi hotplug on PCIE. One such issue is:
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02146.html
Another reason is that users have been using native hotplug on pcie root ports
up until now. They have built and tested their systems based on native hotplug.
They may not want to suddenly move to acpi based hotplug just because it is now
the default in qemu. Supporting the option to chose one or the other through
libvirt makes things simpler for end users.
Ani Sinha (4):
qemu: capablities: detect presence of
acpi-pci-hotplug-with-bridge-support
conf: introduce support for acpi-bridge-hotplug feature
qemu: command: add support for acpi-bridge-hotplug feature
NEWS: add new acpi pci hotplug config option in the release note for
next release
NEWS.rst | 7 ++
docs/formatdomain.rst | 11 +++
docs/schemas/domaincommon.rng | 15
src/conf/domain_conf.c| 89 ++-
src/conf/domain_conf.h| 9 ++
src/qemu/qemu_capabilities.c | 4 +
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 14 +++
src/qemu/qemu_validate.c | 46 ++
.../caps_2.11.0.x86_64.xml| 1 +
.../caps_2.12.0.x86_64.xml| 1 +
.../caps_3.0.0.x86_64.xml | 1 +
.../caps_3.1.0.x86_64.xml | 1 +
.../caps_4.0.0.x86_64.xml | 1 +
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 2 +
.../aarch64-acpi-hotplug-bridge-disable.err | 1 +
.../aarch64-acpi-hotplug-bridge-disable.xml | 33 +++
...pc-i440fx-acpi-hotplug-bridge-disable.args | 31 +++
.../pc-i440fx-acpi-hotplug-bridge-disable.err | 1 +
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
[PATCH v6 4/4] NEWS: add new acpi pci hotplug config option in the release note for next release
Added the following new libvirt conf option to the release note to indicate their availability with the next release: Signed-off-by: Ani Sinha --- NEWS.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index caa61f0646..25de621c91 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -32,6 +32,13 @@ v7.9.0 (unreleased) controller. The default behavior is unchanged (hotplug is allowed). + * Add a new global feature for controlling ACPI PCI hotplug on bridges + +A new config option under +sub-element have been added to allow users control BIOS ACPI based PCI +hotplug for cold plugged bridges. It is only applicable for x86 guests +using qemu driver. + * **Improvements** * **Bug fixes** -- 2.25.1
[PATCH v6 1/4] qemu: capablities: detect presence of acpi-pci-hotplug-with-bridge-support
qemu added support for i440fx specific global boolean flag
PIIX4_PM.acpi-pci-hotplug-with-bridge-support
around version 2.1. This flag is enabled by default. When disabled, it turns
off acpi pci hotplug for cold plugged pci bridges in i440fx machine types.
Very recently, in qemu version 6.1, the same global option was also added for
q35 machine types as well.
ICH9-LPC.acpi-pci-hotplug-with-bridge-support
This option turns on or off acpi based hotplug for cold plugged pcie bridges
like pcie root ports. This flag is also enabled by default. Please refer to
the following qemu changes:
c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
This patch adds the corresponding qemu capabilities in libvirt. For i440fx,
the capability is detected as QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE. For q35,
the capability is detected as QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE.
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 | 2 ++
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
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 | 2 ++
14 files changed, 19 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 82687dbf39..c4d0e1858c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -644,6 +644,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
"virtio-mem-pci", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI */
"memory-backend-file.reserve", /*
QEMU_CAPS_MEMORY_BACKEND_RESERVE */
"piix4.acpi-root-pci-hotplug", /*
QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG */
+ "piix4.acpi-hotplug-bridge", /*
QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE */
+ "ich9.acpi-hotplug-bridge", /*
QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE */
);
@@ -1472,6 +1474,7 @@ 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_PIIX4_ACPI_ROOT_PCI_HOTPLUG, NULL },
+{ "acpi-pci-hotplug-with-bridge-support",
QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE, NULL },
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
@@ -1524,6 +1527,7 @@ static struct virQEMUCapsDevicePropsFlags
virQEMUCapsDevicePropsVirtioGpu[] = {
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsICH9[] = {
{ "disable_s3", QEMU_CAPS_ICH9_DISABLE_S3, NULL },
{ "disable_s4", QEMU_CAPS_ICH9_DISABLE_S4, NULL },
+{ "acpi-pci-hotplug-with-bridge-support",
QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, NULL },
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBNECXHCI[] =
{
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 2bbfc15dc4..e9bd6c8885 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -624,6 +624,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */
QEMU_CAPS_MEMORY_BACKEND_RESERVE, /* -object memory-backend-*.reserve= */
QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, /* -M pc
PIIX4_PM.acpi-root-pci-hotplug */
+QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE, /* -M pc
PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
+QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35
ICH9-LPC.acpi-pci-hotplug-with-bridge-support */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index d6549d6440..65bfe911dd 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -172,6 +172,7 @@
+
2011000
0
43100288
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 354a95cebc..e4d936886b 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++
[PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature
This change introduces a new libvirt sub-element under that
can be used to configure all pci related features.
Currently the only sub-sub element supported by this sub-element is
'acpi-bridge-hotplug' as shown below:
The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.
'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option are required. For pc machine type in x86, this option
is available in qemu for a long time, from version 2.1.
Please see the following changes in qemu repo:
9e047b982452c6 ("piix4: add acpi pci hotplug support")
133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge
hotplug is disabled")
For q35 machine type, this was introduced in qemu 6.1 with the following
changes in qemu repo:
(a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
(b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
The reasons for enabling ACPI based hotplug for PCIe (q35) based machines (as
opposed to native hotplug) are outlined in (b). There are use cases where users
would still want to use native hotplug. Therefore, this config option
enables users to choose either ACPI based hotplug or native hotplug for bridges
(for example for pcie root port controller in q35 machines).
Qemu capability validation checks have also been added along with related unit
tests to exercise the new conf option.
Signed-off-by: Ani Sinha
---
docs/formatdomain.rst | 11 +++
docs/schemas/domaincommon.rng | 15
src/conf/domain_conf.c| 89 ++-
src/conf/domain_conf.h| 9 ++
src/qemu/qemu_validate.c | 46 ++
.../aarch64-acpi-hotplug-bridge-disable.xml | 33 +++
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 33 +++
.../q35-acpi-hotplug-bridge-disable.xml | 47 ++
.../q35-acpi-hotplug-bridge-enable.xml| 47 ++
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 1 +
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 1 +
.../q35-acpi-hotplug-bridge-disable.xml | 1 +
.../q35-acpi-hotplug-bridge-enable.xml| 1 +
tests/qemuxml2xmltest.c | 14 +++
15 files changed, 380 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml
create mode 100644
tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
create mode 100644
tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml
create mode 12
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
create mode 12
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index a02802a954..8d916eefa6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1847,6 +1847,9 @@ Hypervisors may allow certain CPU / machine features to
be toggled on/off.
+
+
+
@@ -1942,6 +1945,14 @@ are:
passthrough Enable IOMMU mappings allowing PCI passthrough on, off; mode -
optional string sync_pt or share_pt :since:`6.3.0`
=== ==
=== ==
+``pci``
+ Various PCI bus related features of the hypervisor.
+
===
+ Feature Description
Value Since
+
===
+ acpi-bridge-hotplug Enable ACPI based hotplug on the cold-plugged PCI
bridges (pc) and pcie-root-ports (q35) on, off :since:`7.8.0 (QEMU 6.1.0)`
+
===
+
``pmu``
Depending on the ``state`` attribute (values ``on``, ``off``, default
``on``)
enable or disable the performance monitoring unit for the guest.
diff --git
Re: [PATCH 2/5] ch_monitor: Correctly close and ref the virCHMonitor
On 10/3/21 10:10 PM, Douglas, William wrote: On Sun, 2021-10-03 at 15:52 -0400, Laine Stump wrote: On 10/3/21 3:43 PM, Laine Stump wrote: cleanup: + if (mon) + virCHMonitorClose(mon); Oops, I also meant to point out that the "if (mon)" is unnecessary here, because (as with all similar functions in libvirt) virCHMonitorClose() can be called with a null argument, and will just be a NOP in that case. If you want me to split and push, I'll fix that up before pushing too. Ah oops, thanks for pointing that out. I'll gladly take advantage of your offer to split them up and push, thanks! OKay, I've pushed everything now. Thanks for following up!
Re: [PATCH] qemu: capabilities: rename piix4-acpi-root-hotplug-en to more appropriate name
On 10/4/21 1:26 PM, Ani Sinha wrote: The capability name piix4-acpi-root-hotplug-en is not conventional and appreared to be confusing to some. "en" suffix is also incorrect as the capability in qemu is used to both enable and disable hotplug on the pci root bus on the i440fx. Hence, rename it to piix4.acpi-root-pci-hotplug so that it is clearer, less confusing and more accurate. Signed-off-by: Ani Sinha --- src/qemu/qemu_capabilities.c | 2 +- tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9d0c96a20c..d7b79ef7c0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -641,7 +641,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "virtio-blk.queue-size", /* QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE */ /* 410 */ - "piix4-acpi-root-hotplug-en", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ + "piix4.acpi-root-pci-hotplug", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ Since the enum matched the capability name exactly except that one is "piix4" and the other is "PIIX", I changed the enum to match this new name before pushing. Reviewed-by: Laine Stump Will be pushed in a few minutes as soon as gitlab CI finishes running on my review branch. ); diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 1e5833a9f0..834fb86636 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -230,7 +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 b54dd8a22e..e9d1a26400 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -238,7 +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 0ad493191d..971d55e0cc 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -240,7 +240,7 @@ - + 6001000 0 43100243
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.
[PATCH] qemu: capabilities: rename piix4-acpi-root-hotplug-en to more appropriate name
The capability name piix4-acpi-root-hotplug-en is not conventional and appreared to be confusing to some. "en" suffix is also incorrect as the capability in qemu is used to both enable and disable hotplug on the pci root bus on the i440fx. Hence, rename it to piix4.acpi-root-pci-hotplug so that it is clearer, less confusing and more accurate. Signed-off-by: Ani Sinha --- src/qemu/qemu_capabilities.c | 2 +- tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9d0c96a20c..d7b79ef7c0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -641,7 +641,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "virtio-blk.queue-size", /* QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE */ /* 410 */ - "piix4-acpi-root-hotplug-en", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ + "piix4.acpi-root-pci-hotplug", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ ); diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 1e5833a9f0..834fb86636 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -230,7 +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 b54dd8a22e..e9d1a26400 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -238,7 +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 0ad493191d..971d55e0cc 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -240,7 +240,7 @@ - + 6001000 0 43100243 -- 2.25.1
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] NEWS: cosmetic - fix indentation
On Mon, 4 Oct 2021, Andrea Bolognani wrote: > On Sat, Oct 02, 2021 at 09:37:13PM +0530, Ani Sinha wrote: > > The indentation of the first item under the categoty "new features" for the > > future release v7.9.0 is not right. Fix it. > > > > Signed-off-by: Ani Sinha > > --- > > NEWS.rst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Andrea Bolognani > > and pushed. Thanks!
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] virsh: Fix --nvram and --keep-nvram help strings
On Fri, Oct 01, 2021 at 09:05:52AM +0200, Michal Privoznik wrote: > The --nvram and --keep-nvram options of the undefine command can > be used regardless of the domain status (the only consumer so far > - qemuDomainUndefineFlags() doesn't care about the domain > status). Yet, their corresponding help strings say something > about inactive domains while manpage says nothing. Remove the > reference to domain state. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2007659 > Signed-off-by: Michal Privoznik > --- > tools/virsh-domain.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
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
Re: [PATCH] NEWS: cosmetic - fix indentation
On Sat, Oct 02, 2021 at 09:37:13PM +0530, Ani Sinha wrote: > The indentation of the first item under the categoty "new features" for the > future release v7.9.0 is not right. Fix it. > > Signed-off-by: Ani Sinha > --- > NEWS.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Andrea Bolognani and pushed. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 0/1] vmx: Fix mapping
On Mon, Oct 04, 2021 at 04:50:51PM +0200, Laszlo Ersek wrote: > On 10/04/21 11:59, Richard W.M. Jones wrote: > > It turns out that changing the qemu implementation is painful, > > particularly if we wish to maintain backwards compatibility of the > > command line and live migration. > > > > Instead I opted to document comprehensively what all the > > different hypervisors do: > > > > > > https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt > > > Unfortunately QEMU made a significant mistake when implementing this > > feature. Because the string is 128 bits wrong, they decided it must > ^^ > > Haha, that's a great typo :) > > > be a UUID (as you can see above there is no evidence that Microsoft > > who wrote the original spec thought it was). Following from this > > incorrect assumption, they stated that the "UUID" must be supplied to > > qemu in big endian format and must be byteswapped when writing it to > > guest memory. Their documentation says that they only do this for > > little endian guests, but this is not true of their implementation > > which byte swaps it for all guests. > > I don't think this is what section "Endian-ness Considerations" in > "docs/specs/vmgenid.txt" says. That text says that the *device* uses > little-endian format. That's independent of the endianness of *CPU* of > the particular guest architecture. > > So the byte-swapping in the QEMU code occurs unconditionally because > QEMU's UUID type is inherently big endian, and the device model in > question is fixed little endian. The guest CPU's endianness is > irrelevant as far as the device is concerned. > > If a BE guest CPU intends to read the generation ID and to interpret it > as a set of integers, then the guest CPU is supposed to byte-swap the > appropriate fields itself. > > > References > > I suggest adding two links in this section, namely to: > - docs/specs/vmgenid.txt > - hw/acpi/vmgenid.c Fair enough - I've made the changes you suggest (same URL as above). Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: Squelch 'eof from qemu monitor' error on normal VM shutdown
On 9/30/21 7:15 PM, Jim Fehlig wrote: > On 9/29/21 21:29, Jim Fehlig wrote: >> Hi All, >> >> Likely Christian received a bug report that motivated commit >> aeda1b8c56, which was later reverted by Michal with commit 72adaf2f10. >> In the past, I recall being asked about "internal error: End of file >> from qemu monitor" on normal VM shutdown and gave a hand wavy response >> using some of Michal's words from the revert commit message. >> >> I recently received a bug report (sorry, but no public link) from a >> concerned user about this error and wondered if there is some way to >> improve it? I went down some dead ends before circling back to >> Christian's patch. When rebased to latest master, I cannot reproduce >> the hangs reported by Michal [1]. Perhaps Nikolay's series to resolve >> hangs/crashes of libvirtd [2] has now made Christian's patch viable? > > Hmm, Nikolay's series improves thread management at daemon shutdown and > doesn't touch VM shutdown logic. But there has been some behavior change > from the time aeda1b8c56 was committed (3.4.0 dev cycle) to current git > master. At least I don't see libvirtd hanging when running Michal's test > on master + rebased aeda1b8c56. That's sort of expected - back in that era we had one event loop for everything. Thus things like virGetLastErrorCode() which use thread local variables did not work as expected and thus we needed to save the error in mon->lastError. But now we have one additional thread per each QEMU just for handling monitor IO, thus virGetLastErrorCode() works now and we could lift some restrictions. Therefore I think we can revisit Christian's patch again. I too see some concerned users who think it's a true error. Michal
Re: [PATCH 0/1] vmx: Fix mapping
On 10/04/21 11:59, Richard W.M. Jones wrote: > It turns out that changing the qemu implementation is painful, > particularly if we wish to maintain backwards compatibility of the > command line and live migration. > > Instead I opted to document comprehensively what all the > different hypervisors do: > > > https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt > Unfortunately QEMU made a significant mistake when implementing this > feature. Because the string is 128 bits wrong, they decided it must ^^ Haha, that's a great typo :) > be a UUID (as you can see above there is no evidence that Microsoft > who wrote the original spec thought it was). Following from this > incorrect assumption, they stated that the "UUID" must be supplied to > qemu in big endian format and must be byteswapped when writing it to > guest memory. Their documentation says that they only do this for > little endian guests, but this is not true of their implementation > which byte swaps it for all guests. I don't think this is what section "Endian-ness Considerations" in "docs/specs/vmgenid.txt" says. That text says that the *device* uses little-endian format. That's independent of the endianness of *CPU* of the particular guest architecture. So the byte-swapping in the QEMU code occurs unconditionally because QEMU's UUID type is inherently big endian, and the device model in question is fixed little endian. The guest CPU's endianness is irrelevant as far as the device is concerned. If a BE guest CPU intends to read the generation ID and to interpret it as a set of integers, then the guest CPU is supposed to byte-swap the appropriate fields itself. > References I suggest adding two links in this section, namely to: - docs/specs/vmgenid.txt - hw/acpi/vmgenid.c Thanks, Laszlo
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
Am 04.10.2021 um 14:18 hat Damien Hedde geschrieben: > > > On 10/1/21 16:42, Peter Krempa wrote: > > On Fri, Sep 24, 2021 at 11:04:25 +0200, Kevin Wolf wrote: > > > Directly call qdev_device_add_from_qdict() for QMP device_add instead of > > > first going through QemuOpts and converting back to QDict. > > > > > > Note that this changes the behaviour of device_add, though in ways that > > > should be considered bug fixes: > > > > > > QemuOpts ignores differences between data types, so you could > > > successfully pass a string "123" for an integer property, or a string > > > "on" for a boolean property (and vice versa). After this change, the > > > correct data type for the property must be used in the JSON input. > > > > > > qemu_opts_from_qdict() also silently ignores any options whose value is > > > a QDict, QList or QNull. > > > > Sorry for chiming in a bit late, but preferrably this commit should be > > postponed to at least the next release so that we decrease the amount of > > libvirt users broken by this. > > > > Granted users are supposed to use new libvirt with new qemu but that's > > not always the case. > > > > Anyways, libvirt is currently mangling all the types to strings with > > device_add. I'm currently working on fixing it and it will hopefully be > > done until next-month's release, but preferrably we increase the window > > of working combinations by postponing this until the next release. > > Switching to qdict is really an improvement I think. > > If we choose to keep legacy behavior working for now, I think we > should find a way to still do this switch. Maybe we can temporarily > keep the str_to_int and str_to_bool conversion when converting the > qdict to the qdev properties afterward? I guess we can keep the detour through QemuOpts for QMP for now, and make sure that the command line code bypasses this bit and still requires correct types for JSON input. It's only this patch that breaks compatibility with libvirt, patch 8 should still be okay. Kevin
[PATCH] docs: describe flag VIR_STORAGE_POOL_CREATE_NORMAL to correct the HTML doc
From: Robin Lee
This patch makes the descriptions of virStoragePoolCreateFlags annotate to the
correct flag in the generated HTML file.
---
Resending a merged change from GitLab.
include/libvirt/libvirt-storage.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/libvirt/libvirt-storage.h
b/include/libvirt/libvirt-storage.h
index b459fe3e8e..f89856b93e 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -70,6 +70,7 @@ typedef enum {
} virStoragePoolDeleteFlags;
typedef enum {
+/* Create the pool but do not perform pool build */
VIR_STORAGE_POOL_CREATE_NORMAL = 0,
/* Create the pool and perform pool build without any flags */
--
2.31.1
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
On 10/1/21 16:42, Peter Krempa wrote: On Fri, Sep 24, 2021 at 11:04:25 +0200, Kevin Wolf wrote: Directly call qdev_device_add_from_qdict() for QMP device_add instead of first going through QemuOpts and converting back to QDict. Note that this changes the behaviour of device_add, though in ways that should be considered bug fixes: QemuOpts ignores differences between data types, so you could successfully pass a string "123" for an integer property, or a string "on" for a boolean property (and vice versa). After this change, the correct data type for the property must be used in the JSON input. qemu_opts_from_qdict() also silently ignores any options whose value is a QDict, QList or QNull. Sorry for chiming in a bit late, but preferrably this commit should be postponed to at least the next release so that we decrease the amount of libvirt users broken by this. Granted users are supposed to use new libvirt with new qemu but that's not always the case. Anyways, libvirt is currently mangling all the types to strings with device_add. I'm currently working on fixing it and it will hopefully be done until next-month's release, but preferrably we increase the window of working combinations by postponing this until the next release. Switching to qdict is really an improvement I think. If we choose to keep legacy behavior working for now, I think we should find a way to still do this switch. Maybe we can temporarily keep the str_to_int and str_to_bool conversion when converting the qdict to the qdev properties afterward ? Damien
Re: [PATCH] failover: allow to pause the VM during the migration
On 30/09/2021 22:17, Laine Stump wrote: On 9/30/21 1:09 PM, Laurent Vivier wrote: If we want to save a snapshot of a VM to a file, we used to follow the following steps: 1- stop the VM: (qemu) stop 2- migrate the VM to a file: (qemu) migrate "exec:cat > snapshot" 3- resume the VM: (qemu) cont After that we can restore the snapshot with: qemu-system-x86_64 ... -incoming "exec:cat snapshot" (qemu) cont This is the basics of what libvirt does for a snapshot, and steps 1+2 are what it does for a "managedsave" (where it saves the snapshot to disk and then terminates the qemu process, for later re-animation). In those cases, it seems like this new parameter could work for us - instead of explicitly pausing the guest prior to migrating it to disk, we would set this new parameter to on, then directly migrate-to-disk (relying on qemu to do the pause). Care will need to be taken to assure that error recovery behaves the same though. In case of error, the VM is restarted like it's done for a standard migration. I can change that if you need. An other point is the VM state sent to the migration stream is "paused", it means that machine needs to be resumed after the stream is loaded (from the file or on destination in the case of a real migration), but it can be also changed to be "running" so the machine will be resumed automatically at the end of the file loading (or real migration) There are a couple of cases when libvirt apparently *doesn't* pause the guest during the migrate-to-disk, both having to do with saving a coredump of the guest. Since I really have no idea of how common/important that is (or even if my assessment of the code is correct), I'm Cc'ing this patch to libvir-list to make sure it catches the attention of someone who knows the answers and implications. It's an interesting point I need to test and think about: in case of a coredump I guess the machine is crashed and doesn't answer to the unplug request and so the failover unplug cannot be done. For the moment the migration will hang until it is canceled. IT can be annoying if we want to debug the cause of the crash... But when failover is configured, it doesn't work anymore. As the failover needs to ask the guest OS to unplug the card the machine cannot be paused. This patch introduces a new migration parameter, "pause-vm", that asks the migration to pause the VM during the migration startup phase after the the card is unplugged. Once the migration is done, we only need to resume the VM with "cont" and the card is plugged back: 1- set the parameter: (qemu) migrate_set_parameter pause-vm on 2- migrate the VM to a file: (qemu) migrate "exec:cat > snapshot" The primary failover card (VFIO) is unplugged and the VM is paused. 3- resume the VM: (qemu) cont The VM restarts and the primary failover card is plugged back The VM state sent in the migration stream is "paused", it means when the snapshot is loaded or if the stream is sent to a destination QEMU, the VM needs to be resumed manually. Signed-off-by: Laurent Vivier --- qapi/migration.json | 20 +++--- include/hw/virtio/virtio-net.h | 1 + hw/net/virtio-net.c | 33 ++ migration/migration.c | 37 +- monitor/hmp-cmds.c | 8 5 files changed, 95 insertions(+), 4 deletions(-) ... Thanks, Laurent
Re: [PATCH 0/1] vmx: Fix mapping
It turns out that changing the qemu implementation is painful, particularly if we wish to maintain backwards compatibility of the command line and live migration. Instead I opted to document comprehensively what all the different hypervisors do: https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt On Thu, Sep 30, 2021 at 10:16:20AM +0100, Richard W.M. Jones wrote: > I was going to suggest something like: > > aa-bb-cc.. > or > aabbcc.. After thinking about this some more, the real implementation on Windows guest and host is two 64 bit little-endian integers[1]. How about implementing this exactly the same way as Hyper-V (and VMware): 0x8877665544332211 0x00ffeeddccbbaa99 This would have to be mapped to the following qemu command line: qemu -device vmgenid,guid=44332211-6655-8877-99aa-bbccddeeff00 which would unmangle to the following in guest physical memory: 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 00 The equivalent back-compat option would be: 44332211-6655-8877-99aa-bbccddeeff00 Rich. [1] No one has ever thought what to do about big-endian guests. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
