[libvirt PATCH v2 2/5] qemu: add virtio-blk-vhost-vdpa capability

[Jonathon Jongsma]

Check whether the qemu binary supports the vdpa block driver. We can't
rely simply on the existence of the virtio-blk-vhost-vdpa block driver
since the first releases of qemu didn't support fd-passing for this
driver. So we have to check for the 'fdset' feature on the driver
object. This feature will be present in the qemu 8.1.0 release and was
merged to qemu in commit 98b126f5.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 87412dd4ec..3a1bfbf74d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -697,6 +697,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 450 */
   "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN 
*/
+  "virtio-blk-vhost-vdpa", /* 
QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */
 );
 
 
@@ -1531,6 +1532,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+rbd/encrypt/format/^luks-any", 
QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-add/arg-type/+qcow2/discard-no-unref", 
QEMU_CAPS_QCOW2_DISCARD_NO_UNREF },
+{ "blockdev-add/arg-type/+virtio-blk-vhost-vdpa/$fdset", 
QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA},
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
 { "device_add/$json-cli-hotplug", QEMU_CAPS_DEVICE_JSON },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e51d3fffdc..3c4f7f625b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -676,6 +676,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 450 */
 QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with 
async-teardown=on|off */
+QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block 
driver */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
index 6f8c5a57b7..d266dd0f31 100644
--- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
@@ -197,6 +197,7 @@
   
   
   
+  
   8001000
   43100245
   v8.1.0
-- 
2.41.0

[libvirt PATCH v2 3/5] qemu: make vdpa connect function more generic

[Jonathon Jongsma]

qemuInterfaceVDPAConnect() was a helper function for connecting to the
vdpa device file. But in order to support other vdpa devices besides
network interfaces (e.g. vdpa block devices) make this function a bit
more generic.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_command.c   | 23 ++-
 src/qemu/qemu_command.h   |  1 +
 src/qemu/qemu_interface.c | 23 ---
 src/qemu/qemu_interface.h |  2 --
 tests/qemuhotplugmock.c   |  4 ++--
 tests/qemuxml2argvmock.c  |  2 +-
 6 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 778958700b..e84374b4cf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8533,7 +8533,7 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
 break;
 
 case VIR_DOMAIN_NET_TYPE_VDPA:
-if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+if ((vdpafd = qemuVDPAConnect(net->data.vdpa.devicepath)) < 0)
 return -1;
 
 netpriv->vdpafd = qemuFDPassNew(net->info.alias, priv);
@@ -10993,3 +10993,24 @@ 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top,
 
 return g_steal_pointer();
 }
+
+
+/* qemuVDPAConnect:
+ * @devicepath: the path to the vdpa device
+ *
+ * returns: file descriptor of the vdpa device
+ */
+int
+qemuVDPAConnect(const char *devicepath)
+{
+int fd;
+
+if ((fd = open(devicepath, O_RDWR)) < 0) {
+virReportSystemError(errno,
+ _("Unable to open '%1$s' for vdpa device"),
+ devicepath);
+return -1;
+}
+
+return fd;
+}
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 55efa45601..341ec43f9a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -248,3 +248,4 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev,
 
 const char * qemuAudioDriverTypeToString(virDomainAudioType type);
 virDomainAudioType qemuAudioDriverTypeFromString(const char *str);
+int qemuVDPAConnect(const char *devicepath) G_NO_INLINE;
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e875de48ee..8856bb95a8 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -648,29 +648,6 @@ qemuInterfaceBridgeConnect(virDomainDef *def,
 }
 
 
-/* qemuInterfaceVDPAConnect:
- * @net: pointer to the VM's interface description
- *
- * returns: file descriptor of the vdpa device
- *
- * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_VDPA
- */
-int
-qemuInterfaceVDPAConnect(virDomainNetDef *net)
-{
-int fd;
-
-if ((fd = open(net->data.vdpa.devicepath, O_RDWR)) < 0) {
-virReportSystemError(errno,
- _("Unable to open '%1$s' for vdpa device"),
- net->data.vdpa.devicepath);
-return -1;
-}
-
-return fd;
-}
-
-
 /*
  * Returns: -1 on error, 0 on success. Populates net->privateData->slirp if
  * the slirp helper is needed.
diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
index d866beb184..6eed3e6bd7 100644
--- a/src/qemu/qemu_interface.h
+++ b/src/qemu/qemu_interface.h
@@ -55,5 +55,3 @@ int qemuInterfaceOpenVhostNet(virDomainObj *def,
 
 int qemuInterfacePrepareSlirp(virQEMUDriver *driver,
   virDomainNetDef *net);
-
-int qemuInterfaceVDPAConnect(virDomainNetDef *net) G_NO_INLINE;
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index 89d287945a..dd7e2c67e0 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -18,8 +18,8 @@
 
 #include 
 
+#include "qemu/qemu_command.h"
 #include "qemu/qemu_hotplug.h"
-#include "qemu/qemu_interface.h"
 #include "qemu/qemu_process.h"
 #include "testutilsqemu.h"
 #include "conf/domain_conf.h"
@@ -94,7 +94,7 @@ qemuProcessKillManagedPRDaemon(virDomainObj *vm G_GNUC_UNUSED)
 }
 
 int
-qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
+qemuVDPAConnect(const char *devicepath G_GNUC_UNUSED)
 {
 /* need a valid fd or sendmsg won't work. Just open /dev/null */
 return open("/dev/null", O_RDONLY);
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 400dd5c020..52c44b2ed0 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -255,7 +255,7 @@ virNetDevBandwidthSetRootQDisc(const char *ifname 
G_GNUC_UNUSED,
 
 
 int
-qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
+qemuVDPAConnect(const char *devicepath G_GNUC_UNUSED)
 {
 if (fcntl(1732, F_GETFD) != -1)
 abort();
-- 
2.41.0

[libvirt PATCH v2 4/5] qemu: consider vdpa block devices for memlock limits

[Jonathon Jongsma]

vDPA block devices will also need the same consideration for memlock
limits as other vdpa devices, so consider these devices when calculating
memlock limits.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c7d64e1b5c..52ea8f649d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9652,7 +9652,7 @@ qemuDomainGetNumNVMeDisks(const virDomainDef *def)
 
 
 static int
-qemuDomainGetNumVDPANetDevices(const virDomainDef *def)
+qemuDomainGetNumVDPADevices(const virDomainDef *def)
 {
 size_t i;
 int n = 0;
@@ -9662,6 +9662,14 @@ qemuDomainGetNumVDPANetDevices(const virDomainDef *def)
 n++;
 }
 
+for (i = 0; i < def->ndisks; i++) {
+virStorageSource *src;
+for (src = def->disks[i]->src; src; src = src->backingStore) {
+if (src->type == VIR_STORAGE_TYPE_VHOST_VDPA)
+n++;
+}
+}
+
 return n;
 }
 
@@ -9704,7 +9712,7 @@ qemuDomainGetMemLockLimitBytes(virDomainDef *def)
 
 nvfio = qemuDomainGetNumVFIOHostdevs(def);
 nnvme = qemuDomainGetNumNVMeDisks(def);
-nvdpa = qemuDomainGetNumVDPANetDevices(def);
+nvdpa = qemuDomainGetNumVDPADevices(def);
 /* For device passthrough using VFIO the guest memory and MMIO memory
  * regions need to be locked persistent in order to allow DMA.
  *
-- 
2.41.0

[libvirt PATCH v2 1/5] conf: add ability to configure a vdpa block disk device

[Jonathon Jongsma]

vDPA block devices can be configured as follows:


  


Signed-off-by: Jonathon Jongsma 
---
 docs/formatdomain.rst | 19 +--
 src/ch/ch_monitor.c   |  1 +
 src/conf/domain_conf.c|  8 
 src/conf/schemas/domaincommon.rng | 13 +
 src/conf/storage_source_conf.c|  7 ++-
 src/conf/storage_source_conf.h|  2 ++
 src/libxl/xen_xl.c|  1 +
 src/qemu/qemu_block.c |  6 ++
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_migration.c |  2 ++
 src/qemu/qemu_snapshot.c  |  4 
 src/qemu/qemu_validate.c  |  1 +
 src/storage_file/storage_source.c |  1 +
 13 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index bc469e5f9f..a65edc6703 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2678,6 +2678,11 @@ paravirtualized driver is specified via the ``disk`` 
element.


  
+ 
+   
+   
+   
+ 

...
 
@@ -2688,8 +2693,9 @@ paravirtualized driver is specified via the ``disk`` 
element.
``type``
   Valid values are "file", "block", "dir" ( :since:`since 0.7.5` ),
   "network" ( :since:`since 0.8.7` ), or "volume" ( :since:`since 1.0.5` ),
-  or "nvme" ( :since:`since 6.0.0` ), or "vhostuser" ( :since:`since 
7.1.0` )
-  and refer to the underlying source for the disk. :since:`Since 0.0.3`
+  or "nvme" ( :since:`since 6.0.0` ), or "vhostuser" ( :since:`since 
7.1.0` ),
+  or "vhostvdpa" ( :since:`since 9.8.0 (QEMU 8.1.0)`) and refer to the
+  underlying source for the disk. :since:`Since 0.0.3`
``device``
   Indicates how the disk is to be exposed to the guest OS. Possible values
   for this attribute are "floppy", "disk", "cdrom", and "lun", defaulting 
to
@@ -2879,6 +2885,15 @@ paravirtualized driver is specified via the ``disk`` 
element.
    XML for this disk type. Additionally features such as 
blockjobs,
   incremental backups and snapshots are not supported for this disk type.
 
+   ``vhostvdpa``
+  Enables the hypervisor to connect to a vDPA block device. Requires shared
+  memory configured for the VM, for more details see ``access`` mode for
+  ``memoryBacking`` element (See `Memory Backing`_).
+
+  The ``source`` element has a mandatory attribute ``dev`` that specifies
+  the fully-qualified path to the vhost-vdpa character device (e.g.
+  ``/dev/vhost-vdpa-0``).
+
With "file", "block", and "volume", one or more optional sub-elements
``seclabel`` (See `Security label`_) can be used to override the domain
security labeling policy for just that source file.
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 200ad6c77b..1691a4efb6 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -197,6 +197,7 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, 
virDomainDiskDef *diskdef)
 case VIR_STORAGE_TYPE_VOLUME:
 case VIR_STORAGE_TYPE_NVME:
 case VIR_STORAGE_TYPE_VHOST_USER:
+case VIR_STORAGE_TYPE_VHOST_VDPA:
 case VIR_STORAGE_TYPE_LAST:
 default:
 virReportEnumRangeError(virStorageType, diskdef->src->type);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c8727de54..1f14ef6f23 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7522,6 +7522,10 @@ virDomainStorageSourceParse(xmlNodePtr node,
 if (virDomainDiskSourceVHostUserParse(node, src, xmlopt, ctxt) < 0)
 return -1;
 break;
+case VIR_STORAGE_TYPE_VHOST_VDPA:
+if (!(src->vdpadev = virXMLPropStringRequired(node, "dev")))
+return -1;
+break;
 case VIR_STORAGE_TYPE_NONE:
 case VIR_STORAGE_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -22386,6 +22390,10 @@ virDomainDiskSourceFormat(virBuffer *buf,
 virDomainDiskSourceVhostuserFormat(, , 
src->vhostuser);
 break;
 
+case VIR_STORAGE_TYPE_VHOST_VDPA:
+virBufferEscapeString(, " dev='%s'", src->vdpadev);
+break;
+
 case VIR_STORAGE_TYPE_NONE:
 case VIR_STORAGE_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index 2f9ba31c0a..1fe9ccb70e 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1811,6 +1811,7 @@
   
   
   
+  
 
   
 
@@ -2381,6 +2382,18 @@
 
   
 
+  
+
+  vhostvdpa
+
+
+  
+
+  
+  
+
+  
+
   
 
   (ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index dcac3a8ff6..f57cb3241d 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -47,7 +47,8 @@ VIR_ENUM_IMPL(virStorage,
   "network",
   "volume",
   "nvme",

[libvirt PATCH v2 5/5] qemu: Implement support for vDPA block devices

[Jonathon Jongsma]

Requires recent qemu with support for the virtio-blk-vhost-vdpa device
and the ability to pass a /dev/fdset/N path for the vdpa path (8.1.0)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1900770
Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_block.c | 20 --
 src/qemu/qemu_process.c   | 34 +
 src/qemu/qemu_validate.c  | 32 ++--
 .../disk-vhostvdpa.x86_64-latest.args | 37 +++
 tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++
 tests/qemuxml2argvtest.c  | 34 +
 tests/testutilsqemu.c | 11 ++
 tests/testutilsqemu.h |  2 +
 8 files changed, 185 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0b4c2dbcf4..d31bbde0f4 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -778,6 +778,20 @@ qemuBlockStorageSourceGetNVMeProps(virStorageSource *src)
 }
 
 
+static virJSONValue *
+qemuBlockStorageSourceGetVhostVdpaProps(virStorageSource *src)
+{
+virJSONValue *ret = NULL;
+qemuDomainStorageSourcePrivate *srcpriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+
+ignore_value(virJSONValueObjectAdd(,
+   "s:driver", "virtio-blk-vhost-vdpa",
+   "s:path", 
qemuFDPassGetPath(srcpriv->fdpass),
+   NULL));
+return ret;
+}
+
+
 static int
 qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSource *src,
virJSONValue *props)
@@ -874,9 +888,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource *src,
 break;
 
 case VIR_STORAGE_TYPE_VHOST_VDPA:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("vhostvdpa disk type not yet supported"));
-return NULL;
+if (!(fileprops = qemuBlockStorageSourceGetVhostVdpaProps(src)))
+return NULL;
+break;
 
 case VIR_STORAGE_TYPE_VHOST_USER:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7a1cdb0302..42837c4a8a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6820,6 +6820,28 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj 
*vm)
 }
 
 
+static int
+qemuProcessPrepareHostStorageSourceVDPA(virStorageSource *src,
+qemuDomainObjPrivate *priv)
+{
+qemuDomainStorageSourcePrivate *srcpriv = NULL;
+virStorageType actualType = virStorageSourceGetActualType(src);
+int vdpafd = -1;
+
+if (actualType != VIR_STORAGE_TYPE_VHOST_VDPA)
+return 0;
+
+if ((vdpafd = qemuVDPAConnect(src->vdpadev)) < 0)
+return -1;
+
+srcpriv = qemuDomainStorageSourcePrivateFetch(src);
+
+srcpriv->fdpass = qemuFDPassNew(src->nodestorage, priv);
+qemuFDPassAddFD(srcpriv->fdpass, , "-vdpa");
+return 0;
+}
+
+
 static int
 qemuProcessPrepareHostStorage(virQEMUDriver *driver,
   virDomainObj *vm,
@@ -6856,6 +6878,18 @@ qemuProcessPrepareHostStorage(virQEMUDriver *driver,
 return -1;
 }
 
+/* connect to any necessary vdpa block devices */
+for (i = vm->def->ndisks; i > 0; i--) {
+size_t idx = i - 1;
+virDomainDiskDef *disk = vm->def->disks[idx];
+virStorageSource *src;
+
+for (src = disk->src; virStorageSourceIsBacking(src); src = 
src->backingStore) {
+if (qemuProcessPrepareHostStorageSourceVDPA(src, vm->privateData) 
< 0)
+return -1;
+}
+}
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 165ab3a66a..5bae56b00f 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3175,13 +3175,39 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef 
*disk,
 }
 
 if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) {
+const char *vhosttype = virStorageTypeToString(disk->src->type);
+
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_BLK)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("vhostuser disk is not supported with this QEMU 
binary"));
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("%1$s disk is not supported with this QEMU 
binary"),
+   vhosttype);
+return -1;
+}
+
+if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, vhosttype) 
< 0)
+return -1;
+}
+
+if (disk->src->type == VIR_STORAGE_TYPE_VHOST_VDPA) {
+const char *vhosttype = virStorageTypeToString(disk->src->type);
+
+

[libvirt PATCH v2 0/5] Add support for vDPA block devices

[Jonathon Jongsma]

see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.

Changes in v2:
 - Don't use virStorageSource->path for vdpa device path to avoid clashing with
   existing path functionality
 - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
   function rather than the qemuDomainPrepareStorageSource() function. This
   also required some additional support in the tests for setting up the
   objects properly for testing.
 - rebased to latest master branch

Jonathon Jongsma (5):
  conf: add ability to configure a vdpa block disk device
  qemu: add virtio-blk-vhost-vdpa capability
  qemu: make vdpa connect function more generic
  qemu: consider vdpa block devices for memlock limits
  qemu: Implement support for vDPA block devices

 docs/formatdomain.rst | 19 +-
 src/ch/ch_monitor.c   |  1 +
 src/conf/domain_conf.c|  8 
 src/conf/schemas/domaincommon.rng | 13 +++
 src/conf/storage_source_conf.c|  7 +++-
 src/conf/storage_source_conf.h|  2 +
 src/libxl/xen_xl.c|  1 +
 src/qemu/qemu_block.c | 20 ++
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   | 24 +++-
 src/qemu/qemu_command.h   |  1 +
 src/qemu/qemu_domain.c| 12 +-
 src/qemu/qemu_interface.c | 23 
 src/qemu/qemu_interface.h |  2 -
 src/qemu/qemu_migration.c |  2 +
 src/qemu/qemu_process.c   | 34 +
 src/qemu/qemu_snapshot.c  |  4 ++
 src/qemu/qemu_validate.c  | 33 +++--
 src/storage_file/storage_source.c |  1 +
 .../caps_8.1.0_x86_64.xml |  1 +
 tests/qemuhotplugmock.c   |  4 +-
 .../disk-vhostvdpa.x86_64-latest.args | 37 +++
 tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++
 tests/qemuxml2argvmock.c  |  2 +-
 tests/qemuxml2argvtest.c  | 34 +
 tests/testutilsqemu.c | 11 ++
 tests/testutilsqemu.h |  2 +
 28 files changed, 285 insertions(+), 37 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml

-- 
2.41.0

[PATCH v1 8/9] qemu_driver: report feature policy when baselining

[Collin Walling]

Some baselined models may require that certain features are disabled to
ensure compatability with all computed models. When converting the model
info to a CPU definition, check the model info feature value for false
and set the CPU def policy to disabled.

Additionally, set the CPU def type to VIR_CPU_TYPE_GUEST so that the XML
formatter will consider the feature policies.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_driver.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0d4da937b0..1c61685489 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11659,15 +11659,19 @@ qemuConnectStealCPUModelFromInfo(virCPUDef *dst,
 
 info = g_steal_pointer(src);
 dst->model = g_steal_pointer(>name);
+dst->type = VIR_CPU_TYPE_GUEST;
 
 for (i = 0; i < info->nprops; i++) {
 char *name = info->props[i].name;
+virCPUFeaturePolicy policy;
 
-if (info->props[i].type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN ||
-!info->props[i].value.boolean)
+if (info->props[i].type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
 continue;
 
-if (virCPUDefAddFeature(dst, name, VIR_CPU_FEATURE_REQUIRE) < 0)
+policy = info->props[i].value.boolean ? VIR_CPU_FEATURE_REQUIRE
+  : VIR_CPU_FEATURE_DISABLE;
+
+if (virCPUDefAddFeature(dst, name, policy) < 0)
 return -1;
 }
 
-- 
2.41.0

[PATCH v1 5/9] cpu: conf: add cpu mode for host-recommended

[Collin Walling]

Add VIR_CPU_MODE_HOST_RECOMMENDED for host-recommended CPU models and
satisfy all switch cases. For the most part, host-recommended will
follow similar paths as host-model, aside from touching any
architecture specifics.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/conf/cpu_conf.c  | 1 +
 src/conf/cpu_conf.h  | 1 +
 src/cpu/cpu.c| 1 +
 src/cpu/cpu_ppc64.c  | 2 ++
 src/qemu/qemu_capabilities.c | 4 
 src/qemu/qemu_command.c  | 1 +
 src/qemu/qemu_domain.c   | 2 ++
 src/qemu/qemu_validate.c | 1 +
 8 files changed, 13 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 7abe489733..c0116808d8 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virCPUMode,
   "host-model",
   "host-passthrough",
   "maximum",
+  "host-recommended",
 );
 
 VIR_ENUM_IMPL(virCPUMatch,
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 3e4c53675c..f73d852e69 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -44,6 +44,7 @@ typedef enum {
 VIR_CPU_MODE_HOST_MODEL,
 VIR_CPU_MODE_HOST_PASSTHROUGH,
 VIR_CPU_MODE_MAXIMUM,
+VIR_CPU_MODE_HOST_RECOMMENDED,
 
 VIR_CPU_MODE_LAST
 } virCPUMode;
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index bb5737e938..805aff1bf5 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -587,6 +587,7 @@ virCPUUpdate(virArch arch,
 return 0;
 
 case VIR_CPU_MODE_HOST_MODEL:
+case VIR_CPU_MODE_HOST_RECOMMENDED:
 relative = true;
 break;
 
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index e13cdbdf6b..dc42e869e7 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -483,6 +483,8 @@ ppc64Compute(virCPUDef *host,
  * look up guest CPU information */
 guest_model = ppc64ModelFromCPU(cpu, map);
 break;
+case VIR_CPU_MODE_HOST_RECOMMENDED:
+break;
 }
 } else {
 /* Other host CPU information */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 59403808ee..d7096a08c2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2340,6 +2340,10 @@ virQEMUCapsIsCPUModeSupported(virQEMUCaps *qemuCaps,
 case VIR_CPU_MODE_MAXIMUM:
 return virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MAX);
 
+case VIR_CPU_MODE_HOST_RECOMMENDED:
+return !!virQEMUCapsGetHostModel(qemuCaps, type,
+ VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED);
+
 case VIR_CPU_MODE_LAST:
 break;
 }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a912ed064f..a45e2fbb95 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6293,6 +6293,7 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver,
 virBufferAdd(buf, cpu->model, -1);
 break;
 
+case VIR_CPU_MODE_HOST_RECOMMENDED:
 case VIR_CPU_MODE_LAST:
 break;
 }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c7d64e1b5c..f7493431ac 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4753,6 +4753,7 @@ qemuDomainDefCPUPostParse(virDomainDef *def,
 break;
 
 case VIR_CPU_MODE_HOST_MODEL:
+case VIR_CPU_MODE_HOST_RECOMMENDED:
 def->cpu->check = VIR_CPU_CHECK_PARTIAL;
 break;
 
@@ -6995,6 +6996,7 @@ qemuDomainObjCheckCPUTaint(virQEMUDriver *driver,
 }
 break;
 case VIR_CPU_MODE_HOST_MODEL:
+case VIR_CPU_MODE_HOST_RECOMMENDED:
 case VIR_CPU_MODE_LAST:
 default:
 break;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 1346bbfb44..387c2f1fa8 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -372,6 +372,7 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
 }
 break;
 
+case VIR_CPU_MODE_HOST_RECOMMENDED:
 case VIR_CPU_MODE_CUSTOM:
 case VIR_CPU_MODE_LAST:
 break;
-- 
2.41.0

[PATCH v1 6/9] domain: capabilities: report host-recommended CPU model

[Collin Walling]

Report the host-recommended CPU definition in the domaincapabilities.
Currently, only s390x supports this model, but the formatting remains
open if other archs decide to support this as well.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/conf/domain_capabilities.c| 16 +++
 src/conf/domain_capabilities.h|  1 +
 src/conf/schemas/domaincaps.rng   | 24 ++
 src/qemu/qemu_capabilities.c  |  8 
 tests/domaincapsdata/qemu_8.1.0.s390x.xml | 55 +++
 5 files changed, 104 insertions(+)

diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 2fa5756184..4b9019f252 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -450,6 +450,22 @@ virDomainCapsCPUFormat(virBuffer *buf,
 virBufferAddLit(buf, "supported='no'/>\n");
 }
 
+if (cpu->hostRecommended) {
+virBufferAsprintf(buf, "hostRecommended) {
+virBufferAddLit(buf, "supported='yes'>\n");
+virBufferAdjustIndent(buf, 2);
+
+virCPUDefFormatBuf(buf, cpu->hostRecommended);
+
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "supported='no'/>\n");
+}
+}
+
 virBufferAsprintf(buf, "custom && cpu->custom->nmodels) {
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 01bcfa2e39..893101ee97 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -201,6 +201,7 @@ struct _virDomainCapsCPU {
 bool maximum;
 virDomainCapsEnum maximumMigratable;
 virCPUDef *hostModel;
+virCPUDef *hostRecommended;
 virDomainCapsCPUModels *custom;
 };
 
diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
index 99ef148d44..91657555dd 100644
--- a/src/conf/schemas/domaincaps.rng
+++ b/src/conf/schemas/domaincaps.rng
@@ -93,6 +93,9 @@
   
   
   
+  
+
+  
   
 
   
@@ -142,6 +145,27 @@
 
   
 
+  
+
+  
+host-recommended
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d7096a08c2..6ac8aaf248 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6250,6 +6250,14 @@ virQEMUCapsFillDomainCPUCaps(virQEMUCaps *qemuCaps,
 domCaps->cpu.custom = NULL;
 }
 }
+
+if (virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, domCaps->virttype,
+  VIR_CPU_MODE_HOST_RECOMMENDED,
+  domCaps->machine)) {
+virCPUDef *cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype,
+ 
VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED);
+domCaps->cpu.hostRecommended = virCPUDefCopy(cpu);
+}
 }
 
 
