[libvirt PATCH v3 5/8] util: probe stub driver from within function that binds to stub driver

2023-08-21 Thread Laine Stump
virPCIProbeStubDriver() and virPCIDeviceBindToStub() both have
very similar code that locally sets a driver name (based on
stubDriverType). These two functions are each also called in just one
place (virPCIDeviceDetach()), with just a small bit of validation code
in between.

To eliminate the "duplicated" code (which is going to be expanded
slightly in upcoming patches to support manually or automatically
picking a VFIO variant driver), this patch modifies
virPCIProbeStubDriver() to take the driver name as an argument
(rather than the virPCIDevice object), and calls it from within
virPCIDeviceBindToStub() (rather than from that function's caller),
using the driverName it has just figured out with the
now-not-duplicated code.

(NB: Since it could be used to probe *any* driver module, the name is
changed to virPCIProbeDriver()).

Signed-off-by: Laine Stump 
---
 src/util/virpci.c | 33 -
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index e165725cd9..ac91480e0b 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1154,28 +1154,19 @@ virPCIDeviceReset(virPCIDevice *dev,
 
 
 static int
-virPCIProbeStubDriver(virPCIStubDriver driver)
+virPCIProbeDriver(const char *driverName)
 {
-const char *drvname = NULL;
 g_autofree char *drvpath = NULL;
 g_autofree char *errbuf = NULL;
 
-if (driver == VIR_PCI_STUB_DRIVER_NONE ||
-!(drvname = virPCIStubDriverTypeToString(driver))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s",
-   _("Attempting to use unknown stub driver"));
-return -1;
-}
-
-drvpath = virPCIDriverDir(drvname);
+drvpath = virPCIDriverDir(driverName);
 
 /* driver previously loaded, return */
 if (virFileExists(drvpath))
 return 0;
 
-if ((errbuf = virKModLoad(drvname))) {
-VIR_WARN("failed to load driver %s: %s", drvname, errbuf);
+if ((errbuf = virKModLoad(driverName))) {
+VIR_WARN("failed to load driver %s: %s", driverName, errbuf);
 goto cleanup;
 }
 
@@ -1187,14 +1178,14 @@ virPCIProbeStubDriver(virPCIStubDriver driver)
 /* If we know failure was because of admin config, let's report that;
  * otherwise, report a more generic failure message
  */
-if (virKModIsProhibited(drvname)) {
+if (virKModIsProhibited(driverName)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to load PCI stub module %1$s: 
administratively prohibited"),
-   drvname);
+   _("Failed to load PCI driver module %1$s: 
administratively prohibited"),
+   driverName);
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to load PCI stub module %1$s"),
-   drvname);
+   _("Failed to load PCI driver module %1$s"),
+   driverName);
 }
 
 return -1;
@@ -1316,6 +1307,9 @@ virPCIDeviceBindToStub(virPCIDevice *dev)
 return -1;
 }
 
+if (virPCIProbeDriver(stubDriverName) < 0)
+return -1;
+
 stubDriverPath = virPCIDriverDir(stubDriverName);
 driverLink = virPCIFile(dev->name, "driver");
 
@@ -1358,9 +1352,6 @@ virPCIDeviceDetach(virPCIDevice *dev,
virPCIDeviceList *activeDevs,
virPCIDeviceList *inactiveDevs)
 {
-if (virPCIProbeStubDriver(dev->stubDriverType) < 0)
-return -1;
-
 if (activeDevs && virPCIDeviceListFind(activeDevs, >address)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Not detaching active device %1$s"), dev->name);
-- 
2.41.0



[libvirt PATCH v3 7/8] node_device: support binding other drivers with virNodeDeviceDetachFlags()

2023-08-21 Thread Laine Stump
In the past, the only allowable values for the "driver" field of
virNodeDeviceDetachFlags() were "kvm" or "vfio" for the QEMU driver,
and "xen" for the libxl driver. Then "kvm" was deprecated and removed,
so the driver name became essentially irrelevant (because it is always
called via a particular hypervisor driver, and so the "xen" or "vfio"
can be (and almost always is) implied.

With the advent of VFIO variant drivers, the ability to explicitly
specify a driver name once again becomes useful - it can be used to
name the exact VFIO driver that we want bound to the device in place
of vfio-pci, so this patch allows those other names to be passed down
the call chain, where the code in virpci.c can make use of them.

The names "vfio", "kvm", and "xen" retain their special meaning, though:

  1) because there may be some application or configuration that still
 calls virNodeDeviceDetachFlags() with driverName="vfio", this
 single value is substituted with the synonym of NULL, which means
 "bind the default driver for this device and hypervisor". This
 will currently result in the vfio-pci driver being bound to the
 device.

  2) in the case of the libxl driver, "xen" means to use the standard
 driver used in the case of Xen ("pciback").

  3) "kvm" as a driver name always results in an error, as legacy KVM
 device assignment was removed from the kernel around 10 years ago.

Signed-off-by: Laine Stump 
---
 src/hypervisor/domain_driver.c |  9 -
 src/hypervisor/domain_driver.h |  2 ++
 src/libxl/libxl_driver.c   |  3 ++-
 src/qemu/qemu_driver.c | 33 +++--
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index a70f75f3ae..429784292a 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -462,6 +462,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
 int
 virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
  virHostdevManager *hostdevMgr,
+ virPCIStubDriver driverType,
  const char *driverName)
 {
 g_autoptr(virPCIDevice) pci = NULL;
@@ -471,7 +472,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
 g_autoptr(virConnect) nodeconn = NULL;
 g_autoptr(virNodeDevice) nodedev = NULL;
 
-if (!driverName)
+if (driverType == VIR_PCI_STUB_DRIVER_NONE)
 return -1;
 
 if (!(nodeconn = virGetConnectNodeDev()))
@@ -504,10 +505,8 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
 if (!pci)
 return -1;
 
-if (STREQ(driverName, "vfio"))
-virPCIDeviceSetStubDriverType(pci, VIR_PCI_STUB_DRIVER_VFIO);
-else if (STREQ(driverName, "xen"))
-virPCIDeviceSetStubDriverType(pci, VIR_PCI_STUB_DRIVER_XEN);
+virPCIDeviceSetStubDriverType(pci, driverType);
+virPCIDeviceSetStubDriverName(pci, driverName);
 
 return virHostdevPCINodeDeviceDetach(hostdevMgr, pci);
 }
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
index 4241c86932..9942f58fda 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -22,6 +22,7 @@
 
 #include "node_device_conf.h"
 #include "virhostdev.h"
+#include "virpci.h"
 
 char *
 virDomainDriverGenerateRootHash(const char *drivername,
@@ -58,6 +59,7 @@ int virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
 
 int virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
  virHostdevManager *hostdevMgr,
+ virPCIStubDriver driverType,
  const char *driverName);
 
 int virDomainDriverAddIOThreadCheck(virDomainDef *def,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3d10f45850..079922dd32 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5876,7 +5876,8 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
 
 /* virNodeDeviceDetachFlagsEnsureACL() is being called by
  * virDomainDriverNodeDeviceDetachFlags() */
-return virDomainDriverNodeDeviceDetachFlags(dev, hostdev_mgr, driverName);
+return virDomainDriverNodeDeviceDetachFlags(dev, hostdev_mgr,
+VIR_PCI_STUB_DRIVER_XEN, NULL);
 }
 
 static int
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f8039160f4..f676744e9e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -70,7 +70,6 @@
 #include "domain_driver.h"
 #include "domain_postparse.h"
 #include "domain_validate.h"
-#include "virpci.h"
 #include "virpidfile.h"
 #include "virprocess.h"
 #include "libvirt_internal.h"
@@ -11400,24 +11399,28 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
 
 virCheckFlags(0, -1);
 
-if (!driverName)
-driverName = "vfio";
-
-/* Only the 'vfio' 

[libvirt PATCH v3 6/8] util: honor stubDriverName when probing/binding stub driver for a device

2023-08-21 Thread Laine Stump
Signed-off-by: Laine Stump 
---
 src/util/virpci.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index ac91480e0b..c721b8e533 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1290,17 +1290,20 @@ virPCIDeviceUnbindFromStub(virPCIDevice *dev)
 static int
 virPCIDeviceBindToStub(virPCIDevice *dev)
 {
-const char *stubDriverName;
+const char *stubDriverName = dev->stubDriverName;
 g_autofree char *stubDriverPath = NULL;
 g_autofree char *driverLink = NULL;
 
-/* Check the device is configured to use one of the known stub drivers */
+
 if (dev->stubDriverType == VIR_PCI_STUB_DRIVER_NONE) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("No stub driver configured for PCI device %1$s"),
dev->name);
 return -1;
-} else if (!(stubDriverName = 
virPCIStubDriverTypeToString(dev->stubDriverType))) {
+}
+
+if (!stubDriverName
+&& !(stubDriverName = 
virPCIStubDriverTypeToString(dev->stubDriverType))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown stub driver configured for PCI device %1$s"),
dev->name);
-- 
2.41.0



[libvirt PATCH v3 3/8] util: rename virPCIDeviceGetDriverPathAndName

2023-08-21 Thread Laine Stump
Instead, call it virPCIDeviceGetCurrentDriverPathAndName() to avoid
confusion with the device name that is stored in the virPCIDevice
object - that one is not necessarily the name of the current driver
for the device, but could instead be the driver that we want to be
bound to the device in the future.

Signed-off-by: Laine Stump 
---
 src/hypervisor/virhostdev.c |  7 ---
 src/libvirt_private.syms|  2 +-
 src/util/virpci.c   | 10 ++
 src/util/virpci.h   |  6 +++---
 tests/virpcitest.c  |  2 +-
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index c437ca9d22..244f057c6c 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -765,9 +765,10 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
  *   information about active / inactive device across
  *   daemon restarts has been implemented */
 
-if (virPCIDeviceGetDriverPathAndName(pci,
- , ) < 0)
+if (virPCIDeviceGetCurrentDriverPathAndName(pci, ,
+) < 0) {
 goto reattachdevs;
+}
 
 stub = virPCIStubDriverTypeFromString(driverName);
 
@@ -2294,7 +2295,7 @@ virHostdevPrepareOneNVMeDevice(virHostdevManager 
*hostdev_mgr,
 g_autofree char *drvName = NULL;
 int stub = VIR_PCI_STUB_DRIVER_NONE;
 
-if (virPCIDeviceGetDriverPathAndName(pci, , ) < 0)
+if (virPCIDeviceGetCurrentDriverPathAndName(pci, , ) < 
0)
 goto cleanup;
 
 if (drvName)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fad5389d68..2b577c4e2d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3064,7 +3064,7 @@ virPCIDeviceFileIterate;
 virPCIDeviceFree;
 virPCIDeviceGetAddress;
 virPCIDeviceGetConfigPath;
-virPCIDeviceGetDriverPathAndName;
+virPCIDeviceGetCurrentDriverPathAndName;
 virPCIDeviceGetIOMMUGroupDev;
 virPCIDeviceGetIOMMUGroupList;
 virPCIDeviceGetLinkCapSta;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 103bc4254e..2ec0dc2053 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -228,7 +228,7 @@ virPCIFile(const char *device, const char *file)
 }
 
 
-/* virPCIDeviceGetDriverPathAndName - put the path to the driver
+/* virPCIDeviceGetCurrentDriverPathAndName - put the path to the driver
  * directory of the driver in use for this device in @path and the
  * name of the driver in @name. Both could be NULL if it's not bound
  * to any driver.
@@ -236,7 +236,9 @@ virPCIFile(const char *device, const char *file)
  * Return 0 for success, -1 for error.
  */
 int
