Re: [PATCH] qemuDomainChangeDiskLive: Modify 'startupPolicy' before changing source
On Mon, Sep 13, 2021 at 4:10 PM Peter Krempa wrote: > On Fri, Sep 10, 2021 at 22:04:01 +0300, Nir Soffer wrote: > > On Fri, Sep 10, 2021 at 4:35 PM Peter Krempa wrote: > > > > > > We don't support all startup policies with all source types so to > > > correctly allow switching from a 'file' based cdrom with 'optional' > > > startup policy to a 'block' based one which doesn't support optional we > > > must update the startup policy field first. Obviously we need to have > > > fallback if the update fails. > > > > Why is there a difference between file and block for startup policy? > > I'm not sure about the historic reasons for why 'block' was not > considered when this was implemented originally. > Hi Peter, I searched in the commits history and found the error message """ 'startupPolicy' is only valid for 'file' type volume """ comes from 43404fee37( https://listman.redhat.com/archives/libvir-list/2013-April/msg00419.html): Author: Osier Yang Date: Fri Apr 5 03:37:58 2013 +0800 Support startupPolicy for 'volume' disk "startupPolicy" is only valid for file type storage volume, otherwise it fails on starting the domain. But it's not clear why the author said "startupPolicy" is not valid for block type. Maybe we can have test on https://listman.redhat.com/archives/libvir-list/2021-September/msg00903.html , to see if it fails to start the domain with startupPolicy and block disk type. > > > Is this documented? > > (citation from 'formatdomain.rst' the source for > https://www.libvirt.org/formatdomain.html ) > >For a "file" or "volume" disk type which represents a cdrom or floppy > (the >``device`` attribute), it is possible to define policy what to do with > the >disk if the source file is not accessible. (NB, ``startupPolicy`` is not >valid for "volume" disk unless the specified storage volume is of "file" >type). This is done by the ``startupPolicy`` attribute ( :since:`since > 0.9.7` >), accepting these values: > >= > = >mandatory fail if missing for any reason (the default) >requisite fail if missing on boot up, drop if missing on > migrate/restore/revert >optional drop if missing at any start attempt >= > = > >:since:`Since 1.1.2` the ``startupPolicy`` is extended to support hard > disks >besides cdrom and floppy. On guest cold bootup, if a certain disk is not >accessible or its disk chain is broken, with startupPolicy 'optional' > the >guest will drop this disk. This feature doesn't support migration > currently. > > As you can notice only "file" and "volume" (pointing to a file) are > allowed. > > > Do we have a way to switch from file to block with current libvirt > > (centos 8 stream, rhel 8.4) without this patch, or do we need to wait > > until this fix is available? (rhel 8.5?) > > You can issue two separate update calls. One updating the startup policy > first and then the second one updating the source. > > On another note, I don't quite understand though why oVirt actually even > uses startup policy in the first place. Since oVirt is already doing a > pretty complicated storage management, checking whether the image file > exists is a trivial operation. > > Similarly, when migrating you can use the destination XML (which can be > optionally used with the migration API) allows you to update storage > source and even provide empty storage for a cdrom. This can be used to > update paths to storage too. This allows superior flexibility when > compared to startup policy. > >
[PATCH v4 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 fd20e50d18..0bf4ee002c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -29,6 +29,13 @@ v7.8.0 (unreleased) active. This information can also be retrieved with the new virsh command ``nodedev-info``. +* 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 v4 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 | 6 ++
src/qemu/qemu_capabilities.h | 4
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, 23 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index db5432c9fc..23ff1c6a08 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -639,6 +639,10 @@ 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-hotplug-bridge", /*
QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE */
+ "ich9-acpi-hotplug-bridge", /*
QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE */
);
@@ -1465,6 +1469,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-pci-hotplug-with-bridge-support",
QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, NULL },
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
@@ -1517,6 +1522,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 097f28bd40..1c169ce0c1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -620,6 +620,10 @@ 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_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 1e08a04c82..af06a36bed 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -175,6 +175,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 43060efbac..b2694226b1 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -187,6 +187,7 @@
+
2011090
0
43100289
diff
[PATCH v4 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 b60ee1192b..a5c395919e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6029,6 +6029,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
@@ -6074,6 +6075,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 v4 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 0f5d833521..86933f1a50 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
[PATCH v4 0/4] introduce support for acpi-bridge-hotplug feature
changelog:
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. 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).
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 | 6 ++
src/qemu/qemu_capabilities.h | 4 +
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 +++
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 33 +++
.../q35-acpi-hotplug-bridge-disable.args | 33 +++
.../q35-acpi-hotplug-bridge-disable.err | 1 +
.../q35-acpi-hotplug-bridge-disable.xml | 47 ++
.../q35-acpi-hotplug-bridge-enable.xml| 47 ++
tests/qemuxml2argvtest.c | 16
.../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 +++
37 files changed, 507 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
create mode 100644
tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml
create mode 100644
Re: [PATCH] conf: fix block type CDROM cannot support startupPolicy
On Tue, Sep 28, 2021 at 10:43 AM Jie Wang wrote:
> block type CDROM also support startupPolicy in the past, so
>
s/block/Block/
"in the past" could be more detailed. It's better if you tell from which
version to which version the
startupPolicy of block type cdrom is supported
> let us fix it.
>
> Signed-off-by: Jie Wang
> ---
> src/conf/domain_conf.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f7025bffe4..dd374e8ab3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30853,9 +30853,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDef
> *def)
> }
>
> if (def->startupPolicy != 0 &&
> -virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE)
> {
> +(virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE
> &&
> + virStorageSourceGetActualType(def->src) !=
> VIR_STORAGE_TYPE_BLOCK)) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("'startupPolicy' is only valid for 'file' type
> volume"));
> + _("'startupPolicy' is only valid for 'file' or
> 'block' type volume"));
> return -1;
> }
>
> --
> 2.24.0.windows.2
>
>
>
RE: [libvirt][PATCH v7 5/5] Add get domaincaps unit test
> -Original Message- > From: Daniel P. Berrangé > Sent: Tuesday, September 28, 2021 10:15 PM > To: Huang, Haibin > Cc: libvir-list@redhat.com; Ding, Jian-feng ; Yang, > Lin A ; Lu, Lianhao ; > pbonz...@redhat.com; pkre...@redhat.com; twied...@redhat.com; > phrd...@redhat.com; mpriv...@redhat.com > Subject: Re: [libvirt][PATCH v7 5/5] Add get domaincaps unit test > > On Wed, Sep 08, 2021 at 09:15:58AM +0800, Haibin Huang wrote: > > Signed-off-by: Haibin Huang > > --- > > tests/domaincapsdata/bhyve_basic.x86_64.xml | 1 + > > tests/domaincapsdata/bhyve_fbuf.x86_64.xml| 1 + > > tests/domaincapsdata/bhyve_uefi.x86_64.xml| 1 + > > tests/domaincapsdata/empty.xml| 1 + > > tests/domaincapsdata/libxl-xenfv.xml | 1 + > > tests/domaincapsdata/libxl-xenpv.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + > > tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + > > tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + > > tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.4.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.4.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.0.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.0.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_4.0.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_4.0.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_4.0.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_4.0.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.1.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.2.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_4.2.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_4.2.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_5.0.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_5.0.0.x86_64.xml| 1
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
On Wed, Sep 29, 2021 at 7:58 PM Igor Mammedov wrote:
>
> On Tue, 28 Sep 2021 11:47:26 +0100
> Daniel P. Berrangé wrote:
>
> > On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote:
> > >
> > >
> > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> > >
> > > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:
> > > > >
> > > > >
> > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> > > > >
> > > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > > > > > > This change introduces libvirt xml support for the following two
> > > > > > > pm options:
> [...]
> > > > > The switch in libvirt for pcie-root-ports
> > > > > currently does not care whether native or acpi hotplug is used. It
> > > > > simply
> > > > > turns on the hotplug for that particular port. Whether ACPI or native
> > > > > is
> > > > > used is controlled by this global flag that Julia has introduced in
> > > > > 6.1.
>
>
> > > > Right so we have
> > > >
>
> *1*)
> following applies to piix4/q35:
> * ACPI hotplug when enabled, affects _only_ cold-plugged 'bridges'
> since it requires 'slots' being described in DSDT table which
> in current impl. is static table built at reset time.
>
> (i.e. built-in or 'bridges' specified on command line,
> where 'bridges' could be PCI-PCI or PCIe-PCI or root/downstream-ports')
>for anything else ('bridges' added with device_add) native hotplug
>is in use (whether it's SHPC or PCI-E native).
>
> ACPI hotplug wiring is done by calling qbus_set_hotplug_handler()
> * for root bus piix4_pm_realize()/ich9_pm_init()
> * for anything else acpi_pcihp_device_plug_cb()
>
>
> > > > * PIIX4
> > > >
> > > > - acpi-root-pci-hotplug=bool
> > > >
> > > > Whether hotplug is enabled for the root bridge or not
> > > >
> > > >for pci-root controller
> > > >
> > > >
> > > > - acpi-pci-hotplug-with-bridge-support=bool
> > > >
> > > > Toggles support for ACPI based hotplug across all bridges.
> > > > If disabled will there will be no hotplug at all for PIIX4 ?
> > > > Or does 'shpc' come into play in that scenario ?
>
> 'SHPC' hotplug kicks in if it's enabled. (defaults to 'on' except 2.9 machine
> type)
>
> on q35/APCI side of things we always advertise -all_ hotplug methods available
>
> build_q35_osc_method()
> /*
> * Always allow native PME, AER (no dependencies)
> * Allow SHPC (PCI bridges can have SHPC controller)
> */
> aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
>
> bits 0, 1 are Native PCI-E hotplug and SHPC respectively
>
> for PIIX4 we don't have _OSC so it's up to guest OS to make up
> supported methods.
>
> In order of preference:
> * Windows supports ACPI hotplug then Native PCI-E (SHPC never worked there)
Hmm. from https://www.mail-archive.com/qemu-devel@nongnu.org/msg746673.html :
Me:
> Stupid question. If both native and acpi is enabled which one does OS chose?
> Does this choice vary between OSes and different flavours of the same OS like
> Windows?
Julia: It will always choose native.
re: SHPC this is the reason I thought SHPC was disabled. In my
experiments, Windows did not seem to care about SHPC.
> * Linux supports ACPI hotplug, SHPC, Native PCI-E
> (SHPC worked poorly due to need to reserve IO for bridges
> io reservation hinting (impl. later by Marcel))
>
> > > >PIIX combinations
> > > >
> > > >(1) acpi-root-pci-hotplug=yes
> > > >acpi-pci-hotplug-with-bridge-support=yes
> > > >
> > > > - All bridges have hotplug
> > > >
> > > >(2) acpi-root-pci-hotplug=yes
> > > >acpi-pci-hotplug-with-bridge-support=no
> > > >
> > > > - No bridges have hotplug
> > > >
> > > >(3) acpi-root-pci-hotplug=no
> > > >acpi-pci-hotplug-with-bridge-support=yes
> > > >
> > > > - All bridges except root have hotplug
>
> requested by Promox guys, to battle against Windows 'feature' that
> lets any user to unplug sole NIC using an icon on taskbar.
I implemented that on qemu side :-)
>
> (Laine mentioned we have similar per port control for PCI-E
> ('hotplug' property) that was requested by other users
> probably for the same reason)
>
> so acpi-root-pci-hotplug is similar to pcie-root-port.hotplug
> with a difference that the former applies to whole root bus
> on PIIX4, where the later could be controlled per root port.
>
> > > >(4) acpi-root-pci-hotplug=no
> > > >acpi-pci-hotplug-with-bridge-support=no
> > > >
> > > > - No bridges have hotplug. Essentially identical to (2)
> > >
> > > no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci
> > > root bus still has hotplug enabled.
> >
> > So you're saying that acpi-root-pci-hotplug=yes overrides the
> > global request acpi-pci-hotplug-with-bridge-support=no and
> > turns ACPI hotplug back on for the pcie-root
>
> historically ACPI hotplug on root bus was always supported
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
On Tue, 28 Sep 2021 11:47:26 +0100
Daniel P. Berrangé wrote:
> On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote:
> >
> >
> > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> >
> > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:
> > > >
> > > >
> > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> > > >
> > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > > > > > This change introduces libvirt xml support for the following two pm
> > > > > > options:
[...]
> > > > The switch in libvirt for pcie-root-ports
> > > > currently does not care whether native or acpi hotplug is used. It
> > > > simply
> > > > turns on the hotplug for that particular port. Whether ACPI or native is
> > > > used is controlled by this global flag that Julia has introduced in
> > > > 6.1.
> > > Right so we have
> > >
*1*)
following applies to piix4/q35:
* ACPI hotplug when enabled, affects _only_ cold-plugged 'bridges'
since it requires 'slots' being described in DSDT table which
in current impl. is static table built at reset time.
(i.e. built-in or 'bridges' specified on command line,
where 'bridges' could be PCI-PCI or PCIe-PCI or root/downstream-ports')
for anything else ('bridges' added with device_add) native hotplug
is in use (whether it's SHPC or PCI-E native).
ACPI hotplug wiring is done by calling qbus_set_hotplug_handler()
* for root bus piix4_pm_realize()/ich9_pm_init()
* for anything else acpi_pcihp_device_plug_cb()
> > > * PIIX4
> > >
> > > - acpi-root-pci-hotplug=bool
> > >
> > > Whether hotplug is enabled for the root bridge or not
> > >
> > >for pci-root controller
> > >
> > >
> > > - acpi-pci-hotplug-with-bridge-support=bool
> > >
> > > Toggles support for ACPI based hotplug across all bridges.
> > > If disabled will there will be no hotplug at all for PIIX4 ?
> > > Or does 'shpc' come into play in that scenario ?
'SHPC' hotplug kicks in if it's enabled. (defaults to 'on' except 2.9 machine
type)
on q35/APCI side of things we always advertise -all_ hotplug methods available
build_q35_osc_method()
/*
* Always allow native PME, AER (no dependencies)
* Allow SHPC (PCI bridges can have SHPC controller)
*/
aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
bits 0, 1 are Native PCI-E hotplug and SHPC respectively
for PIIX4 we don't have _OSC so it's up to guest OS to make up
supported methods.
In order of preference:
* Windows supports ACPI hotplug then Native PCI-E (SHPC never worked there)
* Linux supports ACPI hotplug, SHPC, Native PCI-E
(SHPC worked poorly due to need to reserve IO for bridges
io reservation hinting (impl. later by Marcel))
> > >PIIX combinations
> > >
> > >(1) acpi-root-pci-hotplug=yes
> > >acpi-pci-hotplug-with-bridge-support=yes
> > >
> > > - All bridges have hotplug
> > >
> > >(2) acpi-root-pci-hotplug=yes
> > >acpi-pci-hotplug-with-bridge-support=no
> > >
> > > - No bridges have hotplug
> > >
> > >(3) acpi-root-pci-hotplug=no
> > >acpi-pci-hotplug-with-bridge-support=yes
> > >
> > > - All bridges except root have hotplug
requested by Promox guys, to battle against Windows 'feature' that
lets any user to unplug sole NIC using an icon on taskbar.
(Laine mentioned we have similar per port control for PCI-E
('hotplug' property) that was requested by other users
probably for the same reason)
so acpi-root-pci-hotplug is similar to pcie-root-port.hotplug
with a difference that the former applies to whole root bus
on PIIX4, where the later could be controlled per root port.
> > >(4) acpi-root-pci-hotplug=no
> > >acpi-pci-hotplug-with-bridge-support=no
> > >
> > > - No bridges have hotplug. Essentially identical to (2)
> >
> > no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci
> > root bus still has hotplug enabled.
>
> So you're saying that acpi-root-pci-hotplug=yes overrides the
> global request acpi-pci-hotplug-with-bridge-support=no and
> turns ACPI hotplug back on for the pcie-root
historically ACPI hotplug on root bus was always supported
without any option, i.e. acpi-root-pci-hotplug=yes by default.
acpi-pci-hotplug-with-bridge-support does what its name
claims - i.e. adds hotplug for bridges (at least on PIIX4).
> > > * Q35
clarification [*1*] still applies
> > >
> > >
> > > - acpi-pci-hotplug-with-bridge-support=bool
> > >
> > > Toggles support for ACPI based hotplug. If disabled native
> > > PCIe hotplug is activated instead
> > >
> > >
> > > * pcie-root-port
> > >
> > > - hotplug=bool
> > >
> > >
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
On Wed, Sep 29, 2021 at 02:49:32PM +0200, Igor Mammedov wrote: > On Tue, 28 Sep 2021 11:59:42 +0100 > Daniel P. Berrangé wrote: > > > On Tue, Sep 28, 2021 at 11:47:26AM +0100, Daniel P. Berrangé wrote: > > > On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote: > > > > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote: > > > > > > > > > > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > > > > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote: > > > > > > > > This change introduces libvirt xml support for the following > > > > > > > > two pm options: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +``acpi-hotplug-bridge`` > > > > > > > > + :since:`Since 7.8.0` This option enables or disables BIOS > > > > > > > > ACPI based hotplug support > > > > > > > > + for cold plugged bridges. It is available only for x86 > > > > > > > > guests, both for q35 and pc > > > > > > > > + machine types. For pc machines, the support is available > > > > > > > > from `QEMU 2.12`. For q35 > > > > > > > > + machines, the support is available from `QEMU 6.1`. > > > > > > > > Examples of cold plugged bridges > > > > > > > > + include PCI-PCI bridges for pc machine types (pci-bridge > > > > > > > > controller). For q35 machines, > > > > > > > > + it includes PCIE root ports (pcie-root-port controller). > > > > > > > > This is a global option that > > > > > > > > + affects all bridges. No other bridge specific option is > > > > > > > > required to be specified. > > > > > > > > > > > > > > Can you confirm my understanding of the situation.. > > > > > > > > > > > > > > - i440fx / PCI topology - hotplug always uses ACPI > > > > > > > > > > > > > > > > > > > ACPI is the primary means of enabling hotplug. shpc might also have > > > > > > a role > > > > > > here but I think it is disabled. Igor (cc'd) might throw some > > > > > > lights on > > > > > > how shpc comes to play. > > > > > > > > > > Yes, I think it will be important to understand if 'shpc' becomes > > > > > relevant > > > > > when ACPI hotplug is disabled for PCI > > > > > > > > > > > > > > > > > > - q35 / PCIe topology - hotplug historically used native PCIe > > > > > > > hotplug, > > > > > > > but in 6.1 switched to ACPI > > > > > > > > > > > > > > > > > > > Correct. > > > > > > > > > > > > > Given, the name "acpi-hotplug-bridge", am I right that this > > > > > > > option > > > > > > > has *no* effect, if the q35 machine is using native PCIe hotplug > > > > > > > approach ? > > > > > > > > > > > > Its complicated. > > > > > > With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu. > > > > > > With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu. > > > > > > > > > > Oh, I mis-read and didn't realize this was controlling the QEMU > > > > > "acpi-pci-hotplug-with-bridge-support" configuration. > > > > > > > > > > With this in mind I think the naming is somewhat misleading. Setting > > > > > it > > > > > to off would give users the impression that hotplug is disabled, which > > > > > is not the case for Q35 at least. It is just switching to a different > > > > > hotplug implementation. > > > > > > > > > > At least from Q35 pov, I think it would be better to call it > > > > > > > > > > hotplug-mode="acpi|pcie" > > > > > > > > > > so it is clear that no matter what value it is set to, hotplug > > > > > is still available. > > > > > > > > > > If we also consider PIIX, then depending on the answer wrt shpc > > > > > above, we might want one of > > > > > > > > > > hotplug-mode="acpi|pcie|none" > > > > > hotplug-mode="acpi|pcie|shpc" > > > > > > > > > > > > > If libvirt does not deal with shpc today I think we should not bother > > > > with > > > > shpc at all. We should simply have a boolean mode appropriately named > > > > that > > > > choses between acpi hotplug vs native. > > > > > > I want to understand what's possible at the qemu hardware level, > > > so we don't paint ourselves into a corner. > > > > > > IIUC, with shpc we only have a toggle on "pci-bridge" devices, > > > and those currently have shpc=true by default. There's no shpc > > > setting on the pci-root, and theres no global setting. > > > > Opps, I was mislead. They have shpc=false by default due to machine > > types >= 2.9 overriding it to false > > > If I read it correctly, shcp is on by default > (modulo 2.9 see 2fa356629ed2) Sigh, so I was mislead twice ! I should have just tested it for real, which I have now done below which confirms what you say: $ echo -e "info qtree\r\nquit\r\n" | qemu-system-x86_64 -monitor stdio -device pci-bridge,chassis_nr=4 | grep shpc shpc = true $ echo -e "info qtree\r\nquit\r\n" | qemu-system-x86_64 -monitor stdio
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
On Tue, 28 Sep 2021 11:59:42 +0100 Daniel P. Berrangé wrote: > On Tue, Sep 28, 2021 at 11:47:26AM +0100, Daniel P. Berrangé wrote: > > On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote: > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote: > > > > > > > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote: > > > > > > > This change introduces libvirt xml support for the following two > > > > > > > pm options: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +``acpi-hotplug-bridge`` > > > > > > > + :since:`Since 7.8.0` This option enables or disables BIOS > > > > > > > ACPI based hotplug support > > > > > > > + for cold plugged bridges. It is available only for x86 > > > > > > > guests, both for q35 and pc > > > > > > > + machine types. For pc machines, the support is available from > > > > > > > `QEMU 2.12`. For q35 > > > > > > > + machines, the support is available from `QEMU 6.1`. Examples > > > > > > > of cold plugged bridges > > > > > > > + include PCI-PCI bridges for pc machine types (pci-bridge > > > > > > > controller). For q35 machines, > > > > > > > + it includes PCIE root ports (pcie-root-port controller). This > > > > > > > is a global option that > > > > > > > + affects all bridges. No other bridge specific option is > > > > > > > required to be specified. > > > > > > > > > > > > Can you confirm my understanding of the situation.. > > > > > > > > > > > > - i440fx / PCI topology - hotplug always uses ACPI > > > > > > > > > > > > > > > > ACPI is the primary means of enabling hotplug. shpc might also have a > > > > > role > > > > > here but I think it is disabled. Igor (cc'd) might throw some lights > > > > > on > > > > > how shpc comes to play. > > > > > > > > Yes, I think it will be important to understand if 'shpc' becomes > > > > relevant > > > > when ACPI hotplug is disabled for PCI > > > > > > > > > > > > > > > - q35 / PCIe topology - hotplug historically used native PCIe > > > > > > hotplug, > > > > > > but in 6.1 switched to ACPI > > > > > > > > > > > > > > > > Correct. > > > > > > > > > > > Given, the name "acpi-hotplug-bridge", am I right that this option > > > > > > has *no* effect, if the q35 machine is using native PCIe hotplug > > > > > > approach ? > > > > > > > > > > Its complicated. > > > > > With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu. > > > > > With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu. > > > > > > > > Oh, I mis-read and didn't realize this was controlling the QEMU > > > > "acpi-pci-hotplug-with-bridge-support" configuration. > > > > > > > > With this in mind I think the naming is somewhat misleading. Setting it > > > > to off would give users the impression that hotplug is disabled, which > > > > is not the case for Q35 at least. It is just switching to a different > > > > hotplug implementation. > > > > > > > > At least from Q35 pov, I think it would be better to call it > > > > > > > > hotplug-mode="acpi|pcie" > > > > > > > > so it is clear that no matter what value it is set to, hotplug > > > > is still available. > > > > > > > > If we also consider PIIX, then depending on the answer wrt shpc > > > > above, we might want one of > > > > > > > > hotplug-mode="acpi|pcie|none" > > > > hotplug-mode="acpi|pcie|shpc" > > > > > > > > > > If libvirt does not deal with shpc today I think we should not bother with > > > shpc at all. We should simply have a boolean mode appropriately named that > > > choses between acpi hotplug vs native. > > > > I want to understand what's possible at the qemu hardware level, > > so we don't paint ourselves into a corner. > > > > IIUC, with shpc we only have a toggle on "pci-bridge" devices, > > and those currently have shpc=true by default. There's no shpc > > setting on the pci-root, and theres no global setting. > > Opps, I was mislead. They have shpc=false by default due to machine > types >= 2.9 overriding it to false If I read it correctly, shcp is on by default (modulo 2.9 see 2fa356629ed2) > > > Seems to imply that if we have acpi-hotplug disabled for PIIX, > > then there would be no hotplug on the pci-root, but shpc hotplug > > would still be available on any pci-bridge devices ? > > Regards, > Daniel
libvirt-7.8.0 release candidate 2
I have just tagged v7.8.0-rc2 in the repository and pushed signed tarballs and source RPMs to https://libvirt.org/sources/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 11:10:35AM +0100, Daniel P. Berrangé wrote: > On Wed, Sep 29, 2021 at 10:57:19AM +0100, Richard W.M. Jones wrote: > > Looking at the qemu code the problem IMHO is: > > > > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/docs/specs/vmgenid.txt#L189 > > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/hw/acpi/vmgenid.c#L37 > > > > This byte swapping makes no sense to me. How do we know that the > > guest is little endian? What will this code do for BE guests? I > > think qemu would be better off treating the "GUID" as a list of bytes > > and writing that exactly into the guest memory. > > This is an artifact of the HyperV only caring about x86 and thus leaving > endianness unspecified in the spec for GenID. QEMU docs say > > > Endian-ness Considerations: > --- > > Although not specified in Microsoft's document, it is assumed that the > device is expected to use little-endian format. > > All GUID passed in via command line or monitor are treated as big-endian. > GUID values displayed via monitor are shown in big-endian format. > > > So by extension if libvirt is passing the value from its XML straight > to QEMU, then libvirt has effectively defined that the XML is storing > it big-endian too. > > This could be where the confusion with VMX config is coming into play, > though the byte re-ordering in v2v seems more complex than just an > endianess-fiddle ? qemu's qemu_uuid_bswap function only swaps some of the fields. I think even more that applying qemu_uuid_bswap to these (not really) "UUIDs" is a nonsense. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:46:38AM +0100, Richard W.M. Jones wrote: > I don't know why we decided to use a GUID for this. The feature > itself (https://go.microsoft.com/fwlink/?LinkId=260709) defines it as > an 128 bit / 8 byte number. The only connection to GUIDs is the size. *cough* .. 16 bytes :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 11:07:30AM +0100, Daniel P. Berrangé wrote: > I'm not sure if we actually need the full driver or not for testing > purposes. The the GenID is just in memory somewhere, and the somewhere > is reported via ACPI table entry. For QEMU its easy as the data is > exposed via fw_cfg which can be read from sysfs directly without > even needing to look at ACPI entries to find it. Not sure how we > find it with VMWare/HyperV though. This still has the problem that qemu is mangling the vmgenid. Nevertheless, on qemu-6.1.0-5.fc36.x86_64 I added this to a Linux guest: 11223344-5566-7788-99aa-bbccddeeff00 which turned into: -device vmgenid,guid=11223344-5566-7788-99aa-bbccddeeff00,id=vmgenid0 Inside the guest: # ls /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_guid/ -l total 0 -r. 1 root root 4096 Sep 29 11:16 key -r. 1 root root 4096 Sep 29 11:16 name -r. 1 root root0 Sep 29 11:16 raw -r. 1 root root 4096 Sep 29 11:16 size # hexdump -C /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_guid/raw 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0020 00 00 00 00 00 00 00 00 44 33 22 11 66 55 88 77 |D3".fU.w| 0030 99 aa bb cc dd ee ff 00 00 00 00 00 00 00 00 00 || 0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 1000 But I think what I really need to do is look at the raw physical address: # hexdump -C /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_addr/raw 28 f0 ff 7f 00 00 00 00 |(...| 0008 # dd if=/dev/mem bs=1 skip=$(( 0x7028 )) count=16 | hexdump -C 16+0 records in 16+0 records out 16 bytes copied, 6.0392e-05 s, 265 kB/s 44 33 22 11 66 55 88 77 99 aa bb cc dd ee ff 00 |D3".fU.w| 0010 I think for VMware I'm really going to need the kernel driver, unless there's some way that iasl can be used to extract the information? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:57:19AM +0100, Richard W.M. Jones wrote: > Looking at the qemu code the problem IMHO is: > > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/docs/specs/vmgenid.txt#L189 > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/hw/acpi/vmgenid.c#L37 > > This byte swapping makes no sense to me. How do we know that the > guest is little endian? What will this code do for BE guests? I > think qemu would be better off treating the "GUID" as a list of bytes > and writing that exactly into the guest memory. This is an artifact of the HyperV only caring about x86 and thus leaving endianness unspecified in the spec for GenID. QEMU docs say Endian-ness Considerations: --- Although not specified in Microsoft's document, it is assumed that the device is expected to use little-endian format. All GUID passed in via command line or monitor are treated as big-endian. GUID values displayed via monitor are shown in big-endian format. So by extension if libvirt is passing the value from its XML straight to QEMU, then libvirt has effectively defined that the XML is storing it big-endian too. This could be where the confusion with VMX config is coming into play, though the byte re-ordering in v2v seems more complex than just an endianess-fiddle ? Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:46:39AM +0100, Richard W.M. Jones wrote: > On Wed, Sep 29, 2021 at 10:33:43AM +0100, Daniel P. Berrangé wrote: > > Ultimately as long as the mapping from libvirt XML to guest visible > > string is consistent across drivers, that's sufficient. > > > > > Adding qemu-devel because I'm interesting to see if the qemu > > > developers would prefer to fix this properly in qemu. > > > > No matter what order QEMU accepts the data in, it can be said to be > > functionally correct. If the order is "unusual" though, it ought to > > at least be documented how the QEMU order corresponds to guest visible > > order. > > > > Incidentally I wonder how much to trust VMGENID.EXE and whether that > > actally reports what it gets out of guest memory "as is", or whether > > it has done any byte re-ordering. > > > > I'm curious what Linux kernel sees for the genid on Vmware vs KVM > > hosts, as probably I'd trust that data more ? > > As far as I can tell no driver has successfully made it upstream, > although there have been a few attempts: > > https://lkml.org/lkml/2018/3/1/498 > > Here's a more recent one from 10 months ago: > > > https://lore.kernel.org/linux-doc/3e05451b-a9cd-4719-99d0-72750a304...@amazon.com/ > > If I have time I'll patch a Linux kernel with the second patch and see > how it relates to the VMware and KVM parameters, but it probably won't > be today. I'm not sure if we actually need the full driver or not for testing purposes. The the GenID is just in memory somewhere, and the somewhere is reported via ACPI table entry. For QEMU its easy as the data is exposed via fw_cfg which can be read from sysfs directly without even needing to look at ACPI entries to find it. Not sure how we find it with VMWare/HyperV though. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 0/1] vmx: Fix mapping
Looking at the qemu code the problem IMHO is: https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/docs/specs/vmgenid.txt#L189 https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/hw/acpi/vmgenid.c#L37 This byte swapping makes no sense to me. How do we know that the guest is little endian? What will this code do for BE guests? I think qemu would be better off treating the "GUID" as a list of bytes and writing that exactly into the guest memory. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:33:43AM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 29, 2021 at 10:20:44AM +0100, Richard W.M. Jones wrote:
> > On Wed, Sep 29, 2021 at 10:01:55AM +0200, Michal Privoznik wrote:
> > > Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
> > > was brought up in a private thread that libvirt doesn't report correct
> > > UUIDs. For instance for the following input:
> > >
> > > vm.genid = "-8536691797830587195"
> > > vm.genidX = "-1723453263670062919"
> >
> > (The two lines above come from a VMware .vmx file)
> >
> > The only thing that really matters is what the guest sees. I ran
> > VMGENID.EXE (https://bugzilla.redhat.com/show_bug.cgi?id=1598350#c3
> > https://docs.microsoft.com/en-gb/windows/win32/hyperv_v2/virtual-machine-generation-identifier)
> > inside the guest and it showed:
> >
> > 8987940a09512cc5:e81510634ff550b9
> >
> > Note these numbers are the hex equivalents of the VMware .vmx numbers:
> >
> > >>> print("%x" % (2**64-8536691797830587195))
> > 8987940a09512cc5
> > >>> print("%x" % (2**64-1723453263670062919))
> > e81510634ff550b9
> >
> > > Libvirt would report:
> > >
> > > 8987940a-0951-2cc5-e815-10634ff550b9
> > >
> > > while virt-v2v would report:
> > >
> > > 09512cc5-940a-8987-b950-f54f631015e8
> >
> > After thinking about this a bit more, IMHO the real problem is
> > with qemu. If you pass this to qemu:
> >
> > -device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0
> >
> > then inside the guest VMGENID shows 2cc509518987940a:b950f54f631015e8
> > (wrong)
> >
> > If you pass this to qemu:
> >
> > ...guid=09512cc5-940a-8987-b950-f54f631015e8...
> >
> > then inside the guest it sees 8987940a09512cc5:e81510634ff550b9
> > (the correct values, matching VMware).
> >
> > This is the reason why virt-v2v mangles the GUID, in order to trick
> > libvirt into passing a mangled GUID which gets mangled again by qemu
> > which is the same as unmangling it.
> >
> > So another way to fix this could be for us to fix qemu. We could just
> > pass the two numbers to qemu instead of using GUIDs, eg:
> >
> > -device vmgenid,low=0x8987940a09512cc5,high=0xe81510634ff550b9,id=vmgenid0
> >
> > and then we'd fix the implementation in qemu so that the low/high
> > values match what is seen by VMGENID.EXE in the guest.
> >
> > Or we could have libvirt use the current GUID in but to do the
> > mangling when it gets passed through to qemu (instead of virt-v2v
> > doing the mangling).
>
> On the libvirt side, the #1 most important thing is that a given
> XML string
>
> 8987940a-0951-2cc5-e815-10634ff550b9
>
> results in the same value being exposed to the guest OS, for both
> the QEMU and VMWare drivers. Whehter the guest sees the bytes in
> the same order is not essential, but it would be less of a surprise
> if it did match.
I don't know why we decided to use a GUID for this. The feature
itself (https://go.microsoft.com/fwlink/?LinkId=260709) defines it as
an 128 bit / 8 byte number. The only connection to GUIDs is the size.
> Ultimately as long as the mapping from libvirt XML to guest visible
> string is consistent across drivers, that's sufficient.
>
> > Adding qemu-devel because I'm interesting to see if the qemu
> > developers would prefer to fix this properly in qemu.
>
> No matter what order QEMU accepts the data in, it can be said to be
> functionally correct. If the order is "unusual" though, it ought to
> at least be documented how the QEMU order corresponds to guest visible
> order.
>
> Incidentally I wonder how much to trust VMGENID.EXE and whether that
> actally reports what it gets out of guest memory "as is", or whether
> it has done any byte re-ordering.
>
> I'm curious what Linux kernel sees for the genid on Vmware vs KVM
> hosts, as probably I'd trust that data more ?
As far as I can tell no driver has successfully made it upstream,
although there have been a few attempts:
https://lkml.org/lkml/2018/3/1/498
Here's a more recent one from 10 months ago:
https://lore.kernel.org/linux-doc/3e05451b-a9cd-4719-99d0-72750a304...@amazon.com/
If I have time I'll patch a Linux kernel with the second patch and see
how it relates to the VMware and KVM parameters, but it probably won't
be today.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:20:44AM +0100, Richard W.M. Jones wrote:
> On Wed, Sep 29, 2021 at 10:01:55AM +0200, Michal Privoznik wrote:
> > Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
> > was brought up in a private thread that libvirt doesn't report correct
> > UUIDs. For instance for the following input:
> >
> > vm.genid = "-8536691797830587195"
> > vm.genidX = "-1723453263670062919"
>
> (The two lines above come from a VMware .vmx file)
>
> The only thing that really matters is what the guest sees. I ran
> VMGENID.EXE (https://bugzilla.redhat.com/show_bug.cgi?id=1598350#c3
> https://docs.microsoft.com/en-gb/windows/win32/hyperv_v2/virtual-machine-generation-identifier)
> inside the guest and it showed:
>
> 8987940a09512cc5:e81510634ff550b9
>
> Note these numbers are the hex equivalents of the VMware .vmx numbers:
>
> >>> print("%x" % (2**64-8536691797830587195))
> 8987940a09512cc5
> >>> print("%x" % (2**64-1723453263670062919))
> e81510634ff550b9
>
> > Libvirt would report:
> >
> > 8987940a-0951-2cc5-e815-10634ff550b9
> >
> > while virt-v2v would report:
> >
> > 09512cc5-940a-8987-b950-f54f631015e8
>
> After thinking about this a bit more, IMHO the real problem is
> with qemu. If you pass this to qemu:
>
> -device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0
>
> then inside the guest VMGENID shows 2cc509518987940a:b950f54f631015e8 (wrong)
>
> If you pass this to qemu:
>
> ...guid=09512cc5-940a-8987-b950-f54f631015e8...
>
> then inside the guest it sees 8987940a09512cc5:e81510634ff550b9
> (the correct values, matching VMware).
>
> This is the reason why virt-v2v mangles the GUID, in order to trick
> libvirt into passing a mangled GUID which gets mangled again by qemu
> which is the same as unmangling it.
>
> So another way to fix this could be for us to fix qemu. We could just
> pass the two numbers to qemu instead of using GUIDs, eg:
>
> -device vmgenid,low=0x8987940a09512cc5,high=0xe81510634ff550b9,id=vmgenid0
>
> and then we'd fix the implementation in qemu so that the low/high
> values match what is seen by VMGENID.EXE in the guest.
>
> Or we could have libvirt use the current GUID in but to do the
> mangling when it gets passed through to qemu (instead of virt-v2v
> doing the mangling).
On the libvirt side, the #1 most important thing is that a given
XML string
8987940a-0951-2cc5-e815-10634ff550b9
results in the same value being exposed to the guest OS, for both
the QEMU and VMWare drivers. Whehter the guest sees the bytes in
the same order is not essential, but it would be less of a surprise
if it did match.
Ultimately as long as the mapping from libvirt XML to guest visible
string is consistent across drivers, that's sufficient.
> Adding qemu-devel because I'm interesting to see if the qemu
> developers would prefer to fix this properly in qemu.
No matter what order QEMU accepts the data in, it can be said to be
functionally correct. If the order is "unusual" though, it ought to
at least be documented how the QEMU order corresponds to guest visible
order.
Incidentally I wonder how much to trust VMGENID.EXE and whether that
actally reports what it gets out of guest memory "as is", or whether
it has done any byte re-ordering.
I'm curious what Linux kernel sees for the genid on Vmware vs KVM
hosts, as probably I'd trust that data more ?
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:01:55AM +0200, Michal Privoznik wrote:
> Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
> was brought up in a private thread that libvirt doesn't report correct
> UUIDs. For instance for the following input:
>
> vm.genid = "-8536691797830587195"
> vm.genidX = "-1723453263670062919"
(The two lines above come from a VMware .vmx file)
The only thing that really matters is what the guest sees. I ran
VMGENID.EXE (https://bugzilla.redhat.com/show_bug.cgi?id=1598350#c3
https://docs.microsoft.com/en-gb/windows/win32/hyperv_v2/virtual-machine-generation-identifier)
inside the guest and it showed:
8987940a09512cc5:e81510634ff550b9
Note these numbers are the hex equivalents of the VMware .vmx numbers:
>>> print("%x" % (2**64-8536691797830587195))
8987940a09512cc5
>>> print("%x" % (2**64-1723453263670062919))
e81510634ff550b9
> Libvirt would report:
>
> 8987940a-0951-2cc5-e815-10634ff550b9
>
> while virt-v2v would report:
>
> 09512cc5-940a-8987-b950-f54f631015e8
After thinking about this a bit more, IMHO the real problem is
with qemu. If you pass this to qemu:
-device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0
then inside the guest VMGENID shows 2cc509518987940a:b950f54f631015e8 (wrong)
If you pass this to qemu:
...guid=09512cc5-940a-8987-b950-f54f631015e8...
then inside the guest it sees 8987940a09512cc5:e81510634ff550b9
(the correct values, matching VMware).
This is the reason why virt-v2v mangles the GUID, in order to trick
libvirt into passing a mangled GUID which gets mangled again by qemu
which is the same as unmangling it.
So another way to fix this could be for us to fix qemu. We could just
pass the two numbers to qemu instead of using GUIDs, eg:
-device vmgenid,low=0x8987940a09512cc5,high=0xe81510634ff550b9,id=vmgenid0
and then we'd fix the implementation in qemu so that the low/high
values match what is seen by VMGENID.EXE in the guest.
Or we could have libvirt use the current GUID in but to do the
mangling when it gets passed through to qemu (instead of virt-v2v
doing the mangling).
Adding qemu-devel because I'm interesting to see if the qemu
developers would prefer to fix this properly in qemu.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
[PATCH 1/1] vmx: Fix mapping
Not a long ago I've introduced parsing of vmx.genid and
vmx.genidX properties. They map onto our element. The
implementation was quite straightforward: the UUID which is 128
bits long is split into two equally long parts which I then put
next to each other and used virUUIDParse() to fill the UUID
buffer. However, as it turns out it is not that simple - VMX
apparently does swap some byte pairs in both Hi and Lo parts. Do
the reverse so that the UUID is true to its original value.
Mind you, this algorithm is heavily inspired by virt-v2v code:
https://github.com/libguestfs/virt-v2v/blob/981e0c6b2d2ae0093e3f31829a8d00b552898552/input/parse_domain_from_vmx.ml#L364
Fixes: 7d661d6e20fe82e5472d5ab6dcd97ed76291f256
Reported-by: Richard W.M. Jones
Signed-off-by: Michal Privoznik
---
src/vmx/vmx.c| 16
tests/vmx2xmldata/esx-in-the-wild-10.xml | 2 +-
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index d3540acd84..f0c30e60b6 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1347,6 +1347,9 @@ virVMXParseGenID(virConf *conf,
{
long long vmid[2] = { 0 };
g_autofree char *uuidstr = NULL;
+size_t i;
+/* This mapping comes from virt-v2v sources. */
+const int uuidmap[] = {8, 10, 12, 14, 4, 6, 0, 2, 30, 28, 26, 24, 22, 20,
18, 16};
if (virVMXGetConfigLong(conf, "vm.genid", [0], 0, true) < 0 ||
virVMXGetConfigLong(conf, "vm.genidX", [1], 0, true) < 0)
@@ -1356,10 +1359,15 @@ virVMXParseGenID(virConf *conf,
return 0;
uuidstr = g_strdup_printf("%.16llx%.16llx", vmid[0], vmid[1]);
-if (virUUIDParse(uuidstr, def->genid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not parse UUID from string '%s'"), uuidstr);
-return -1;
+/* Ideally we would just call virUUIDParse() and be done, but it's not that
+ * simple. Some bytes in genid and genidX are swapped and we need to swap
+ * them back. This matches what virt-v2v does. */
+for (i = 0; i < G_N_ELEMENTS(uuidmap); i++) {
+int idx = uuidmap[i];
+int val_hi = g_ascii_xdigit_value(uuidstr[idx]);
+int val_lo = g_ascii_xdigit_value(uuidstr[idx + 1]);
+
+def->genid[i] = 16 * val_hi + val_lo;
}
def->genidRequested = true;
diff --git a/tests/vmx2xmldata/esx-in-the-wild-10.xml
b/tests/vmx2xmldata/esx-in-the-wild-10.xml
index 47ed637920..59cdc68bf3 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-10.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-10.xml
@@ -1,7 +1,7 @@
w2019biosvmware
421a6177-5aa9-abb7-5924-fc376c18a1b4
- 13c67c91-9f47-526f-b0d6-e4dd2e4bb4f9
+ 9f47526f-7c91-13c6-f9b4-4b2edde4d6b0
4194304
4194304
2
--
2.32.0
[PATCH 0/1] vmx: Fix mapping
Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it was brought up in a private thread that libvirt doesn't report correct UUIDs. For instance for the following input: vm.genid = "-8536691797830587195" vm.genidX = "-1723453263670062919" Libvirt would report: 8987940a-0951-2cc5-e815-10634ff550b9 while virt-v2v would report: 09512cc5-940a-8987-b950-f54f631015e8 Another example: vm.genid = "-6284418052148993891" vm.genidX = "-649955058627554545" Libvirt: a8c94313-ed9b-609d-f6fa-e5515a89530f virt-v2v: ed9b609d-4313-a8c9-0f53-895a51e5faf6 To test my patch I modified tests/vmx2xmldata/esx-in-the-wild-10.vmx (because it already contains vmx.genid, but any file can be modified really), and then ran: libvirt.git/_build/tests $ VIR_TEST_DEBUG=1 VIR_TEST_RANGE=58 ./vmx2xmltest Mind you, the fix is almost direct rewrite of virt-v2v's algorithm, except it's optimized for C and not OCaml :-) You can find various attempts/versions of that on my gitlab: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/vmx_genid/ I'm sending only the last one because that looks the best IMO. Michal Prívozník (1): vmx: Fix mapping src/vmx/vmx.c| 16 tests/vmx2xmldata/esx-in-the-wild-10.xml | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) -- 2.32.0
Re: [PATCH v3 0/5] Add support for two i386 pm options which control acpi hotplug
On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > On Tue, Sep 28, 2021 at 06:14:12PM +0530, Ani Sinha wrote: > > On Tue, Sep 28, 2021 at 17:49 Daniel P. Berrangé > > wrote: > > > > > On Tue, Sep 28, 2021 at 05:31:43PM +0530, Ani Sinha wrote: > > > > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > > > On Thu, Sep 23, 2021 at 04:46:38PM -0400, Laine Stump wrote: > > > > > > On 9/11/21 11:26 PM, Ani Sinha wrote: > > > > > > > Hi all: > > > > > > > > > > > > > > This patchset introduces libvirt xml support for the following two > > > pm conf > > > > > > > options: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (before I get into a more radical discussion about different options > > > - since > > > > > > we aren't exactly duplicating the QEMU option name anyway, what if > > > we made > > > > > > these names more consistent, e.g. "acpi-hotplug-bridge" and > > > > > > "acpi-hotplug-root"?) > > > > > > > > > > > > I've thought quite a bit about whether to put these attributes here, > > > or > > > > > > somewhere else, and I'm still undecided. > > > > > > > > > > > > My initial reaction to this was "PM == Power Management, and power > > > > > > management is all about suspend mode support. Hotplug isn't power > > > > > > management." But then you look at the name of the QEMU option and PM > > > is > > > > > > right there in the name, and I guess it's *kind of related* > > > (effectively > > > > > > suspending/resuming a single device), so maybe I'm thinking too > > > narrowly. > > > > > > > > > > I had the same reaction. Even if QEMU hangs it off a "_PM" device, > > > > > I feel it is a pretty wierd location from libvirt POV to put this. > > > > > > > > > > > So are there alternate places that might fit the purpose of these > > > > > > new > > > > > > options better, rather than directly mimicking the QEMU option > > > placement > > > > > > (for better or worse)? A couple alternative possibilities: > > > > > > > > > > > > 1) > > > > > > > > > > > > One possibility would be to include these new flags within the > > > existing > > > > > > subelement of , which is already used to control > > > whether > > > > > > the guest exposes ACPI to the guest *at all* (via adding "-no-acpi" > > > to the > > > > > > QEMU commandline when is missing - NB: this feature flag is > > > currently > > > > > > supported only on x86 and aarch64 QEMU platforms, and ignored for > > > all other > > > > > > hypervisors). > > > > > > > > > > > > Possibly the new flags could be put in something like this: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > But: > > > > > > > > > > > > * currently there are no subelements to . So this isn't > > > "extending > > > > > > according to an existing pattern". > > > > > > > > > > > > * even though the element uses presence of a subelement > > > > > > to > > > > > > indicate "enabled" and absence of the subelement to indicate > > > "disabled". But > > > > > > in the case of these new acpi bridge options we would need to > > > explicitly > > > > > > have the "enabled='yes/no'" rather than just using presence of the > > > option to > > > > > > mean "enabled" and absence to mean "disabled" because the default > > > > > > for > > > > > > "root-hotplug" up until now has been *enabled*, and the default for > > > > > > hotplug-bridge is different depending on machinetype. We need to > > > continue > > > > > > working properly (and identically) with old/existing XML, but if we > > > didn't > > > > > > have an "enabled" attribute for these new flags, there would be no > > > way to > > > > > > tell the difference between "not specified" and "disabled", and so > > > no way to > > > > > > disable the feature for a QEMU where the default was "enabled". (Why > > > does > > > > > > this matter? Because I don't like the inconsistency that would arise > > > from > > > > > > some feature flags using absense to mean "disabled" and some using > > > it to > > > > > > mean "use the default".) > > > > > > > > > > > > * Having something in in the domain XML kind of implies > > > that the > > > > > > associated capability flags should be represented in the > > > section > > > > > > of the domain capabilities. For example, is listed under > > > > > > > > > in the output of virsh capabilities, separately from the flag > > > indicating > > > > > > presence of the -no-acpi option. I'm not sure if we would need to > > > > > > add > > > > > > something there for these options if we moved them into > > > (seems a > > > > > > bit redundant to me to have it in both places, but I'm sure there > > > > > > are > > > > > > $reasons). > > > > > > > > > > Essentially has become a dumping ground for adhoc global > > > > > properties. So in that sense it probably is the best fit for this. > > > > > > > > > > If we don't want to touch th existing