diff --git a/tests/domaincapsdata/qemu_8.1.0.s390x.xml 
b/tests/domaincapsdata/qemu_8.1.0.s390x.xml
index 37c7c3b8bf..68a456645c 100644
--- a/tests/domaincapsdata/qemu_8.1.0.s390x.xml
+++ b/tests/domaincapsdata/qemu_8.1.0.s390x.xml
@@ -90,6 +90,61 @@
   
   
 
+
+  gen16a-base
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
 
   gen16a-base
   gen16a
-- 
2.41.0

[PATCH v1 2/9] qemu: capabilities: add static-recommended capability

[Collin Walling]

Check for the QEMU capability to query for a static-recommended CPU
model via CPU model expansion. Cache this capability for later.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_capabilities.c| 2 ++
 src/qemu/qemu_capabilities.h| 1 +
 tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies | 6 +-
 tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 87412dd4ec..4ace8eea4a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -697,6 +697,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 450 */
   "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN 
*/
+  "query-cpu-model-expansion.static-recommended", /* 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED */
 );
 
 
@@ -1557,6 +1558,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "object-add/arg-type/+iothread/thread-pool-max", 
QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX },
 { "query-migrate/ret-type/blocked-reasons", 
QEMU_CAPS_MIGRATION_BLOCKED_REASONS },
 { "screendump/arg-type/format/^png", QEMU_CAPS_SCREENSHOT_FORMAT_PNG },
+{ "query-cpu-model-expansion/arg-type/type/^static-recommended", 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED },
 };
 
 typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e51d3fffdc..4fa64b6435 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -676,6 +676,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 450 */
 QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with 
async-teardown=on|off */
+QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED, /* 
query-cpu-model-expansion supports type static-recommended*/
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies 
b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies
index 57ce64e88e..8cd7312bea 100644
--- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies
+++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies
@@ -15398,12 +15398,16 @@
 },
 {
   "name": "full"
+},
+{
+  "name": "static-recommended"
 }
   ],
   "meta-type": "enum",
   "values": [
 "static",
-"full"
+"full",
+"static-recommended"
   ]
 },
 {
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml 
b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml
index 427ee9d5c7..0bb4233383 100644
--- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml
@@ -112,6 +112,7 @@
   
   
   
+  
   850
   39100245
   v8.0.0-1270-g1c12355b
-- 
2.41.0

[PATCH v1 7/9] qemu: process: allow guest to use host-recommended CPU model

[Collin Walling]

A guest may enable the host-recommended CPU model via the following
domain XML:

  

Once the guest is running, the deprecated features will nest under
the  tag paired with the "disable" policy, e.g.:

  
gen16a-base







...


  

Though existing guests must restart for the CPU model to update with
these changes, this at least removes a requirement of the user to
manually stay up-to-date on which features are recommended to be
disabled as future hardware updates come into play -- so long as the
hypervisor maintains the most up-to-date CPU model definitions.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/cpu/cpu.c |  1 +
 src/qemu/qemu_command.c   |  4 +++
 src/qemu/qemu_process.c   |  9 --
 src/qemu/qemu_validate.c  |  7 
 ...c-cpu-kvm-ccw-virtio-8.1.s390x-latest.args | 32 +++
 .../s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml  | 17 ++
 tests/qemuxml2argvtest.c  |  2 ++
 ...ec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml | 25 +++
 tests/qemuxml2xmltest.c   |  1 +
 9 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args
 create mode 100644 
tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml
 create mode 100644 
tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 805aff1bf5..4ac34a5efb 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -981,6 +981,7 @@ virCPUTranslate(virArch arch,
 
 if (cpu->mode == VIR_CPU_MODE_HOST_MODEL ||
 cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH ||
+cpu->mode == VIR_CPU_MODE_HOST_RECOMMENDED ||
 cpu->mode == VIR_CPU_MODE_MAXIMUM)
 return 0;
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a45e2fbb95..ace9deb246 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6294,6 +6294,10 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver,
 break;
 
 case VIR_CPU_MODE_HOST_RECOMMENDED:
+if (ARCH_IS_S390(def->os.arch))
+virBufferAddLit(buf, "host-recommended");
+break;
+
 case VIR_CPU_MODE_LAST:
 break;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7a1cdb0302..4ff8c1c87a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6303,6 +6303,7 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
 if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
 def->cpu->mode != VIR_CPU_MODE_MAXIMUM) {
 g_autoptr(virDomainCapsCPUModels) cpuModels = NULL;
+virQEMUCapsHostCPUType cpuType;
 
 if (def->cpu->check == VIR_CPU_CHECK_PARTIAL &&
 virCPUCompare(hostarch,
@@ -6311,9 +6312,13 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
   def->cpu, true) < 0)
 return -1;
 
+if (def->cpu->mode == VIR_CPU_MODE_HOST_RECOMMENDED)
+cpuType = VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED;
+else
+cpuType = VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE;
+
 if (virCPUUpdate(def->os.arch, def->cpu,
- virQEMUCapsGetHostModel(qemuCaps, def->virtType,
- 
VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE)) < 0)
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType, 
cpuType)) < 0)
 return -1;
 
 cpuModels = virQEMUCapsGetCPUModels(qemuCaps, def->virtType, NULL, 
NULL);
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 387c2f1fa8..a60445f686 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -373,6 +373,13 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
 break;
 
 case VIR_CPU_MODE_HOST_RECOMMENDED:
+if (!virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("host-recommended CPU is not supported by 
QEMU binary"));
+return -1;
+}
+break;
+
 case VIR_CPU_MODE_CUSTOM:
 case VIR_CPU_MODE_LAST:
 break;
diff --git 
a/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args 
b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args
new file mode 100644
index 00..78546380d7
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-test/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-test/.cache \

[PATCH v1 9/9] qemu_driver: expand baselined model to static_recommended for s390x

[Collin Walling]

In order for the CPU model expansion command to consider any deprecated
CPU features, s390x will need to use the special static recommended
expansion type after baselining.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_driver.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1c61685489..c4428c066f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11687,7 +11687,8 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps,
 bool expand_features,
 virCPUDef **cpus,
 int ncpus,
-virDomainCapsCPUModels *cpuModels)
+virDomainCapsCPUModels *cpuModels,
+virArch arch)
 {
 g_autoptr(qemuProcessQMP) proc = NULL;
 g_autoptr(virCPUDef) baseline = NULL;
@@ -11739,6 +11740,9 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps,
 expansion_type = expand_features ? 
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
  : 
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
 
+if (ARCH_IS_S390(arch) && expansion_type == 
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC)
+expansion_type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC;
+
 if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type,
 baseline, true,
 false, false, ) < 0)