-virPCIDeviceGetDriverPathAndName(virPCIDevice *dev, char **path, char **name)
+virPCIDeviceGetCurrentDriverPathAndName(virPCIDevice *dev,
+char **path,
+char **name)
 {
 int ret = -1;
 g_autofree char *drvlink = NULL;
@@ -1032,7 +1034,7 @@ virPCIDeviceReset(virPCIDevice *dev,
  * reset it whenever appropriate, so doing it ourselves would just
  * be redundant.
  */
-if (virPCIDeviceGetDriverPathAndName(dev, , ) < 0)
+if (virPCIDeviceGetCurrentDriverPathAndName(dev, , ) < 0)
 goto cleanup;
 
 if (virPCIStubDriverTypeFromString(drvName) == VIR_PCI_STUB_DRIVER_VFIO) {
@@ -1137,7 +1139,7 @@ virPCIDeviceUnbind(virPCIDevice *dev)
 g_autofree char *drvpath = NULL;
 g_autofree char *driver = NULL;
 
-if (virPCIDeviceGetDriverPathAndName(dev, , ) < 0)
+if (virPCIDeviceGetCurrentDriverPathAndName(dev, , ) < 0)
 return -1;
 
 if (!driver)
diff --git a/src/util/virpci.h b/src/util/virpci.h
index f8f98f39de..19c910202a 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -280,9 +280,9 @@ virPCIVPDResource * virPCIDeviceGetVPD(virPCIDevice *dev);
 
 int virPCIDeviceUnbind(virPCIDevice *dev);
 int virPCIDeviceRebind(virPCIDevice *dev);
-int virPCIDeviceGetDriverPathAndName(virPCIDevice *dev,
- char **path,
- char **name);
+int virPCIDeviceGetCurrentDriverPathAndName(virPCIDevice *dev,
+char **path,
+char **name);
 
 int virPCIDeviceIsPCIExpress(virPCIDevice *dev);
 int virPCIDeviceHasPCIExpressLink(virPCIDevice *dev);
diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 92cc8c07c6..d69a1b5118 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -37,7 +37,7 @@ testVirPCIDeviceCheckDriver(virPCIDevice *dev, const char 
*expected)
 g_autofree char *path = NULL;
 g_autofree char *driver = NULL;
 
-if (virPCIDeviceGetDriverPathAndName(dev, , ) < 0)
+if (virPCIDeviceGetCurrentDriverPathAndName(dev, , ) < 0)
 return -1;
 
 if (STRNEQ_NULLABLE(driver, 

[libvirt PATCH v3 8/8] qemu: turn two multiline log messages into single line

2023-08-21 Thread Laine Stump
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;
 }
 
-- 
2.41.0



[libvirt PATCH v3 0/8] Support for VFIO variant drivers, Part 1

2023-08-21 Thread Laine Stump
A "VFIO variant" driver is a kernel driver for a device that supports
all the APIs of the basic vfio-pci driver (which enables assigning a
host PCI device to a QEMU guest) plus "some extra stuff" (e.g. to
enable things like saving/restoring device state in order to support
live migration.)

Way back last year I posted a couple attempts to support VFIO variant
drivers; here is V2 (along with a later followup discussion from a
couple months ago):

https://listman.redhat.com/archives/libvir-list/2022-August/233661.html
https://listman.redhat.com/archives/libvir-list/2023-May/240108.html

The mlx5-vfio-pci driver has now been upstream for quite awhile (and
even in the downstream Fedora 38 kernel, for example), as are the
sysfs bits that allow us to determine whether or not a driver is a
VFIO variant, and I've updated the patch(es) to use this.

I've also been working on auto-binding to the "best-match" VFIO
variant driver based on comparing the device's modalias file in sysfs
to the contents of the kernel's modules.alias file, but that isn't
quite ready (partly code that isn't yet working, but also partly
indecision about exactly where in the XML to put the driver name when
it is specified; I won't take up more space here with that though).

In the meantime, there are people who want to use the mlx5-vfio-pci
driver (and Cedric Le Goater also has written vfio-pci-igbvf and
vfio-pci-e1000e drivers (which area very useful for testing), although
I don't think he has posted them anywhere yet), so I would like to get
the basic patches here merged in upstream now while I continue working
on "Part 2".

These patches provide two improvements that make testing/using VFIO
drivers much more convenient:

1) The specific driver can be given in the virsh nodedev-detach
command (or the virNodeDeviceDetachFlags() API call), e.g.:

virsh nodedev-detach pci__04_11_5 --driver vfio-pci-igbvf

2) If the  (or " ... " has
"managed='no'", then libvirt will recognize any VFIO variant driver
(rather than the current behavior of rejecting anything that isn't
exactly "vfio-pci")

With these two capabilities, it's simple and straightforward to bind a
device to a VFIO variant driver, and then start a guest that uses that
device.

Change in V2:

* complete remake, more refactoring

* use existence of "vfio-dev" subdirectory of device directory in
  sysfs to determine whether the currently-bound driver is a vfio
  variant.

* support binding to a user-specified driver during nodedev-detach,
  rather than only supporting vfio-pci.

Laine Stump (8):
  util: use "stubDriverType" instead of just "stubDriver"
  util: add stub driver name to virPCIDevice object
  util: rename virPCIDeviceGetDriverPathAndName
  util: permit existing binding to VFIO variant driver
  util: probe stub driver from within function that binds to stub driver
  util: honor stubDriverName when probing/binding stub driver for a
device
  node_device: support binding other drivers with
virNodeDeviceDetachFlags()
  qemu: turn two multiline log messages into single line

 src/hypervisor/domain_driver.c |   9 +-
 src/hypervisor/domain_driver.h |   2 +
 src/hypervisor/virhostdev.c|  35 +++-
 src/libvirt_private.syms   |   9 +-
 src/libxl/libxl_driver.c   |   3 +-
 src/qemu/qemu_driver.c |  37 
 src/util/virnvme.c |   2 +-
 src/util/virpci.c  | 156 +
 src/util/virpci.h  |  18 ++--
 tests/virhostdevtest.c |   2 +-
 tests/virpcitest.c |  10 +--
 11 files changed, 185 insertions(+), 98 deletions(-)

-- 
2.41.0



[libvirt PATCH v3 4/8] util: permit existing binding to VFIO variant driver

2023-08-21 Thread Laine Stump
Before a PCI device can be assigned to a guest with VFIO, that device
must be bound to the vfio-pci driver rather than to the device's
normal host driver. The vfio-pci driver provides APIs that permit QEMU
to perform all the necessary operations to make the device accessible
to the guest.

In the past vfio-pci was the only driver that supplied these APIs, but
there are now vendor/device-specific "VFIO variant" drivers that
provide the basic vfio-pci driver functionality/API while adding
support for device-specific operations (for example these
device-specific drivers may support live migration of certain
devices).  All that is needed to make this functionality available is
to bind the vendor-specific "VFIO variant" driver to the device
(rather than the generic vfio-pci driver, which will continue to work,
just without the extra functionality).

But until now libvirt has required that all PCI devices being assigned
to a guest with VFIO specifically have the "vfio-pci" driver bound to
the device. So even if the user manually binds a shiny new
vendor-specific VFIO variant driver to the device (and puts
"managed='no'" in the config to prevent libvirt from changing the
binding), libvirt will just fail during startup of the guest (or
during hotplug) because the driver bound to the device isn't exactly
"vfio-pci".

Beginning with kernel 6.1, it's possible to determine from the sysfs
directory for a device whether the currently-bound driver is the
vfio-pci driver or a VFIO variant - the device directory will have a
subdirectory called "vfio-dev". We can use that to appropriately widen
the list of drivers that libvirt will allow for VFIO device
assignment.

This patch doesn't remove the explicit check for the exact "vfio-pci"
driver (since that would cause systems with pre-6.1 kernels to behave
incorrectly), but adds an additional check for the vfio-dev directory,
so that any VFIO variant driver is acceptable for libvirt to continue
setting up for VFIO device assignment.

Signed-off-by: Laine Stump 
---
 src/hypervisor/virhostdev.c | 28 +
 src/libvirt_private.syms|  1 +
 src/util/virpci.c   | 78 ++---
 src/util/virpci.h   |  3 ++
 4 files changed, 87 insertions(+), 23 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 244f057c6c..b95d6bf3d6 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -743,9 +743,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
mgr->inactivePCIHostdevs) < 0)
 goto reattachdevs;
 } else {
-g_autofree char *driverPath = NULL;
-g_autofree char *driverName = NULL;
-int stub;
+g_autofree char *drvName = NULL;
+virPCIStubDriver drvType;
 
 /* Unmanaged devices should already have been marked as
  * inactive: if that's the case, we can simply move on */
@@ -765,19 +764,17 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
  *   information about active / inactive device across
  *   daemon restarts has been implemented */
 
-if (virPCIDeviceGetCurrentDriverPathAndName(pci, ,
-) < 0) {
+if (virPCIDeviceGetCurrentDriverNameAndType(pci, ,
+) < 0) {
 goto reattachdevs;
 }
 
-stub = virPCIStubDriverTypeFromString(driverName);
-
-if (stub > VIR_PCI_STUB_DRIVER_NONE &&
-stub < VIR_PCI_STUB_DRIVER_LAST) {
+if (drvType > VIR_PCI_STUB_DRIVER_NONE) {
 
 /* The device is bound to a known stub driver: store this
  * information and add a copy to the inactive list */
-virPCIDeviceSetStubDriverType(pci, stub);
+virPCIDeviceSetStubDriverType(pci, drvType);
+virPCIDeviceSetStubDriverName(pci, drvName);
 
 VIR_DEBUG("Adding PCI device %s to inactive list",
   virPCIDeviceGetName(pci));
@@ -2291,18 +2288,13 @@ virHostdevPrepareOneNVMeDevice(virHostdevManager 
*hostdev_mgr,
 /* Let's check if all PCI devices are NVMe disks. */
 for (i = 0; i < virPCIDeviceListCount(pciDevices); i++) {
 virPCIDevice *pci = virPCIDeviceListGet(pciDevices, i);
-g_autofree char *drvPath = NULL;
 g_autofree char *drvName = NULL;
-int stub = VIR_PCI_STUB_DRIVER_NONE;
+virPCIStubDriver drvType;
 
-if (virPCIDeviceGetCurrentDriverPathAndName(pci, , ) < 
0)
+if (virPCIDeviceGetCurrentDriverNameAndType(pci, , ) < 
0)
 goto cleanup;
 
-if (drvName)
-stub = virPCIStubDriverTypeFromString(drvName);
-
-if (stub == VIR_PCI_STUB_DRIVER_VFIO ||
-STREQ_NULLABLE(drvName, "nvme"))
+

[libvirt PATCH v3 2/8] util: add stub driver name to virPCIDevice object

2023-08-21 Thread Laine Stump
There can be many different drivers that are of the type "VFIO", so
add the driver name to the object and allow getting/setting it.

Signed-off-by: Laine Stump 
---
 src/libvirt_private.syms |  2 ++
 src/util/virpci.c| 16 
 src/util/virpci.h|  3 +++
 3 files changed, 21 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 983109df86..fad5389d68 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3072,6 +3072,7 @@ virPCIDeviceGetManaged;
 virPCIDeviceGetName;
 virPCIDeviceGetRemoveSlot;
 virPCIDeviceGetReprobe;
+virPCIDeviceGetStubDriverName;
 virPCIDeviceGetStubDriverType;
 virPCIDeviceGetUnbindFromStub;
 virPCIDeviceGetUsedBy;
@@ -3098,6 +3099,7 @@ virPCIDeviceReset;
 virPCIDeviceSetManaged;
 virPCIDeviceSetRemoveSlot;
 virPCIDeviceSetReprobe;
+virPCIDeviceSetStubDriverName;
 virPCIDeviceSetStubDriverType;
 virPCIDeviceSetUnbindFromStub;
 virPCIDeviceSetUsedBy;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 88a020fb86..103bc4254e 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -88,6 +88,7 @@ struct _virPCIDevice {
 bool  managed;
 
 virPCIStubDriver stubDriverType;
+char*stubDriverName; /* if blank, use default for type */
 
 /* used by reattach function */
 bool  unbind_from_stub;
@@ -1507,6 +1508,7 @@ virPCIDeviceCopy(virPCIDevice *dev)
 copy->path = g_strdup(dev->path);
 copy->used_by_drvname = g_strdup(dev->used_by_drvname);
 copy->used_by_domname = g_strdup(dev->used_by_domname);
+copy->stubDriverName = g_strdup(dev->stubDriverName);
 return copy;
 }
 
@@ -1521,6 +1523,7 @@ virPCIDeviceFree(virPCIDevice *dev)
 g_free(dev->path);
 g_free(dev->used_by_drvname);
 g_free(dev->used_by_domname);
+g_free(dev->stubDriverName);
 g_free(dev);
 }
 
@@ -1580,6 +1583,19 @@ virPCIDeviceGetStubDriverType(virPCIDevice *dev)
 return dev->stubDriverType;
 }
 
+void
+virPCIDeviceSetStubDriverName(virPCIDevice *dev,
+   const char *driverName)
+{
+dev->stubDriverName = g_strdup(driverName);
+}
+
+const char *
+virPCIDeviceGetStubDriverName(virPCIDevice *dev)
+{
+return dev->stubDriverName;
+}
+
 bool
 virPCIDeviceGetUnbindFromStub(virPCIDevice *dev)
 {
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 485f535bc9..f8f98f39de 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -137,6 +137,9 @@ bool virPCIDeviceGetManaged(virPCIDevice *dev);
 void virPCIDeviceSetStubDriverType(virPCIDevice *dev,
virPCIStubDriver driverType);
 virPCIStubDriver virPCIDeviceGetStubDriverType(virPCIDevice *dev);
+void virPCIDeviceSetStubDriverName(virPCIDevice *dev,
+   const char *driverName);
+const char *virPCIDeviceGetStubDriverName(virPCIDevice *dev);
 virPCIDeviceAddress *virPCIDeviceGetAddress(virPCIDevice *dev);
 int virPCIDeviceSetUsedBy(virPCIDevice *dev,
   const char *drv_name,
-- 
2.41.0



[libvirt PATCH v3 1/8] util: use "stubDriverType" instead of just "stubDriver"

2023-08-21 Thread Laine Stump
In the past we just kept track of the type of the "stub driver" (the
driver that is bound to a device in order to assign it to a
guest). The next commit will add a stubDriverName to go along with
type, so lets use stubDriverType for the existing enum to make it
easier to keep track of whether we're talking about the name or the
type.

Signed-off-by: Laine Stump 
---
 src/hypervisor/domain_driver.c |  4 ++--
 src/hypervisor/virhostdev.c|  8 
 src/libvirt_private.syms   |  4 ++--
 src/util/virnvme.c |  2 +-
 src/util/virpci.c  | 16 
 src/util/virpci.h  |  6 +++---
 tests/virhostdevtest.c |  2 +-
 tests/virpcitest.c |  8 
 8 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 66e09ffb04..a70f75f3ae 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -505,9 +505,9 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
 return -1;
 
 if (STREQ(driverName, "vfio"))
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
+virPCIDeviceSetStubDriverType(pci, VIR_PCI_STUB_DRIVER_VFIO);
 else if (STREQ(driverName, "xen"))
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
+virPCIDeviceSetStubDriverType(pci, VIR_PCI_STUB_DRIVER_XEN);
 
 return virHostdevPCINodeDeviceDetach(hostdevMgr, pci);
 }
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index eac3474783..c437ca9d22 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -244,9 +244,9 @@ virHostdevGetPCIHostDevice(const virDomainHostdevDef 
*hostdev,
 virPCIDeviceSetManaged(actual, hostdev->managed);
 
 if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO);
+virPCIDeviceSetStubDriverType(actual, VIR_PCI_STUB_DRIVER_VFIO);
 } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) {
-virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN);
+virPCIDeviceSetStubDriverType(actual, VIR_PCI_STUB_DRIVER_XEN);
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("pci backend driver '%1$s' is not supported"),
@@ -679,7 +679,7 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
 for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
 virPCIDevice *pci = virPCIDeviceListGet(pcidevs, i);
 bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
-bool usesVFIO = (virPCIDeviceGetStubDriver(pci) == 
VIR_PCI_STUB_DRIVER_VFIO);
+bool usesVFIO = (virPCIDeviceGetStubDriverType(pci) == 
VIR_PCI_STUB_DRIVER_VFIO);
 struct virHostdevIsPCINodeDeviceUsedData data = {mgr, drv_name, 
dom_name, false};
 int hdrType = -1;
 
@@ -776,7 +776,7 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
 
 /* The device is bound to a known stub driver: store this
  * information and add a copy to the inactive list */
-virPCIDeviceSetStubDriver(pci, stub);
+virPCIDeviceSetStubDriverType(pci, stub);
 
 VIR_DEBUG("Adding PCI device %s to inactive list",
   virPCIDeviceGetName(pci));
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index da60c965dd..983109df86 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3072,7 +3072,7 @@ virPCIDeviceGetManaged;
 virPCIDeviceGetName;
 virPCIDeviceGetRemoveSlot;
 virPCIDeviceGetReprobe;
-virPCIDeviceGetStubDriver;
+virPCIDeviceGetStubDriverType;
 virPCIDeviceGetUnbindFromStub;
 virPCIDeviceGetUsedBy;
 virPCIDeviceGetVPD;
@@ -3098,7 +3098,7 @@ virPCIDeviceReset;
 virPCIDeviceSetManaged;
 virPCIDeviceSetRemoveSlot;
 virPCIDeviceSetReprobe;
-virPCIDeviceSetStubDriver;
+virPCIDeviceSetStubDriverType;
 virPCIDeviceSetUnbindFromStub;
 virPCIDeviceSetUsedBy;
 virPCIDeviceUnbind;
diff --git a/src/util/virnvme.c b/src/util/virnvme.c
index f7f8dc5ea9..37333d515b 100644
--- a/src/util/virnvme.c
+++ b/src/util/virnvme.c
@@ -292,7 +292,7 @@ virNVMeDeviceCreatePCIDevice(const virNVMeDevice *nvme)
 return NULL;
 
 /* NVMe devices must be bound to vfio */
-virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
+virPCIDeviceSetStubDriverType(pci, VIR_PCI_STUB_DRIVER_VFIO);
 virPCIDeviceSetManaged(pci, nvme->managed);
 
 return g_steal_pointer();
diff --git a/src/util/virpci.c b/src/util/virpci.c
index cc2b07bbba..88a020fb86 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -87,7 +87,7 @@ struct _virPCIDevice {
 
 bool  managed;
 
-virPCIStubDriver stubDriver;
+virPCIStubDriver stubDriverType;
 
 /* used by reattach function */
 bool  unbind_from_stub;
@@ -1233,12 +1233,12 @@ virPCIDeviceBindToStub(virPCIDevice *dev)
 

Re: [PATCH 14/24] qemuValidateDomainVCpuTopology: Remove misconfiguration warning

2023-08-21 Thread Daniel P . Berrangé
On Mon, Aug 21, 2023 at 02:57:09PM +0200, Ján Tomko wrote:
> On a Thursday in 2023, Peter Krempa wrote:
> > VIR_WARN of a misconfiguration of a VM is not really going to be seen in
> > most cases as it's simply logged, and if the VM works users are not
> > likely to dig through logs.
> 
> Should we put it under VIR_DOMAIN_START_WERROR? :)

If a config scenario is explicitly deprecated  and expected to be
remove, then we should be using the 'taint' mechanism to alert
users/apps. This is queryable by apps and also ends up in the per
VM log files as a warning.



With 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/9] Clean up _virDomainMemoryDef struct

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Michal Privoznik wrote:

*** BLURB HERE ***

Michal Prívozník (9):
 virt-aa-helper: Rework setting virDomainMemoryDef labels
 virt-aa-helper: Set label on VIRTIO_PMEM device too
 qemu_hotplug: validate address on memory device change
 qemu_hotplug: Don't validate inaccessible fields in
   qemuDomainChangeMemoryLiveValidateChange()
 conf: Compare memory device address in
   virDomainMemoryFindByDefInternal()
 qemu_driver: validate mem->model on MEMORY_DEVICE_SIZE_CHANGE event
 src: Move _virDomainMemoryDef source nodes into an union
 src: Move _virDomainMemoryDef target nodes into an union
 src: Rename some members of _virDomainMemoryDef struct

src/conf/domain_conf.c   | 285 +--
src/conf/domain_conf.h   |  72 +---
src/conf/domain_postparse.c  |   6 +-
src/conf/domain_validate.c   |  18 +-
src/qemu/qemu_cgroup.c   |  12 +-
src/qemu/qemu_command.c  | 131 ++
src/qemu/qemu_domain.c   |  13 +-
src/qemu/qemu_driver.c   |  15 +-
src/qemu/qemu_hotplug.c  |  62 ++-
src/qemu/qemu_namespace.c|   4 +-
src/qemu/qemu_process.c  |  14 +-
src/qemu/qemu_validate.c |   6 +-
src/security/security_apparmor.c |  24 ++-
src/security/security_dac.c  |   9 +-
src/security/security_selinux.c  |  52 +++---
src/security/virt-aa-helper.c|  19 ++-
16 files changed, 474 insertions(+), 268 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 7/9] src: Move _virDomainMemoryDef source nodes into an union

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Michal Privoznik wrote:

The _virDomainMemoryDef struct is getting a bit messy. It has
various members and only some of them are valid for given model.
Worse, some are re-used for different models. We tried to make
this more bearable by putting a comment next to each member
describing what models the member is valid for, but that gets
messy too.

Therefore, do what we do elsewhere: introduce an union of structs
and move individual members into their respective groups.

This allows us to shorten some names (e.g. nvdimmPath or
sourceNodes) as their purpose is obvious due to their placement.
But to make this commit as small as possible, that'll be
addressed later.

Signed-off-by: Michal Privoznik 
---
src/conf/domain_conf.c   | 125 +++
src/conf/domain_conf.h   |  29 +--
src/conf/domain_validate.c   |   4 +-
src/qemu/qemu_cgroup.c   |  12 ++-
src/qemu/qemu_command.c  |  97 ++--
src/qemu/qemu_hotplug.c  |  10 +--
src/qemu/qemu_namespace.c|   4 +-
src/qemu/qemu_process.c  |  10 ++-
src/qemu/qemu_validate.c |   4 +-
src/security/security_apparmor.c |  24 --
src/security/security_dac.c  |   9 ++-
src/security/security_selinux.c  |  52 ++---
src/security/virt-aa-helper.c|   5 +-
13 files changed, 272 insertions(+), 113 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a4a26d875..a1dad679dd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,8 +3490,27 @@ void virDomainMemoryDefFree(virDomainMemoryDef *def)
if (!def)
return;

-g_free(def->nvdimmPath);
-virBitmapFree(def->sourceNodes);
+switch (def->model) {
+case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+virBitmapFree(def->source.dimm.sourceNodes);
+break;
+case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+g_free(def->source.nvdimm.nvdimmPath);
+break;
+case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+g_free(def->source.virtio_pmem.nvdimmPath);
+break;
+case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+virBitmapFree(def->source.dimm.sourceNodes);


def->source.virtio_mem.sourceNodes


+break;
+case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
+virBitmapFree(def->source.sgx_epc.sourceNodes);
+break;
+case VIR_DOMAIN_MEMORY_MODEL_NONE:
+case VIR_DOMAIN_MEMORY_MODEL_LAST:
+break;
+}
+
g_free(def->uuid);
virDomainDeviceInfoClear(>info);
g_free(def);


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 24/24] qemuxml2xmltest: Modernize all remaining fake capability tests

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Convert all cases using DO_TEST() to use DO_TEST_CAPS_LATEST() and
remove DO_TEST() to prevent further use.

Most of the changes are related to CPU being present in the output XML.

Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/boot-floppy-q35.xml|  16 +
.../disk-virtio-scsi-reservations.xml |   5 +-
.../memory-hotplug-nvdimm-access.xml  |   5 +-
.../memory-hotplug-nvdimm-align.xml   |   5 +-
.../memory-hotplug-nvdimm-label.xml   |   5 +-
.../memory-hotplug-nvdimm-pmem.xml|   5 +-
.../memory-hotplug-nvdimm-readonly.xml|   5 +-
.../memory-hotplug-nvdimm.xml |   5 +-
tests/qemuxml2argvdata/numatune-hmat.xml  |   5 +-
...oindex.xml => autoindex.x86_64-latest.xml} |   5 +-
xml => boot-floppy-q35.x86_64-latest.xml} |   0
...el.xml => chardev-label.x86_64-latest.xml} |   5 +-
...up.xml => clock-catchup.x86_64-latest.xml} |   5 +-
... controller-virtio-scsi.x86_64-latest.xml} |   5 +-
...=> disk-cdrom-bus-other.x86_64-latest.xml} |   5 +-
...l => disk-network-iscsi.x86_64-latest.xml} |   5 +-
...> disk-scsi-device-auto.x86_64-latest.xml} |   5 +-
...l => disk-scsi-disk-vpd.x86_64-latest.xml} |   5 +-
xml => disk-usb-device.x86_64-latest.xml} |   5 +-
...irtio-scsi-reservations.x86_64-latest.xml} |   0
...egl-headless-rendernode.x86_64-latest.xml} |   5 +-
...graphics-listen-network.x86_64-latest.xml} |   5 +-
...raphics-listen-network2.x86_64-latest.xml} |   5 +-
...graphics-sdl-fullscreen.x86_64-latest.xml} |   5 +-
...sdl.xml => graphics-sdl.x86_64-latest.xml} |   5 +-
...ics-vnc-auto-socket-cfg.x86_64-latest.xml} |   5 +-
...raphics-vnc-auto-socket.x86_64-latest.xml} |   5 +-
...aphics-vnc-egl-headless.x86_64-latest.xml} |   5 +-
...hics-vnc-no-listen-attr.x86_64-latest.xml} |   5 +-
...generated-socket-active.x86_64-latest.xml} |   5 +-
...nerated-socket-inactive.x86_64-latest.xml} |   5 +-
...ml => graphics-vnc-sasl.x86_64-latest.xml} |   5 +-
... => graphics-vnc-socket.x86_64-latest.xml} |   5 +-
...xml => graphics-vnc-tls.x86_64-latest.xml} |   5 +-
... graphics-vnc-websocket.x86_64-latest.xml} |   5 +-
...vnc.xml => graphics-vnc.x86_64-latest.xml} |   5 +-
...=> hostdev-mdev-display.x86_64-latest.xml} |   5 +-
...hostdev-mdev-precreated.x86_64-latest.xml} |   7 +-
...-pci-address-unassigned.x86_64-latest.xml} |   5 +-
...stdev-pci-multifunction.x86_64-latest.xml} |   5 +-
...ev-scsi-autogen-address.x86_64-latest.xml} |   5 +-
...hostdev-scsi-large-unit.x86_64-latest.xml} |   5 +-
...xml => hostdev-scsi-lsi.x86_64-latest.xml} |   5 +-
... hostdev-scsi-shareable.x86_64-latest.xml} |   5 +-
...dev-scsi-vhost-scsi-pci.x86_64-latest.xml} |   5 +-
...ostdev-scsi-virtio-scsi.x86_64-latest.xml} |   5 +-
...fio.xml => hostdev-vfio.x86_64-latest.xml} |   5 +-
... => memory-hotplug-dimm.x86_64-latest.xml} |   5 +-
...y-hotplug-nvdimm-access.x86_64-latest.xml} |   0
...ry-hotplug-nvdimm-align.x86_64-latest.xml} |   0
...ry-hotplug-nvdimm-label.x86_64-latest.xml} |   0
...ory-hotplug-nvdimm-pmem.x86_64-latest.xml} |   0
...hotplug-nvdimm-readonly.x86_64-latest.xml} |   0
...> memory-hotplug-nvdimm.x86_64-latest.xml} |   0
xml => misc-disable-s3.x86_64-latest.xml} |   5 +-
...> misc-disable-suspends.x86_64-latest.xml} |   5 +-
...4.xml => misc-enable-s4.x86_64-latest.xml} |   5 +-
...at.xml => numatune-hmat.x86_64-latest.xml} |   0
...ble.xml => panic-double.x86_64-latest.xml} |   5 +-
...xml => panic-no-address.x86_64-latest.xml} |   5 +-
.../{panic.xml => panic.x86_64-latest.xml}|   5 +-
...xml => pci-autoadd-addr.x86_64-latest.xml} |   5 +-
xml => pci-autoadd-idx.x86_64-latest.xml} |   5 +-
...ml => pci-autofill-addr.x86_64-latest.xml} |   5 +-
...> pci-bridge-many-disks.x86_64-latest.xml} |   5 +-
...ridge.xml => pci-bridge.x86_64-latest.xml} |   5 +-
...xml => pci-expander-bus.x86_64-latest.xml} |   3 +-
...ci-many.xml => pci-many.x86_64-latest.xml} |   5 +-
...ml => pcie-expander-bus.x86_64-latest.xml} |  13 +-
...root-port-model-generic.x86_64-latest.xml} |   6 +
...root-port-model-ioh3420.x86_64-latest.xml} |   8 +-
...t.xml => pcie-root-port.x86_64-latest.xml} |   8 +-
.../pcie-root.x86_64-latest.xml   |  43 +++
tests/qemuxml2xmloutdata/pcie-root.xml|  27 --
...-switch-downstream-port.x86_64-latest.xml} |   8 +-
...cie-switch-upstream-port.x86_64-latest.xml |  66 
...35.xml => pcihole64-q35.x86_64-latest.xml} |  31 +-
tests/qemuxml2xmloutdata/pcihole64-q35.xml|  39 --
...> q35-pci-force-address.x86_64-latest.xml} |   5 +-
...i.xml => q35-usb2-multi.x86_64-latest.xml} |   3 +
...xml => q35-usb2-reorder.x86_64-latest.xml} |   3 +
...35-usb2.xml => q35-usb2.x86_64-latest.xml} |   3 +
...pstream-port.xml => q35.x86_64-latest.xml} |  24 +-
...xml => serial-spiceport.x86_64-latest.xml} |   5 +-
...=> smartcard-controller.x86_64-latest.xml} |   7 +-
...t-certificates-database.x86_64-latest.xml} |   5 +-

