Re: [libvirt PATCH 0/2] ci: Some build.sh fixes
On a Thursday in 2023, Andrea Bolognani wrote: Initially reported in https://listman.redhat.com/archives/libvir-list/2023-August/241307.html Andrea Bolognani (2): ci: Fix precedence between arguments passed to meson ci: Fix quoting and option name ci/build.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[libvirt PATCH 0/2] Document basic VFIO variant driver support
update the manpage for the virsh nodedev-detach --driver option, and add a blurb to the NEWS file for the upcoming release Laine Stump (2): docs: update description of virsh nodedev-detach --driver option NEWS: document support for VFIO variant drivers NEWS.rst| 11 +++ docs/manpages/virsh.rst | 25 + 2 files changed, 28 insertions(+), 8 deletions(-) -- 2.41.0
[libvirt PATCH 2/2] NEWS: document support for VFIO variant drivers
Signed-off-by: Laine Stump --- NEWS.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 3571c01b29..e40c8ac259 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,17 @@ v9.7.0 (unreleased) * **New features** + * qemu: basic support for use of "VFIO variant" drivers + +A VFIO variant driver is a device-specific driver that can +be used in place of the generic vfio-pci driver, and provides +extra functionality to support things like live migration of +guests with vfio-assigned devices. It can currently be used by: + +1) setting ``managed='no'`` in the XML configuration for the device +2) pre-binding the variant driver using the ``--driver`` option of + ``virsh nodedev-detach``. + * **Improvements** * **Bug fixes** -- 2.41.0
[libvirt PATCH 1/2] docs: update description of virsh nodedev-detach --driver option
--driver can now be used to specify a specific driver to bind to the
device being detached from the host driver (e.g. vfio-pci-igbvf), not
just the *type* of driver (e.g. "vfio" or "xen", which are unnecessary
anyway, since they are implicit in which hypervisor driver is in use)
Signed-off-by: Laine Stump
---
docs/manpages/virsh.rst | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index f4e5a0bd62..ec7c823602 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5388,14 +5388,23 @@ nodedev-detach
nodedev-detach nodedev [--driver backend_driver]
-Detach *nodedev* from the host, so that it can safely be used by
-guests via passthrough. This is reversed with
-``nodedev-reattach``, and is done automatically for managed devices.
-
-Different backend drivers expect the device to be bound to different
-dummy devices. For example, QEMU's "vfio" backend driver expects the
-device to be bound to vfio-pci. The *--driver* parameter can be used
-to specify the desired backend driver.
+Detach *nodedev* from the host driver and bind it to a special driver
+that provides the API needed by the hypervisor for assigning the
+device to a virtual machine (using in the domain XML
+definition). This is reversed with ``nodedev-reattach``, and is done
+automatically by the hypervisor driver for managed devices (those
+devices with "managed='yes'" in their XML definition).
+
+Different hypervisors expect the device being assigned to be bound to
+different drivers. For example, QEMU's "vfio" backend requires the
+device to be bound to the driver "vfio-pci" or to a "VFIO variant"
+driver (this is a driver that supports the full API provided by
+vfio-pci, plus some other APIs to support things like live
+migration). The *--driver* parameter can be used to specify a
+particular driver (e.g. a device-specific VFIO variant driver) the
+device should be bound to. When *--driver* is omitted, the default
+driver for the hypervisor is used ("vfio-pci" for QEMU, "pciback" for
+Xen).
nodedev-dumpxml
--
2.41.0
Re: [libvirt PATCH v2 1/2] capabilities: reword disksnapshot feature to mention creating snapshots
On Thu, Aug 24, 2023 at 18:21:40 +0200, Pavel Hrdina wrote: > Libvirt supports creating snapshots for a long time but the wording of > the feature may imply that libvirt supports external snapshots in > general but that is not true as users were not able to use APIs to > delete or revert external snapshots. > > Signed-off-by: Pavel Hrdina > --- > docs/formatcaps.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst > index f7e5342654..bb8bc663d2 100644 > --- a/docs/formatcaps.rst > +++ b/docs/formatcaps.rst > @@ -131,9 +131,9 @@ The element will typically wrap up the > following elements: >:since:`Since 0.8.8` > ``disksnapshot`` >If this element is present, the ``default`` attribute describes whether > - external disk snapshots are supported. If absent, external snapshots > may > - still be supported, but it requires attempting the API and checking > for an > - error to find out for sure. :since:`Since 1.2.3` > + creating external disk snapshots is supported. If absent, creating > external > + snapshots may still be supported, but it requires attempting the API > and Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 2/2] capabilities: report full external snapshot support
On Thu, Aug 24, 2023 at 18:21:41 +0200, Pavel Hrdina wrote: > Now that deleting and reverting external snapshots is implemented we can > report that in capabilities so management applications can use that > information and start using external snapshots. > > Signed-off-by: Pavel Hrdina > --- > docs/formatcaps.rst| 6 ++ > src/conf/capabilities.c| 1 + > src/conf/capabilities.h| 1 + > src/conf/schemas/capability.rng| 5 + > src/qemu/qemu_capabilities.c | 1 + > tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + > tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + > tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + > tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + > tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + > tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + > tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + > tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + > tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + > 14 files changed, 23 insertions(+) > > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst > index bb8bc663d2..255180135d 100644 > --- a/docs/formatcaps.rst > +++ b/docs/formatcaps.rst > @@ -134,6 +134,11 @@ The element will typically wrap up the > following elements: >creating external disk snapshots is supported. If absent, creating > external >snapshots may still be supported, but it requires attempting the API > and >checking for an error to find out for sure. :since:`Since 1.2.3` > + ``externalSnapshot`` > + If this element is present, the hypervisor supports deleting and > + reverting external snapshots including memory state. Creating external > + snapshots is supported for long time. "Support for creation of external snapshots is reported via the `disksnapshot` feature flag." Reviewed-by: Peter Krempa
[libvirt PATCH v2 2/2] capabilities: report full external snapshot support
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina
---
docs/formatcaps.rst| 6 ++
src/conf/capabilities.c| 1 +
src/conf/capabilities.h| 1 +
src/conf/schemas/capability.rng| 5 +
src/qemu/qemu_capabilities.c | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 +
14 files changed, 23 insertions(+)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index bb8bc663d2..255180135d 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,11 @@ The element will typically wrap up the
following elements:
creating external disk snapshots is supported. If absent, creating
external
snapshots may still be supported, but it requires attempting the API and
checking for an error to find out for sure. :since:`Since 1.2.3`
+ ``externalSnapshot``
+ If this element is present, the hypervisor supports deleting and
+ reverting external snapshots including memory state. Creating external
+ snapshots is supported for long time. Management applications can now
+ switch from internal snapshots to external snapshots. :since:`Since
9.7.0`
Examples
@@ -318,6 +323,7 @@ capabilities enabled in the chip and BIOS you will see:
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
+
+
+
+
+
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40eacf0f4d..05cc11218a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,6 +1121,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
+virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml
[libvirt PATCH v2 1/2] capabilities: reword disksnapshot feature to mention creating snapshots
Libvirt supports creating snapshots for a long time but the wording of the feature may imply that libvirt supports external snapshots in general but that is not true as users were not able to use APIs to delete or revert external snapshots. Signed-off-by: Pavel Hrdina --- docs/formatcaps.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst index f7e5342654..bb8bc663d2 100644 --- a/docs/formatcaps.rst +++ b/docs/formatcaps.rst @@ -131,9 +131,9 @@ The element will typically wrap up the following elements: :since:`Since 0.8.8` ``disksnapshot`` If this element is present, the ``default`` attribute describes whether - external disk snapshots are supported. If absent, external snapshots may - still be supported, but it requires attempting the API and checking for an - error to find out for sure. :since:`Since 1.2.3` + creating external disk snapshots is supported. If absent, creating external + snapshots may still be supported, but it requires attempting the API and + checking for an error to find out for sure. :since:`Since 1.2.3` Examples -- 2.41.0
[libvirt PATCH v2 0/2] reword and add external snapshot related capabilities
Pavel Hrdina (2): capabilities: reword disksnapshot feature to mention creating snapshots capabilities: report full external snapshot support docs/formatcaps.rst| 12 +--- src/conf/capabilities.c| 1 + src/conf/capabilities.h| 1 + src/conf/schemas/capability.rng| 5 + src/qemu/qemu_capabilities.c | 1 + tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + 14 files changed, 26 insertions(+), 3 deletions(-) -- 2.41.0
Re: [libvirt PATCH] capabilities: report full external snapshot support
On Thu, Aug 24, 2023 at 05:44:32PM +0200, Peter Krempa wrote: > On Thu, Aug 24, 2023 at 17:29:49 +0200, Pavel Hrdina wrote: > > Now that deleting and reverting external snapshots is implemented we can > > report that in capabilities so management applications can use that > > information and start using external snapshots. > > > > Signed-off-by: Pavel Hrdina > > --- > > docs/formatcaps.rst| 6 ++ > > src/conf/capabilities.c| 1 + > > src/conf/capabilities.h| 1 + > > src/conf/schemas/capability.rng| 5 + > > src/qemu/qemu_capabilities.c | 1 + > > tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + > > tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + > > tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + > > tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + > > tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + > > tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + > > 14 files changed, 23 insertions(+) > > > > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst > > index f7e5342654..0e056914b8 100644 > > --- a/docs/formatcaps.rst > > +++ b/docs/formatcaps.rst > > @@ -134,6 +134,11 @@ The element will typically wrap up the > > following elements: > >external disk snapshots are supported. If absent, external snapshots > > may > >still be supported, but it requires attempting the API and checking > > for an > >error to find out for sure. :since:`Since 1.2.3` > > + ``externalSnapshot`` > > + If this element is present, the hypervisor supports creating, > > deleting and > > + reverting external snapshots including memory state and management > > + applications can switch from internal snapshots to external > > snapshots. > > + :since:`Since 9.7.0` > > This implies that 'creation' of external snapshots is also included in > the "Sice 9.7.0". Reword it such that you explicitly mention that > creation of external snapshots is a long-existing feature. > > You can also entirely omit 'creation' from the text as it's covered by > 'disksnapshot' above. Yeah the fact the feature name is `disksnapshot` is a bit unfortunate and it doesn't mention that only creation is supported if that feature is available. I'll reword it and post v2. Thanks, Pavel signature.asc Description: PGP signature
Re: [libvirt PATCH 09/20] ci: build.sh: Join MESON_ARGS and MESON_OPTS
On Wed, Aug 23, 2023 at 11:38:17AM +0200, Erik Skultety wrote: > On Mon, Aug 21, 2023 at 05:17:06AM -0700, Andrea Bolognani wrote: > > On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote: > > > -meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ > > > +MESON_ARGS="$MESON_ARGS $MESON_OPTS" > > > + > > > +meson setup build --werror -Dsystem=true $MESON_ARGS || \ > > > > This has inverted the priority of the two lists of arguments. > > > > Before the change, an option (e.g. -Dfoo=enabled) could be added to > > $MESON_ARGS at the job level and it would override the same option > > (e.g. -Dfoo=disabled) defined as part of $MESON_OPTS in the container > > image. Now the option in the container image will always take > > precedence, which is undesirable. > > Good points. However, I'm already close to proposing something vastly > different > from this, dropping most of these variables as they are to make the local > executions pretty much >95% compatible to what happens in GitLab and > overriding > these shell variables simply isn't part of it because it only makes sense > right > now, but I think with the changes I have it'll only make sense in interactive > container shell sessions in which case it's left to the developer to set and > pass the right options IMO. This all sounds good, but in the meantime the feature is not working as intended. I've posted a quick fix here: https://listman.redhat.com/archives/libvir-list/2023-August/241402.html -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 1/2] ci: Fix precedence between arguments passed to meson
Commit 9c9848f955fd merged $MESON_OPTS into $MESON_ARGS, and while doing so changed their behavior: while until then the contents of $MESON_ARGS had precedence over those of $MESON_OPTS, now the opposite is true. Restore the original behavior and document it. The argument for merging the two variables in the first place was that having both present on the meson command line could be confusing; however, that should no longer be the case now that we have reasonably extensive comments explaining the role of each of the variables and how they interact with each other, so return the meson command line to its original form. Signed-off-by: Andrea Bolognani --- ci/build.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ci/build.sh b/ci/build.sh index ed9b1f4473..0e07b2e59d 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -12,10 +12,12 @@ export VIR_TEST_DEBUG=1 # populated either from a GitLab's job configuration or from command line as # `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local # containerized environment +# +# The contents of $MESON_ARGS (defined locally) should take precedence over +# those of $MESON_OPTS (defined when the container was built), so they're +# passed to meson after them -MESON_ARGS="$MESON_ARGS $MESON_OPTS" - -meson setup build --werror -Dsystem=true $MESON_ARGS || \ +meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \ (cat build/meson-logs/meson-log.txt && exit 1) ninja -C build $NINJA_ARGS -- 2.41.0
[libvirt PATCH 2/2] ci: Fix quoting and option name
Multiple values passed to --meson-args need to be quoted so that the shell will interpret them correctly. The option's name was also reported incorrectly, so fix that as well. Signed-off-by: Andrea Bolognani --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index 0e07b2e59d..d5ed8ad104 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -10,7 +10,7 @@ export VIR_TEST_DEBUG=1 # # $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's # populated either from a GitLab's job configuration or from command line as -# `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local +# `$ helper build --meson-args='-Dopt1 -Dopt2'` when run in a local # containerized environment # # The contents of $MESON_ARGS (defined locally) should take precedence over -- 2.41.0
[libvirt PATCH 0/2] ci: Some build.sh fixes
Initially reported in https://listman.redhat.com/archives/libvir-list/2023-August/241307.html Andrea Bolognani (2): ci: Fix precedence between arguments passed to meson ci: Fix quoting and option name ci/build.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) -- 2.41.0
Re: [libvirt PATCH] capabilities: report full external snapshot support
On Thu, Aug 24, 2023 at 17:29:49 +0200, Pavel Hrdina wrote: > Now that deleting and reverting external snapshots is implemented we can > report that in capabilities so management applications can use that > information and start using external snapshots. > > Signed-off-by: Pavel Hrdina > --- > docs/formatcaps.rst| 6 ++ > src/conf/capabilities.c| 1 + > src/conf/capabilities.h| 1 + > src/conf/schemas/capability.rng| 5 + > src/qemu/qemu_capabilities.c | 1 + > tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + > tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + > tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + > tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + > tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + > tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + > tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + > tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + > tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + > 14 files changed, 23 insertions(+) > > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst > index f7e5342654..0e056914b8 100644 > --- a/docs/formatcaps.rst > +++ b/docs/formatcaps.rst > @@ -134,6 +134,11 @@ The element will typically wrap up the > following elements: >external disk snapshots are supported. If absent, external snapshots > may >still be supported, but it requires attempting the API and checking > for an >error to find out for sure. :since:`Since 1.2.3` > + ``externalSnapshot`` > + If this element is present, the hypervisor supports creating, deleting > and > + reverting external snapshots including memory state and management > + applications can switch from internal snapshots to external snapshots. > + :since:`Since 9.7.0` This implies that 'creation' of external snapshots is also included in the "Sice 9.7.0". Reword it such that you explicitly mention that creation of external snapshots is a long-existing feature. You can also entirely omit 'creation' from the text as it's covered by 'disksnapshot' above.
[libvirt PATCH] capabilities: report full external snapshot support
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina
---
docs/formatcaps.rst| 6 ++
src/conf/capabilities.c| 1 +
src/conf/capabilities.h| 1 +
src/conf/schemas/capability.rng| 5 +
src/qemu/qemu_capabilities.c | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 +
14 files changed, 23 insertions(+)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index f7e5342654..0e056914b8 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,11 @@ The element will typically wrap up the
following elements:
external disk snapshots are supported. If absent, external snapshots may
still be supported, but it requires attempting the API and checking for
an
error to find out for sure. :since:`Since 1.2.3`
+ ``externalSnapshot``
+ If this element is present, the hypervisor supports creating, deleting
and
+ reverting external snapshots including memory state and management
+ applications can switch from internal snapshots to external snapshots.
+ :since:`Since 9.7.0`
Examples
@@ -318,6 +323,7 @@ capabilities enabled in the chip and BIOS you will see:
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
+
+
+
+
+
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40eacf0f4d..05cc11218a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,6 +1121,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
+virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml
b/tests/qemucaps2xmloutdata/caps.ppc64.xml
index 85623f3980..90859f9594 100644
---
Re: [PATCH 0/5] qemuxml2xmltest: Finish modernization of few last forgotten cases
On a Thursday in 2023, Peter Krempa wrote:
Peter Krempa (5):
qemuxml2xmltest: Rework file name generation in 'testInfoSetPaths'
qemuxml2xmltest: Use DO_TEST_CAPS_ARCH_LATEST_FULL for arm GIC tests
qemuxml2argvtest: Pass expected state via struct testQemuInfo's
'flags' member
qemuxml2xmltest: Modernize rest of 'seclabel-*' tests
qemuxml2xmltest: Merge DO_TEST macro into DO_TEST_CAPS_INTERNAL
...amic-baselabel-inactive.x86_64-latest.xml} | 5 +-
...amic-labelskip-inactive.x86_64-latest.xml} | 5 +-
...c-none-relabel-inactive.x86_64-latest.xml} | 5 +-
...namic-override-inactive.x86_64-latest.xml} | 5 +-
...ynamic-relabel-inactive.x86_64-latest.xml} | 5 +-
tests/qemuxml2xmltest.c | 149 ++
tests/testutilsqemu.h | 1 +
7 files changed, 73 insertions(+), 102 deletions(-)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-baselabel-inactive.xml =>
seclabel-dynamic-baselabel-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-labelskip-inactive.xml =>
seclabel-dynamic-labelskip-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-none-relabel-inactive.xml =>
seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-override-inactive.xml =>
seclabel-dynamic-override-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-relabel-inactive.xml =>
seclabel-dynamic-relabel-inactive.x86_64-latest.xml} (89%)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
[PATCH 4/5] qemuxml2xmltest: Modernize rest of 'seclabel-*' tests
Use real capabilities for these last few tests that were not modernized
due to use of 'WHEN_INACTIVE'.
Signed-off-by: Peter Krempa
---
...el-dynamic-baselabel-inactive.x86_64-latest.xml} | 5 -
...el-dynamic-labelskip-inactive.x86_64-latest.xml} | 5 -
...dynamic-none-relabel-inactive.x86_64-latest.xml} | 5 -
...bel-dynamic-override-inactive.x86_64-latest.xml} | 5 -
...abel-dynamic-relabel-inactive.x86_64-latest.xml} | 5 -
tests/qemuxml2xmltest.c | 13 +
6 files changed, 25 insertions(+), 13 deletions(-)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-baselabel-inactive.xml =>
seclabel-dynamic-baselabel-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-labelskip-inactive.xml =>
seclabel-dynamic-labelskip-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-none-relabel-inactive.xml =>
seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-override-inactive.xml =>
seclabel-dynamic-override-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-relabel-inactive.xml =>
seclabel-dynamic-relabel-inactive.x86_64-latest.xml} (89%)
diff --git a/tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.xml
b/tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.x86_64-latest.xml
similarity index 90%
rename from tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.xml
rename to
tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.x86_64-latest.xml
index 5fa8455f77..7ab0558e66 100644
--- a/tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.xml
+++
b/tests/qemuxml2xmloutdata/seclabel-dynamic-baselabel-inactive.x86_64-latest.xml
@@ -8,6 +8,9 @@
hvm
+
+qemu64
+
destroy
restart
@@ -20,7 +23,7 @@
-
+
diff --git a/tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.xml
b/tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.x86_64-latest.xml
similarity index 90%
rename from tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.xml
rename to
tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.x86_64-latest.xml
index 5fa8455f77..7ab0558e66 100644
--- a/tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.xml
+++
b/tests/qemuxml2xmloutdata/seclabel-dynamic-labelskip-inactive.x86_64-latest.xml
@@ -8,6 +8,9 @@
hvm
+
+qemu64
+
destroy
restart
@@ -20,7 +23,7 @@
-
+
diff --git
a/tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.xml
b/tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml
similarity index 92%
rename from tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.xml
rename to
tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml
index 09c019e474..005e13334c 100644
--- a/tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.xml
+++
b/tests/qemuxml2xmloutdata/seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml
@@ -16,13 +16,16 @@
+
+qemu64
+
destroy
restart
destroy
/usr/bin/qemu-system-x86_64
-
+
diff --git a/tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.xml
b/tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.x86_64-latest.xml
similarity index 92%
rename from tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.xml
rename to
tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.x86_64-latest.xml
index 56f15db911..7c1d16d0b2 100644
--- a/tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.xml
+++
b/tests/qemuxml2xmloutdata/seclabel-dynamic-override-inactive.x86_64-latest.xml
@@ -8,6 +8,9 @@
hvm
+
+qemu64
+
destroy
restart
@@ -32,7 +35,7 @@
-
+
diff --git a/tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.xml
b/tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.x86_64-latest.xml
similarity index 89%
rename from tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.xml
rename to
tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.x86_64-latest.xml
index a2b4a3e19e..d94c33d47b 100644
--- a/tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.xml
+++
b/tests/qemuxml2xmloutdata/seclabel-dynamic-relabel-inactive.x86_64-latest.xml
@@ -8,6 +8,9 @@
hvm
+
+qemu64
+
destroy
restart
@@ -20,7 +23,7 @@
-
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e6018f8240..f6be677694 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -503,20 +503,17 @@ mymain(void)
[PATCH 0/5] qemuxml2xmltest: Finish modernization of few last forgotten cases
Peter Krempa (5):
qemuxml2xmltest: Rework file name generation in 'testInfoSetPaths'
qemuxml2xmltest: Use DO_TEST_CAPS_ARCH_LATEST_FULL for arm GIC tests
qemuxml2argvtest: Pass expected state via struct testQemuInfo's
'flags' member
qemuxml2xmltest: Modernize rest of 'seclabel-*' tests
qemuxml2xmltest: Merge DO_TEST macro into DO_TEST_CAPS_INTERNAL
...amic-baselabel-inactive.x86_64-latest.xml} | 5 +-
...amic-labelskip-inactive.x86_64-latest.xml} | 5 +-
...c-none-relabel-inactive.x86_64-latest.xml} | 5 +-
...namic-override-inactive.x86_64-latest.xml} | 5 +-
...ynamic-relabel-inactive.x86_64-latest.xml} | 5 +-
tests/qemuxml2xmltest.c | 149 ++
tests/testutilsqemu.h | 1 +
7 files changed, 73 insertions(+), 102 deletions(-)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-baselabel-inactive.xml =>
seclabel-dynamic-baselabel-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-labelskip-inactive.xml =>
seclabel-dynamic-labelskip-inactive.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-none-relabel-inactive.xml =>
seclabel-dynamic-none-relabel-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-override-inactive.xml =>
seclabel-dynamic-override-inactive.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{seclabel-dynamic-relabel-inactive.xml =>
seclabel-dynamic-relabel-inactive.x86_64-latest.xml} (89%)
--
2.41.0
[PATCH 1/5] qemuxml2xmltest: Rework file name generation in 'testInfoSetPaths'
Pass the state-based suffix directly as string.
Document the logic how the filename is chosen.
Signed-off-by: Peter Krempa
---
tests/qemuxml2xmltest.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b1989cbb5f..ccf9cfcef0 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -75,10 +75,22 @@ testXML2XMLInactive(const void *opaque)
}
+/**
+ * testInfoSetPaths:
+ * @info: test info structure to populate
+ * @suffix: suffix used to create output file name e.g. ".x86-64_latest"
+ * @statesuffix: suffix to create output file name based on tested state
("active" | "inactive")
+ *
+ * This function populates @info with the correct input and output file paths.
+ *
+ * The output file is chosen based on whether a version with @statesuffix
exists.
+ * If yes, it's used, if no the @statesuffix is omitted and it's expected that
+ * both the "active" and "inactive" versions are the same.
+ */
static void
testInfoSetPaths(struct testQemuInfo *info,
const char *suffix,
- int when)
+ const char *statesuffix)
{
VIR_FREE(info->infile);
VIR_FREE(info->outfile);
@@ -88,7 +100,7 @@ testInfoSetPaths(struct testQemuInfo *info,
info->outfile = g_strdup_printf("%s/qemuxml2xmloutdata/%s-%s%s.xml",
abs_srcdir, info->name,
-when == WHEN_ACTIVE ? "active" :
"inactive", suffix);
+statesuffix, suffix);
if (!virFileExists(info->outfile)) {
VIR_FREE(info->outfile);
@@ -146,12 +158,12 @@ mymain(void)
testQemuInfoSetArgs(, , __VA_ARGS__); \
\
if (when & WHEN_INACTIVE) { \
-testInfoSetPaths(, suffix, WHEN_INACTIVE); \
+testInfoSetPaths(, suffix, "inactive"); \
virTestRunLog(, "QEMU XML-2-XML-inactive " _name,
testXML2XMLInactive, ); \
} \
\
if (when & WHEN_ACTIVE) { \
-testInfoSetPaths(, suffix, WHEN_ACTIVE); \
+testInfoSetPaths(, suffix, "active"); \
virTestRunLog(, "QEMU XML-2-XML-active " _name,
testXML2XMLActive, ); \
} \
testQemuInfoClear(); \
--
2.41.0
[PATCH 3/5] qemuxml2argvtest: Pass expected state via struct testQemuInfo's 'flags' member
Rather than having a separate argument to DO_TEST pass the state via
newly added flags 'FLAG_SKIP_CONFIG_ACTIVE'. The '_INACTIVE' equivalent
was not added as there's no test which'd use it.
Remove the old 'WHEN_' flags and move the decision logic out of the
DO_TEST macro as any addition to the logic makes the compiler take much
longer to compile qemuxml2xmltest.
Signed-off-by: Peter Krempa
---
tests/qemuxml2xmltest.c | 36 ++--
tests/testutilsqemu.h | 1 +
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index c67909404f..e6018f8240 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -19,13 +19,6 @@
static virQEMUDriver driver;
-enum {
-WHEN_INACTIVE = 1,
-WHEN_ACTIVE = 2,
-WHEN_BOTH = 3,
-};
-
-
static int
testXML2XMLCommon(const struct testQemuInfo *info)
{
@@ -46,6 +39,9 @@ testXML2XMLActive(const void *opaque)
{
const struct testQemuInfo *info = opaque;
+if (info->flags & FLAG_SKIP_CONFIG_ACTIVE)
+return EXIT_AM_SKIP;
+
if (testXML2XMLCommon(info) < 0 ||
testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt,
info->infile, info->outfile, true,
@@ -150,27 +146,23 @@ mymain(void)
virSetConnectSecret(conn);
virSetConnectStorage(conn);
-#define DO_TEST_FULL(_name, suffix, when, ...) \
+#define DO_TEST_FULL(_name, suffix, ...) \
do { \
static struct testQemuInfo info = { \
.name = _name, \
}; \
testQemuInfoSetArgs(, , __VA_ARGS__); \
\
-if (when & WHEN_INACTIVE) { \
-testInfoSetPaths(, suffix, "inactive"); \
-virTestRunLog(, "QEMU XML-2-XML-inactive " _name,
testXML2XMLInactive, ); \
-} \
+testInfoSetPaths(, suffix, "inactive"); \
+virTestRunLog(, "QEMU XML-2-XML-inactive " _name,
testXML2XMLInactive, ); \
\
-if (when & WHEN_ACTIVE) { \
-testInfoSetPaths(, suffix, "active"); \
-virTestRunLog(, "QEMU XML-2-XML-active " _name,
testXML2XMLActive, ); \
-} \
+testInfoSetPaths(, suffix, "active"); \
+virTestRunLog(, "QEMU XML-2-XML-active " _name, testXML2XMLActive,
); \
testQemuInfoClear(); \
} while (0)
#define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \
-DO_TEST_FULL(name, "." arch "-" ver, WHEN_BOTH, \
+DO_TEST_FULL(name, "." arch "-" ver, \
ARG_CAPS_ARCH, arch, \
ARG_CAPS_VER, ver, \
__VA_ARGS__, \
@@ -511,17 +503,17 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("ppc64-tpmproxy-single", "ppc64");
DO_TEST_CAPS_ARCH_LATEST("ppc64-tpmproxy-with-tpm", "ppc64");
-DO_TEST_FULL("seclabel-dynamic-baselabel", "", WHEN_INACTIVE, ARG_END);
-DO_TEST_FULL("seclabel-dynamic-override", "", WHEN_INACTIVE, ARG_END);
-DO_TEST_FULL("seclabel-dynamic-labelskip", "", WHEN_INACTIVE, ARG_END);
-DO_TEST_FULL("seclabel-dynamic-relabel", "", WHEN_INACTIVE, ARG_END);
+DO_TEST_FULL("seclabel-dynamic-baselabel", "", ARG_FLAGS,
FLAG_SKIP_CONFIG_ACTIVE, ARG_END);
+DO_TEST_FULL("seclabel-dynamic-override", "", ARG_FLAGS,
FLAG_SKIP_CONFIG_ACTIVE, ARG_END);
+DO_TEST_FULL("seclabel-dynamic-labelskip", "", ARG_FLAGS,
FLAG_SKIP_CONFIG_ACTIVE, ARG_END);
+DO_TEST_FULL("seclabel-dynamic-relabel", "", ARG_FLAGS,
FLAG_SKIP_CONFIG_ACTIVE, ARG_END);
DO_TEST_CAPS_LATEST("seclabel-static");
DO_TEST_CAPS_LATEST("seclabel-static-labelskip");
DO_TEST_CAPS_LATEST("seclabel-none");
DO_TEST_CAPS_LATEST("seclabel-dac-none");
DO_TEST_CAPS_LATEST("seclabel-dynamic-none");
DO_TEST_CAPS_LATEST("seclabel-device-multiple");
-DO_TEST_FULL("seclabel-dynamic-none-relabel", "", WHEN_INACTIVE,
+DO_TEST_FULL("seclabel-dynamic-none-relabel", "", ARG_FLAGS,
FLAG_SKIP_CONFIG_ACTIVE,
ARG_QEMU_CAPS, QEMU_CAPS_DEVICE_CIRRUS_VGA,
QEMU_CAPS_SPICE, QEMU_CAPS_LAST,
ARG_END);
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index e0d116336e..7845ac7cb6 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -59,6 +59,7 @@ typedef enum {
FLAG_REAL_CAPS = 1 << 3,
FLAG_SKIP_LEGACY_CPUS = 1 << 4,
FLAG_SLIRP_HELPER = 1 << 5,
+FLAG_SKIP_CONFIG_ACTIVE = 1 << 6, /* Skip 'active' config test in
qemuxml2xmltest */
} testQemuInfoFlags;
struct testQemuConf {
--
2.41.0
[PATCH 5/5] qemuxml2xmltest: Merge DO_TEST macro into DO_TEST_CAPS_INTERNAL
Now all tests invoke a real-capability version. Remove DO_TEST.
Signed-off-by: Peter Krempa
---
tests/qemuxml2xmltest.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index f6be677694..895e24d522 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -146,28 +146,24 @@ mymain(void)
virSetConnectSecret(conn);
virSetConnectStorage(conn);
-#define DO_TEST_FULL(_name, suffix, ...) \
+#define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \
do { \
static struct testQemuInfo info = { \
.name = _name, \
}; \
-testQemuInfoSetArgs(, , __VA_ARGS__); \
+testQemuInfoSetArgs(, , \
+ARG_CAPS_ARCH, arch, \
+ARG_CAPS_VER, ver, \
+__VA_ARGS__, ARG_END); \
\
-testInfoSetPaths(, suffix, "inactive"); \
+testInfoSetPaths(, "." arch "-" ver, "inactive"); \
virTestRunLog(, "QEMU XML-2-XML-inactive " _name,
testXML2XMLInactive, ); \
\
-testInfoSetPaths(, suffix, "active"); \
+testInfoSetPaths(, "." arch "-" ver, "active"); \
virTestRunLog(, "QEMU XML-2-XML-active " _name, testXML2XMLActive,
); \
testQemuInfoClear(); \
} while (0)
-#define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \
-DO_TEST_FULL(name, "." arch "-" ver, \
- ARG_CAPS_ARCH, arch, \
- ARG_CAPS_VER, ver, \
- __VA_ARGS__, \
- ARG_END)
-
#define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
DO_TEST_CAPS_INTERNAL(name, arch, "latest", __VA_ARGS__)
--
2.41.0
[PATCH 2/5] qemuxml2xmltest: Use DO_TEST_CAPS_ARCH_LATEST_FULL for arm GIC tests
Use the new macro instead of open coding it.
Signed-off-by: Peter Krempa
---
tests/qemuxml2xmltest.c | 84 +++--
1 file changed, 21 insertions(+), 63 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ccf9cfcef0..c67909404f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -782,69 +782,27 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("aarch64-traditional-pci", "aarch64");
DO_TEST_CAPS_ARCH_LATEST("aarch64-video-default", "aarch64");
-DO_TEST_FULL("aarch64-gic-none", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_NONE, ARG_END);
-DO_TEST_FULL("aarch64-gic-none-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V2, ARG_END);
-DO_TEST_FULL("aarch64-gic-none-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V3, ARG_END);
-DO_TEST_FULL("aarch64-gic-none-both", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
-DO_TEST_FULL("aarch64-gic-none-tcg", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
-DO_TEST_FULL("aarch64-gic-default", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_NONE, ARG_END);
-DO_TEST_FULL("aarch64-gic-default-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V2, ARG_END);
-DO_TEST_FULL("aarch64-gic-default-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V3, ARG_END);
-DO_TEST_FULL("aarch64-gic-default-both", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
-DO_TEST_FULL("aarch64-gic-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_NONE, ARG_END);
-DO_TEST_FULL("aarch64-gic-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V2, ARG_END);
-DO_TEST_FULL("aarch64-gic-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V3, ARG_END);
-DO_TEST_FULL("aarch64-gic-v2", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
-DO_TEST_FULL("aarch64-gic-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_NONE, ARG_END);
-DO_TEST_FULL("aarch64-gic-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V2, ARG_END);
-DO_TEST_FULL("aarch64-gic-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V3, ARG_END);
-DO_TEST_FULL("aarch64-gic-v3", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
-DO_TEST_FULL("aarch64-gic-host", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_NONE, ARG_END);
-DO_TEST_FULL("aarch64-gic-host", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V2, ARG_END);
-DO_TEST_FULL("aarch64-gic-host", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_V3, ARG_END);
-DO_TEST_FULL("aarch64-gic-host", ".aarch64-latest", WHEN_BOTH,
- ARG_CAPS_ARCH, "aarch64", ARG_CAPS_VER, "latest",
- ARG_GIC, GIC_BOTH, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-none", "aarch64", ARG_GIC,
GIC_NONE, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-none-v2", "aarch64", ARG_GIC,
GIC_V2, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-none-v3", "aarch64", ARG_GIC,
GIC_V3, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-none-both", "aarch64", ARG_GIC,
GIC_BOTH, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-none-tcg", "aarch64", ARG_GIC,
GIC_BOTH, ARG_END);
+DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-gic-default", "aarch64", ARG_GIC,
GIC_NONE, ARG_END);
+
Re: [PATCH 0/2] virschematest: Fix schema coverage of 'ch' driver's tests
On a Thursday in 2023, Peter Krempa wrote: Peter Krempa (2): tests: chxml2xmlin: Fix path format for fake paths virschematest: Validate files in 'chxml2xmlin' and 'chxml2xmlout' directories tests/chxml2xmlin/basic.xml | 4 ++-- tests/virschematest.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH 2/2] virschematest: Validate files in 'chxml2xmlin' and 'chxml2xmlout' directories
The test files for the 'ch' driver were not validated against the schema
and thus also didn't conform to the schema.
Signed-off-by: Peter Krempa
---
tests/virschematest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index c8c1527613..c7bfd372c9 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -237,6 +237,8 @@ static const struct testSchemaEntry schemaDomain[] = {
{ .dir = "tests/xml2vmxdata" },
{ .dir = "tests/bhyveargv2xmldata" },
{ .dir = "tests/qemuagentdata" },
+{ .dir = "tests/chxml2xmlin" },
+{ .dir = "tests/chxml2xmlout" },
};
static const struct testSchemaEntry schemaDomainCaps[] = {
--
2.41.0
[PATCH 1/2] tests: chxml2xmlin: Fix path format for fake paths
Our XML schema requires absolute paths for the and disk source values. Fix the 'ch' test to have absolute paths. Signed-off-by: Peter Krempa --- tests/chxml2xmlin/basic.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/chxml2xmlin/basic.xml b/tests/chxml2xmlin/basic.xml index 911aa7c621..4cd2f3ae5f 100644 --- a/tests/chxml2xmlin/basic.xml +++ b/tests/chxml2xmlin/basic.xml @@ -6,7 +6,7 @@ 2 hvm -hypervisor-fw +/path/to/hypervisor-fw @@ -16,7 +16,7 @@ /usr/local/bin/cloud-hypervisor - + -- 2.41.0
[PATCH 0/2] virschematest: Fix schema coverage of 'ch' driver's tests
Peter Krempa (2): tests: chxml2xmlin: Fix path format for fake paths virschematest: Validate files in 'chxml2xmlin' and 'chxml2xmlout' directories tests/chxml2xmlin/basic.xml | 4 ++-- tests/virschematest.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.41.0
Re: [PATCH 0/6] Fix one corner case when parsing 'mdevctl' output
On Thu, Aug 24, 2023 at 10:57 AM Michal Privoznik wrote: > See 2/6 for explanation. > > Michal Prívozník (6): > nodedevmdevctltest: Rename mdevctl-list-empty test case > nodeDeviceParseMdevctlJSON: Accept empty string > nodedevmdevctltest: Introduce a test case for empty mdevctl output > node_device_driver: Deduplicate mediated devices listing > virMdevctlList: Don't check for !output > virjsontest: Introduce a test case for an empty array > > src/node_device/node_device_driver.c | 40 ++- > .../mdevctl-list-empty-array.json | 1 + > .../mdevctl-list-empty-array.out.xml | 0 > .../mdevctl-list-empty.json | 1 - > tests/nodedevmdevctltest.c| 1 + > tests/virjsontest.c | 1 + > 6 files changed, 16 insertions(+), 28 deletions(-) > create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty-array.json > create mode 100644 > tests/nodedevmdevctldata/mdevctl-list-empty-array.out.xml Reviewed-by: Kristina Hanicova Kristina
Re: [PATCH] conf: add virDomainDiskBlockIoCheckABIStability()
On 8/24/23 12:57, Kristina Hanicova wrote: > Add missing ABI stability check for blockio properties for disk > devices. > > Signed-off-by: Kristina Hanicova > --- > src/conf/domain_conf.c | 25 + > 1 file changed, 25 insertions(+) > Reviewed-by: Michal Privoznik Michal
Re: [PATCH 2/6] nodeDeviceParseMdevctlJSON: Accept empty string
On Thu, Aug 24, 2023 at 10:57 AM Michal Privoznik
wrote:
> It is possible for 'mdevctl' to output nothing, an empty string
> (e.g. when no mediated devices are defined on the host). What is
> weird is that when passing '--defined' then 'mdevctl' outputs an
> empty JSON array instead. Nevertheless, we should accept both and
> threat them the same, i.e. as no mediated devices.
>
*treat
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/523
> Signed-off-by: Michal Privoznik
> ---
> src/node_device/node_device_driver.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 2ef9197adc..593bc64e25 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -43,6 +43,7 @@
> #include "virutil.h"
> #include "vircommand.h"
> #include "virlog.h"
> +#include "virstring.h"
>
> #define VIR_FROM_THIS VIR_FROM_NODEDEV
>
> @@ -1176,6 +1177,12 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
> size_t j;
> virJSONValue *obj;
>
> +if (virStringIsEmpty(jsonstring)) {
> +VIR_DEBUG("mdevctl has no defined mediated devices");
> +*devs = NULL;
> +return 0;
> +}
> +
> json_devicelist = virJSONValueFromString(jsonstring);
>
> if (!json_devicelist || !virJSONValueIsArray(json_devicelist)) {
> --
> 2.41.0
>
>
Reviewed-by: Kristina Hanicova
Kristina
[PATCH] conf: add virDomainDiskBlockIoCheckABIStability()
Add missing ABI stability check for blockio properties for disk
devices.
Signed-off-by: Kristina Hanicova
---
src/conf/domain_conf.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2e60927799..71bd49bf95 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19775,6 +19775,28 @@
virDomainVirtioOptionsCheckABIStability(virDomainVirtioOptions *src,
}
+static bool
+virDomainDiskBlockIoCheckABIStability(virDomainDiskDef *src,
+ virDomainDiskDef *dst)
+{
+if (src->blockio.logical_block_size != dst->blockio.logical_block_size) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target disk logical_block_size %1$u does not match
source %2$u"),
+ dst->blockio.logical_block_size,
src->blockio.logical_block_size);
+return false;
+}
+
+if (src->blockio.physical_block_size != dst->blockio.physical_block_size) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target disk physical_block_size %1$u does not match
source %2$u"),
+ dst->blockio.physical_block_size,
src->blockio.physical_block_size);
+return false;
+}
+return true;
+}
+
+
+
static bool
virDomainDiskDefCheckABIStability(virDomainDiskDef *src,
virDomainDiskDef *dst)
@@ -19858,6 +19880,9 @@ virDomainDiskDefCheckABIStability(virDomainDiskDef *src,
if (!virDomainDeviceInfoCheckABIStability(>info, >info))
return false;
+if (!virDomainDiskBlockIoCheckABIStability(src, dst))
+return false;
+
return true;
}
--
2.41.0
[PATCH 5/6] virMdevctlList: Don't check for !output
After 'mdevctl' was ran, its stdout is captured in @output which is then compared against NULL and if it is NULL a negative value is returned (to indicate error to the caller). But this is effectively a dead code, because virCommand (specifically virCommandProcessIO()) makes sure both stdout and stderr buffers are properly '\0' terminated. Therefore, this can never evaluate to true. Also, if there really is no output from 'mdevctl' (which was handled in one of earlier commits, but let just assume it wasn't), then we should not error out and treat such scenario as 'no mdevs defined/active'. Signed-off-by: Michal Privoznik --- src/node_device/node_device_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ac50c96837..a59cd0875d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1650,9 +1650,6 @@ virMdevctlList(bool defined, return -1; } -if (!output) -return -1; - return nodeDeviceParseMdevctlJSON(output, devs); } -- 2.41.0
[PATCH 6/6] virjsontest: Introduce a test case for an empty array
Previous commits were all about empty strings and empty JSON
arrays. Introduce a test case for "[]" to make sure we pare it
correctly.
Signed-off-by: Michal Privoznik
---
tests/virjsontest.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/virjsontest.c b/tests/virjsontest.c
index 294889a795..6b6a64d3d3 100644
--- a/tests/virjsontest.c
+++ b/tests/virjsontest.c
@@ -553,6 +553,7 @@ mymain(void)
DO_TEST_PARSE("integer", "1", NULL);
DO_TEST_PARSE("boolean", "true", NULL);
DO_TEST_PARSE("null", "null", NULL);
+DO_TEST_PARSE("[]", "[]", NULL);
DO_TEST_PARSE("escaping symbols", "[\"\\\"\\t\\n\"]", NULL);
DO_TEST_PARSE("escaped strings", "[\"{\\\"blurb\\\":\\\"test\\\"}\"]",
NULL);
--
2.41.0
[PATCH 4/6] node_device_driver: Deduplicate mediated devices listing
We have virMdevctlListDefined() to list defined mdevs, and
virMdevctlListActive() to list active mdevs. Both have the same
body except for one boolean argument passed to
nodeDeviceGetMdevctlListCommand(). Join the two functions under
virMdevctlList() name and introduce @defined argument that is
then just passed to the cmd line builder function.
Signed-off-by: Michal Privoznik
---
src/node_device/node_device_driver.c | 30 ++--
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 593bc64e25..ac50c96837 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1636,32 +1636,14 @@ nodeDeviceGenerateName(virNodeDeviceDef *def,
static int
-virMdevctlListDefined(virNodeDeviceDef ***devs, char **errmsg)
+virMdevctlList(bool defined,
+ virNodeDeviceDef ***devs,
+ char **errmsg)
{
int status;
g_autofree char *output = NULL;
g_autofree char *errbuf = NULL;
-g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true, ,
);
-
-if (virCommandRun(cmd, ) < 0 || status != 0) {
-*errmsg = g_steal_pointer();
-return -1;
-}
-
-if (!output)
-return -1;
-
-return nodeDeviceParseMdevctlJSON(output, devs);
-}
-
-
-static int
-virMdevctlListActive(virNodeDeviceDef ***devs, char **errmsg)
-{
-int status;
-g_autofree char *output = NULL;
-g_autofree char *errbuf = NULL;
-g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(false,
, );
+g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(defined,
, );
if (virCommandRun(cmd, ) < 0 || status != 0) {
*errmsg = g_steal_pointer();
@@ -1750,7 +1732,7 @@ nodeDeviceUpdateMediatedDevices(void)
return 0;
}
-if ((data.ndefs = virMdevctlListDefined(, )) < 0) {
+if ((data.ndefs = virMdevctlList(true, , )) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to query mdevs from mdevctl: %1$s"), errmsg);
return -1;
@@ -1767,7 +1749,7 @@ nodeDeviceUpdateMediatedDevices(void)
return -1;
/* Update active/transient mdev devices */
-if ((act_ndefs = virMdevctlListActive(_defs, )) < 0) {
+if ((act_ndefs = virMdevctlList(false, _defs, )) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to query mdevs from mdevctl: %1$s"), errmsg);
return -1;
--
2.41.0
[PATCH 2/6] nodeDeviceParseMdevctlJSON: Accept empty string
It is possible for 'mdevctl' to output nothing, an empty string
(e.g. when no mediated devices are defined on the host). What is
weird is that when passing '--defined' then 'mdevctl' outputs an
empty JSON array instead. Nevertheless, we should accept both and
threat them the same, i.e. as no mediated devices.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/523
Signed-off-by: Michal Privoznik
---
src/node_device/node_device_driver.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 2ef9197adc..593bc64e25 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -43,6 +43,7 @@
#include "virutil.h"
#include "vircommand.h"
#include "virlog.h"
+#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_NODEDEV
@@ -1176,6 +1177,12 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
size_t j;
virJSONValue *obj;
+if (virStringIsEmpty(jsonstring)) {
+VIR_DEBUG("mdevctl has no defined mediated devices");
+*devs = NULL;
+return 0;
+}
+
json_devicelist = virJSONValueFromString(jsonstring);
if (!json_devicelist || !virJSONValueIsArray(json_devicelist)) {
--
2.41.0
[PATCH 1/6] nodedevmdevctltest: Rename mdevctl-list-empty test case
The mdevctl-list-empty test case is there to test whether an
empty JSON array "[]" is handled correctly by mdevctl handling
code. Well, mdevctl can output both, an empty JSON array or no
output at all.
Therefore, rename "mdevctl-list-empty" test case to
"mdevctl-list-empty-array" which is more descriptive and also
frees up slot for actual empty output (handled in next commits).
Signed-off-by: Michal Privoznik
---
.../{mdevctl-list-empty.json => mdevctl-list-empty-array.json} | 0
...vctl-list-empty.out.xml => mdevctl-list-empty-array.out.xml} | 0
tests/nodedevmdevctltest.c | 2 +-
3 files changed, 1 insertion(+), 1 deletion(-)
rename tests/nodedevmdevctldata/{mdevctl-list-empty.json =>
mdevctl-list-empty-array.json} (100%)
rename tests/nodedevmdevctldata/{mdevctl-list-empty.out.xml =>
mdevctl-list-empty-array.out.xml} (100%)
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json
b/tests/nodedevmdevctldata/mdevctl-list-empty-array.json
similarity index 100%
rename from tests/nodedevmdevctldata/mdevctl-list-empty.json
rename to tests/nodedevmdevctldata/mdevctl-list-empty-array.json
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
b/tests/nodedevmdevctldata/mdevctl-list-empty-array.out.xml
similarity index 100%
rename from tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
rename to tests/nodedevmdevctldata/mdevctl-list-empty-array.out.xml
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 4dc524b5a5..c04b05c417 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -470,7 +470,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
-DO_TEST_PARSE_JSON("mdevctl-list-empty");
+DO_TEST_PARSE_JSON("mdevctl-list-empty-array");
DO_TEST_PARSE_JSON("mdevctl-list-multiple");
DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
--
2.41.0
[PATCH 3/6] nodedevmdevctltest: Introduce a test case for empty mdevctl output
As explained earlier, 'mdevctl' can output nothing. Add a test
case to nodedevmdevctltest which covers this situation.
Signed-off-by: Michal Privoznik
---
tests/nodedevmdevctldata/mdevctl-list-empty.json| 0
tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
tests/nodedevmdevctltest.c | 1 +
3 files changed, 1 insertion(+)
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json
b/tests/nodedevmdevctldata/mdevctl-list-empty.json
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index c04b05c417..e403328e5a 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -470,6 +470,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
+DO_TEST_PARSE_JSON("mdevctl-list-empty");
DO_TEST_PARSE_JSON("mdevctl-list-empty-array");
DO_TEST_PARSE_JSON("mdevctl-list-multiple");
--
2.41.0
[PATCH 0/6] Fix one corner case when parsing 'mdevctl' output
See 2/6 for explanation. Michal Prívozník (6): nodedevmdevctltest: Rename mdevctl-list-empty test case nodeDeviceParseMdevctlJSON: Accept empty string nodedevmdevctltest: Introduce a test case for empty mdevctl output node_device_driver: Deduplicate mediated devices listing virMdevctlList: Don't check for !output virjsontest: Introduce a test case for an empty array src/node_device/node_device_driver.c | 40 ++- .../mdevctl-list-empty-array.json | 1 + .../mdevctl-list-empty-array.out.xml | 0 .../mdevctl-list-empty.json | 1 - tests/nodedevmdevctltest.c| 1 + tests/virjsontest.c | 1 + 6 files changed, 16 insertions(+), 28 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty-array.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty-array.out.xml -- 2.41.0
Re: [PATCH v2 0/5] Improve closing of FDs for child processes
On Tue, Aug 22, 2023 at 3:35 PM Michal Privoznik wrote: > This is a v2 of: > > https://listman.redhat.com/archives/libvir-list/2023-June/240351.html > > Hopefully, I've implemented all Dan's suggestions. > > Michal Prívozník (5): > virfile: Introduce virCloseRange() > virfile: Introduce virCloseFrom() > vircommand: Unify mass FD closing > vircommand: Introduce virCommandMassCloseRange() > src: Detect close_range syscall during virGlobalInit() > > src/libvirt.c| 4 + > src/libvirt_private.syms | 4 + > src/util/vircommand.c| 160 --- > src/util/virfile.c | 110 +++ > src/util/virfile.h | 5 ++ > tests/commandtest.c | 2 + > 6 files changed, 222 insertions(+), 63 deletions(-) > > Reviewed-by: Kristina Hanicova Kristina
Re: [libvirt PATCH v3 8/8] qemu: turn two multiline log messages into single line
On 8/23/23 22:06, Laine Stump wrote:
> On 8/23/23 3:52 AM, Michal Prívozník wrote:
>> On 8/21/23 21:32, Laine Stump wrote:
>>> Normally I wouldn't bother with a change like this, but I was touching
>>> the function anyway, and wanted to leave it looking nice and tidy.
>>>
>>> Signed-off-by: Laine Stump
>>> ---
>>> src/qemu/qemu_driver.c | 6 ++
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index f676744e9e..a60cbf0ed4 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -11411,8 +11411,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
>>> */
>>> if (STREQ_NULLABLE(driverName, "kvm")) {
>>> virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>>> - _("'legacy KVM' device assignment is no longer "
>>> - "supported on this system"));
>>> + _("'legacy KVM' device assignment is no
>>> longer supported on this system"));
>>> return -1;
>>> }
>>> @@ -11423,8 +11422,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr
>>> dev,
>>> if (!qemuHostdevHostSupportsPassthroughVFIO()) {
>>> virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>>> - _("VFIO device assignment is currently not "
>>> - "supported on this system"));
>>> + _("VFIO device assignment is currently not
>>> supported on this system"));
>>> return -1;
>>> }
>>>
>>
>> This got me thinking, whether we should do one huge commit which would
>> fix ALL the error messages in all files and just be done with it for
>> good. Again, future work, you patch is good as is.
>
> Yep, I had that thought too. I do worry that single giant mega-patches
> like that can create merge conflicts later though (since cherry-picking
> the mega-patch to resolve one conflict in context can create several
> other conflicts), but as you say that can be done (including the
> discussion of its relative merits) at another time.
>
Yeah, that would be a problem, but also such conflicts would be trivial
to resolve.
Michal