@@ -11827,7 +11831,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir,
 cfg->user, cfg->group,
 expand_features, cpus, ncpus,
-cpuModels)))
+cpuModels, arch)))
 goto cleanup;
 } else {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-- 
2.41.0

[PATCH v1 4/9] qemu: capabilities: query and cache host-recommended CPU model

[Collin Walling]

Query the host-recommended CPU model from QEMU, if it is supported. This
model will be stored in the QEMU capabilities file underneath the
closing  tag, labeled . An example follows,
(shortened as to not overwhelm the commit message):

  







...


  

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/conf/schemas/cputypes.rng |   1 +
 src/qemu/qemu_capabilities.c  | 108 +++---
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_capspriv.h  |   6 +-
 tests/cputest.c   |   4 +-
 .../caps_8.1.0_s390x.replies  |  74 
 .../qemucapabilitiesdata/caps_8.1.0_s390x.xml |  54 +
 7 files changed, 231 insertions(+), 19 deletions(-)

diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng
index db1aa57158..5e89d67c8e 100644
--- a/src/conf/schemas/cputypes.rng
+++ b/src/conf/schemas/cputypes.rng
@@ -10,6 +10,7 @@
 host-model
 host-passthrough
 maximum
+host-recommended
   
 
   
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40cdcffbfe..59403808ee 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -724,6 +724,11 @@ struct _virQEMUCapsHostCPUData {
 qemuMonitorCPUModelInfo *info;
 /* Physical address size of the host CPU or 0 if unknown or not 
applicable. */
 unsigned int physAddrSize;
+/* Host CPU model with delta changes reported by the hypervisor to ensure
+ * migration compatibility with newer machines that may exclude certain
+ * features available on older machines.
+ */
+qemuMonitorCPUModelInfo *hostRec;
 /* Host CPU definition reported in domain capabilities. */
 virCPUDef *reported;
 /* Migratable host CPU definition used for updating guest CPU. */
@@ -732,6 +737,8 @@ struct _virQEMUCapsHostCPUData {
  * combined with features reported by QEMU. This is used for backward
  * compatible comparison between a guest CPU and a host CPU. */
 virCPUDef *full;
+/* CPU definition converted from hostRec model info */
+virCPUDef *recommended;
 };
 
 typedef struct _virQEMUCapsAccel virQEMUCapsAccel;
@@ -1834,6 +1841,12 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUData *dst,
 
 if (src->full)
 dst->full = virCPUDefCopy(src->full);
+
+if (src->hostRec)
+dst->hostRec = qemuMonitorCPUModelInfoCopy(src->hostRec);
+
+if (src->recommended)
+dst->recommended = virCPUDefCopy(src->recommended);
 }
 
 
@@ -1841,9 +1854,11 @@ static void
 virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUData *cpuData)
 {
 qemuMonitorCPUModelInfoFree(cpuData->info);
+qemuMonitorCPUModelInfoFree(cpuData->hostRec);
 virCPUDefFree(cpuData->reported);
 virCPUDefFree(cpuData->migratable);
 virCPUDefFree(cpuData->full);
+virCPUDefFree(cpuData->recommended);
 
 memset(cpuData, 0, sizeof(*cpuData));
 }
@@ -2190,6 +2205,9 @@ virQEMUCapsGetHostModel(virQEMUCaps *qemuCaps,
 /* 'full' is non-NULL only if we have data from both QEMU and
  * virCPUGetHost */
 return cpuData->full ? cpuData->full : cpuData->reported;
+
+case VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED:
+return cpuData->recommended;
 }
 
 return NULL;
@@ -2202,7 +2220,8 @@ virQEMUCapsSetHostModel(virQEMUCaps *qemuCaps,
 unsigned int physAddrSize,
 virCPUDef *reported,
 virCPUDef *migratable,
-virCPUDef *full)
+virCPUDef *full,
+virCPUDef *recommended)
 {
 virQEMUCapsHostCPUData *cpuData;
 
@@ -2211,6 +2230,7 @@ virQEMUCapsSetHostModel(virQEMUCaps *qemuCaps,
 cpuData->reported = reported;
 cpuData->migratable = migratable;
 cpuData->full = full;
+cpuData->recommended = recommended;
 }
 
 
@@ -3048,6 +3068,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps,
 {
 const char *model = virQEMUCapsTypeIsAccelerated(virtType) ? "host" : 
"max";
 g_autoptr(qemuMonitorCPUModelInfo) modelInfo = NULL;
+g_autoptr(qemuMonitorCPUModelInfo) hostRecInfo = NULL;
 g_autoptr(qemuMonitorCPUModelInfo) nonMigratable = NULL;
 g_autoptr(virCPUDef) cpu = NULL;
 qemuMonitorCPUModelExpansionType type;
@@ -3133,6 +3154,19 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps,
 return -1;
 }
 
+/* Retrieve the hypervisor recommended host CPU */
+if (virQEMUCapsTypeIsAccelerated(virtType) &&
+ARCH_IS_S390(qemuCaps->arch) &&
+virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED)) {
+type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC;
+
+if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, false, 
fail_no_props,
+) < 0)
+

[PATCH v1 1/9] qemu: monitor: enable query for static-recommended CPU model

[Collin Walling]

Future S390 hardware generations may decide to drop CPU features
completely, thus rendering guests running with such features incapable
of migrating to newer machines. This results in a problem whereas the
user must manually disable features ahead of time to prepare a guest for
migration in a mixed environment.

To circumvent this issue, a "host-recommended" CPU model has been
introduced, which is the host-model with delta changes that disable
features that have been flagged as deprecated.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_monitor.h  | 1 +
 src/qemu/qemu_monitor_json.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6c590933aa..d3f63e0703 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1144,6 +1144,7 @@ struct _qemuMonitorCPUModelInfo {
 typedef enum {
 QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
 QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL,
+QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC,
 QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
 } qemuMonitorCPUModelExpansionType;
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5b9edadcf7..7b2aee7a42 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5062,7 +5062,9 @@ qemuMonitorJSONQueryCPUModelExpansionOne(qemuMonitor *mon,
 case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL:
 typeStr = "static";
 break;
-
+case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC:
+typeStr = "static-recommended";
+break;
 case QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL:
 typeStr = "full";
 break;
-- 
2.41.0

[PATCH v1 0/9] query & cache host-recommended CPU model

[Collin Walling]

Notes:
https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html
 - For information regarding the QEMU "static-recommended" (aka 
host-recommended)
   CPU model, please see the analogous QEMU patches: 

 https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html

 - Patches include caps added to QEMU 8.1 test files. This is a stand-in for the
   interim and the appropriate caps files will be updated in the future pending 
   QEMU acceptance.

Description:

Allow libvirt to query, cache, and report the "host-recommended" CPU model, 
which
reports CPU features the hypervisor recommends older CPU models to disable in 
order
to ensure migration to new-generation machines that will no longer support said
features.

The host-recommended CPU is a retrieved via a query to QEMU that is a 
combination 
of two parameters: the standard model name "host" and a new model type
"static-recommended". The resulting CPU is similar to the host-model which now
reports deprecated CPU features disabled (e.g. featurex=off). Once this model 
has
been queried, the libvirt ABI will parse the feature on|off parameter and set 
the
feature policy as appropriate. The resulting model is cached in the respective 
QEMU
capabilities file. It can be reported via the virsh domcapabilities command

Two "host models" are now represented: the original "host-model" that reports 
the
hypervisor-supported features as-is, and the new "host-recommended" that reports
the hypervisor-supported features with any deprecated features paired with the
disabled policy.

Additionally, hypervisor-cpu-baseline has been modified to output the feature
policy and report disabled features. Baseline will also expand the resulting
model with the host-recommended parameters if possible.

The following s390x features are flagged as deprecated and will be dropped
outright on future models: csske, te, cte, bpb.

Examples:

To enable this CPU model, define a guest with the following  element:

  

A guest will be generated with a CPU e.g.:


  gen16a-base






















































An example of the hypervisor-cpu-baseline command which will now
invoke a CPU model expansion with the host-recommended parameters:

# virsh hypervisor-cpu-baseline host-model.xml 

  z14.2-base
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  


Collin Walling (9):
  qemu: monitor: enable query for static-recommended CPU model
  qemu: capabilities: add static-recommended capability
  qemu: capabilities: refactor virQEMUCapsLoadHostCPUModelInfo
  qemu: capabilities: query and cache host-recommended CPU model
  cpu: conf: add cpu mode for host-recommended
  domain: capabilities: report host-recommended CPU model
  qemu: process: allow guest to use host-recommended CPU model
  qemu_driver: report feature policy when baselining
  qemu_driver: expand baselined model to static_recommended for s390x

 src/conf/cpu_conf.c   |   1 +
 src/conf/cpu_conf.h   |   1 +
 src/conf/domain_capabilities.c|  16 ++
 src/conf/domain_capabilities.h|   1 +
 src/conf/schemas/cputypes.rng |   1 +
 src/conf/schemas/domaincaps.rng   |  24 +++
 src/cpu/cpu.c |   2 +
 src/cpu/cpu_ppc64.c   |   2 +
 src/qemu/qemu_capabilities.c  | 144 +++---
 src/qemu/qemu_capabilities.h  |   4 +
 src/qemu/qemu_capspriv.h  |   6 +-
 src/qemu/qemu_command.c   |   5 +
 src/qemu/qemu_domain.c|   2 +
 src/qemu/qemu_driver.c|  18 ++-
 src/qemu/qemu_monitor.h   |   1 +
 src/qemu/qemu_monitor_json.c  |   4 +-
 src/qemu/qemu_process.c   |   9 +-
 src/qemu/qemu_validate.c  |   8 +
 tests/cputest.c   |   4 +-
 tests/domaincapsdata/qemu_8.1.0.s390x.xml |  55 +++
 .../caps_8.1.0_s390x.replies  |  80 +-
 .../qemucapabilitiesdata/caps_8.1.0_s390x.xml |  55 +++
 ...c-cpu-kvm-ccw-virtio-8.1.s390x-latest.args |  32 
 .../s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml  |  17 +++
 tests/qemuxml2argvtest.c 

[PATCH v1 3/9] qemu: capabilities: refactor virQEMUCapsLoadHostCPUModelInfo

[Collin Walling]

This will be used to load the host recommended CPU model, introduced in
a subsequent patch. Also move ctxt auto restore to end of variable
declarations to avoid a compiler warning.

Signed-off-by: Collin Walling 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_capabilities.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4ace8eea4a..40cdcffbfe 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3894,19 +3894,18 @@ virQEMUCapsSetCPUModelInfo(virQEMUCaps *qemuCaps,
 
 
 static int
-virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps,
-xmlXPathContextPtr ctxt,
-const char *typeStr)
+virQEMUCapsLoadCPUModelInfo(qemuMonitorCPUModelInfo **modelInfo,
+xmlXPathContextPtr ctxt,
+char *xpath)
 {
 xmlNodePtr hostCPUNode;
 g_autofree xmlNodePtr *nodes = NULL;
-VIR_XPATH_NODE_AUTORESTORE(ctxt)
 g_autoptr(qemuMonitorCPUModelInfo) hostCPU = NULL;
-g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr);
 size_t i;
 int n;
 virTristateBool migratability;
 int val;
+VIR_XPATH_NODE_AUTORESTORE(ctxt)
 
 if (!(hostCPUNode = virXPathNode(xpath, ctxt))) {
 return 0;
@@ -3991,11 +3990,22 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps,
 }
 }
 
-caps->hostCPU.info = g_steal_pointer();
+*modelInfo = g_steal_pointer();
 return 0;
 }
 
 
+static int
+virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps,
+xmlXPathContextPtr ctxt,
+const char *typeStr)
+{
+g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr);
+
+return virQEMUCapsLoadCPUModelInfo(>hostCPU.info, ctxt, xpath);
+}
+
+
 static int
 virQEMUCapsLoadCPUModels(virArch arch,
  virQEMUCapsAccel *caps,
-- 
2.41.0

Re: [libvirt PATCHv1 8/8] docs: virtiofs: add section about ID remapping

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:51:28PM +0200, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  docs/kbase/virtiofs.rst | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst
> index 5940092db5..ecfb8e4236 100644
> --- a/docs/kbase/virtiofs.rst
> +++ b/docs/kbase/virtiofs.rst
> @@ -59,6 +59,35 @@ Sharing a host directory with a guest
>  
> Note: this requires virtiofs support in the guest kernel (Linux v5.4 or 
> later)
>  
> +ID mapping
> +==
> +
> +In unprivileged mode (``qemu:///session``), mapping user/group IDs is 
> available
> +(since libvirt version TBD). After reserving an ID range from the host for 
> your
> +regular user

Is the GUID/GID mapping available as in optional, or available as
in mandatory ?

I would expect libvirt to "do the right thing" and automatically load
the /etc/subuid data for the current user and NOT require any extra
XML mapping to be set for unprivileged usage.

By all means have an XML config for it, but it should be optional and
something that is essentially never used except for niche scenarios

> +
> +::
> +
> +  $ cat /etc/subuid
> +  jtomko:10:65536
> +  $ cat /etc/subgid
> +  jtomko:10:65536
> +
> +you can let virtiofsd map guest UIDs from 0 to 65535
> +to host IDs 10 to 165535 for example:
> +
> +::
> +
> +  
> +
> +...
> +
> +  
> +  
> +
> +  
> +
> +
>  Optional parameters
>  ===
>  
> -- 
> 2.41.0
> 

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: [libvirt PATCH v2 10/35] ci: build.sh: Add a wrapper function over the 'potfile' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:11PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart. There's one notable difference such that we pass '-j1' to
> the meson compile command otherwise we'd have to execute the 'run_build'
> function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets
> in a serial manner.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 11 +++
>  1 file changed, 11 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 13/35] ci: build.sh: Drop changing working directory to CI_CONT_DIR

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:14PM +0200, Erik Skultety wrote:
> Firstly, this would mangle with "sourcing" this file in either
> execution environment later down the road. Secondly, we won't need this
> as future ci/helper patches will generate a throwaway script that will
> take care of a correct execution of a build job in a similar fashion as
> if the job ran in a GitLab environment.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 16/35] ci: Rename build.sh -> jobs.sh

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:17PM +0200, Erik Skultety wrote:
> After the recent changes, this script no longer executes any logic
> anymore, it merely defines the jobs run in the GitLab environment. In
> order to use it, one has to source the file in the environment and then
> run one of the job "functions". For that, the 'build.sh' name is no
> longer descriptive enough and 'jobs.sh' feels more suitable and less
> misleading.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/{build.sh => jobs.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename ci/{build.sh => jobs.sh} (100%)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 14/35] ci: build.sh: Drop direct invocation of meson/ninja commands

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:15PM +0200, Erik Skultety wrote:
> We've moved all invocations to the respective helper function which
> we'll execute both from gitlab CI jobs and local environments so we
> don't need to have them on the global level as it would also not work
> with "sourcing" this file to populate the environment with function
> definitions.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 12/35] ci: build.sh: Add a wrapper function over the 'website' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:13PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 7 +++
>  1 file changed, 7 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 11/35] ci: build.sh: Add a wrapper function over the 'rpmbuild' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:12PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 9 +
>  1 file changed, 9 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 09/35] ci: build.sh: Add a wrapper function over the 'codestyle' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:10PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 8 
>  1 file changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrangé 

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: [libvirt PATCH v2 08/35] ci: build.sh: Add a wrapper function over the 'test' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:09PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index ab56c5e5eb..29b6a3306d 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -52,3 +52,8 @@ run_dist() {
>  git update-index --refresh
>  run_cmd meson dist -C build --no-tests
>  }
> +
> +run_test() {
> +test -d $GIT_ROOT/build/build.ninja || run_meson_setup

test -f

> +run_cmd meson test -C build $TEST_ARGS
> +}

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 07/35] ci: build.sh: Add a helper function to create the dist tarball

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:08PM +0200, Erik Skultety wrote:
> This helper function does not correspond to a particular GitLab job, it
> just logically separates the necessary step of creating a dist tarball
> from the RPM build job that takes over.
> One notable change here is the need to update git's file index which
> causes issues in local container executions which rely on a shallow
> copy of the libvirt repo created as:
> 
> $ git clone --local
> 
> Even if all changes have been committed, git often complained
> otherwise. Updating the index in a GitLab environment is a NOP.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index f908bbc5d4..ab56c5e5eb 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -42,3 +42,13 @@ run_build() {
>  test -d $GIT_ROOT/build/build.ninja || run_meson_setup
>  run_cmd meson compile -C build $BUILD_ARGS
>  }
> +
> +run_dist() {
> +test -d $GIT_ROOT/build/build.ninja || run_meson_setup

test -f


> +
> +# dist is unhappy in local container environment complaining about
> +# uncommitted changes in the repo which is often not the case - 
> refreshing
> +# git's index solves the problem
> +git update-index --refresh
> +run_cmd meson dist -C build --no-tests
> +}