Re: [PATCH 23/24] qemuxml2xmltest: Modernize all 'net-' tests

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
.../net-virtio-teaming-hostdev.xml|  5 -
...th.xml => net-bandwidth.x86_64-latest.xml} |  5 -
...2.xml => net-bandwidth2.x86_64-latest.xml} |  5 -
...xml => net-hostdev-vfio.x86_64-latest.xml} |  5 -
tests/qemuxml2xmloutdata/net-user-passt.xml   |  1 -
... => net-vdpa-multiqueue.x86_64-latest.xml} |  5 -
...et-vdpa.xml => net-vdpa.x86_64-latest.xml} |  5 -
...-virtio-teaming-hostdev.x86_64-latest.xml} |  0
...-virtio-teaming-network.x86_64-latest.xml} |  5 -
...l => net-virtio-teaming.x86_64-latest.xml} |  5 -
tests/qemuxml2xmltest.c   | 19 ---
11 files changed, 40 insertions(+), 20 deletions(-)
rename tests/qemuxml2xmloutdata/{net-bandwidth.xml => 
net-bandwidth.x86_64-latest.xml} (94%)
rename tests/qemuxml2xmloutdata/{net-bandwidth2.xml => 
net-bandwidth2.x86_64-latest.xml} (93%)
rename tests/qemuxml2xmloutdata/{net-hostdev-vfio.xml => 
net-hostdev-vfio.x86_64-latest.xml} (92%)
delete mode 12 tests/qemuxml2xmloutdata/net-user-passt.xml
rename tests/qemuxml2xmloutdata/{net-vdpa-multiqueue.xml => 
net-vdpa-multiqueue.x86_64-latest.xml} (88%)
rename tests/qemuxml2xmloutdata/{net-vdpa.xml => net-vdpa.x86_64-latest.xml} 
(88%)
rename tests/qemuxml2xmloutdata/{net-virtio-teaming-hostdev.xml => 
net-virtio-teaming-hostdev.x86_64-latest.xml} (100%)
rename tests/qemuxml2xmloutdata/{net-virtio-teaming-network.xml => 
net-virtio-teaming-network.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{net-virtio-teaming.xml => 
net-virtio-teaming.x86_64-latest.xml} (94%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 22/24] qemuxml2argvtest: Modernize 'net-*'

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Convert all tests using fake capabilities to use DO_TEST_CAPS_LATEST.

Note that rename detection in git didn't work too well here and the
files may not correspond.

Signed-off-by: Peter Krempa 
---
.../net-client.x86_64-latest.args | 38 
...args => net-eth-hostip.x86_64-latest.args} | 16 ++---
...args => net-eth-ifname.x86_64-latest.args} | 19 +++---
.../net-eth-names.x86_64-latest.args  | 40 
.../net-eth-unmanaged-tap.args| 35 ---
.../net-eth-unmanaged-tap.x86_64-latest.args  | 37 +++
tests/qemuxml2argvdata/net-eth.args   | 35 ---
.../net-eth.x86_64-latest.args| 37 +++
.../net-hostdev-bootorder.args| 34 ---
... net-hostdev-bootorder.x86_64-latest.args} | 16 ++---
...err => net-hostdev-fail.x86_64-latest.err} |  0
.../net-hostdev-multidomain.args  | 35 ---
...net-hostdev-multidomain.x86_64-latest.args | 37 +++
.../net-hostdev-vfio-multidomain.args | 35 ---
...ostdev-vfio-multidomain.x86_64-latest.args | 37 +++
tests/qemuxml2argvdata/net-hostdev-vfio.args  | 35 ---
.../net-hostdev-vfio.x86_64-latest.args   | 37 +++
tests/qemuxml2argvdata/net-hostdev.args   | 35 ---
.../net-hostdev.x86_64-latest.args| 37 +++
tests/qemuxml2argvdata/net-mcast.args | 36 ---
.../net-mcast.x86_64-latest.args  | 38 
tests/qemuxml2argvdata/net-server.args| 36 ---
.../net-server.x86_64-latest.args | 38 
tests/qemuxml2argvdata/net-udp.args   | 36 ---
.../net-udp.x86_64-latest.args| 38 
tests/qemuxml2argvdata/net-user.args  | 35 ---
...r => net-vhostuser-fail.x86_64-latest.err} |  0
.../net-vhostuser-multiq.args | 47 --
.../net-vhostuser-multiq.x86_64-latest.args   | 49 +++
tests/qemuxml2argvdata/net-vhostuser.args | 38 
tests/qemuxml2argvdata/net-virtio-device.args | 36 ---
.../net-virtio-device.x86_64-latest.args  | 38 
...irtio-disable-offloads.x86_64-latest.args} | 18 +++---
tests/qemuxml2argvdata/net-virtio-netdev.args | 36 ---
.../net-virtio-netdev.x86_64-latest.args  | 38 
...xqueuesize-invalid-size.x86_64-latest.err} |  0
.../net-virtio-rxtxqueuesize.args | 36 ---
...et-virtio-rxtxqueuesize.x86_64-latest.args | 38 
.../net-virtio-teaming-hostdev.args   | 40 
...-virtio-teaming-hostdev.x86_64-latest.args | 42 +
.../qemuxml2argvdata/net-virtio-teaming.args  | 40 
.../net-virtio-teaming.x86_64-latest.args | 42 +
tests/qemuxml2argvdata/net-virtio.args| 35 ---
...tip.args => net-virtio.x86_64-latest.args} | 16 ++---
tests/qemuxml2argvtest.c  | 61 +--
45 files changed, 735 insertions(+), 767 deletions(-)
create mode 100644 tests/qemuxml2argvdata/net-client.x86_64-latest.args


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 21/24] qemuxml2xmltest: Modernize all 'video-*' cases

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
.../qemuxml2argvdata/video-qxl-resolution.xml |  5 ++-
...-device-pciaddr-default.x86_64-latest.xml} |  5 ++-
...ml => video-none-device.x86_64-latest.xml} |  5 ++-
xml => video-qxl-heads.x86_64-latest.xml} |  5 ++-
...ml => video-qxl-noheads.x86_64-latest.xml} |  5 ++-
...=> video-qxl-resolution.x86_64-latest.xml} |  0
...video-virtio-gpu-device.x86_64-latest.xml} |  5 ++-
...video-virtio-gpu-sdl-gl.x86_64-latest.xml} |  5 ++-
...eo-virtio-gpu-secondary.x86_64-latest.xml} |  5 ++-
...deo-virtio-gpu-spice-gl.x86_64-latest.xml} |  5 ++-
... video-virtio-gpu-virgl.x86_64-latest.xml} |  5 ++-
tests/qemuxml2xmltest.c   | 33 ++-
12 files changed, 50 insertions(+), 33 deletions(-)
rename tests/qemuxml2xmloutdata/{video-device-pciaddr-default.xml => 
video-device-pciaddr-default.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{video-none-device.xml => 
video-none-device.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{video-qxl-heads.xml => 
video-qxl-heads.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{video-qxl-noheads.xml => 
video-qxl-noheads.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{video-qxl-resolution.xml => 
video-qxl-resolution.x86_64-latest.xml} (100%)
rename tests/qemuxml2xmloutdata/{video-virtio-gpu-device.xml => 
video-virtio-gpu-device.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{video-virtio-gpu-sdl-gl.xml => 
video-virtio-gpu-sdl-gl.x86_64-latest.xml} (90%)
rename tests/qemuxml2xmloutdata/{video-virtio-gpu-secondary.xml => 
video-virtio-gpu-secondary.x86_64-latest.xml} (88%)
rename tests/qemuxml2xmloutdata/{video-virtio-gpu-spice-gl.xml => 
video-virtio-gpu-spice-gl.x86_64-latest.xml} (91%)
rename tests/qemuxml2xmloutdata/{video-virtio-gpu-virgl.xml => 
video-virtio-gpu-virgl.x86_64-latest.xml} (90%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 20/24] qemuxml2argvtest: Moderinze 'video-*' cases

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Use latest capabilities for all tests.

Signed-off-by: Peter Krempa 
---
.../video-device-pciaddr-default.args | 37 --
...-device-pciaddr-default.x86_64-latest.args | 39 +++
...nvalid-multiple-devices.x86_64-latest.err} |  0
...ideo-multiple-primaries.x86_64-latest.err} |  0
...s => video-none-device.x86_64-latest.args} | 14 ++--
...video-qxl-device-vgamem.x86_64-latest.args | 37 ++
.../video-qxl-device.x86_64-latest.args   | 37 ++
.../video-qxl-heads.x86_64-latest.args| 39 +++
tests/qemuxml2argvdata/video-qxl-noheads.args | 35 --
...s => video-qxl-noheads.x86_64-latest.args} | 18 ++---
.../video-qxl-resolution.args | 35 --
...> video-qxl-resolution.x86_64-latest.args} | 18 ++---
...o-qxl-sec-device-vgamem.x86_64-latest.args | 38 ++
.../video-qxl-sec-device.x86_64-latest.args   | 38 ++
...ideo-vga-device-vgamem.x86_64-latest.args} | 17 ++---
tests/qemuxml2argvdata/video-vga-device.args  | 35 --
...gs => video-vga-device.x86_64-latest.args} | 17 ++---
.../video-vga-qxl-heads.x86_64-latest.args| 39 +++
.../video-virtio-gpu-device.args  | 35 --
...ideo-virtio-gpu-device.x86_64-latest.args} | 16 +++--
...ideo-virtio-gpu-sdl-gl.x86_64-latest.args} | 16 +++--
.../video-virtio-gpu-secondary.args   | 33 -
...o-virtio-gpu-secondary.x86_64-latest.args} | 18 ++---
...eo-virtio-gpu-spice-gl.x86_64-latest.args} | 16 +++--
.../video-virtio-gpu-virgl.args   | 35 --
.../video-virtio-gpu-virgl.x86_64-latest.args | 37 ++
tests/qemuxml2argvdata/video-virtio-vga.args  | 35 --
...gs => video-virtio-vga.x86_64-latest.args} | 16 +++--
tests/qemuxml2argvtest.c  | 69 ++-
29 files changed, 413 insertions(+), 406 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/video-device-pciaddr-default.args
create mode 100644 
tests/qemuxml2argvdata/video-device-pciaddr-default.x86_64-latest.args
rename tests/qemuxml2argvdata/{video-invalid-multiple-devices.err => 
video-invalid-multiple-devices.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{video-multiple-primaries.err => 
video-multiple-primaries.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{video-none-device.args => 
video-none-device.x86_64-latest.args} (61%)
create mode 100644 
tests/qemuxml2argvdata/video-qxl-device-vgamem.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-qxl-device.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-qxl-heads.x86_64-latest.args
delete mode 100644 tests/qemuxml2argvdata/video-qxl-noheads.args
rename tests/qemuxml2argvdata/{video-qxl-heads.args => 
video-qxl-noheads.x86_64-latest.args} (55%)
delete mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.args
rename tests/qemuxml2argvdata/{video-vga-qxl-heads.args => 
video-qxl-resolution.x86_64-latest.args} (55%)
create mode 100644 
tests/qemuxml2argvdata/video-qxl-sec-device-vgamem.x86_64-latest.args
create mode 100644 
tests/qemuxml2argvdata/video-qxl-sec-device.x86_64-latest.args
rename tests/qemuxml2argvdata/{video-qxl-sec-device.args => 
video-vga-device-vgamem.x86_64-latest.args} (59%)
delete mode 100644 tests/qemuxml2argvdata/video-vga-device.args
rename tests/qemuxml2argvdata/{video-qxl-sec-device-vgamem.args => 
video-vga-device.x86_64-latest.args} (59%)
create mode 100644 tests/qemuxml2argvdata/video-vga-qxl-heads.x86_64-latest.args
delete mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-device.args
rename tests/qemuxml2argvdata/{video-qxl-device-vgamem.args => 
video-virtio-gpu-device.x86_64-latest.args} (59%)
rename tests/qemuxml2argvdata/{video-virtio-gpu-sdl-gl.args => 
video-virtio-gpu-sdl-gl.x86_64-latest.args} (58%)
delete mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-secondary.args
rename tests/qemuxml2argvdata/{video-vga-device-vgamem.args => 
video-virtio-gpu-secondary.x86_64-latest.args} (51%)
rename tests/qemuxml2argvdata/{video-virtio-gpu-spice-gl.args => 
video-virtio-gpu-spice-gl.x86_64-latest.args} (60%)
delete mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-virgl.args
create mode 100644 
tests/qemuxml2argvdata/video-virtio-gpu-virgl.x86_64-latest.args
delete mode 100644 tests/qemuxml2argvdata/video-virtio-vga.args
rename tests/qemuxml2argvdata/{video-qxl-device.args => 
video-virtio-vga.x86_64-latest.args} (59%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 18/24] qemu: Retire 'ivshmem' device

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

The device was removed in qemu-4.0 and is superseded by 'ivshmem-plain'
and 'ivshmem-doorbell'.

Always report error when the old version is used and drop the irrelevant
tests.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_command.c| 40 +---
src/qemu/qemu_hotplug.c| 14 +-
src/qemu/qemu_migration.c  |  5 --
src/qemu/qemu_validate.c   | 10 ++--
tests/qemuxml2argvdata/shmem.args  | 43 -
tests/qemuxml2argvdata/shmem.err   |  1 -
tests/qemuxml2argvdata/shmem.xml   | 56 --
tests/qemuxml2argvtest.c   |  2 -
tests/qemuxml2xmloutdata/shmem.xml | 74 --
tests/qemuxml2xmltest.c|  1 -
10 files changed, 5 insertions(+), 241 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/shmem.args
delete mode 100644 tests/qemuxml2argvdata/shmem.err
delete mode 100644 tests/qemuxml2argvdata/shmem.xml
delete mode 100644 tests/qemuxml2xmloutdata/shmem.xml



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 19/24] qemu: capabilities: Retire unused QEMU_CAPS_DEVICE_IVSHMEM

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

qemu removed the support for the old 'ivshmem' device in 4.0 release.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c | 3 +--
src/qemu/qemu_capabilities.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 16/24] qemuxml2(argv|xml)test: Modernize 'fd-memory*' test cases

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Use latest real capabilities.

Signed-off-by: Peter Krempa 
---
...emory-no-numa-topology.x86_64-latest.args} | 16 
.../fd-memory-no-numa-topology.xml|  5 ++-
...d-memory-numa-topology.x86_64-latest.args} | 15 
.../fd-memory-numa-topology.xml   |  5 ++-
...-memory-numa-topology2.x86_64-latest.args} | 17 +
.../fd-memory-numa-topology2.xml  |  5 ++-
.../fd-memory-numa-topology3.args | 37 --
...d-memory-numa-topology3.x86_64-latest.args | 38 +++
.../fd-memory-numa-topology3.xml  |  5 ++-
tests/qemuxml2argvtest.c  |  8 ++--
...memory-no-numa-topology.x86_64-latest.xml} |  0
...fd-memory-numa-topology.x86_64-latest.xml} |  0
...d-memory-numa-topology2.x86_64-latest.xml} |  0
...d-memory-numa-topology3.x86_64-latest.xml} |  0
tests/qemuxml2xmltest.c   |  8 ++--
15 files changed, 83 insertions(+), 76 deletions(-)
rename tests/qemuxml2argvdata/{fd-memory-no-numa-topology.args => 
fd-memory-no-numa-topology.x86_64-latest.args} (51%)
rename tests/qemuxml2argvdata/{fd-memory-numa-topology.args => 
fd-memory-numa-topology.x86_64-latest.args} (55%)
rename tests/qemuxml2argvdata/{fd-memory-numa-topology2.args => 
fd-memory-numa-topology2.x86_64-latest.args} (51%)
delete mode 100644 tests/qemuxml2argvdata/fd-memory-numa-topology3.args
create mode 100644 
tests/qemuxml2argvdata/fd-memory-numa-topology3.x86_64-latest.args
rename tests/qemuxml2xmloutdata/{fd-memory-no-numa-topology.xml => 
fd-memory-no-numa-topology.x86_64-latest.xml} (100%)
rename tests/qemuxml2xmloutdata/{fd-memory-numa-topology.xml => 
fd-memory-numa-topology.x86_64-latest.xml} (100%)
rename tests/qemuxml2xmloutdata/{fd-memory-numa-topology2.xml => 
fd-memory-numa-topology2.x86_64-latest.xml} (100%)
rename tests/qemuxml2xmloutdata/{fd-memory-numa-topology3.xml => 
fd-memory-numa-topology3.x86_64-latest.xml} (100%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 17/24] qemuxml2(argv|xml)test: Modernize 'shmem' test cases

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Upgrade the relevant test cases to use latest capabilities. Note that
the 'shmem' (ivshmem) device is no longer supported and will be dropped
later.