With the test fix

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 06/35] ci: build.sh: Add a wrapper function over the 'build' job

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:07PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 477ccbc7d1..f908bbc5d4 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -37,3 +37,8 @@ run_meson_setup() {
>  run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS 
> || \
>  (cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1)
>  }
> +
> +run_build() {
> +test -d $GIT_ROOT/build/build.ninja || run_meson_setup

test -f  

> +run_cmd meson compile -C build $BUILD_ARGS
> +}

With that test change

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 05/35] ci: build.sh: Add a wrapper function over meson's setup

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:06PM +0200, Erik Skultety wrote:
> The reason for this wrapper is that all job functions introduced in
> future patches will refer to this one instead of open-coding the same
> 'meson setup' invocation N times. It also prevents 'setup' to be called
> multiple times as some future job functions might actually do just that
> in a transitive manner.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 04/35] ci: build.sh: Add a wrapper function executing 'shell' commands

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:05PM +0200, Erik Skultety wrote:
> This would normally be not needed at all, but the problem here is the
> Shell-in-YAML which GitLab interprets. It outputs every command that
> appears as a line in the 'script' segment in a color-coded fashion for
> easy identification of problems. Well, that useful feature is lost when
> there's indirection and one script calls into another in which case it
> would only output the respective script name which would make failure
> investigation harder. This simple helper tackles that by echoing the
> command to be run by any script/function with a color escape sequence
> so that we don't lose track of the *actual* shell commands being run as
> part of the GitLab job pipelines. An example of what the output then
> might look like:
> [RUN COMMAND]: 'meson compile -C build install-web'
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [libvirt PATCH v2 01/35] ci: build.sh: Add variables from .gitlab-ci.yml

[Erik Skultety]

On Mon, Sep 11, 2023 at 03:02:15PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 11, 2023 at 03:43:02PM +0200, Erik Skultety wrote:
> > These are common variables we wish to use in containerized environments
> > both in GitLab and locally. Having these defined in a single place
> > rather than twice is highly preferable.
> > 
> > Signed-off-by: Erik Skultety 
> > Erik Skultety :
> 
> A script went wrong here and other following patches. I'll assume you'll
> fix it, so will ack stuff regardless

Sigh, must have run the same borked git command line in 2 separate terminals,
sure, consider this fixed.

Erik

Re: [libvirt PATCHv1 1/8] qemu: fix indentation

[Michal Prívozník]

On 9/11/23 15:51, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_validate.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 

You could at least mention in which function, because I doubt this is
the only misalignment we have in src/qemu/. Or in that file alone.

Michal

Re: [libvirt PATCHv1 3/8] conf: move idmap parsing earlier

[Michal Prívozník]

On 9/11/23 15:51, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/conf/domain_conf.c | 98 +-
>  1 file changed, 49 insertions(+), 49 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2c8727de54..dd67e7f21b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8581,6 +8581,55 @@ virDomainNetGenerateMAC(virDomainXMLOption *xmlopt,
>  }
>  
>  
> +static int virDomainIdMapEntrySort(const void *a, const void *b)
> +{
> +const virDomainIdMapEntry *entrya = a;
> +const virDomainIdMapEntry *entryb = b;
> +
> +if (entrya->start > entryb->start)
> +return 1;
> +else if (entrya->start < entryb->start)
> +return -1;
> +else
> +return 0;
> +}
> +

Please put an extra empty line here.

> +/* Parse the XML definition for user namespace id map.
> + *
> + * idmap has the form of
> + *
> + *   
> + *   
> + */
> +static virDomainIdMapEntry *
> +virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
> +  xmlNodePtr *node,
> +  size_t num)
> +{
> +size_t i;
> +virDomainIdMapEntry *idmap = NULL;
> +VIR_XPATH_NODE_AUTORESTORE(ctxt)
> +
> +idmap = g_new0(virDomainIdMapEntry, num);
> +
> +for (i = 0; i < num; i++) {
> +ctxt->node = node[i];
> +if (virXPathUInt("string(./@start)", ctxt, [i].start) < 0 ||
> +virXPathUInt("string(./@target)", ctxt, [i].target) < 0 ||
> +virXPathUInt("string(./@count)", ctxt, [i].count) < 0) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("invalid idmap start/target/count settings"));
> +VIR_FREE(idmap);
> +return NULL;
> +}
> +}
> +
> +qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort);
> +
> +return idmap;
> +}
> +
> +

Michal

Re: [libvirt PATCHv1 6/8] qemu: virtiofs: do not force UID 0

[Michal Prívozník]

On 9/11/23 15:51, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_virtiofs.c | 4 
>  1 file changed, 4 deletions(-)

While I love short commit messages, this one is rather short. I'd
appreciate more insight into why this is no longer needed.

Michal

Re: [libvirt PATCHv1 4/8] conf: add idmap element to filesystem

[Michal Prívozník]

On 9/11/23 15:51, Ján Tomko wrote:
> Let unprivileged virtiofs use user namespace.
> 
> Signed-off-by: Ján Tomko 
> ---
>  docs/formatdomain.rst |  7 +++
>  src/conf/domain_conf.c| 51 +++
>  src/conf/domain_conf.h|  1 +
>  src/conf/schemas/domaincommon.rng |  3 ++
>  .../vhost-user-fs-fd-memory.xml   |  4 ++
>  5 files changed, 66 insertions(+)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index bc469e5f9f..0f10b3043f 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3501,6 +3501,10 @@ A directory on the host that can be accessed directly 
> from the guest.
>   
>   
>   
> + 
> + 
> + 
> + 
>   
>   
>   
> @@ -3650,6 +3654,9 @@ A directory on the host that can be accessed directly 
> from the guest.
> Where the ``source`` can be accessed in the guest. For most drivers this 
> is
> an automatic mount point, but for QEMU/KVM this is merely an arbitrary 
> string
> tag that is exported to the guest as a hint for where to mount.
> +``idmap``
> +   For ``virtiofs``, an ``idmap`` element can be specified to map IDs in the 
> user
> +   namespace. See the `Container boot`_ section for the syntax of the 
> element.

:since:

>  ``readonly``
> Enables exporting filesystem as a readonly mount for guest, by default
> read-write access is given (currently only works for QEMU/KVM driver).
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index dd67e7f21b..2379a9204f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2585,6 +2585,8 @@ void virDomainFSDefFree(virDomainFSDef *def)
>  virObjectUnref(def->privateData);
>  g_free(def->binary);
>  g_free(def->sock);
> +g_free(def->idmap.uidmap);
> +g_free(def->idmap.gidmap);
>  
>  g_free(def);
>  }
> @@ -8767,6 +8769,8 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
>  xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt);
>  xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt);
>  xmlNodePtr binary_sandbox_node = virXPathNode("./binary/sandbox", 
> ctxt);
> +ssize_t n;
> +xmlNodePtr *nodes = NULL;
>  
>  if (queue_size && virStrToLong_ull(queue_size, NULL, 10, 
> >queue_size) < 0) {
>  virReportError(VIR_ERR_XML_ERROR,
> @@ -8812,6 +8816,30 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
> VIR_XML_PROP_NONZERO,
> >sandbox) < 0)
>  goto error;
> +
> +if ((n = virXPathNodeSet("./idmap/uid", ctxt, )) < 0)
> +return NULL;
> +
> +if (n) {
> +def->idmap.uidmap = virDomainIdmapDefParseXML(ctxt, nodes, n);
> +if (!def->idmap.uidmap)
> +return NULL;

If this return is taken, then ...

> +
> +def->idmap.nuidmap = n;
> +}
> +VIR_FREE(nodes);

... this is never called.

> +
> +if  ((n = virXPathNodeSet("./idmap/gid", ctxt, )) < 0)
> +return NULL;
> +
> +if (n) {
> +def->idmap.gidmap =  virDomainIdmapDefParseXML(ctxt, nodes, n);
> +if (!def->idmap.gidmap)
> +return NULL;

Same here.

> +
> +def->idmap.ngidmap = n;
> +}
> +VIR_FREE(nodes);
>  }
>  
>  if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM
> @@ -23164,6 +23192,29 @@ virDomainFSDefFormat(virBuffer *buf,
>  virXMLFormatElement(buf, "driver", , );
>  virXMLFormatElement(buf, "binary", , );
>  
> +if (def->idmap.uidmap) {
> +size_t i;
> +
> +virBufferAddLit(buf, "\n");
> +virBufferAdjustIndent(buf, 2);
> +for (i = 0; i < def->idmap.nuidmap; i++) {
> +virBufferAsprintf(buf,
> +  "\n",
> +  def->idmap.uidmap[i].start,
> +  def->idmap.uidmap[i].target,
> +  def->idmap.uidmap[i].count);
> +}
> +for (i = 0; i < def->idmap.ngidmap; i++) {
> +virBufferAsprintf(buf,
> +  "\n",
> +  def->idmap.gidmap[i].start,
> +  def->idmap.gidmap[i].target,
> +  def->idmap.gidmap[i].count);
> +}
> +virBufferAdjustIndent(buf, -2);
> +virBufferAddLit(buf, "\n");
> +}
> +
>  switch (def->type) {
>  case VIR_DOMAIN_FS_TYPE_MOUNT:
>  case VIR_DOMAIN_FS_TYPE_BIND:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8937968e3b..b84719b01d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -925,6 +925,7 @@ struct _virDomainFSDef {
>  virTristateSwitch flock;
>  virDomainFSSandboxMode 

Re: [libvirt PATCHv1 0/8] support running virtiofsd in session mode

[Michal Prívozník]

On 9/11/23 15:51, Ján Tomko wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=2034630
> https://gitlab.com/libvirt/libvirt/-/issues/535
> https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/292
> 
> Ján Tomko (8):
>   qemu: fix indentation
>   conf: move idmap definition earlier
>   conf: move idmap parsing earlier
>   conf: add idmap element to filesystem
>   qemu: format uid/gid map for virtiofs
>   qemu: virtiofs: do not force UID 0
>   qemu: allow running virtiofsd in session mode
>   docs: virtiofs: add section about ID remapping
> 
>  docs/formatdomain.rst |   7 +
>  docs/kbase/virtiofs.rst   |  29 
>  src/conf/domain_conf.c| 149 --
>  src/conf/domain_conf.h|  29 ++--
>  src/conf/schemas/domaincommon.rng |   3 +
>  src/qemu/qemu_validate.c  |  25 ++-
>  src/qemu/qemu_virtiofs.c  |  17 +-
>  .../vhost-user-fs-fd-memory.xml   |   4 +
>  8 files changed, 181 insertions(+), 82 deletions(-)
> 

So how does this idmap end up on virtiofsd cmd line? Maybe you forgot to
send some additional patches?

Michal

Re: [libvirt PATCH v2 01/35] ci: build.sh: Add variables from .gitlab-ci.yml

[Daniel P . Berrangé]

On Mon, Sep 11, 2023 at 03:43:02PM +0200, Erik Skultety wrote:
> These are common variables we wish to use in containerized environments
> both in GitLab and locally. Having these defined in a single place
> rather than twice is highly preferable.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :

A script went wrong here and other following patches. I'll assume you'll
fix it, so will ack stuff regardless

> ---
>  ci/build.sh | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index d5ed8ad104..5a9298c4b4 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -2,7 +2,17 @@
>  
>  cd "$CI_CONT_SRCDIR"
>  
> -export VIR_TEST_DEBUG=1
> +export CCACHE_BASEDIR="$(pwd)"
> +export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
> +export CCACHE_MAXSIZE="500M"
> +export PATH="$CCACHE_WRAPPERSDIR:$PATH"
> +
> +# Enable these conditionally since their best use case is during
> +# non-interactive workloads without having a Shell
> +if ! [ -t 1 ]; then
> +export VIR_TEST_VERBOSE="1"
> +export VIR_TEST_DEBUG="1"
> +fi
>  
>  # $MESON_OPTS is an env that can optionally be set in the container,
>  # populated at build time from the Dockerfile. A typical use case would

Reviewed-by: Daniel P. Berrangé 


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

[libvirt PATCHv1 8/8] docs: virtiofs: add section about ID remapping

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 docs/kbase/virtiofs.rst | 29 +
 1 file changed, 29 insertions(+)

diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst
index 5940092db5..ecfb8e4236 100644
--- a/docs/kbase/virtiofs.rst
+++ b/docs/kbase/virtiofs.rst
@@ -59,6 +59,35 @@ Sharing a host directory with a guest
 
Note: this requires virtiofs support in the guest kernel (Linux v5.4 or 
later)
 
+ID mapping
+==
+
+In unprivileged mode (``qemu:///session``), mapping user/group IDs is available
+(since libvirt version TBD). After reserving an ID range from the host for your
+regular user
+
+::
+
+  $ cat /etc/subuid
+  jtomko:10:65536
+  $ cat /etc/subgid
+  jtomko:10:65536
+
+you can let virtiofsd map guest UIDs from 0 to 65535
+to host IDs 10 to 165535 for example:
+
+::
+
+  
+
+...
+
+  
+  
+
+  
+
+
 Optional parameters
 ===
 
-- 
2.41.0

[libvirt PATCHv1 7/8] qemu: allow running virtiofsd in session mode

[Ján Tomko]

https://gitlab.com/libvirt/libvirt/-/issues/535

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_validate.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index e3c34eca3d..da7b0ff419 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4229,7 +4229,7 @@ qemuValidateDomainDeviceDefGraphics(const 
virDomainGraphicsDef *graphics,
 static int
 qemuValidateDomainDeviceDefFS(virDomainFSDef *fs,
   const virDomainDef *def,
-  virQEMUDriver *driver,
+  virQEMUDriver *driver G_GNUC_UNUSED,
   virQEMUCaps *qemuCaps)
 {
 if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
@@ -4286,11 +4286,6 @@ qemuValidateDomainDeviceDefFS(virDomainFSDef *fs,
_("virtiofs does not yet support read-only 
mode"));
 return -1;
 }
-if (!driver->privileged) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs is not yet supported in session 
mode"));
-return -1;
-}
 if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtiofs only supports passthrough 
accessmode"));
-- 
2.41.0

[libvirt PATCHv1 6/8] qemu: virtiofs: do not force UID 0

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_virtiofs.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 94c8b4711e..4871bad801 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -255,10 +255,6 @@ qemuVirtioFSStart(virQEMUDriver *driver,
 if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, )))
 goto error;
 
-/* so far only running as root is supported */
-virCommandSetUID(cmd, 0);
-virCommandSetGID(cmd, 0);
-
 virCommandSetPidFile(cmd, pidfile);
 virCommandSetOutputFD(cmd, );
 virCommandSetErrorFD(cmd, );
-- 
2.41.0

[libvirt PATCHv1 4/8] conf: add idmap element to filesystem

[Ján Tomko]

Let unprivileged virtiofs use user namespace.

Signed-off-by: Ján Tomko 
---
 docs/formatdomain.rst |  7 +++
 src/conf/domain_conf.c| 51 +++
 src/conf/domain_conf.h|  1 +
 src/conf/schemas/domaincommon.rng |  3 ++
 .../vhost-user-fs-fd-memory.xml   |  4 ++
 5 files changed, 66 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index bc469e5f9f..0f10b3043f 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3501,6 +3501,10 @@ A directory on the host that can be accessed directly 
from the guest.
  
  
  
+ 
+ 
+ 
+ 
  
  
  
@@ -3650,6 +3654,9 @@ A directory on the host that can be accessed directly 
from the guest.
Where the ``source`` can be accessed in the guest. For most drivers this is
an automatic mount point, but for QEMU/KVM this is merely an arbitrary 
string
tag that is exported to the guest as a hint for where to mount.
+``idmap``
+   For ``virtiofs``, an ``idmap`` element can be specified to map IDs in the 
user
+   namespace. See the `Container boot`_ section for the syntax of the element.
 ``readonly``
Enables exporting filesystem as a readonly mount for guest, by default
read-write access is given (currently only works for QEMU/KVM driver).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dd67e7f21b..2379a9204f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2585,6 +2585,8 @@ void virDomainFSDefFree(virDomainFSDef *def)
 virObjectUnref(def->privateData);
 g_free(def->binary);
 g_free(def->sock);
+g_free(def->idmap.uidmap);
+g_free(def->idmap.gidmap);
 
 g_free(def);
 }
@@ -8767,6 +8769,8 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
 xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt);
 xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt);
 xmlNodePtr binary_sandbox_node = virXPathNode("./binary/sandbox", 
ctxt);
+ssize_t n;
+xmlNodePtr *nodes = NULL;
 
 if (queue_size && virStrToLong_ull(queue_size, NULL, 10, 
>queue_size) < 0) {
 virReportError(VIR_ERR_XML_ERROR,
@@ -8812,6 +8816,30 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
VIR_XML_PROP_NONZERO,
>sandbox) < 0)
 goto error;
+
+if ((n = virXPathNodeSet("./idmap/uid", ctxt, )) < 0)
+return NULL;
+
+if (n) {
+def->idmap.uidmap = virDomainIdmapDefParseXML(ctxt, nodes, n);
+if (!def->idmap.uidmap)
+return NULL;
+
+def->idmap.nuidmap = n;
+}
+VIR_FREE(nodes);
+
+if  ((n = virXPathNodeSet("./idmap/gid", ctxt, )) < 0)
+return NULL;
+
+if (n) {
+def->idmap.gidmap =  virDomainIdmapDefParseXML(ctxt, nodes, n);
+if (!def->idmap.gidmap)
+return NULL;
+
+def->idmap.ngidmap = n;
+}
+VIR_FREE(nodes);
 }
 
 if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM
@@ -23164,6 +23192,29 @@ virDomainFSDefFormat(virBuffer *buf,
 virXMLFormatElement(buf, "driver", , );
 virXMLFormatElement(buf, "binary", , );
 
+if (def->idmap.uidmap) {
+size_t i;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+for (i = 0; i < def->idmap.nuidmap; i++) {
+virBufferAsprintf(buf,
+  "\n",
+  def->idmap.uidmap[i].start,
+  def->idmap.uidmap[i].target,
+  def->idmap.uidmap[i].count);
+}
+for (i = 0; i < def->idmap.ngidmap; i++) {
+virBufferAsprintf(buf,
+  "\n",
+  def->idmap.gidmap[i].start,
+  def->idmap.gidmap[i].target,
+  def->idmap.gidmap[i].count);
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
 switch (def->type) {
 case VIR_DOMAIN_FS_TYPE_MOUNT:
 case VIR_DOMAIN_FS_TYPE_BIND:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8937968e3b..b84719b01d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -925,6 +925,7 @@ struct _virDomainFSDef {
 virTristateSwitch flock;
 virDomainFSSandboxMode sandbox;
 int thread_pool_size;
+virDomainIdMapDef idmap;
 virDomainVirtioOptions *virtio;
 virObject *privateData;
 };
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index 2f9ba31c0a..2ca0e92f00 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -3052,6 +3052,9 @@
   
   
   

[libvirt PATCHv1 2/8] conf: move idmap definition earlier

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 src/conf/domain_conf.h | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ca195a52d2..8937968e3b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -802,6 +802,20 @@ struct _virDomainControllerDef {
 virDomainVirtioOptions *virtio;
 };
 
+struct _virDomainIdMapEntry {
+unsigned int start;
+unsigned int target;
+unsigned int count;
+};
+
+struct _virDomainIdMapDef {
+size_t nuidmap;
+virDomainIdMapEntry *uidmap;
+
+size_t ngidmap;
+virDomainIdMapEntry *gidmap;
+};
+
 
 /* Types of disk backends */
 typedef enum {
@@ -2695,20 +2709,6 @@ virDomainMemoryDef 
*virDomainMemoryDefNew(virDomainMemoryModel model);
 void virDomainMemoryDefFree(virDomainMemoryDef *def);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainMemoryDef, virDomainMemoryDefFree);
 
-struct _virDomainIdMapEntry {
-unsigned int start;
-unsigned int target;
-unsigned int count;
-};
-
-struct _virDomainIdMapDef {
-size_t nuidmap;
-virDomainIdMapEntry *uidmap;
-
-size_t ngidmap;
-virDomainIdMapEntry *gidmap;
-};
-
 
 typedef enum {
 VIR_DOMAIN_PANIC_MODEL_DEFAULT,
-- 
2.41.0

[libvirt PATCHv1 3/8] conf: move idmap parsing earlier

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 src/conf/domain_conf.c | 98 +-
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c8727de54..dd67e7f21b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8581,6 +8581,55 @@ virDomainNetGenerateMAC(virDomainXMLOption *xmlopt,
 }
 
 
+static int virDomainIdMapEntrySort(const void *a, const void *b)
+{
+const virDomainIdMapEntry *entrya = a;
+const virDomainIdMapEntry *entryb = b;
+
+if (entrya->start > entryb->start)
+return 1;
+else if (entrya->start < entryb->start)
+return -1;
+else
+return 0;
+}
+
+/* Parse the XML definition for user namespace id map.
+ *
+ * idmap has the form of
+ *
+ *   
+ *   
+ */
+static virDomainIdMapEntry *
+virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
+  xmlNodePtr *node,
+  size_t num)
+{
+size_t i;
+virDomainIdMapEntry *idmap = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+idmap = g_new0(virDomainIdMapEntry, num);
+
+for (i = 0; i < num; i++) {
+ctxt->node = node[i];
+if (virXPathUInt("string(./@start)", ctxt, [i].start) < 0 ||
+virXPathUInt("string(./@target)", ctxt, [i].target) < 0 ||
+virXPathUInt("string(./@count)", ctxt, [i].count) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("invalid idmap start/target/count settings"));
+VIR_FREE(idmap);
+return NULL;
+}
+}
+
+qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort);
+
+return idmap;
+}
+
+
 static virDomainFSDef *
 virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
xmlNodePtr node,
@@ -15711,55 +15760,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
 return 0;
 }
 
-
-static int virDomainIdMapEntrySort(const void *a, const void *b)
-{
-const virDomainIdMapEntry *entrya = a;
-const virDomainIdMapEntry *entryb = b;
-
-if (entrya->start > entryb->start)
-return 1;
-else if (entrya->start < entryb->start)
-return -1;
-else
-return 0;
-}
-
-/* Parse the XML definition for user namespace id map.
- *
- * idmap has the form of
- *
- *   
- *   
- */
-static virDomainIdMapEntry *
-virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
-  xmlNodePtr *node,
-  size_t num)
-{
-size_t i;
-virDomainIdMapEntry *idmap = NULL;
-VIR_XPATH_NODE_AUTORESTORE(ctxt)
-
-idmap = g_new0(virDomainIdMapEntry, num);
-
-for (i = 0; i < num; i++) {
-ctxt->node = node[i];
-if (virXPathUInt("string(./@start)", ctxt, [i].start) < 0 ||
-virXPathUInt("string(./@target)", ctxt, [i].target) < 0 ||
-virXPathUInt("string(./@count)", ctxt, [i].count) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("invalid idmap start/target/count settings"));
-VIR_FREE(idmap);
-return NULL;
-}
-}
-
-qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort);
-
-return idmap;
-}
-
 /* Parse the XML definition for an IOThread ID
  *
  * Format is :
-- 
2.41.0

[libvirt PATCHv1 5/8] qemu: format uid/gid map for virtiofs

[Ján Tomko]

Pass the ID map to virtiofsd.

https://bugzilla.redhat.com/show_bug.cgi?id=2034630
https://gitlab.com/libvirt/libvirt/-/issues/535

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_virtiofs.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 230f85c291..94c8b4711e 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -169,6 +169,19 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg,
 if (cfg->virtiofsdDebug)
 virCommandAddArg(cmd, "-d");
 
+if (fs->idmap.nuidmap > 0) {
+virCommandAddArgFormat(cmd, "--uid-map=:%u:%u:%u:",
+   fs->idmap.uidmap[0].start,
+   fs->idmap.uidmap[0].target,
+   fs->idmap.uidmap[0].count);
+}
+if (fs->idmap.ngidmap > 0) {
+virCommandAddArgFormat(cmd, "--gid-map=:%u:%u:%u:",
+   fs->idmap.gidmap[0].start,
+   fs->idmap.gidmap[0].target,
+   fs->idmap.gidmap[0].count);
+}
+
 return g_steal_pointer();
 }
 
-- 
2.41.0

[libvirt PATCHv1 1/8] qemu: fix indentation

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_validate.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 1346bbfb44..e3c34eca3d 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4282,24 +4282,24 @@ qemuValidateDomainDeviceDefFS(virDomainFSDef *fs,
 case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
 if (!fs->sock) {
 if (fs->readonly) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs does not yet support read-only mode"));
-return -1;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs does not yet support read-only 
mode"));
+return -1;
 }
 if (!driver->privileged) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs is not yet supported in session mode"));
-return -1;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs is not yet supported in session 
mode"));
+return -1;
 }
 if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs only supports passthrough accessmode"));
-return -1;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs only supports passthrough 
accessmode"));
+return -1;
 }
 if (fs->wrpolicy != VIR_DOMAIN_FS_WRPOLICY_DEFAULT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("virtiofs does not support wrpolicy"));
-return -1;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs does not support wrpolicy"));
+return -1;
 }
 }
 
-- 
2.41.0

[libvirt PATCHv1 0/8] support running virtiofsd in session mode

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=2034630
https://gitlab.com/libvirt/libvirt/-/issues/535
https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/292

Ján Tomko (8):
  qemu: fix indentation
  conf: move idmap definition earlier
  conf: move idmap parsing earlier
  conf: add idmap element to filesystem
  qemu: format uid/gid map for virtiofs
  qemu: virtiofs: do not force UID 0
  qemu: allow running virtiofsd in session mode
  docs: virtiofs: add section about ID remapping

 docs/formatdomain.rst |   7 +
 docs/kbase/virtiofs.rst   |  29 
 src/conf/domain_conf.c| 149 --
 src/conf/domain_conf.h|  29 ++--
 src/conf/schemas/domaincommon.rng |   3 +
 src/qemu/qemu_validate.c  |  25 ++-
 src/qemu/qemu_virtiofs.c  |  17 +-
 .../vhost-user-fs-fd-memory.xml   |   4 +
 8 files changed, 181 insertions(+), 82 deletions(-)

-- 
2.41.0

[libvirt PATCH v2 33/35] ci: helper: Drop the --meson-args/--ninja-args CLI options

[Erik Skultety]

These originally allowed customizing the ci/Makefile script which was
the core of the local container executions. The problem was that
however flexible this may have been, it never mirrored what was being
done as part of the GitLab jobs. Motivated by the effort of mirroring
GitLab jobs locally, these would only ever make sense to be set/used in
interactive shell container sessions where the developer is perfectly
capable of using the right meson/ninja CLI options directly without
going through another shell variable indirection as it was the case
with these ci/helper options.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/ci/helper b/ci/helper
index b90dc56ede..c734629731 100755
--- a/ci/helper
+++ b/ci/helper
@@ -75,21 +75,6 @@ class Parser:
 help="path to lcitool (default: $PATH)",
 )
 
-# Options that are common to all actions that call the
-# project's build system
-mesonparser = argparse.ArgumentParser(add_help=False)
-mesonparser.add_argument(
-"--meson-args",
-default="",
-help="additional arguments passed to meson "
- "(eg --meson-args='-Dopt1=enabled -Dopt2=disabled')",
-)
-mesonparser.add_argument(
-"--ninja-args",
-default="",
-help="additional arguments passed to ninja",
-)
-
 # Options that are common to actions communicating with a GitLab
 # instance
 gitlabparser = argparse.ArgumentParser(add_help=False)
-- 
2.41.0

[libvirt PATCH v2 26/35] ci: helper: Add a required_deps higher order helper/decorator

[Erik Skultety]

Since we'll depend on GitPython for repo cloning, we need to make sure
to emit a user friendly error if the module is not installed. This
patch introduces a helper which future patches will use as a decorator.
Inspiration for this helper has been taken out of lcitool where we use
an identical helper for this purpose.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/ci/helper b/ci/helper
index 75552774f6..4727145b28 100755
--- a/ci/helper
+++ b/ci/helper
@@ -14,6 +14,28 @@ import textwrap
 import util
 
 
+def required_deps(*deps):
+module2pkg = {
+"git": "GitPython"
+}
+
+def inner_decorator(func):
+def wrapped(*args, **kwargs):
+cmd = func.__name__[len('_action_'):]
+for dep in deps:
+try:
+import importlib
+importlib.import_module(dep)
+except ImportError:
+pkg = module2pkg[dep]
+msg = f"'{pkg}' not found (required by the '{cmd}' 
command)"
+print(msg, file=sys.stderr)
+sys.exit(1)
+func(*args, **kwargs)
+return wrapped
+return inner_decorator
+
+
 class Parser:
 def __init__(self):
 # Options that are common to all actions that use containers
-- 
2.41.0

[libvirt PATCH v2 22/35] .gitlab-ci.yml: Convert the potfile job to the build.sh usage

[Erik Skultety]

Individual shell command executions are replaced by respective
functions in the ci/build.sh base script. This will make sure we use
the same recipes in GitLab jobs as well as in local executions.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 35f1a1e135..1cdabed941 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -156,9 +156,8 @@ potfile:
   before_script:
 - *script_variables
   script:
-- meson setup build --werror || (cat build/meson-logs/meson-log.txt && 
exit 1)
-- meson compile -C build libvirt-pot-dep
-- meson compile -C build libvirt-pot
+- source ci/jobs.sh
+- run_potfile
   after_script:
 - test "$CI_JOB_STATUS" != "success" && exit 1;
 - cp po/libvirt.pot libvirt.pot
-- 
2.41.0

[libvirt PATCH v2 29/35] ci: helper: Rework _lcitool_run method logic

[Erik Skultety]

This method wasn't even utilized before this patch. This patch adds all
the necessary logic to successfully execute a container workload via
lcitool (which will later allow us to ditch ci/Makefile). Because
container executions via lcitool creates the following inside the
container:

$ ls
script datadir

where 'datadir' is the workload directory (in this case a local git
repo clone) and 'script' is the code that runs whatever the workload is
over 'datadir'.

In order to satisfy the ^above, our helper generates a trivial
temporary 'script' that will source ci/build.sh and run whatever was
specified as --job essentially to simulate the exact steps a GitLab
pipeline job would go through.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 55 +--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/ci/helper b/ci/helper
index 392702ae41..fce370f995 100755
--- a/ci/helper
+++ b/ci/helper
@@ -11,6 +11,9 @@ import subprocess
 import sys
 import textwrap
 
+from pathlib import Path
+from tempfile import TemporaryDirectory
+
 import util
 
 
@@ -201,8 +204,56 @@ class Application:
 return repo.clone(dest, local=True)
 
 def _lcitool_run(self, args):
-output = subprocess.check_output([self._args.lcitool] + args)
-return output.decode("utf-8")
+positional_args = ["container"]
+opts = ["--user", self._args.login]
+tmpdir = TemporaryDirectory(prefix="scratch",
+dir=Path(self.repo.working_dir, "ci"))
+
+repo_dest_path = Path(tmpdir.name, "libvirt.git").as_posix()
+repo_clone = self._prepare_repo_copy(self.repo, repo_dest_path)
+opts.extend(["--workload-dir", repo_clone.working_dir])
+
+if self._args.job == "shell":
+positional_args.append("shell")
+else:
+job2func = {
+"test": "run_test",
+"build": "run_build",
+"codestyle": "run_codestyle",
+"potfile": "run_potfile",
+"rpmbuild": "run_rpmbuild",
+"website": "run_website_build",
+}
+
+if self._args.engine != "auto":
+positional_args.extend(["--engine", self._args.engine])
+
+with open(Path(tmpdir.name, "script"), "w") as f:
+script_path = f.name
+contents = textwrap.dedent(f"""\
+#!/bin/sh
+
+cd datadir
+. ci/jobs.sh
+
+{job2func[self._args.job]}
+""")
+
+f.write(contents)
+
+positional_args.append("run")
+opts.extend(["--script", script_path])
+
+
opts.append(f"{self._args.image_prefix}{self._args.target}:{self._args.image_tag}")
+proc = None
+try:
+proc = subprocess.run([self._args.lcitool] + positional_args + 
opts)
+except KeyboardInterrupt:
+sys.exit(1)
+finally:
+# this will take care of the generated script file above as well
+tmpdir.cleanup()
+return proc.returncode
 
 def _check_stale_images(self):
 namespace = self._args.namespace
-- 
2.41.0

[libvirt PATCH v2 23/35] ci: helper: Drop _lcitool_get_targets method

[Erik Skultety]

This method unused anywhere, so drop it.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 4 
 1 file changed, 4 deletions(-)

diff --git a/ci/helper b/ci/helper
index fb562d55e1..8986772153 100755
--- a/ci/helper
+++ b/ci/helper
@@ -163,10 +163,6 @@ class Application:
 output = subprocess.check_output([self._args.lcitool] + args)
 return output.decode("utf-8")
 
-def _lcitool_get_targets(self):
-output = self._lcitool_run(["targets"])
-return output.splitlines()
-
 def _check_stale_images(self):
 namespace = self._args.namespace
 gitlab_uri = self._args.gitlab_uri
-- 
2.41.0

[libvirt PATCH v2 06/35] ci: build.sh: Add a wrapper function over the 'build' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 477ccbc7d1..f908bbc5d4 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -37,3 +37,8 @@ run_meson_setup() {
 run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS || 
\
 (cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1)
 }
+
+run_build() {
+test -d $GIT_ROOT/build/build.ninja || run_meson_setup
+run_cmd meson compile -C build $BUILD_ARGS
+}
-- 
2.41.0

[libvirt PATCH v2 17/35] .gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection

[Erik Skultety]

This is one of the preparation steps that if not done would otherwise
collide with local container executions where we:
1) don't collect artifacts
2) are not limited by GitLab's environment and hence moving build
   artifacts to unusual places would only cause confusion when doing
   local build inspection in an interactive container shell session

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 944a7b7817..1c6af8f8b3 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -31,11 +31,16 @@ include:
 - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip";
   then
 rpmbuild --clean --nodeps --define "_without_mingw 1" -ta 
build/meson-dist/libvirt-*.tar.xz;
-mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/;
   else
 meson compile -C build;
 meson test -C build --no-suite syntax-check --print-errorlogs;
   fi
+  after_script:
+- test "$CI_JOB_STATUS" != "success" && exit 1;
+- if test -x /usr/bin/rpmbuild && test "$RPM" != "skip";
+  then
+mv "$HOME"/rpmbuild/RPMS/x86_64/ libvirt-rpms/;
+  fi
 
 .native_build_job_prebuilt_env:
   extends:
@@ -77,6 +82,8 @@ include:
 - *script_variables
 - meson setup build --werror -Dsystem=true || (cat 
build/meson-logs/meson-log.txt && exit 1)
 - DESTDIR=$(pwd)/install meson compile -C build install-web
+  after_script:
+- test "$CI_JOB_STATUS" != "success" && exit 1;
 - mv install/usr/share/doc/libvirt/html/ website
   artifacts:
 expose_as: 'Website'
@@ -155,6 +162,8 @@ potfile:
 - meson setup build --werror || (cat build/meson-logs/meson-log.txt && 
exit 1)
 - meson compile -C build libvirt-pot-dep
 - meson compile -C build libvirt-pot
+  after_script:
+- test "$CI_JOB_STATUS" != "success" && exit 1;
 - cp po/libvirt.pot libvirt.pot
   artifacts:
 expose_as: 'Potfile'
-- 
2.41.0

[libvirt PATCH v2 10/35] ci: build.sh: Add a wrapper function over the 'potfile' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart. There's one notable difference such that we pass '-j1' to
the meson compile command otherwise we'd have to execute the 'run_build'
function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets
in a serial manner.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index e6c3225691..d6361f3ade 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -65,3 +65,14 @@ run_codestyle() {
 run_build
 run_test
 }
+
+run_potfile() {
+# since meson would run jobs for each of the following target in parallel,
+# we'd have dependency issues such that one target might depend on a
+# generated file which hasn't been generated yet by the other target, hence
+# we limit potfile job to a single build job (luckily potfile build has
+# negligible performance impact)
+BUILD_ARGS="-j1 libvirt-pot-dep libvirt-pot"
+
+run_build
+}
-- 
2.41.0

[libvirt PATCH v2 07/35] ci: build.sh: Add a helper function to create the dist tarball

[Erik Skultety]

This helper function does not correspond to a particular GitLab job, it
just logically separates the necessary step of creating a dist tarball
from the RPM build job that takes over.
One notable change here is the need to update git's file index which
causes issues in local container executions which rely on a shallow
copy of the libvirt repo created as:

$ git clone --local

Even if all changes have been committed, git often complained
otherwise. Updating the index in a GitLab environment is a NOP.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index f908bbc5d4..ab56c5e5eb 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -42,3 +42,13 @@ run_build() {
 test -d $GIT_ROOT/build/build.ninja || run_meson_setup
 run_cmd meson compile -C build $BUILD_ARGS
 }
+
+run_dist() {
+test -d $GIT_ROOT/build/build.ninja || run_meson_setup
+
+# dist is unhappy in local container environment complaining about
+# uncommitted changes in the repo which is often not the case - refreshing
+# git's index solves the problem
+git update-index --refresh
+run_cmd meson dist -C build --no-tests
+}
-- 
2.41.0

[libvirt PATCH v2 15/35] ci: build.sh: Drop MESON_ARGS definition from global level

[Erik Skultety]

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/build.sh | 10 --
 1 file changed, 10 deletions(-)

diff --git a/ci/build.sh b/ci/build.sh
index ac649ed9a9..96ee289c4f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -13,16 +13,6 @@ if ! [ -t 1 ]; then
 fi
 
 GIT_ROOT="$(git rev-parse --show-toplevel)"
-
-# $MESON_OPTS is an env that can optionally be set in the container,
-# populated at build time from the Dockerfile. A typical use case would
-# be to pass options to trigger cross-compilation
-#
-# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's
-# populated from a GitLab's job configuration
-
-MESON_ARGS="$MESON_ARGS $MESON_OPTS"
-
 run_cmd() {
 printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*"
 $@
-- 
2.41.0

[libvirt PATCH v2 31/35] ci: helper: Add a job argparse subparser

[Erik Skultety]

The idea behind this subcommand is to follow whatever build job we have
defined in the GitLab CI pipeline, so that we only have a single source
of truth for the recipes. Adds 'shell' as an extra option for
interactive container build debugging.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/ci/helper b/ci/helper
index f7b0204ea0..bc5de008b2 100755
--- a/ci/helper
+++ b/ci/helper
@@ -139,6 +139,21 @@ class Parser:
 )
 shellparser.set_defaults(func=Application._action_shell)
 
+jobparser = subparsers.add_parser(
+"run",
+help="Run a GitLab CI job or 'shell' in a local environment",
+parents=[containerparser],
+formatter_class=argparse.ArgumentDefaultsHelpFormatter,
+)
+jobparser.add_argument(
+"--job",
+choices=["build", "codestyle", "potfile", "rpmbuild",
+ "shell", "test", "website"],
+default="build",
+help="Run a GitLab CI job or 'shell' in a local environment",
+)
+jobparser.set_defaults(func=Application._action_run)
+
 # list-images action
 listimagesparser = subparsers.add_parser(
 "list-images",
-- 
2.41.0

[libvirt PATCH v2 35/35] ci: Drop the now unused Makefile

[Erik Skultety]

All the functionality this script provided has been incorporated either
in the Python ci/helper tool or lcitool directly.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/Makefile | 245 
 1 file changed, 245 deletions(-)
 delete mode 100644 ci/Makefile

diff --git a/ci/Makefile b/ci/Makefile
deleted file mode 100644
index 8f1be4318d..00
--- a/ci/Makefile
+++ /dev/null
@@ -1,245 +0,0 @@
-# -*- makefile -*-
-# vim: filetype=make
-
-# The root directory of the libvirt.git checkout
-CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
-
-# The root directory for all CI-related contents
-CI_ROOTDIR = $(CI_GIT_ROOT)/ci
-
-# The directory holding content on the host that we will
-# expose to the container.
-CI_SCRATCHDIR = $(CI_ROOTDIR)/scratch
-
-# The directory holding the clone of the git repo that
-# we will expose to the container
-CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
-
-# The directory holding the source inside the
-# container, i.e. where we want to expose
-# the $(CI_HOST_SRCDIR) directory from the host
-CI_CONT_SRCDIR = $(CI_USER_HOME)/libvirt
-
-# Script containing build instructions
-CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh
-
-# Location of the container images we're going to pull
-# Can be useful to override to use a locally built
-# image instead
-CI_IMAGE_PREFIX = registry.gitlab.com/libvirt/libvirt/ci-
-
-# The default tag is ':latest' but if the container
-# repo above uses different conventions this can override it
-CI_IMAGE_TAG = :latest
-
-# We delete the virtual root after completion, set
-# to 0 if you need to keep it around for debugging
-CI_CLEAN = 1
-
-# We'll always freshly clone the virtual root each
-# time in case it was not cleaned up before. Set
-# to 1 if you want to try restarting a previously
-# preserved env
-CI_REUSE = 0
-
-# We need the user's login and home directory to prepare the
-# environment the way some programs expect it
-CI_USER_LOGIN = $(shell whoami)
-CI_USER_HOME = $(shell eval echo "~$(CI_USER_LOGIN)")
-
-# We also need the container process to run with current host IDs
-# so that it can access the passed in build directory
-CI_UID = $(shell id -u "$(CI_USER_LOGIN)")
-CI_GID = $(shell id -g "$(CI_USER_LOGIN)")
-
-CI_ENGINE = auto
-# Container engine we are going to use, can be overridden per make
-# invocation, if it is not we try podman and then default to docker.
-ifeq ($(CI_ENGINE),auto)
-   override CI_ENGINE = $(shell podman version >/dev/null 2>&1 && echo 
podman || echo docker)
-endif
-
-# IDs you run as do not need to exist in
-# the container's /etc/passwd & /etc/group files, but
-# if they do not, then libvirt's 'ninja test' will fail
-# many tests.
-
-# We do not directly mount /etc/{passwd,group} as Docker
-# is liable to mess with SELinux labelling which will
-# then prevent the host accessing them. And podman cannot
-# relabel the files due to it running rootless. So
-# copying them first is safer and less error-prone.
-CI_PWDB_MOUNTS = \
-   --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
-   --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
-   $(NULL)
-
-CI_HOME_MOUNTS = \
-   --volume $(CI_SCRATCHDIR)/home:$(CI_USER_HOME):z \
-   $(NULL)
-
-CI_SCRIPT_MOUNTS = \
-   --volume $(CI_SCRATCHDIR)/build:$(CI_USER_HOME)/build:z \
-   $(NULL)
-
-# Docker containers can have very large ulimits
-# for nofiles - as much as 1048576. This makes
-# libvirt very slow at exec'ing programs.
-CI_ULIMIT_FILES = 1024
-
-ifeq ($(CI_ENGINE),podman)
-   # Podman cannot reuse host namespace when running non-root
-   # containers.  Until support for --keep-uid is added we can
-   # just create another mapping that will do that for us.
-   # Beware, that in {uid,git}map=container_id:host_id:range, the
-   # host_id does actually refer to the uid in the first mapping
-   # where 0 (root) is mapped to the current user and rest is
-   # offset.
-   #
-   # In order to set up this mapping, we need to keep all the
-   # user IDs to prevent possible errors as some images might
-   # expect UIDs up to 9 (looking at you fedora), so we don't
-   # want the overflowuid to be used for them.  For mapping all
-   # the other users properly, some math needs to be done.
-   # Don't worry, it's just addition and subtraction.
-   #
-   # 65536 ought to be enough (tm), but for really rare cases the
-   # maximums might need to be higher, but that only happens when
-   # your /etc/sub{u,g}id allow users to have more IDs.  Unless
-   # --keep-uid is supported, let's do this in a way that should
-   # work for everyone.
-   CI_MAX_UID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" 
/etc/subuid)
-   CI_MAX_GID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" 
/etc/subgid)
-   ifeq ($(CI_MAX_UID),)
-   CI_MAX_UID = 65536
-   endif
-   ifeq 

[libvirt PATCH v2 34/35] ci: helper: Drop the _make_run method

[Erik Skultety]

We've successfully migrated over to lcitool to take care of the
container workload execution, so dropping this 'make' prep code is a
prerequisite of finally getting rid of the ci/Makefile script.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 25 -
 1 file changed, 25 deletions(-)

diff --git a/ci/helper b/ci/helper
index c734629731..93508515ca 100755
--- a/ci/helper
+++ b/ci/helper
@@ -6,7 +6,6 @@
 import argparse
 import os
 import pathlib
-import pty
 import subprocess
 import sys
 import textwrap
@@ -148,30 +147,6 @@ class Application:
 self._args = Parser().parse()
 self._repo = None
 
-def _make_run(self, target):
-args = [
-"-C",
-self._basedir,
-target,
-]
-
-if self._args.action in ["build", "test", "shell"]:
-args.extend([
-f"CI_ENGINE={self._args.engine}",
-f"CI_USER_LOGIN={self._args.login}",
-f"CI_IMAGE_PREFIX={self._args.image_prefix}",
-f"CI_IMAGE_TAG={self._args.image_tag}",
-])
-
-if self._args.action in ["build", "test"]:
-args.extend([
-f"MESON_ARGS={self._args.meson_args}",
-f"NINJA_ARGS={self._args.ninja_args}",
-])
-
-if pty.spawn(["make"] + args) != 0:
-sys.exit("error: 'make' failed")
-
 @staticmethod
 def _prepare_repo_copy(repo, dest):
 return repo.clone(dest, local=True)
-- 
2.41.0

[libvirt PATCH v2 32/35] ci: helper: Drop original actions

[Erik Skultety]

Previous patches added a single 'run' command parametrized with GitLab
job specs via '--job' that cover all of these original actions, adding
some more in the process. Drop the original actions as we don't need
them anymore.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 36 
 1 file changed, 36 deletions(-)

diff --git a/ci/helper b/ci/helper
index bc5de008b2..b90dc56ede 100755
--- a/ci/helper
+++ b/ci/helper
@@ -112,33 +112,6 @@ class Parser:
 )
 subparsers.required = True
 
-# build action
-buildparser = subparsers.add_parser(
-"build",
-help="run a build in a container",
-parents=[containerparser, mesonparser],
-formatter_class=argparse.ArgumentDefaultsHelpFormatter,
-)
-buildparser.set_defaults(func=Application._action_build)
-
-# test action
-testparser = subparsers.add_parser(
-"test",
-help="run a build in a container (including tests)",
-parents=[containerparser, mesonparser],
-formatter_class=argparse.ArgumentDefaultsHelpFormatter,
-)
-testparser.set_defaults(func=Application._action_test)
-
-# shell action
-shellparser = subparsers.add_parser(
-"shell",
-help="start a shell in a container",
-parents=[containerparser],
-formatter_class=argparse.ArgumentDefaultsHelpFormatter,
-)
-shellparser.set_defaults(func=Application._action_shell)
-
 jobparser = subparsers.add_parser(
 "run",
 help="Run a GitLab CI job or 'shell' in a local environment",
@@ -301,15 +274,6 @@ class Application:
 """)
 print(msg.replace("STALE_DETAILS", stale_details))
 
-def _action_build(self):
-self._make_run(f"ci-build@{self._args.target}")
-
-def _action_test(self):
-self._make_run(f"ci-test@{self._args.target}")
-
-def _action_shell(self):
-self._make_run(f"ci-shell@{self._args.target}")
-
 @required_deps("git")
 def _action_run(self):
 return self._lcitool_run(self._args.job)
-- 
2.41.0

[libvirt PATCH v2 24/35] ci: helper: Don't make ':' literal a static part of the image tag

[Erik Skultety]

':' is just a connecting character, we can add it to the appropriate
place later in the Python script later, but it doesn't make sense to be
part of the image 'tag' string.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/helper b/ci/helper
index 8986772153..d3de6d96cb 100755
--- a/ci/helper
+++ b/ci/helper
@@ -40,7 +40,7 @@ class Parser:
 )
 containerparser.add_argument(
 "--image-tag",
-default=":latest",
+default="latest",
 help="use container images with non-default tags",
 )
 
-- 
2.41.0

[libvirt PATCH v2 30/35] ci: helper: Add an action to run the container workload via lcitool

[Erik Skultety]

Just like with the other CLI sub-commands, add an action to run a
GitLab spec job in a local container via lcitool.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 4 
 1 file changed, 4 insertions(+)

diff --git a/ci/helper b/ci/helper
index fce370f995..f7b0204ea0 100755
--- a/ci/helper
+++ b/ci/helper
@@ -295,6 +295,10 @@ class Application:
 def _action_shell(self):
 self._make_run(f"ci-shell@{self._args.target}")
 
+@required_deps("git")
+def _action_run(self):
+return self._lcitool_run(self._args.job)
+
 def _action_list_images(self):
 registry_uri = util.get_registry_uri(self._args.namespace,
  self._args.gitlab_uri)
-- 
2.41.0

[libvirt PATCH v2 08/35] ci: build.sh: Add a wrapper function over the 'test' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index ab56c5e5eb..29b6a3306d 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -52,3 +52,8 @@ run_dist() {
 git update-index --refresh
 run_cmd meson dist -C build --no-tests
 }
+
+run_test() {
+test -d $GIT_ROOT/build/build.ninja || run_meson_setup
+run_cmd meson test -C build $TEST_ARGS
+}
-- 
2.41.0

[libvirt PATCH v2 21/35] .gitlab-ci.yml: Convert the codestyle job to the build.sh usage

[Erik Skultety]

Individual shell command executions are replaced by respective
functions in the ci/build.sh base script. This will make sure we use
the same recipes in GitLab jobs as well as in local executions.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d41cc63ec2..35f1a1e135 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -114,10 +114,8 @@ website_local_env:
 .codestyle_job:
   stage: sanity_checks
   script:
-- *script_variables
-- meson setup build --werror || (cat build/meson-logs/meson-log.txt && 
exit 1)
-- meson compile -C build libvirt-pot-dep
-- meson test -C build --suite syntax-check --no-rebuild --print-errorlogs
+- source ci/jobs.sh
+- run_codestyle
 
 codestyle_prebuilt_env:
   extends:
-- 
2.41.0

[libvirt PATCH v2 28/35] ci: helper: Add a helper to create a local repo clone Pythonic way

[Erik Skultety]

A proper Python equivalent of 'git clone --local'.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 4 
 1 file changed, 4 insertions(+)

diff --git a/ci/helper b/ci/helper
index 6aca089db4..392702ae41 100755
--- a/ci/helper
+++ b/ci/helper
@@ -196,6 +196,10 @@ class Application:
 if pty.spawn(["make"] + args) != 0:
 sys.exit("error: 'make' failed")
 
+@staticmethod
+def _prepare_repo_copy(repo, dest):
+return repo.clone(dest, local=True)
+
 def _lcitool_run(self, args):
 output = subprocess.check_output([self._args.lcitool] + args)
 return output.decode("utf-8")
-- 
2.41.0

[libvirt PATCH v2 14/35] ci: build.sh: Drop direct invocation of meson/ninja commands

[Erik Skultety]

We've moved all invocations to the respective helper function which
we'll execute both from gitlab CI jobs and local environments so we
don't need to have them on the global level as it would also not work
with "sourcing" this file to populate the environment with function
definitions.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ci/build.sh b/ci/build.sh
index fd326dad8d..ac649ed9a9 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -21,10 +21,7 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
 # $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's
 # populated from a GitLab's job configuration
 
-meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \
-(cat build/meson-logs/meson-log.txt && exit 1)
-
-ninja -C build $NINJA_ARGS
+MESON_ARGS="$MESON_ARGS $MESON_OPTS"
 
 run_cmd() {
 printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*"
-- 
2.41.0

[libvirt PATCH v2 27/35] ci: helper: Add Python code hangling git clones

[Erik Skultety]

This helper will be utilized by a future patch which will add the
lcitool container execution logic. The reason why the required_deps
decorator isn't being used here is because this is a property.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 9 +
 1 file changed, 9 insertions(+)

diff --git a/ci/helper b/ci/helper
index 4727145b28..6aca089db4 100755
--- a/ci/helper
+++ b/ci/helper
@@ -159,9 +159,18 @@ class Parser:
 
 
 class Application:
+@property
+def repo(self):
+if self._repo is None:
+from git import Repo
+
+self._repo = Repo(search_parent_directories=True)
+return self._repo
+
 def __init__(self):
 self._basedir = pathlib.Path(__file__).resolve().parent
 self._args = Parser().parse()
+self._repo = None
 
 def _make_run(self, target):
 args = [
-- 
2.41.0

[libvirt PATCH v2 25/35] ci: helper: Add --lcitool-path CLI option

[Erik Skultety]

We'll soon be relying solely on lcitool so we need to be able to run it
from a user-provided location if it's not installed in a known
location.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/helper | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ci/helper b/ci/helper
index d3de6d96cb..75552774f6 100755
--- a/ci/helper
+++ b/ci/helper
@@ -43,6 +43,12 @@ class Parser:
 default="latest",
 help="use container images with non-default tags",
 )
+containerparser.add_argument(
+"--lcitool-path",
+dest="lcitool",
+default="lcitool",
+help="path to lcitool (default: $PATH)",
+)
 
 # Options that are common to all actions that call the
 # project's build system
-- 
2.41.0

[libvirt PATCH v2 20/35] .gitlab-ci.yml: Convert the website build job to the build.sh usage

[Erik Skultety]

Individual shell command executions are replaced by respective
functions in the ci/build.sh base script. This will make sure we use
the same recipes in GitLab jobs as well as in local executions.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f4c5b47f15..d41cc63ec2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -79,9 +79,8 @@ include:
 #
https://gitlab.com/libvirt/libvirt/-/jobs/artifacts/master/download?job=website
 .website_job:
   script:
-- *script_variables
-- meson setup build --werror -Dsystem=true || (cat 
build/meson-logs/meson-log.txt && exit 1)
-- DESTDIR=$(pwd)/install meson compile -C build install-web
+- source ci/jobs.sh
+- run_website_build
   after_script:
 - test "$CI_JOB_STATUS" != "success" && exit 1;
 - mv install/usr/share/doc/libvirt/html/ website
-- 
2.41.0

[libvirt PATCH v2 05/35] ci: build.sh: Add a wrapper function over meson's setup

[Erik Skultety]

The reason for this wrapper is that all job functions introduced in
future patches will refer to this one instead of open-coding the same
'meson setup' invocation N times. It also prevents 'setup' to be called
multiple times as some future job functions might actually do just that
in a transitive manner.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 5883542b45..477ccbc7d1 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -32,3 +32,8 @@ run_cmd() {
 printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*"
 $@
 }
+
+run_meson_setup() {
+run_cmd meson setup build --error -Dsystem=true $MESON_OPTS $MESON_ARGS || 
\
+(cat "${GIT_ROOT}/build/meson-logs/meson-log.txt" && exit 1)
+}
-- 
2.41.0

[libvirt PATCH v2 18/35] .gitlab-ci.yml: Convert the native build job to the build.sh usage

[Erik Skultety]

Individual shell command executions are replaced by respective
functions in the ci/build.sh base script. This will make sure we use
the same recipes in GitLab jobs as well as in local executions.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 1c6af8f8b3..c837812091 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -25,15 +25,13 @@ include:
   - ccache/
 key: "$CI_JOB_NAME"
   script:
-- *script_variables
-- meson setup build --werror $MESON_ARGS || (cat 
build/meson-logs/meson-log.txt && exit 1)
-- meson dist -C build --no-tests
+- source ci/jobs.sh
 - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip";
   then
-rpmbuild --clean --nodeps --define "_without_mingw 1" -ta 
build/meson-dist/libvirt-*.tar.xz;
+run_rpmbuild;
   else
-meson compile -C build;
-meson test -C build --no-suite syntax-check --print-errorlogs;
+run_build;
+run_test;
   fi
   after_script:
 - test "$CI_JOB_STATUS" != "success" && exit 1;
-- 
2.41.0

[libvirt PATCH v2 12/35] ci: build.sh: Add a wrapper function over the 'website' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index c558b4c9ca..8e1619d483 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -85,3 +85,10 @@ run_rpmbuild() {
 --define '_without_mingw 1' \
 -ta build/meson-dist/libvirt-*.tar.xz
 }
+
+run_website_build() {
+export DESTDIR="${GIT_ROOT}/install"
+BUILD_ARGS="install-web"
+
+run_build
+}
-- 
2.41.0

[libvirt PATCH v2 09/35] ci: build.sh: Add a wrapper function over the 'codestyle' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 29b6a3306d..e6c3225691 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -57,3 +57,11 @@ run_test() {
 test -d $GIT_ROOT/build/build.ninja || run_meson_setup
 run_cmd meson test -C build $TEST_ARGS
 }
+
+run_codestyle() {
+BUILD_ARGS="libvirt-pot-dep"
+TEST_ARGS="--suite syntax-check --no-rebuild --print-errorlogs"
+
+run_build
+run_test
+}
-- 
2.41.0

[libvirt PATCH v2 13/35] ci: build.sh: Drop changing working directory to CI_CONT_DIR

[Erik Skultety]

Firstly, this would mangle with "sourcing" this file in either
execution environment later down the road. Secondly, we won't need this
as future ci/helper patches will generate a throwaway script that will
take care of a correct execution of a build job in a similar fashion as
if the job ran in a GitLab environment.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ci/build.sh b/ci/build.sh
index 8e1619d483..fd326dad8d 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -1,7 +1,5 @@
 #!/bin/sh
 
-cd "$CI_CONT_SRCDIR"
-
 export CCACHE_BASEDIR="$(pwd)"
 export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
 export CCACHE_MAXSIZE="500M"
-- 
2.41.0

[libvirt PATCH v2 19/35] .gitlab-ci.yml: Convert the cross build job to the build.sh usage

[Erik Skultety]

Individual shell command executions are replaced by respective
functions in the ci/build.sh base script. This will make sure we use
the same recipes in GitLab jobs as well as in local executions.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 .gitlab-ci.yml | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c837812091..f4c5b47f15 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -56,10 +56,12 @@ include:
   - ccache/
 key: "$CI_JOB_NAME"
   script:
-- *script_variables
-- meson setup build --werror $MESON_OPTS || (cat 
build/meson-logs/meson-log.txt && exit 1)
-- meson compile -C build
-- if test "$CROSS" = "i686" ; then meson test -C build --no-suite 
syntax-check --print-errorlogs ; fi
+- source ci/jobs.sh
+- run_build
+- if test "$CROSS" = "i686" ;
+  then
+run_test;
+  fi
 
 .cross_build_job_prebuilt_env:
   extends:
-- 
2.41.0

[libvirt PATCH v2 04/35] ci: build.sh: Add a wrapper function executing 'shell' commands

[Erik Skultety]

This would normally be not needed at all, but the problem here is the
Shell-in-YAML which GitLab interprets. It outputs every command that
appears as a line in the 'script' segment in a color-coded fashion for
easy identification of problems. Well, that useful feature is lost when
there's indirection and one script calls into another in which case it
would only output the respective script name which would make failure
investigation harder. This simple helper tackles that by echoing the
command to be run by any script/function with a color escape sequence
so that we don't lose track of the *actual* shell commands being run as
part of the GitLab job pipelines. An example of what the output then
might look like:
[RUN COMMAND]: 'meson compile -C build install-web'

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 7cf07ba5a8..5883542b45 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -27,3 +27,8 @@ meson setup build --werror -Dsystem=true $MESON_OPTS 
$MESON_ARGS || \
 (cat build/meson-logs/meson-log.txt && exit 1)
 
 ninja -C build $NINJA_ARGS
+
+run_cmd() {
+printf "\e[32m[RUN COMMAND]: '%s'\e[0m\n" "$*"
+$@
+}
-- 
2.41.0

[libvirt PATCH v2 11/35] ci: build.sh: Add a wrapper function over the 'rpmbuild' job

[Erik Skultety]

This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index d6361f3ade..c558b4c9ca 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -76,3 +76,12 @@ run_potfile() {
 
 run_build
 }
+
+run_rpmbuild() {
+run_dist
+run_cmd rpmbuild \
+--clean \
+--nodeps \
+--define '_without_mingw 1' \
+-ta build/meson-dist/libvirt-*.tar.xz
+}
-- 
2.41.0

[libvirt PATCH v2 16/35] ci: Rename build.sh -> jobs.sh

[Erik Skultety]

After the recent changes, this script no longer executes any logic
anymore, it merely defines the jobs run in the GitLab environment. In
order to use it, one has to source the file in the environment and then
run one of the job "functions". For that, the 'build.sh' name is no
longer descriptive enough and 'jobs.sh' feels more suitable and less
misleading.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/{build.sh => jobs.sh} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename ci/{build.sh => jobs.sh} (100%)

diff --git a/ci/build.sh b/ci/jobs.sh
similarity index 100%
rename from ci/build.sh
rename to ci/jobs.sh
-- 
2.41.0

[libvirt PATCH v2 03/35] ci: build.sh: Don't mention that MESON_ARGS are available via CLI

[Erik Skultety]

Previous patches have removed the code that allowed injecting arbitrary
meson arguments, same for ninja args.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/build.sh | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/ci/build.sh b/ci/build.sh
index 0549b01ca0..7cf07ba5a8 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -21,13 +21,7 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
 # be to pass options to trigger cross-compilation
 #
 # $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-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
+# populated from a GitLab's job configuration
 
 meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \
 (cat build/meson-logs/meson-log.txt && exit 1)
-- 
2.41.0

[libvirt PATCH v2 01/35] ci: build.sh: Add variables from .gitlab-ci.yml

[Erik Skultety]

These are common variables we wish to use in containerized environments
both in GitLab and locally. Having these defined in a single place
rather than twice is highly preferable.

Signed-off-by: Erik Skultety 
Erik Skultety :
---
 ci/build.sh | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/ci/build.sh b/ci/build.sh
index d5ed8ad104..5a9298c4b4 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -2,7 +2,17 @@
 
 cd "$CI_CONT_SRCDIR"
 
-export VIR_TEST_DEBUG=1
+export CCACHE_BASEDIR="$(pwd)"
+export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
+export CCACHE_MAXSIZE="500M"
+export PATH="$CCACHE_WRAPPERSDIR:$PATH"
+
+# Enable these conditionally since their best use case is during
+# non-interactive workloads without having a Shell
+if ! [ -t 1 ]; then
+export VIR_TEST_VERBOSE="1"
+export VIR_TEST_DEBUG="1"
+fi
 
 # $MESON_OPTS is an env that can optionally be set in the container,
 # populated at build time from the Dockerfile. A typical use case would
-- 
2.41.0

[libvirt PATCH v2 00/35] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions

[Erik Skultety]

since v2 [1]:
- moved the custom meson.build directory check before running setup to
  individual callers for better robustness, but actually let meson deal with an
  existing setup directory most of the time
- drops the ci/Makefile
- only set our verbose env variables when running without a TTY
- switched the bright yellow (93m) color to green (32m)
- added KeyboardInterrupt handling in the ci/helper Python code

The changes above require the following lcitool MR to be merged first as
they're built on top of it [2]

[1] https://listman.redhat.com/archives/libvir-list/2023-August/241471.html
[2] https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/431

Erik Skultety (35):
  ci: build.sh: Add variables from .gitlab-ci.yml
  ci: build.sh: Add GIT_ROOT env helper variable
  ci: build.sh: Don't mention that MESON_ARGS are available via CLI
  ci: build.sh: Add a wrapper function executing 'shell' commands
  ci: build.sh: Add a wrapper function over meson's setup
  ci: build.sh: Add a wrapper function over the 'build' job
  ci: build.sh: Add a helper function to create the dist tarball
  ci: build.sh: Add a wrapper function over the 'test' job
  ci: build.sh: Add a wrapper function over the 'codestyle' job
  ci: build.sh: Add a wrapper function over the 'potfile' job
  ci: build.sh: Add a wrapper function over the 'rpmbuild' job
  ci: build.sh: Add a wrapper function over the 'website' job
  ci: build.sh: Drop changing working directory to CI_CONT_DIR
  ci: build.sh: Drop direct invocation of meson/ninja commands
  ci: build.sh: Drop MESON_ARGS definition from global level
  ci: Rename build.sh -> jobs.sh
  .gitlab-ci.yml: Add 'after_script' stage to prep for artifact
collection
  .gitlab-ci.yml: Convert the native build job to the build.sh usage
  .gitlab-ci.yml: Convert the cross build job to the build.sh usage
  .gitlab-ci.yml: Convert the website build job to the build.sh usage
  .gitlab-ci.yml: Convert the codestyle job to the build.sh usage
  .gitlab-ci.yml: Convert the potfile job to the build.sh usage
  ci: helper: Drop _lcitool_get_targets method
  ci: helper: Don't make ':' literal a static part of the image tag
  ci: helper: Add --lcitool-path CLI option
  ci: helper: Add a required_deps higher order helper/decorator
  ci: helper: Add Python code hangling git clones
  ci: helper: Add a helper to create a local repo clone Pythonic way
  ci: helper: Rework _lcitool_run method logic
  ci: helper: Add an action to run the container workload via lcitool
  ci: helper: Add a job argparse subparser
  ci: helper: Drop original actions
  ci: helper: Drop the --meson-args/--ninja-args CLI options
  ci: helper: Drop the _make_run method
  ci: Drop the now unused Makefile

 .gitlab-ci.yml |  47 +-
 ci/Makefile| 245 -
 ci/build.sh|  23 -
 ci/helper  | 181 +---
 ci/jobs.sh |  79 
 5 files changed, 211 insertions(+), 364 deletions(-)
 delete mode 100644 ci/Makefile
 delete mode 100644 ci/build.sh
 create mode 100644 ci/jobs.sh

-- 
2.41.0

[libvirt PATCH v2 02/35] ci: build.sh: Add GIT_ROOT env helper variable

[Erik Skultety]

We'll use this one in many of the job functions future patches will
introduce, it's a neat shortcut to avoid using relative paths.

Signed-off-by: Erik Skultety 
Reviewed-by: Daniel P. Berrangé 
Erik Skultety :
---
 ci/build.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 5a9298c4b4..0549b01ca0 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -14,6 +14,8 @@ if ! [ -t 1 ]; then
 export VIR_TEST_DEBUG="1"
 fi
 
+GIT_ROOT="$(git rev-parse --show-toplevel)"
+
 # $MESON_OPTS is an env that can optionally be set in the container,
 # populated at build time from the Dockerfile. A typical use case would
 # be to pass options to trigger cross-compilation
-- 
2.41.0

Re: [PATCH 0/2] Check for capng_*() retvals

[Martin Kletzander]

On Mon, Sep 11, 2023 at 12:18:28PM +0200, Michal Privoznik wrote:

There's a commit inside of (yet-unreleased) libcap-ng which marks some
functions as 'warned unused result'. Fedora rawhide already picked up
the commit, but since we are not checking for all retvals we got a build
failure on rawhide.

https://src.fedoraproject.org/rpms/libcap-ng/c/fed9b23c8d0020e07c937a3ac0d6dcc4534715fb?branch=rawhide

Green pipeline:

https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/999435445

Michal Prívozník (2):
 lxc_container: Check retval of capng_get_caps_process()
 virutil: Check retval of capng_apply()


Reviewed-by: Martin Kletzander 

Checking return values is nicer anyway.



src/lxc/lxc_container.c | 8 +++-
src/util/virutil.c  | 8 ++--
2 files changed, 13 insertions(+), 3 deletions(-)

--
2.41.0



signature.asc
Description: PGP signature

Re: [PATCH 0/2] Check for capng_*() retvals

[Ján Tomko]

On a Monday in 2023, Michal Privoznik wrote:

There's a commit inside of (yet-unreleased) libcap-ng which marks some
functions as 'warned unused result'. Fedora rawhide already picked up
the commit, but since we are not checking for all retvals we got a build
failure on rawhide.

https://src.fedoraproject.org/rpms/libcap-ng/c/fed9b23c8d0020e07c937a3ac0d6dcc4534715fb?branch=rawhide

Green pipeline:

https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/999435445

Michal Prívozník (2):
 lxc_container: Check retval of capng_get_caps_process()
 virutil: Check retval of capng_apply()

src/lxc/lxc_container.c | 8 +++-
src/util/virutil.c  | 8 ++--
2 files changed, 13 insertions(+), 3 deletions(-)



Please add a separating ':' between "capabilities" and the number,
as we do elsewhere when printing capng's error codes.

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[PATCH 2/2] virutil: Check retval of capng_apply()

[Michal Privoznik]

Inside of virSetUIDGIDWithCaps() there's a naked call to
capng_apply(), i.e. without any retval check. This is potentially
dangerous as capng_apply() may fail. Do the check and report an
error.

This also fixes the build on bleeding edge distros - like Fedora
rawhide - where the function is declared with 'warn unused
result' [1].

1: 
https://github.com/stevegrubb/libcap-ng/commit/a0743c335c9a16a2fda9b25120a5523742119e47

Signed-off-by: Michal Privoznik 
---
 src/util/virutil.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index b5b65fb415..edc39b981f 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1200,8 +1200,12 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t 
*groups, int ngroups,
  * do this if we failed to get the capability above, so ignore the
  * return value.
  */