Signed-off-by: Peter Krempa 
---
...> shmem-invalid-address.x86_64-latest.err} |  0
.../shmem-invalid-address.xml |  1 +
...r => shmem-invalid-size.x86_64-latest.err} |  0
tests/qemuxml2argvdata/shmem-invalid-size.xml |  1 +
...y.err => shmem-msi-only.x86_64-latest.err} |  0
tests/qemuxml2argvdata/shmem-msi-only.xml |  1 +
.../shmem-plain-doorbell.args | 46 --
.../shmem-plain-doorbell.x86_64-latest.args   | 48 +++
...err => shmem-small-size.x86_64-latest.err} |  0
tests/qemuxml2argvdata/shmem-small-size.xml   |  1 +
tests/qemuxml2argvtest.c  | 15 ++
...=> shmem-plain-doorbell.x86_64-latest.xml} |  5 +-
tests/qemuxml2xmltest.c   |  3 +-
13 files changed, 62 insertions(+), 59 deletions(-)
rename tests/qemuxml2argvdata/{shmem-invalid-address.err => 
shmem-invalid-address.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{shmem-invalid-size.err => 
shmem-invalid-size.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{shmem-msi-only.err => 
shmem-msi-only.x86_64-latest.err} (100%)
delete mode 100644 tests/qemuxml2argvdata/shmem-plain-doorbell.args
create mode 100644 
tests/qemuxml2argvdata/shmem-plain-doorbell.x86_64-latest.args
rename tests/qemuxml2argvdata/{shmem-small-size.err => 
shmem-small-size.x86_64-latest.err} (100%)
rename tests/qemuxml2xmloutdata/{shmem-plain-doorbell.xml => 
shmem-plain-doorbell.x86_64-latest.xml} (94%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 15/24] qemuValidateDomainVCpuTopology: Always validate vcpu count against topology

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Historically we've used QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS as witness
that the topology must cover the maximum number ov vcpus. qemu started
to enforce this in qemu-2.5, thus we can now always do the check.

This change also requires aligning the topology in certain test files.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_validate.c  | 42 ---
.../fd-memory-numa-topology2.args |  2 +-
.../fd-memory-numa-topology2.xml  |  2 +-
.../fd-memory-numa-topology3.args |  2 +-
.../fd-memory-numa-topology3.xml  |  2 +-
5 files changed, 22 insertions(+), 28 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 14/24] qemuValidateDomainVCpuTopology: Remove misconfiguration warning

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

VIR_WARN of a misconfiguration of a VM is not really going to be seen in
most cases as it's simply logged, and if the VM works users are not
likely to dig through logs.


Should we put it under VIR_DOMAIN_START_WERROR? :)