-if (!need_setpcap)
-capng_apply(CAPNG_SELECT_BOUNDS);
+if (!need_setpcap &&
+(capng_ret = capng_apply(CAPNG_SELECT_BOUNDS)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot apply process capabilities %1$d"), capng_ret);
+return -1;
+}
 
 /* Drop the caps that allow setuid/gid (unless they were requested) */
 if (need_setgid)
-- 
2.41.0

[PATCH 1/2] lxc_container: Check retval of capng_get_caps_process()

[Michal Privoznik]

Added in v0.6.5~14 the call to capng_get_caps_process() inside of
lxcContainerDropCapabilities() is not really explained in the
commit message. But looking into the libcap-ng sources it's to
initialize the internal state of the library.

But with recent libcap-ng commit [1] (which some bleeding edge
distros - like Fedora rawhide - already picked up) the function
has been marked as 'warn unused result'. Well, check for its
retval then.

1: 
https://github.com/stevegrubb/libcap-ng/commit/a0743c335c9a16a2fda9b25120a5523742119e47

Signed-off-by: Michal Privoznik 
---
 src/lxc/lxc_container.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 21220661f7..4c37fcd012 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1725,7 +1725,13 @@ static int lxcContainerDropCapabilities(virDomainDef 
*def,
 CAP_SYSLOG,
 CAP_WAKE_ALARM};
 
-capng_get_caps_process();
+/* Init the internal state of capng */
+if ((ret = capng_get_caps_process()) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to get current process capabilities %1$d"),
+   ret);
+return -1;
+}
 
 /* Make sure we drop everything if required by the user */
 if (policy == VIR_DOMAIN_CAPABILITIES_POLICY_DENY)
-- 
2.41.0

[PATCH 0/2] Check for capng_*() retvals

[Michal Privoznik]

There's a commit inside of (yet-unreleased) libcap-ng which marks some
functions as 'warned unused result'. Fedora rawhide already picked up
the commit, but since we are not checking for all retvals we got a build
failure on rawhide.

https://src.fedoraproject.org/rpms/libcap-ng/c/fed9b23c8d0020e07c937a3ac0d6dcc4534715fb?branch=rawhide

Green pipeline:

https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/999435445

Michal Prívozník (2):
  lxc_container: Check retval of capng_get_caps_process()
  virutil: Check retval of capng_apply()

 src/lxc/lxc_container.c | 8 +++-
 src/util/virutil.c  | 8 ++--
 2 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.41.0

Re: [libvirt PATCH v4] ch: Fix cloud-hypervisor version processing

[Martin Kletzander]

On Fri, Sep 08, 2023 at 05:29:08PM -0500, Praveen K Paladugu wrote:

Refactor the version processing logic in ch driver to support versions
from non-release cloud-hypervisor binaries. This version also supports
versions with branch prefixes in them.

Signed-off-by: Praveen K Paladugu 


Reviewed-by: Martin Kletzander 

and pushed now.  Thanks


---
src/ch/ch_conf.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index a8565d9537..f421af5121 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -172,6 +172,29 @@ virCHDriverConfigDispose(void *obj)