Signed-off-by: Peter Krempa 
---
src/qemu/qemu_validate.c | 8 
1 file changed, 8 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 12/24] qemuxml2(argv|xml)test: Modernize 'graphics-dbus*' tests

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Use latest caps for this rather recent graphics protocol.

Signed-off-by: Peter Krempa 
---
... graphics-dbus-address.x86_64-latest.args} | 12 ---
.../graphics-dbus-address.xml |  5 ++-
...=> graphics-dbus-audio.x86_64-latest.args} | 14 
.../qemuxml2argvdata/graphics-dbus-audio.xml  |  5 ++-
... graphics-dbus-chardev.x86_64-latest.args} | 14 
.../graphics-dbus-chardev.xml |  5 ++-
tests/qemuxml2argvdata/graphics-dbus-p2p.args | 30 
...s => graphics-dbus-p2p.x86_64-latest.args} | 16 -
tests/qemuxml2argvdata/graphics-dbus-p2p.xml  |  5 ++-
.../graphics-dbus-usbredir.x86_64-latest.args | 36 +++
args => graphics-dbus.x86_64-latest.args} | 12 ---
tests/qemuxml2argvdata/graphics-dbus.xml  |  5 ++-
tests/qemuxml2argvtest.c  | 22 
...> graphics-dbus-address.x86_64-latest.xml} |  0
... => graphics-dbus-audio.x86_64-latest.xml} |  0
...> graphics-dbus-chardev.x86_64-latest.xml} |  0
...ml => graphics-dbus-p2p.x86_64-latest.xml} |  0
...us.xml => graphics-dbus.x86_64-latest.xml} |  0
tests/qemuxml2xmltest.c   | 20 +++
19 files changed, 104 insertions(+), 97 deletions(-)


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 13/24] qemuxml2(argv|xml)test: Sanitize testing of default video type on x86_64

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Use real capabilities and remove the redundant test case.

Signed-off-by: Peter Krempa 
---
.../default-video-type-x86_64-caps-test-0.err |  1 -
.../default-video-type-x86_64-caps-test-1.xml | 16 --
...fault-video-type-x86_64.x86_64-latest.args | 32 +++
...st-0.xml => default-video-type-x86_64.xml} |  0
tests/qemuxml2argvtest.c  |  2 +-
.../default-video-type-x86_64-caps-test-0.xml | 31 --
...fault-video-type-x86_64.x86_64-latest.xml} |  3 ++
tests/qemuxml2xmltest.c   |  7 +---
8 files changed, 37 insertions(+), 55 deletions(-)
delete mode 100644 
tests/qemuxml2argvdata/default-video-type-x86_64-caps-test-0.err
delete mode 100644 
tests/qemuxml2argvdata/default-video-type-x86_64-caps-test-1.xml
create mode 100644 
tests/qemuxml2argvdata/default-video-type-x86_64.x86_64-latest.args
rename tests/qemuxml2argvdata/{default-video-type-x86_64-caps-test-0.xml => 
default-video-type-x86_64.xml} (100%)
delete mode 100644 
tests/qemuxml2xmloutdata/default-video-type-x86_64-caps-test-0.xml
rename tests/qemuxml2xmloutdata/{default-video-type-x86_64-caps-test-1.xml => 
default-video-type-x86_64.x86_64-latest.xml} (91%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 11/24] qemuxml2xmltest: Convert rest of 'DO_TEST_NOCAPS' cases to 'DO_TEST_CAPS_LATEST'

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Finish the conversion of cases which didn't need any special
capabilities to use real capabilities.

Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/net-mtu.xml|   5 +-
tests/qemuxml2argvdata/pages-discard.xml  |   5 +-
...table.xml => acpi-table.x86_64-latest.xml} |   5 +-
...cdrom.xml => boot-cdrom.x86_64-latest.xml} |   5 +-
...oppy.xml => boot-floppy.x86_64-latest.xml} |   5 +-

[...]

...ues.xml => vhost_queues.x86_64-latest.xml} |   5 +-
...o-lun.xml => virtio-lun.x86_64-latest.xml} |   5 +-
...atchdog.xml => watchdog.x86_64-latest.xml} |   5 +-
tests/qemuxml2xmltest.c   | 178 +-
92 files changed, 398 insertions(+), 191 deletions(-)
rename tests/qemuxml2xmloutdata/{acpi-table.xml => 
acpi-table.x86_64-latest.xml} (87%)
rename tests/qemuxml2xmloutdata/{boot-cdrom.xml => 
boot-cdrom.x86_64-latest.xml} (88%)
rename tests/qemuxml2xmloutdata/{boot-floppy.xml => 
boot-floppy.x86_64-latest.xml} (90%)

[...]

rename tests/qemuxml2xmloutdata/{tap-vhost.xml => tap-vhost.x86_64-latest.xml} 
(93%)
rename tests/qemuxml2xmloutdata/{vhost_queues.xml => 
vhost_queues.x86_64-latest.xml} (93%)
rename tests/qemuxml2xmloutdata/{virtio-lun.xml => 
virtio-lun.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{watchdog.xml => watchdog.x86_64-latest.xml} 
(88%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 10/24] qemuxml2xmltest: Use real caps for 'vxhs' disk tests

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Version-lock the test to qemu-5.0.0 as it's the latest qemu that
supports 'vxhs' and thus the test can't use 'latest'.

Signed-off-by: Peter Krempa 
---
...vxhs.xml => disk-network-tlsx509-vxhs.x86_64-5.0.0.xml} | 7 +--
...network-vxhs.xml => disk-network-vxhs.x86_64-5.0.0.xml} | 7 +--
tests/qemuxml2xmltest.c| 4 ++--
3 files changed, 12 insertions(+), 6 deletions(-)
rename tests/qemuxml2xmloutdata/{disk-network-tlsx509-vxhs.xml => 
disk-network-tlsx509-vxhs.x86_64-5.0.0.xml} (90%)
rename tests/qemuxml2xmloutdata/{disk-network-vxhs.xml => 
disk-network-vxhs.x86_64-5.0.0.xml} (84%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 09/24] virschematest: Improve detection of 'invalid' XMLs

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

The output files from 'qemuxml2argvtest' may have the real capability
suffix e.g. 'pci-rom-disabled-invalid.x86_64-latest.xml' which would not
be detected as being invalid and thus causing a test failure.

Change the logic to find '-invalid.' so that we can properly use
'virschematest' with test cases using real capabilities.

Signed-off-by: Peter Krempa 
---
tests/virschematest.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 08/24] qemuxml2xmloutdata: Workaround wrong detection of 'disk-cdrom-empty-network-invalid' in virschematest

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

The 'disk-cdrom-empty-network-invalid' is a special case were the input
XML is invalid according to the schema, but after processing a valid XML
is produced.

This corner case doesn't play well with 'virschematest' which uses the
file suffix to determine whether the file is invalid.

Upcoming patch will change the 'virschematest' condition, which would
start detecting this XML as invalid.

Use the '-active'/'-inactive' suffix for the file, which is possible
with qemuxml2xmltest so that an upcoming patch will not cause test
failure.

Signed-off-by: Peter Krempa 
---
... disk-cdrom-empty-network-invalid-active.x86_64-latest.xml} | 0
...disk-cdrom-empty-network-invalid-inactive.x86_64-latest.xml | 1 +
tests/qemuxml2xmltest.c| 3 +++
3 files changed, 4 insertions(+)
rename 
tests/qemuxml2xmloutdata/{disk-cdrom-empty-network-invalid.x86_64-latest.xml => 
disk-cdrom-empty-network-invalid-active.x86_64-latest.xml} (100%)
create mode 12 
tests/qemuxml2xmloutdata/disk-cdrom-empty-network-invalid-inactive.x86_64-latest.xml



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 07/24] qemuxml2xmltest: Modernize all 'DO_TEST_NOCAPS' tests

2023-08-21 Thread Ján Tomko

On a Thursday in 2023, Peter Krempa wrote:

Convert all tests using the 'DO_TEST_NOCAPS' "fake" capability
invocation to use DO_TEST_CAPS_LATEST and remove the DO_TEST_NOCAPS
macro to prevent further use.

Most of the output file changes are related to default USB controller
type and the CPU becoming defined in the XML.

Signed-off-by: Peter Krempa 
---
... => balloon-device-auto.x86_64-latest.xml} |   5 +-
...> balloon-device-period.x86_64-latest.xml} |   5 +-
...xml => blkiotune-device.x86_64-latest.xml} |   5 +-

[...]

...r.xml => usb-controller.x86_64-latest.xml} |   5 +-
...l => usb-ich9-ehci-addr.x86_64-latest.xml} |   3 +
...sb-none.xml => usb-none.x86_64-latest.xml} |   3 +
tests/qemuxml2xmltest.c   | 154 +-
84 files changed, 423 insertions(+), 198 deletions(-)
rename tests/qemuxml2xmloutdata/{balloon-device-auto.xml => 
balloon-device-auto.x86_64-latest.xml} (89%)
rename tests/qemuxml2xmloutdata/{balloon-device-period.xml => 
balloon-device-period.x86_64-latest.xml} (89%)
rename tests/qemuxml2xmloutdata/{blkiotune-device.xml => 
blkiotune-device.x86_64-latest.xml} (92%)
rename tests/qemuxml2xmloutdata/{blkiotune.xml => blkiotune.x86_64-latest.xml} 
(89%)

[...]

rename tests/qemuxml2xmloutdata/{smp.xml => smp.x86_64-latest.xml} (89%)
rename tests/qemuxml2xmloutdata/{usb-controller.xml => 
usb-controller.x86_64-latest.xml} (85%)
rename tests/qemuxml2xmloutdata/{usb-ich9-ehci-addr.xml => 
usb-ich9-ehci-addr.x86_64-latest.xml} (96%)
rename tests/qemuxml2xmloutdata/{usb-none.xml => usb-none.x86_64-latest.xml} 
(89%)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [libvirt PATCH v6 34/36] qemu: implement ssh-agent auth for ssh disks with nbdkit

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:20:01 -0500, Jonathon Jongsma wrote:
> It's not possible to use password-protected ssh keys directly with
> libvirt because libvirt doesn't have any way to prompt a user for the
> password. To accomodate password-protected key files, an administrator
> can add these keys to an ssh agent and then configure the domain with
> the path to the ssh-agent socket.
> 
> Note that this requires an administrator or management app to
> configure the ssh-agent with an appropriate socket path and add the
> necessary keys to it. In addition, it does not currently work with
> selinux enabled. The ssh-agent socket would need a label that libvirt
> would be allowed to access rather than unconfined_t.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/conf/domain_conf.c  | 11 ---
>  src/conf/storage_source_conf.c  |  1 +
>  src/conf/storage_source_conf.h  |  1 +
>  src/qemu/qemu_nbdkit.c  | 10 ++
>  .../disk-network-ssh-key.args.disk0 |  6 +++---
>  .../disk-network-ssh-key.args.disk1 |  9 +
>  tests/qemuxml2argvdata/disk-network-ssh-key.xml | 17 ++---
>  7 files changed, 46 insertions(+), 9 deletions(-)
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh-key.args.disk1
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 08cf1be656..a70d7bf613 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7257,8 +7257,11 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
>  if (!(src->ssh_user = virXMLPropStringRequired(tmpnode, 
> "username")))
>  return -1;
>  
> -if (!(src->ssh_keyfile = virXMLPropStringRequired(tmpnode, 
> "keyfile")))
> -return -1;
> +/* optional path to an ssh key file */
> +src->ssh_keyfile = virXMLPropString(tmpnode, "keyfile");
> +
> +/* optional ssh-agent socket location */
> +src->ssh_agent = virXMLPropString(tmpnode, "agentsock");

By doing this you'll lose validation that either of the two coices from
the schema is present. Thus the user can just provide a username ...

>  }
>  }
>  
> @@ -22175,13 +22178,15 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
>  if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
>  if (src->ssh_known_hosts_file)
>  virBufferEscapeString(childBuf, "\n", 
> src->ssh_known_hosts_file);
> -if (src->ssh_keyfile) {
> +if (src->ssh_keyfile || src->ssh_agent) {
>  virBufferAddLit(childBuf, "  if (src->ssh_user)
>  virBufferEscapeString(childBuf, " username='%s'", 
> src->ssh_user);
>  if (src->ssh_keyfile)
>  virBufferEscapeString(childBuf, " keyfile='%s'", 
> src->ssh_keyfile);
> +if (src->ssh_agent)
> +virBufferEscapeString(childBuf, " agentsock='%s'", 
> src->ssh_agent);

virBufferEscapeString is NULL tolerant

>  
>  virBufferAddLit(childBuf, "/>\n");
>  }


[..]

> diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
> index 8c805664af..061faa66cb 100644
> --- a/src/conf/storage_source_conf.h
> +++ b/src/conf/storage_source_conf.h
> @@ -411,6 +411,7 @@ struct _virStorageSource {
>  bool ssh_host_key_check_disabled;
>  char *ssh_known_hosts_file;
>  char *ssh_keyfile;
> +char *ssh_agent;

Missing impl in virStorageSourceCopy.

>  
>  /* nfs_user and nfs_group store the strings passed in by the user for 
> NFS params.
>   * nfs_uid and nfs_gid represent the converted/looked up ID numbers 
> which are used

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v6 33/36] schema: add ssh-agent configuration for ssh disks

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:20:00 -0500, Jonathon Jongsma wrote:
> Add the ability to specify a path to a ssh-agent socket in order to use
> the ssh-agent to authenticate to remote ssh disks. Example
> configuration:
> 
> 
> 
> 
> ...
> 
> ...
> 
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  docs/formatdomain.rst | 13 -
>  src/conf/schemas/domaincommon.rng | 11 ---
>  2 files changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v6 32/36] qemu: implement keyfile auth for ssh disks with nbdkit

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:19:59 -0500, Jonathon Jongsma wrote:
> For ssh disks that are served by nbdkit, we can support logging in with
> an ssh key file. Pass the path to the configured key file and the
> username to the nbdkit process.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/conf/domain_conf.c| 32 ++
>  src/conf/storage_source_conf.c|  1 +
>  src/conf/storage_source_conf.h|  5 ++-
>  src/qemu/qemu_nbdkit.c| 15 +++--
>  .../disk-network-ssh-key.args.disk0   |  9 +
>  .../disk-network-ssh.args.disk2   |  9 +
>  tests/qemunbdkittest.c|  1 +
>  .../qemuxml2argvdata/disk-network-ssh-key.xml | 33 +++
>  8 files changed, 94 insertions(+), 11 deletions(-)
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh-key.args.disk0
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk2
>  create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-key.xml



> @@ -22164,8 +22172,20 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
>  if (src->timeout)
>  virBufferAsprintf(childBuf, "\n", 
> src->timeout);
>  
> -if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH && 
> src->ssh_known_hosts_file)
> -virBufferEscapeString(childBuf, "\n", 
> src->ssh_known_hosts_file);
> +if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
> +if (src->ssh_known_hosts_file)
> +virBufferEscapeString(childBuf, "\n", 
> src->ssh_known_hosts_file);
> +if (src->ssh_keyfile) {
> +virBufferAddLit(childBuf, " +
> +if (src->ssh_user)
> +virBufferEscapeString(childBuf, " username='%s'", 
> src->ssh_user);

virBufferEscapeString skips the formatting of the whole XL parameter if
the 3rd argument is NULL, so the NULL checks here ..

> +if (src->ssh_keyfile)

... and here are not needed.

> +virBufferEscapeString(childBuf, " keyfile='%s'", 
> src->ssh_keyfile);
> +
> +virBufferAddLit(childBuf, "/>\n");
> +}
> +}
>  }



> diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
> index 8a9c7d07e2..8c805664af 100644
> --- a/src/conf/storage_source_conf.h
> +++ b/src/conf/storage_source_conf.h
> @@ -406,12 +406,11 @@ struct _virStorageSource {
>  
>  bool hostcdrom; /* backing device is a cdrom */
>  
> -/* passthrough variables for the ssh driver which we don't handle 
> properly */
> -/* these must not be used apart from formatting the output JSON in the 
> qemu driver */
> +/* ssh variables */
>  char *ssh_user;
>  bool ssh_host_key_check_disabled;
> -/* additional ssh variables */
>  char *ssh_known_hosts_file;
> +char *ssh_keyfile;

The new field *MUST* be copied in virStorageSourceCopy.


Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v6 30/36] qemu: implement knownHosts for ssh disks with nbdkit

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:19:57 -0500, Jonathon Jongsma wrote:
> For ssh disks that are served by nbdkit, use the configured value for
> knownHosts and pass it to the nbdkit process.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/conf/domain_conf.c|  8 ++
>  src/conf/storage_source_conf.c|  1 +
>  src/conf/storage_source_conf.h|  2 ++
>  src/qemu/qemu_extdevice.c |  4 +--
>  src/qemu/qemu_hotplug.c   |  4 +--
>  src/qemu/qemu_nbdkit.c| 25 +++
>  src/qemu/qemu_nbdkit.h|  6 +++--
>  .../disk-network-ssh-password.args.disk0  |  3 ++-
>  .../disk-network-ssh.args.disk0   |  3 ++-
>  .../disk-network-ssh-password.xml |  1 +
>  tests/qemuxml2argvdata/disk-network-ssh.xml   |  1 +
>  11 files changed, 45 insertions(+), 13 deletions(-)
> 
> diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
> index f13e7c756a..8a9c7d07e2 100644
> --- a/src/conf/storage_source_conf.h
> +++ b/src/conf/storage_source_conf.h
> @@ -410,6 +410,8 @@ struct _virStorageSource {
>  /* these must not be used apart from formatting the output JSON in the 
> qemu driver */
>  char *ssh_user;
>  bool ssh_host_key_check_disabled;
> +/* additional ssh variables */
> +char *ssh_known_hosts_file;

The new field *must* be copied in virStorageSourceCopy. See the comment
at the top of this struct declaration.

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v6 28/36] qemu: implement password auth for ssh disks with nbdkit

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:19:55 -0500, Jonathon Jongsma wrote:
> For ssh disks that are served by nbdkit, lookup the password from the
> configured secret and securely pass it to the nbdkit process using fd
> passing.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/qemu/qemu_nbdkit.c| 87 ++-
>  .../disk-network-ssh-password.args.disk0  |  8 ++
>  ...k-network-ssh-password.args.disk0.pipe.778 |  1 +
>  .../disk-network-ssh.args.disk1   |  8 ++
>  .../disk-network-ssh.args.disk1.pipe.778  |  1 +
>  tests/qemunbdkittest.c|  1 +
>  ...sk-network-ssh-password.x86_64-latest.args | 35 
>  .../disk-network-ssh-password.xml | 34 
>  tests/qemuxml2argvtest.c  |  1 +
>  9 files changed, 137 insertions(+), 39 deletions(-)
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh-password.args.disk0
>  create mode 100644 
> tests/qemunbdkitdata/disk-network-ssh-password.args.disk0.pipe.778
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1.pipe.778
>  create mode 100644 
> tests/qemuxml2argvdata/disk-network-ssh-password.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-password.xml
> 
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index 8bb91de994..9dbe3af1dd 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -936,6 +936,46 @@ qemuNbdkitCommandPassDataByPipe(virCommand *cmd,
>  }
>  
>  
> +static int
> +qemuNbdkitProcessBuildCommandAuth(virStorageAuthDef *authdef,
> +  virCommand *cmd)
> +{
> +g_autoptr(virConnect) conn = NULL;
> +g_autofree uint8_t *secret = NULL;
> +size_t secretlen = 0;
> +int secrettype;
> +
> +if (!authdef)
> +return 0;
> +
> +if ((secrettype = virSecretUsageTypeFromString(authdef->secrettype)) < 
> 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("invalid secret type %1$s"),
> +   authdef->secrettype);
> +return -1;
> +}
> +
> +conn = virGetConnectSecret();
> +if (virSecretGetSecretString(conn,
> + >seclookupdef,
> + secrettype,
> + ,
> + ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("failed to get auth secret for storage"));

This overwrites the error from virSecretGetSecretString with a much
worse error, e.g. not showing which secred we are looking for and in
addition to that with an VIR_ERR_INTERNAL_ERROR.

> +return -1;
> +}
> +
> +virCommandAddArgPair(cmd, "user", authdef->username);
> +
> +if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
> +, secretlen) < 0)
> +return -1;
> +
> +return 0;
> +}
> +