#define MIN_VERSION ((15 * 100) + (0 * 1000) + (0))

+/**
+ * chPreProcessVersionString:
+ *
+ * Returns: a pointer to numerical version without branch/commit info
+ */
+static char *
+chPreProcessVersionString(char *version)
+{
+char *tmp = strrchr(version, '/');
+
+if (tmp)
+version = tmp + 1;
+
+if (version[0] == 'v')
+version++;
+
+tmp = strchr(version, '-');
+if (tmp)
+*tmp = '\0';
+
+return version;
+}
+
int
chExtractVersion(virCHDriver *driver)
{
@@ -193,13 +216,20 @@ chExtractVersion(virCHDriver *driver)

tmp = help;

-/* expected format: cloud-hypervisor v.. */
-if ((tmp = STRSKIP(tmp, "cloud-hypervisor v")) == NULL) {
+/* Below are example version formats and expected outputs:
+ *  cloud-hypervisor v32.0.0 (expected: 32.0.0)
+ *  cloud-hypervisor v33.0-104-ge0e3779e-dirty (expected: 33.0)
+ *  cloud-hypervisor testing/v32.0.131-1-ga5d6db5c-dirty (expected: 
32.0.131)
+ */
+if ((tmp = STRSKIP(tmp, "cloud-hypervisor ")) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
   _("Unexpected output of cloud-hypervisor binary"));
return -1;
}

+tmp = chPreProcessVersionString(tmp);
+VIR_DEBUG("Cloud-Hypervisor version detected: %s", tmp);
+
if (virStringParseVersion(, tmp, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
   _("Unable to parse cloud-hypervisor version: %1$s"), 
tmp);
--
2.41.0



signature.asc
Description: PGP signature

Re: [PATCH v2] interface: fix udev_device_get_sysattr_value return value check

[Martin Kletzander]

On Fri, Sep 08, 2023 at 04:15:29PM +0300, Dmitry Frolov wrote:

Reviewing the code I found that return value of function
udev_device_get_sysattr_value() is dereferenced without a check.
udev_device_get_sysattr_value() may return NULL by number of reasons.

v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE()

Signed-off-by: Dmitry Frolov 
---
src/interface/interface_backend_udev.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/interface/interface_backend_udev.c 
b/src/interface/interface_backend_udev.c
index a0485ddd21..df7066727e 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -23,6 +23,7 @@
#include 
#include 

+#include "virlog.h"
#include "virerror.h"
#include "virfile.h"
#include "datatypes.h"
@@ -40,6 +41,8 @@

#define VIR_FROM_THIS VIR_FROM_INTERFACE

+VIR_LOG_INIT("interface.interface_backend_udev");
+
struct udev_iface_driver {
struct udev *udev;
/* pid file FD, ensures two copies of the driver can't use the same root */
@@ -355,10 +358,13 @@ udevConnectListAllInterfaces(virConnectPtr conn,
g_autoptr(virInterfaceDef) def = NULL;

path = udev_list_entry_get_name(dev_entry);
-dev = udev_device_new_from_syspath(udev, path);
+if (!(dev = udev_device_new_from_syspath(udev, path))) {
+VIR_DEBUG("Skipping interface '%s'", NULLSTR(path));


Since you're at it, and path can be null (I guess), does it even make
sense to get the device from syspath when there is no path?  Shouldn't
we skip it right away?  And it would warrant even a VIR_DEBUG in such
case IMHO since the device is not made up, we got it from udev by
enumeration.

Also since there's going to be more VIR_DEBUG/WARN we should also
mention why the device is being skipped.


+continue;
+}
name = udev_device_get_sysname(dev);


As Peter pointed out this can be NULL and if this function is used for
getting the list of devices (not just the amount) this fails way later,
virGetInterface() sets an error message and we add NULL into the list
and happily carry on.  If we're checking that udev is not returning NULL
for devices that are returned by udev's enumeration then we should also
check the sysname existing if the caller wants it, something like:

if (ifaces && !name) {
VIR_WARN();
continue;
}


macaddr = udev_device_get_sysattr_value(dev, "address");
-status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
+status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), 
"up");

def = udevGetMinimalDefForDevice(dev);
if (!virConnectListAllInterfacesCheckACL(conn, def)) {
@@ -964,9 +970,9 @@ udevGetIfaceDef(struct udev *udev, const char *name)

/* MTU */
mtu_str = udev_device_get_sysattr_value(dev, "mtu");
-if (virStrToLong_ui(mtu_str, NULL, 10, ) < 0) {
+if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
-_("Could not parse MTU value '%1$s'"), mtu_str);
+_("Could not parse MTU value '%1$s'"), NULLSTR(mtu_str));
goto error;
}
ifacedef->mtu = mtu;
@@ -1089,7 +1095,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
   goto cleanup;

/* Check if it's active or not */
-status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
+status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), 
"up");

udev_device_unref(dev);

--
2.34.1



signature.asc
Description: PGP signature