Reviewed-by: Peter Krempa 



Re: [libvirt PATCH v6 25/36] qemu: Monitor nbdkit process for exit

2023-08-21 Thread Peter Krempa
On Thu, Jul 20, 2023 at 17:19:52 -0500, Jonathon Jongsma wrote:
> Adds the ability to monitor the nbdkit process so that we can take
> action in case the child exits unexpectedly.
> 
> When the nbdkit process exits, we pause the vm, restart nbdkit, and then
> resume the vm. This allows the vm to continue working in the event of a
> nbdkit failure.
> 
> Eventually we may want to generalize this functionality since we may
> need something similar for e.g. qemu-storage-daemon, etc.
> 
> The process is monitored with the pidfd_open() syscall if it exists
> (since linux 5.3). Otherwise it resorts to checking whether the process
> is alive once a second. The one-second time period was chosen somewhat
> arbitrarily.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  meson.build |   7 +++
>  src/qemu/qemu_nbdkit.c  | 136 ++--
>  src/qemu/qemu_nbdkit.h  |   4 +-
>  src/qemu/qemu_process.c |   4 +-
>  4 files changed, 143 insertions(+), 8 deletions(-)



> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index c3b43ff3c0..1199acd501 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c

[...]

> @@ -613,6 +616,104 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
>  }
>  
>  
> +static void
> +qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
> + virDomainObj *vm)
> +{
> +qemuDomainObjPrivate *vmpriv = vm->privateData;
> +virQEMUDriver *driver = vmpriv->driver;
> +
> +/* clean up resources associated with process */
> +qemuNbdkitProcessStop(proc);
> +
> +if (qemuNbdkitProcessStart(proc, vm, driver) < 0)
> +VIR_WARN("Unable to restart nbkdit process");

It'd be great to get this message to the user in other way than the
logs.

The 'virDomainGetMessages' API would be a great fit IMO, but at this
point it doesn't have the possibility to do driver specific messages.

In this instance we could taint the VM, as taints are propagated
via the virDomainGetMessages API, when nbdkit fails to start,
which'd be then propagated to the user

> +}
> +
> +
> +typedef struct {
> +qemuNbdkitProcess *proc;
> +virDomainObj *vm;
> +} qemuNbdkitProcessEventData;
> +
> +
> +static qemuNbdkitProcessEventData*
> +qemuNbdkitProcessEventDataNew(qemuNbdkitProcess *proc,
> +  virDomainObj *vm)
> +{
> +qemuNbdkitProcessEventData *d = g_new(qemuNbdkitProcessEventData, 1);
> +d->proc = proc;
> +d->vm = virObjectRef(vm);
> +return d;
> +}
> +
> +
> +static void
> +qemuNbdkitProcessEventDataFree(qemuNbdkitProcessEventData *d)
> +{
> +virObjectUnref(d->vm);
> +g_free(d);
> +}
> +
> +
> +#if WITH_DECL_SYS_PIDFD_OPEN
> +static void
> +qemuNbdkitProcessPidfdCb(int watch G_GNUC_UNUSED,
> + int fd,
> + int events G_GNUC_UNUSED,
> + void *opaque)
> +{
> +qemuNbdkitProcessEventData *d = opaque;
> +
> +VIR_FORCE_CLOSE(fd);
> +VIR_DEBUG("nbdkit process %i died", d->proc->pid);
> +qemuNbdkitProcessRestart(d->proc, d->vm);

This seems to access the 'vm' object unlocked. Since this callback seems
to be triggerrable at an arbitrary point, access to 'vm' locked by
another thread is possible.

Consider defering the reaction to this event to the per-VM event
handling thread so that the startup of the VM and possible locking of
the 'vm' object is not done in the event loop.

> +}
> +#endif /* WITH_DECL_SYS_PIDFD_OPEN */
> +
> +
> +static int
> +qemuNbdkitProcessStartMonitor(qemuNbdkitProcess *proc,
> +  virDomainObj *vm)
> +{
> +#if WITH_DECL_SYS_PIDFD_OPEN
> +int pidfd;
> +
> +pidfd = syscall(SYS_pidfd_open, proc->pid, 0);
> +if (pidfd < 0) {
> +virReportSystemError(errno, _("pidfd_open failed for %1$i"), 
> proc->pid);
> +return -1;
> +}
> +
> +proc->eventwatch = virEventAddHandle(pidfd,
> + VIR_EVENT_HANDLE_READABLE,
> + qemuNbdkitProcessPidfdCb,
> + qemuNbdkitProcessEventDataNew(proc, 
> vm),
> + 
> (virFreeCallback)qemuNbdkitProcessEventDataFree);
> +
> +VIR_DEBUG("Monitoring nbdkit process %i for exit", proc->pid);
> +
> +return 0;
> +#else
> +virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +   _("pidfd_open system call required for nbdkit support"));
> +return -1;

This seems to be unreachable. Please add a comment.

> +#endif /* WITH_DECL_SYS_PIDFD_OPEN */
> +}
> +
> +
> +static void
> +qemuNbdkitProcessStopMonitor(qemuNbdkitProcess *proc)
> +{
> +#if WITH_DECL_SYS_PIDFD_OPEN
> +if (proc->eventwatch > 0) {
> +virEventRemoveHandle(proc->eventwatch);
> +proc->eventwatch = 0;
> +}
> +#endif /* WITH_DECL_SYS_PIDFD_OPEN */
> +}
> +
> +
>  static qemuNbdkitProcess *
>  qemuNbdkitProcessNew(virStorageSource *source,
>  

Re: [libvirt PATCH 09/20] ci: build.sh: Join MESON_ARGS and MESON_OPTS

2023-08-21 Thread Andrea Bolognani
Resurrecting this thread now what you've pushed some of the patches.

On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
> +# $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
> +# containerized environment

You need quotes around the value. As is, the shell will interpret
'--meson-configure-args=-Dopt1' and '-Dopt2' as separate arguments
and things will not work the way you expect them to.

> -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.

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [libvirt PATCH] qemu: remove pointless qemuDomainLogContextMode

2023-08-21 Thread Michal Prívozník
On 8/16/23 14:24, Ján Tomko wrote:
> Since its introduction in 4d1b771fbb610537b7425e649a490143588b8ed3
> it has only been used to differentiate between START and non-START.
> 
> Last use of QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH was removed by:
> 
>   commit f709377301b919a9fcbfc366e33057f7848bee28
> qemu: Fix qemuDomainObjTaint with virtlogd
> 
> QEMU_DOMAIN_LOG_CONTEXT_MODE_STOP is unused since:
> 
>   commit cf3ea0769c54a328733bcb0cd27f546e70090c89
> qemu: process: Append the "shutting down" message using the new APIs
> 
> Now, the only caller passes QEMU_DOMAIN_LOG_CONTEXT_MODE_START.
> Assume that's always the case and remove the 'mode' argument.
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_domain.c  | 26 +++---
>  src/qemu/qemu_domain.h  |  9 +
>  src/qemu/qemu_process.c |  3 +--
>  3 files changed, 13 insertions(+), 25 deletions(-)

Reviewed-by: Michal Privoznik 

Michal



Re: [PATCH] test: qemu: Update qemu-8.1 test data on x86_64

2023-08-21 Thread Peter Krempa
On Thu, Aug 10, 2023 at 15:01:24 +0200, Ján Tomko wrote:
> On a Thursday in 2023, Peter Krempa wrote:
> > Update to v8.1.0-rc2-114-g64d3be986f
> > 
> > Notable changes:
> > - 'dirty-limit' migration feature added
> >- 'vcpu-dirty-limit', 'x-vcpu-dirty-limit-period' parameters added
> >- 'dirty-limit-ring-full-time', 'dirty-limit-throttle-time-per-round' 
> > statistics added
> > - migration statistic of number of skipped zero pages is now deprecated
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> > .../caps_8.1.0_x86_64.replies | 55 +--
> > .../caps_8.1.0_x86_64.xml |  4 +-
> > 2 files changed, 53 insertions(+), 6 deletions(-)
> > 
> 
> Reviewed-by: Ján Tomko 

I forgot to push this originally.

I've updated to qemu v8.1.0-rc4 with no change to the caps dump besides
the version difference, so I'll push the updated version.



Sunset libvirt-snmp?

2023-08-21 Thread Michal Prívozník
It's been a while since libvirt-snmp was actively developed. Now it
receives only libvirt-ci related commits. The code compiles with
net-snmp-5.9.3 but the freshly released net-snmp-5.9.4 [1] breaks
compilation [2]. Now, libvirt-snmp has this crazy architecture, where
some sources are manually generated from src/LIBVIRT-MIB.txt, then
edited (added code to talk to libvirt) and then added to git.

This is labor extensive and since I don't think libvirt-snmp is actually
used I'd like to sunset it. According to repology [3] only Gentoo (and
its clones) has the latest version (released ~5 years ago). And I doubt
it has any real users there.

Thoughts?

1: https://sourceforge.net/projects/net-snmp/files/net-snmp/5.9.4/
2: https://bugs.gentoo.org/show_bug.cgi?id=912582
2: https://repology.org/project/libvirt-snmp/versions

Michal