RE: [RFC 00/18] vfio: Adopt iommufd
> From: Alex Williamson > Sent: Wednesday, April 27, 2022 12:22 AM > > > > > > My expectation would be that libvirt uses: > > > > > > -object iommufd,id=iommufd0,fd=NNN > > > -device vfio-pci,fd=MMM,iommufd=iommufd0 > > > > > > Whereas simple QEMU command line would be: > > > > > > -object iommufd,id=iommufd0 > > > -device vfio-pci,iommufd=iommufd0,host=:02:00.0 > > > > > > The iommufd object would open /dev/iommufd itself. Creating an > > > implicit iommufd object is someone problematic because one of the > > > things I forgot to highlight in my previous description is that the > > > iommufd object is meant to be shared across not only various vfio > > > devices (platform, ccw, ap, nvme, etc), but also across subsystems, ex. > > > vdpa. > > > > Out of curiosity - in concept one iommufd is sufficient to support all > > ioas requirements across subsystems while having multiple iommufd's > > instead lose the benefit of centralized accounting. The latter will also > > cause some trouble when we start virtualizing ENQCMD which requires > > VM-wide PASID virtualization thus further needs to share that > > information across iommufd's. Not unsolvable but really no gain by > > adding such complexity. So I'm curious whether Qemu provide > > a way to restrict that certain object type can only have one instance > > to discourage such multi-iommufd attempt? > > I don't see any reason for QEMU to restrict iommufd objects. The QEMU > philosophy seems to be to let users create whatever configuration they > want. For libvirt though, the assumption would be that a single > iommufd object can be used across subsystems, so libvirt would never > automatically create multiple objects. I like the flexibility what the objection approach gives in your proposal. But with the said complexity in mind (with no foreseen benefit), I wonder whether an alternative approach which treats iommufd as a global property instead of an object is acceptable in Qemu, i.e.: -iommufd on/off -device vfio-pci,iommufd,[fd=MMM/host=:02:00.0] All devices with iommufd specified then implicitly share a single iommufd object within Qemu. This still allows vfio devices to be specified via fd but just requires Libvirt to grant file permission on /dev/iommu. Is it a worthwhile tradeoff to be considered or just not a typical way in Qemu philosophy e.g. any object associated with a device must be explicitly specified? Thanks Kevin
[libvirt RFCv4 14/20] qemu: capabilities: add multifd to the probed migration capabilities
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 3 +++
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 +
34 files changed, 39 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b91db851bb..1aba26a86a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -672,6 +672,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"virtio-iommu-pci", /* QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI */
"virtio-iommu.boot-bypass", /*
QEMU_CAPS_VIRTIO_IOMMU_BOOT_BYPASS */
"virtio-net.rss", /* QEMU_CAPS_VIRTIO_NET_RSS */
+
+ /* 430 */
+ "migrate-multifd", /* QEMU_CAPS_MIGRATE_MULTIFD */
);
@@ -1230,6 +1233,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
{ "rdma-pin-all", QEMU_CAPS_MIGRATE_RDMA },
+{ "multifd", QEMU_CAPS_MIGRATE_MULTIFD },
};
/* Use virQEMUCapsQMPSchemaQueries for querying parameters of events */
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9b240e47fb..b089f83da1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -648,6 +648,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_VIRTIO_IOMMU_BOOT_BYPASS, /* virtio-iommu.boot-bypass */
QEMU_CAPS_VIRTIO_NET_RSS, /* virtio-net rss feature */
+/* 430 */
+QEMU_CAPS_MIGRATE_MULTIFD, /* migrate can set multifd parameter */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
index 5adf904fc4..4ca2cfa81c 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
@@ -148,6 +148,7 @@
+
400
0
61700240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index a84adc2610..1db978eb4c 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -153,6 +153,7 @@
+
400
0
42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index c494254c4d..251d4dfd29 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
@@ -145,6 +145,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index d2582fa297..a4af47c6a4 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
@@ -145,6 +145,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index 4f36186044..2bab764867 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s3
[libvirt RFCv4 13/20] qemu: wire up saveimage code with the multifd helper
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_saveimage.c | 130 --
src/qemu/qemu_saveimage.h | 11
2 files changed, 137 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 5a569fa52e..65d9a3fef5 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -17,6 +17,7 @@
*/
#include
+#include
#include "qemu_saveimage.h"
#include "qemu_domain.h"
@@ -365,6 +366,93 @@ int virQEMUSaveFdFini(virQEMUSaveFd *saveFd, virDomainObj
*vm, int ret)
return ret;
}
+/*
+ * qemuSaveImageFreeMultiFd: free all multifd virQEMUSaveFds.
+ * @multiFd: the array of saveFds
+ * @vm: the virDomainObj, to release lock
+ * @nconn: number of multifd channels
+ * @ret: the current operation result (< 0 is failure)
+ *
+ * If multiFd is NULL, the return value will be unchanged.
+ *
+ * Returns ret, or -1 if an error is detected.
+ */
+int qemuSaveImageFreeMultiFd(virQEMUSaveFd *multiFd, virDomainObj *vm, int
nconn, int ret)
+{
+int idx;
+
+if (!multiFd)
+return ret;
+
+for (idx = 0; idx < nconn; idx++) {
+ret = virQEMUSaveFdFini(&multiFd[idx], vm, ret);
+}
+/*
+ * do it again to unlink all in the error case,
+ * if error happened in the middle of previous loop.
+ */
+for (idx = 0; idx < nconn; idx++) {
+ret = virQEMUSaveFdFini(&multiFd[idx], vm, ret);
+}
+g_free(multiFd);
+return ret;
+}
+
+/*
+ * qemuSaveImageCloseMultiFd: perform normal close on all multifd
virQEMUSaveFds.
+ * If multiFd is NULL, the function will return success.
+ *
+ * Returns -1 on error, 0 on success.
+ */
+
+int qemuSaveImageCloseMultiFd(virQEMUSaveFd *multiFd, int nconn, virDomainObj
*vm)
+{
+int idx;
+
+if (!multiFd)
+return 0;
+
+for (idx = 0; idx < nconn; idx++) {
+if (virQEMUSaveFdClose(&multiFd[idx], vm) < 0) {
+return -1;
+}
+}
+return 0;
+}
+
+/*
+ * qemuSaveImageCreateMultiFd: allocate and initialize all multifd
virQEMUSaveFds.
+ *
+ * Returns the new array of virQEMUSaveFds, or NULL on error.
+ */
+
+virQEMUSaveFd *
+qemuSaveImageCreateMultiFd(virQEMUDriver *driver, virDomainObj *vm,
+ virCommand *cmd, const char *path,
+ int oflags, virQEMUDriverConfig *cfg,
+ int nconn)
+{
+virQEMUSaveFd *multiFd = g_new0(virQEMUSaveFd, nconn);
+int idx;
+
+for (idx = 0; idx < nconn; idx++) {
+virQEMUSaveFd *m = &multiFd[idx];
+if (virQEMUSaveFdInit(m, path, idx + 1, oflags, cfg) < 0 ||
+qemuSecuritySetImageFDLabel(driver->securityManager, vm->def,
m->fd) < 0) {
+
+virQEMUSaveFdFini(m, vm, -1);
+goto error;
+}
+virCommandAddArgFormat(cmd, "%d", m->fd);
+virCommandPassFD(cmd, m->fd, 0);
+}
+return multiFd;
+
+ error:
+qemuSaveImageFreeMultiFd(multiFd, vm, nconn, -1);
+return NULL;
+}
+
/* Helper function to execute a migration to file with a correct save header
* the caller needs to make sure that the processors are stopped and do all
other
@@ -381,6 +469,7 @@ qemuSaveImageCreate(virQEMUDriver *driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
+virQEMUSaveFd *multiFd = NULL;
unsigned int oflags = O_WRONLY | O_TRUNC | O_CREAT;
int ret = -1;
@@ -402,10 +491,43 @@ qemuSaveImageCreate(virQEMUDriver *driver,
if (virQEMUSaveDataWrite(data, saveFd.fd, saveFd.path) < 0)
goto cleanup;
+if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *helper_path = NULL;
+qemuDomainObjPrivate *priv = vm->privateData;
+g_autofree char *sun_path = g_strdup_printf("%s/save-multifd.sock",
priv->libDir);
+char buf[1];
+int helper_out = -1;
+if (!(helper_path = virFileFindResource("libvirt_multifd_helper",
+abs_top_builddir "/src",
+LIBEXECDIR)))
+goto cleanup;
+cmd = virCommandNewArgList(helper_path, sun_path, NULL);
+virCommandAddArgFormat(cmd, "%d", nconn);
+virCommandAddArgFormat(cmd, "%d", saveFd.fd);
+virCommandPassFD(cmd, saveFd.fd, 0);
+virCommandSetOutputFD(cmd, &helper_out); /* should create pipe
automagically */
+
+/* Perform parallel multifd migration to files (main fd + channels) */
+if (!(multiFd = qemuSaveImageCreateMultiFd(driver, vm, cmd,
saveFd.path, oflags, cfg, nconn)))
+goto cleanup;
+if (virCommandRunAsync(cmd, NULL) < 0)
+goto cleanup;
+if (saferead(helper_out, &buf, 1) != 1 || buf[0] != 'R')
+goto cleanup;
+if (chown(sun_path, cfg->user, cfg->group) < 0)
+goto cleanup;
+
[libvirt RFCv4 17/20] qemu: implement qemuSaveImageLoadMultiFd
use multifd to restore parallel saves.
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_driver.c| 12 +++--
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_migration.h | 6 +++
src/qemu/qemu_saveimage.c | 111 +-
src/qemu/qemu_saveimage.h | 9 +++-
5 files changed, 133 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7ff7072da0..2d90865ece 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5815,7 +5815,7 @@ static int
qemuDomainRestoreInternal(virConnectPtr conn,
const char *path,
const char *dxml,
- int nchannels G_GNUC_UNUSED,
+ int nchannels,
unsigned int flags,
int (*ensureACL)(virConnectPtr, virDomainDef *))
{
@@ -5907,8 +5907,14 @@ qemuDomainRestoreInternal(virConnectPtr conn,
flags) < 0)
goto cleanup;
-ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
- false, reset_nvram, true, VIR_ASYNC_JOB_START);
+if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
+ret = qemuSaveImageLoadMultiFd(conn, vm, oflags, data, reset_nvram,
+ &saveFd, nchannels,
VIR_ASYNC_JOB_START);
+
+} else {
+ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
+ false, reset_nvram, true,
VIR_ASYNC_JOB_START);
+}
qemuProcessEndJob(vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 542428ab8e..7bab913fe5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1933,7 +1933,7 @@ qemuMigrationSrcWaitForCompletion(virQEMUDriver *driver,
}
-static int
+int
qemuMigrationDstWaitForCompletion(virQEMUDriver *driver,
virDomainObj *vm,
virDomainAsyncJob asyncJob,
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index c3c48c19c0..38f4877cf0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -191,6 +191,12 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
int retcode,
bool v3proto);
+int
+qemuMigrationDstWaitForCompletion(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainAsyncJob asyncJob,
+ bool postcopy);
+
int
qemuMigrationSrcConfirm(virQEMUDriver *driver,
virDomainObj *vm,
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index d58e070bf7..2bc81035ae 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -551,6 +551,106 @@ qemuSaveImageCreate(virQEMUDriver *driver,
}
+int qemuSaveImageLoadMultiFd(virConnectPtr conn, virDomainObj *vm, int oflags,
+ virQEMUSaveData *data, bool reset_nvram,
+ virQEMUSaveFd *saveFd, int nchannels,
+ virDomainAsyncJob asyncJob)
+{
+virQEMUDriver *driver = conn->privateData;
+qemuDomainObjPrivate *priv = vm->privateData;
+virQEMUSaveFd *multiFd = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *helper_path = NULL;
+g_autofree char *sun_path = g_strdup_printf("%s/restore-multifd.sock",
cfg->saveDir);
+int ret = -1;
+
+if (!(helper_path = virFileFindResource("libvirt_multifd_helper",
+abs_top_builddir "/src",
+LIBEXECDIR)))
+goto cleanup;
+cmd = virCommandNewArgList(helper_path, sun_path, NULL);
+virCommandAddArgFormat(cmd, "%d", nchannels);
+virCommandAddArgFormat(cmd, "%d", saveFd->fd);
+virCommandPassFD(cmd, saveFd->fd, 0);
+
+/* Perform parallel multifd migration from files (main fd + channels) */
+if (!(multiFd = qemuSaveImageCreateMultiFd(driver, vm, cmd, saveFd->path,
oflags, cfg, nchannels)))
+goto cleanup;
+if (qemuSaveImageStartVM(conn, driver, vm, NULL, data, sun_path,
+ false, reset_nvram, false, asyncJob) < 0)
+goto cleanup;
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_MULTIFD)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("QEMU multifd not supported"));
+goto cleanup;
+} else {
+g_autoptr(qemuMigrationParams) migParams = qemuMigrationParamsNew();
+bool bwParam = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH);
+
+if (bwParam) {
+if (qemuMigrationParamsSetULL(migParams,
+ QEMU_MIGRATION_PARAM_MAX_BANDWIDTH,
+
[libvirt RFCv4 01/20] iohelper: introduce new struct to carry copy operation parameters
this is in preparation for a minor refactoring of the copy
function itself out of runIO().
Signed-off-by: Claudio Fontana
---
src/util/iohelper.c | 95 +
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 2c91bf4f93..c13746a547 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -45,6 +45,16 @@
# define O_DIRECT 0
#endif
+struct runIOParams {
+bool isBlockDev;
+bool isDirect;
+bool isWrite;
+int fdin;
+const char *fdinname;
+int fdout;
+const char *fdoutname;
+};
+
static int
runIO(const char *path, int fd, int oflags)
{
@@ -53,13 +63,9 @@ runIO(const char *path, int fd, int oflags)
size_t buflen = 1024*1024;
intptr_t alignMask = 64*1024 - 1;
int ret = -1;
-int fdin, fdout;
-const char *fdinname, *fdoutname;
-unsigned long long total = 0;
-bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0);
-off_t end = 0;
+off_t total = 0;
struct stat sb;
-bool isBlockDev = false;
+struct runIOParams p;
#if WITH_POSIX_MEMALIGN
if (posix_memalign(&base, alignMask + 1, buflen))
@@ -77,34 +83,23 @@ runIO(const char *path, int fd, int oflags)
fd, path);
goto cleanup;
}
-isBlockDev = S_ISBLK(sb.st_mode);
+p.isBlockDev = S_ISBLK(sb.st_mode);
+p.isDirect = O_DIRECT && (oflags & O_DIRECT);
switch (oflags & O_ACCMODE) {
case O_RDONLY:
-fdin = fd;
-fdinname = path;
-fdout = STDOUT_FILENO;
-fdoutname = "stdout";
-/* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
-if (!isBlockDev && direct && ((end = lseek(fd, 0, SEEK_CUR)) != 0)) {
-virReportSystemError(end < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT read needs entire seekable
file"));
-goto cleanup;
-}
+p.isWrite = false;
+p.fdin = fd;
+p.fdinname = path;
+p.fdout = STDOUT_FILENO;
+p.fdoutname = "stdout";
break;
case O_WRONLY:
-fdin = STDIN_FILENO;
-fdinname = "stdin";
-fdout = fd;
-fdoutname = path;
-/* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
-if (!isBlockDev && direct && (end = lseek(fd, 0, SEEK_END)) != 0) {
-virReportSystemError(end < 0 ? errno : EINVAL, "%s",
- _("O_DIRECT write needs empty seekable
file"));
-goto cleanup;
-}
+p.isWrite = true;
+p.fdin = STDIN_FILENO;
+p.fdinname = "stdin";
+p.fdout = fd;
+p.fdoutname = path;
break;
case O_RDWR:
@@ -114,6 +109,22 @@ runIO(const char *path, int fd, int oflags)
(oflags & O_ACCMODE));
goto cleanup;
}
+/* To make the implementation simpler, we give up on any
+ * attempt to use O_DIRECT in a non-trivial manner. */
+if (!p.isBlockDev && p.isDirect) {
+off_t off;
+if (p.isWrite) {
+if ((off = lseek(fd, 0, SEEK_END)) != 0) {
+virReportSystemError(off < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT write needs empty seekable
file"));
+goto cleanup;
+}
+} else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) {
+virReportSystemError(off < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT read needs entire seekable
file"));
+goto cleanup;
+}
+}
while (1) {
ssize_t got;
@@ -124,16 +135,16 @@ runIO(const char *path, int fd, int oflags)
* writes will be aligned.
* In other cases using saferead reduces number of syscalls.
*/
-if (fdin == fd && direct) {
-if ((got = read(fdin, buf, buflen)) < 0 &&
+if (!p.isWrite && p.isDirect) {
+if ((got = read(p.fdin, buf, buflen)) < 0 &&
errno == EINTR)
continue;
} else {
-got = saferead(fdin, buf, buflen);
+got = saferead(p.fdin, buf, buflen);
}
if (got < 0) {
-virReportSystemError(errno, _("Unable to read %s"), fdinname);
+virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
goto cleanup;
}
if (got == 0)
@@ -142,35 +153,35 @@ runIO(const char *path, int fd, int oflags)
total += got;
/* handle last write size align in direct case */
-if (got < buflen && direct && fdout == fd) {
+if (got < buflen && p.isDirect && p.isWrite) {
ssize_t aligned_got = (got + alignMask) & ~alignMask;
memset(buf + got, 0, aligned_got - got);
-if (sa
[libvirt RFCv4 02/20] iohelper: refactor copy operation as a separate function
Signed-off-by: Claudio Fontana
---
src/util/iohelper.c | 131 +---
1 file changed, 75 insertions(+), 56 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index c13746a547..1584321839 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -55,17 +55,23 @@ struct runIOParams {
const char *fdoutname;
};
-static int
-runIO(const char *path, int fd, int oflags)
+/**
+ * runIOCopy: execute the IO copy based on the passed parameters
+ * @p: the IO parameters
+ *
+ * Execute the copy based on the passed parameters.
+ *
+ * Returns: size transfered, or < 0 on error.
+ */
+
+static off_t
+runIOCopy(const struct runIOParams p)
{
g_autofree void *base = NULL; /* Location to be freed */
char *buf = NULL; /* Aligned location within base */
size_t buflen = 1024*1024;
intptr_t alignMask = 64*1024 - 1;
-int ret = -1;
off_t total = 0;
-struct stat sb;
-struct runIOParams p;
#if WITH_POSIX_MEMALIGN
if (posix_memalign(&base, alignMask + 1, buflen))
@@ -77,6 +83,67 @@ runIO(const char *path, int fd, int oflags)
buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
#endif
+while (1) {
+ssize_t got;
+
+/* If we read with O_DIRECT from file we can't use saferead as
+ * it can lead to unaligned read after reading last bytes.
+ * If we write with O_DIRECT use should use saferead so that
+ * writes will be aligned.
+ * In other cases using saferead reduces number of syscalls.
+ */
+if (!p.isWrite && p.isDirect) {
+if ((got = read(p.fdin, buf, buflen)) < 0 &&
+errno == EINTR)
+continue;
+} else {
+got = saferead(p.fdin, buf, buflen);
+}
+
+if (got < 0) {
+virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
+return -2;
+}
+if (got == 0)
+break;
+
+total += got;
+
+/* handle last write size align in direct case */
+if (got < buflen && p.isDirect && p.isWrite) {
+ssize_t aligned_got = (got + alignMask) & ~alignMask;
+
+memset(buf + got, 0, aligned_got - got);
+
+if (safewrite(p.fdout, buf, aligned_got) < 0) {
+virReportSystemError(errno, _("Unable to write %s"),
p.fdoutname);
+return -3;
+}
+
+if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
+virReportSystemError(errno, _("Unable to truncate %s"),
p.fdoutname);
+return -4;
+}
+
+break;
+}
+
+if (safewrite(p.fdout, buf, got) < 0) {
+virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
+return -3;
+}
+}
+return total;
+}
+
+static int
+runIO(const char *path, int fd, int oflags)
+{
+int ret = -1;
+off_t total = 0;
+struct stat sb;
+struct runIOParams p;
+
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno,
_("Unable to access file descriptor %d path %s"),
@@ -125,57 +192,9 @@ runIO(const char *path, int fd, int oflags)
goto cleanup;
}
}
-
-while (1) {
-ssize_t got;
-
-/* If we read with O_DIRECT from file we can't use saferead as
- * it can lead to unaligned read after reading last bytes.
- * If we write with O_DIRECT use should use saferead so that
- * writes will be aligned.
- * In other cases using saferead reduces number of syscalls.
- */
-if (!p.isWrite && p.isDirect) {
-if ((got = read(p.fdin, buf, buflen)) < 0 &&
-errno == EINTR)
-continue;
-} else {
-got = saferead(p.fdin, buf, buflen);
-}
-
-if (got < 0) {
-virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
-goto cleanup;
-}
-if (got == 0)
-break;
-
-total += got;
-
-/* handle last write size align in direct case */
-if (got < buflen && p.isDirect && p.isWrite) {
-ssize_t aligned_got = (got + alignMask) & ~alignMask;
-
-memset(buf + got, 0, aligned_got - got);
-
-if (safewrite(p.fdout, buf, aligned_got) < 0) {
-virReportSystemError(errno, _("Unable to write %s"),
p.fdoutname);
-goto cleanup;
-}
-
-if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
-virReportSystemError(errno, _("Unable to truncate %s"),
p.fdoutname);
-goto cleanup;
-}
-
-break;
-}
-
-if (safewrite(p.fdout, buf, got) < 0) {
-virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
-goto cleanup;
-}
-}
+total = runIOCopy(p);
+if (total <
[libvirt RFCv4 18/20] tools: add parallel parameter to virsh save command
Signed-off-by: Claudio Fontana
---
docs/manpages/virsh.rst | 23 ++-
tools/virsh-domain.c| 49 +
2 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e8568559fa..2bce701057 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -3801,15 +3801,18 @@ save
::
save domain state-file [--bypass-cache] [--xml file]
+ [--parallel] [--parallel-connections connections]
[{--running | --paused}] [--verbose]
-Saves a running domain (RAM, but not disk state) to a state file so that
-it can be restored
-later. Once saved, the domain will no longer be running on the
-system, thus the memory allocated for the domain will be free for
-other domains to use. ``virsh restore`` restores from this state file.
+Saves a paused or running domain (RAM, but not disk state) to one or more
+state files, so that it can be restored later.
+Once saved, the domain will no longer be running on the system,
+thus the memory allocated for the domain will be free for
+other domains to use. ``virsh restore`` restores from state file/s.
+
If *--bypass-cache* is specified, the save will avoid the file system
-cache, although this may slow down the operation.
+cache; depending on the specific scenario this may slow down or speed up
+the operation.
The progress may be monitored using ``domjobinfo`` virsh command and canceled
with ``domjobabort`` command (sent by another virsh instance). Another option
@@ -3831,6 +3834,14 @@ based on the state the domain was in when the save was
done; passing
either the *--running* or *--paused* flag will allow overriding which
state the ``restore`` should use.
+*--parallel* option will cause the save data to be sent over multiple
+parallel connections to multiple files. The main save file is specified
+with ``state-file``, and a number of additional connections can be
+set using *--parallel-connections*, which will save to files named
+``state-file``.1 , ``state-file``.2 ... up to ``connections``.
+
+Parallel connections may help in speeding up the save operation.
+
Domain saved state files assume that disk images will be unchanged
between the creation and restore point. For a more complete system
restore point, where the disk state is saved alongside the memory
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index df8df9c2f3..1e3adaa1be 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4164,6 +4164,14 @@ static const vshCmdOptDef opts_save[] = {
.type = VSH_OT_BOOL,
.help = N_("avoid file system cache when saving")
},
+{.name = "parallel",
+ .type = VSH_OT_BOOL,
+ .help = N_("enable parallel save to files")
+},
+{.name = "parallel-connections",
+ .type = VSH_OT_INT,
+ .help = N_("number of connections/files for parallel save")
+},
{.name = "xml",
.type = VSH_OT_STRING,
.completer = virshCompletePathLocalExisting,
@@ -4193,6 +4201,11 @@ doSave(void *opaque)
g_autoptr(virshDomain) dom = NULL;
const char *name = NULL;
const char *to = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int maxparams = 0;
+int intOpt = 0;
+int rv = -1;
unsigned int flags = 0;
const char *xmlfile = NULL;
g_autofree char *xml = NULL;
@@ -4206,29 +4219,46 @@ doSave(void *opaque)
goto out_sig;
#endif /* !WIN32 */
-if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0)
+if ((rv = vshCommandOptStringReq(ctl, cmd, "file", &to)) < 0) {
goto out;
-
+} else {
+if (virTypedParamsAddString(¶ms, &nparams, &maxparams,
+VIR_SAVE_PARAM_FILE, to) < 0)
+goto out;
+}
if (vshCommandOptBool(cmd, "bypass-cache"))
flags |= VIR_DOMAIN_SAVE_BYPASS_CACHE;
+if (vshCommandOptBool(cmd, "parallel"))
+flags |= VIR_DOMAIN_SAVE_PARALLEL;
+if ((rv = vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt)) <
0) {
+goto out;
+} else if (rv > 0) {
+if (virTypedParamsAddInt(¶ms, &nparams, &maxparams,
+ VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt)
< 0)
+goto out;
+}
if (vshCommandOptBool(cmd, "running"))
flags |= VIR_DOMAIN_SAVE_RUNNING;
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
-if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0)
+if ((rv = vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile)) < 0)
goto out;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
goto out;
-if (xmlfile &&
-virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) {
-vshReportError(ctl);
-goto out;
+if (xmlfile) {
+if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) {
+vshReportError(ctl);
+goto out;
+} els
[libvirt RFCv4 06/20] libvirt: introduce virDomainSaveParametersFlags public API
add new API in order to be able to extend parameters to the domain
save operation. We will use it to fit the existing arguments of
VirDomainSaveFlags, and then add parallel saves functionality.
Signed-off-by: Claudio Fontana
---
include/libvirt/libvirt-domain.h | 45
src/driver-hypervisor.h | 7 +
src/libvirt-domain.c | 51
src/libvirt_public.syms | 5
4 files changed, 108 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 9aa214f3df..b870a73b64 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1481,6 +1481,8 @@ typedef enum {
VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */
VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */
VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from
template */
+/** Since: v8.3.0 */
+VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to
multiple files */
} virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain,
@@ -1489,6 +1491,10 @@ int virDomainSaveFlags
(virDomainPtr domain,
const char *to,
const char *dxml,
unsigned int flags);
+int virDomainSaveParametersFlags (virDomainPtr domain,
+ virTypedParameterPtr
params,
+ int nparams,
+ unsigned int flags);
int virDomainRestore(virConnectPtr conn,
const char *from);
int virDomainRestoreFlags (virConnectPtr conn,
@@ -1496,6 +1502,45 @@ int virDomainRestoreFlags
(virConnectPtr conn,
const char *dxml,
unsigned int flags);
+/**
+ * VIR_SAVE_PARAM_FILE:
+ *
+ * the parameter used to specify the savestate file to save to or restore from.
+ * For parallel saves, this is the main file, with the extra connections
adding suffix
+ * .1 .2 .3 ... up to VIR_SAVE_PARAM_PARALLEL_CONNECTIONS.
+ *
+ * Since: v8.3.0
+ */
+# define VIR_SAVE_PARAM_FILE "file"
+
+/**
+ * VIR_SAVE_PARAM_DXML:
+ *
+ * an optional parameter used to adjust guest xml on restore.
+ * If the hypervisor supports it, it can be used to alter
+ * host-specific portions of the domain XML that will be used when
+ * restoring an image. For example, it is possible to alter the
+ * device while the domain is stopped.
+ *
+ * Since: v8.3.0
+ */
+# define VIR_SAVE_PARAM_DXML "dxml"
+
+/**
+ * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:
+ *
+ * this optional parameter mirrors the migration parameter
+ * VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS.
+ *
+ * This parameter is used when saving or restoring state files
+ * in parallel using the flag VIR_DOMAIN_SAVE_PARALLEL .
+ * It specifies the number of extra files to save/restore
+ * using parallel connections.
+ *
+ * Since: v8.3.0
+ */
+# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
+
/* See below for virDomainSaveImageXMLFlags */
char * virDomainSaveImageGetXMLDesc(virConnectPtr conn,
const char *file,
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 4423eb0885..a4e1d21e76 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -240,6 +240,12 @@ typedef int
const char *dxml,
unsigned int flags);
+typedef int
+(*virDrvDomainSaveParametersFlags)(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags);
+
typedef int
(*virDrvDomainRestore)(virConnectPtr conn,
const char *from);
@@ -1489,6 +1495,7 @@ struct _virHypervisorDriver {
virDrvDomainGetControlInfo domainGetControlInfo;
virDrvDomainSave domainSave;
virDrvDomainSaveFlags domainSaveFlags;
+virDrvDomainSaveParametersFlags domainSaveParametersFlags;
virDrvDomainRestore domainRestore;
virDrvDomainRestoreFlags domainRestoreFlags;
virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cbd7902d2d..9e4fcfd022 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
return -1;
}
+/**
+ * virDomainSaveParametersFlags:
+ * @domain: a domain objec
[libvirt RFCv4 20/20] qemu: add migration parameter multifd-compression
use zstd which is the only really interesting one.
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_migration.c | 6 +++
src/qemu/qemu_migration_params.c | 49 +--
src/qemu/qemu_migration_params.h | 6 +++
src/qemu/qemu_saveimage.c | 6 +++
.../caps_5.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
.../caps_5.0.0.riscv64.xml| 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
.../caps_5.2.0.riscv64.xml| 1 +
.../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 1 +
.../caps_6.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 +
.../caps_6.2.0.x86_64.xml | 1 +
.../caps_7.0.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 +
.../caps_7.0.0.x86_64.xml | 1 +
27 files changed, 66 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1aba26a86a..3b99e66ab5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -675,6 +675,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 430 */
"migrate-multifd", /* QEMU_CAPS_MIGRATE_MULTIFD */
+ "migration-param.multifd-compression", /*
QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION */
);
@@ -1612,6 +1613,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsQMPSchemaQueries[] = {
{ "migrate-set-parameters/arg-type/downtime-limit",
QEMU_CAPS_MIGRATION_PARAM_DOWNTIME },
{ "migrate-set-parameters/arg-type/xbzrle-cache-size",
QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE },
{
"migrate-set-parameters/arg-type/block-bitmap-mapping/bitmaps/transform",
QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING },
+{ "migrate-set-parameters/arg-type/multifd-compression",
QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION },
{ "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS },
{ "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP },
{ "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b089f83da1..e226b1a51a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -650,6 +650,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 430 */
QEMU_CAPS_MIGRATE_MULTIFD, /* migrate can set multifd parameter */
+QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION, /* multifd-compression in
migrate-set-parameters */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7bab913fe5..f9c86de67e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5950,6 +5950,12 @@ qemuMigrationSrcToFileAux(virQEMUDriver *driver,
virDomainObj *vm,
QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS,
nchannels) < 0)
return -1;
+if (virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION)) {
+if (qemuMigrationParamsSetString(migParams,
+
QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION,
+ "zstd") < 0)
+return -1;
+}
}
if (needParams && qemuMigrationParamsApply(driver, vm, asyncJob,
migParams) < 0)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 36174a66d8..f6b9dc337d 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -115,6 +115,7 @@ VIR_ENUM_IMPL(qemuMigrationParam,
"xbzrle-cache-size",
"max-postcopy-bandwidth",
"multifd-channels",
+ "multifd-compression",
);
typedef struct _qemuMigrationParamsAlwaysOnItem
qemuMigrationParamsAlwaysOnItem;
@@ -234,6 +235,7 @@ static const qemuMigrationParamType
qemuMigrationParamTypes[] = {
[QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE] = QEMU_MIGRATION_PARAM_TYPE_ULL,
[QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH] =
QEMU_MIGRATION_PARAM_TYPE_ULL,
[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS] = QEMU_
[libvirt RFCv4 19/20] tools: add parallel parameter to virsh restore command
Signed-off-by: Claudio Fontana
---
docs/manpages/virsh.rst | 11 +-
tools/virsh-domain.c| 47 ++---
2 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 2bce701057..21bfc8b16b 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -3752,12 +3752,14 @@ restore
::
restore state-file [--bypass-cache] [--xml file]
+ [--parallel] [--parallel-connections connections]
[{--running | --paused}] [--reset-nvram]
Restores a domain from a ``virsh save`` state file. See *save* for more info.
If *--bypass-cache* is specified, the restore will avoid the file system
-cache, although this may slow down the operation.
+cache; depending on the specific scenario this may slow down or speed up
+the operation.
*--xml* ``file`` is usually omitted, but can be used to supply an
alternative XML file for use on the restored guest with changes only
@@ -3773,6 +3775,13 @@ domain should be started in.
If *--reset-nvram* is specified, any existing NVRAM file will be deleted
and re-initialized from its pristine template.
+*--parallel* option will cause the save data to be loaded from multiple
+state files over parallel connections. The main save file is specified
+with ``state-file``, and the number of additional channels can be set
+using *--parallel-connections*
+
+Parallel connections may help in speeding up the save operation.
+
``Note``: To avoid corrupting file system contents within the domain, you
should not reuse the saved state file for a second ``restore`` unless you
have also reverted all storage volumes back to the same contents as when
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1e3adaa1be..3a5df609a7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5302,6 +5302,14 @@ static const vshCmdOptDef opts_restore[] = {
.type = VSH_OT_BOOL,
.help = N_("avoid file system cache when restoring")
},
+{.name = "parallel",
+ .type = VSH_OT_BOOL,
+ .help = N_("enable parallel restore")
+},
+{.name = "parallel-connections",
+ .type = VSH_OT_INT,
+ .help = N_("number of connections/files for parallel restore")
+},
{.name = "xml",
.type = VSH_OT_STRING,
.completer = virshCompletePathLocalExisting,
@@ -5326,17 +5334,36 @@ static bool
cmdRestore(vshControl *ctl, const vshCmd *cmd)
{
const char *from = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int maxparams = 0;
+int intOpt = 0;
unsigned int flags = 0;
const char *xmlfile = NULL;
g_autofree char *xml = NULL;
virshControl *priv = ctl->privData;
-int rc;
+int rc = -1;
-if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
-return false;
+if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) {
+goto out;
+} else {
+if (virTypedParamsAddString(¶ms, &nparams, &maxparams,
+VIR_SAVE_PARAM_FILE, from) < 0)
+goto out;
+}
if (vshCommandOptBool(cmd, "bypass-cache"))
flags |= VIR_DOMAIN_SAVE_BYPASS_CACHE;
+if (vshCommandOptBool(cmd, "parallel"))
+flags |= VIR_DOMAIN_SAVE_PARALLEL;
+if ((rc = vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt)) <
0) {
+goto out;
+} else if (rc > 0) {
+rc = -1;
+if (virTypedParamsAddInt(¶ms, &nparams, &maxparams,
+ VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt)
< 0)
+goto out;
+}
if (vshCommandOptBool(cmd, "running"))
flags |= VIR_DOMAIN_SAVE_RUNNING;
if (vshCommandOptBool(cmd, "paused"))
@@ -5345,13 +5372,15 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SAVE_RESET_NVRAM;
if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0)
-return false;
+goto out;
if (xmlfile &&
virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0)
-return false;
+goto out;
-if (flags || xml) {
+if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
+rc = virDomainRestoreParametersFlags(priv->conn, params, nparams,
flags);
+} else if (flags || xml) {
rc = virDomainRestoreFlags(priv->conn, from, xml, flags);
} else {
rc = virDomainRestore(priv->conn, from);
@@ -5359,11 +5388,13 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
if (rc < 0) {
vshError(ctl, _("Failed to restore domain from %s"), from);
-return false;
+goto out;
}
vshPrintExtra(ctl, _("Domain restored from %s\n"), from);
-return true;
+ out:
+virTypedParamsFree(params, nparams);
+return rc >= 0;
}
/*
--
2.34.1
[libvirt RFCv4 16/20] qemu: add parameter to qemuMigrationDstRun to skip waiting
The distinction on whether to wait for the migration completion
or not was made on the async job type,
but with the future addition of multifd migration from files,
we need a way to avoid waiting, so we can prepare multifd
migration parameters before starting the transfers.
Adapt all callers.
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_driver.c| 8
src/qemu/qemu_migration.c | 18 ++
src/qemu/qemu_migration.h | 3 ++-
src/qemu/qemu_process.c | 3 ++-
src/qemu/qemu_process.h | 5 +++--
src/qemu/qemu_saveimage.c | 4 +++-
src/qemu/qemu_saveimage.h | 1 +
src/qemu/qemu_snapshot.c | 4 ++--
8 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6851270be3..7ff7072da0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1630,7 +1630,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr
conn,
}
if (qemuProcessStart(conn, driver, vm, NULL, VIR_ASYNC_JOB_START,
- NULL, -1, NULL, NULL,
+ NULL, -1, NULL, false, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
start_flags) < 0) {
virDomainAuditStart(vm, "booted", false);
@@ -5908,7 +5908,7 @@ qemuDomainRestoreInternal(virConnectPtr conn,
goto cleanup;
ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
- false, reset_nvram, VIR_ASYNC_JOB_START);
+ false, reset_nvram, true, VIR_ASYNC_JOB_START);
qemuProcessEndJob(vm);
@@ -6229,7 +6229,7 @@ qemuDomainObjRestore(virConnectPtr conn,
virDomainObjAssignDef(vm, &def, true, NULL);
ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
- start_paused, reset_nvram, asyncJob);
+ start_paused, reset_nvram, true, asyncJob);
cleanup:
virQEMUSaveDataFree(data);
@@ -6492,7 +6492,7 @@ qemuDomainObjStart(virConnectPtr conn,
}
ret = qemuProcessStart(conn, driver, vm, NULL, asyncJob,
- NULL, -1, NULL, NULL,
+ NULL, -1, NULL, false, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags);
virDomainAuditStart(vm, "booted", ret >= 0);
if (ret >= 0) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 371db6a41c..542428ab8e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2139,7 +2139,8 @@ int
qemuMigrationDstRun(virQEMUDriver *driver,
virDomainObj *vm,
const char *uri,
-virDomainAsyncJob asyncJob)
+virDomainAsyncJob asyncJob,
+bool wait)
{
qemuDomainObjPrivate *priv = vm->privateData;
int rv;
@@ -2160,14 +2161,15 @@ qemuMigrationDstRun(virQEMUDriver *driver,
if (rv < 0)
return -1;
-if (asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) {
-/* qemuMigrationDstWaitForCompletion is called from the Finish phase */
-return 0;
+if (wait) {
+/*
+ * the Migration Finish phase, as well as the multifd load from files,
+ * need to call qemuMigrationDstWaitForCompletion separately, not here.
+ */
+if (qemuMigrationDstWaitForCompletion(driver, vm, asyncJob, false) < 0)
+return -1;
}
-if (qemuMigrationDstWaitForCompletion(driver, vm, asyncJob, false) < 0)
-return -1;
-
return 0;
}
@@ -3041,7 +3043,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver,
}
if (qemuMigrationDstRun(driver, vm, incoming->uri,
-VIR_ASYNC_JOB_MIGRATION_IN) < 0)
+VIR_ASYNC_JOB_MIGRATION_IN, false) < 0)
goto stopjob;
if (qemuProcessFinishStartup(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN,
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index ddc8e65489..c3c48c19c0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -255,7 +255,8 @@ int
qemuMigrationDstRun(virQEMUDriver *driver,
virDomainObj *vm,
const char *uri,
-virDomainAsyncJob asyncJob);
+virDomainAsyncJob asyncJob,
+bool wait);
void
qemuMigrationAnyPostcopyFailed(virQEMUDriver *driver,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b0b00eb0a2..0a1e7985fb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7788,6 +7788,7 @@ qemuProcessStart(virConnectPtr conn,
const char *migrateFrom,
int migrateFd,
const char *migratePath,
+ bool wait_incoming,
virDomainMomentObj *snapshot,
virNetDevVPortProfileOp vmop,
unsigned int flags)
@@ -7850,7 +785
[libvirt RFCv4 15/20] qemu: implement qemuMigrationSrcToFilesMultiFd
implement a function similar to qemuMigrationSrcToFile
that migrates to multiple files using QEMU multifd.
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_migration.c| 129 ---
src/qemu/qemu_migration.h| 7 ++
src/qemu/qemu_migration_params.c | 22 ++
src/qemu/qemu_migration_params.h | 9 +++
src/qemu/qemu_saveimage.c| 5 +-
5 files changed, 124 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b735bdb391..371db6a41c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5896,13 +5896,14 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
return dom;
}
-
/* Helper function called while vm is active. */
-int
-qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm,
- int fd,
- virCommand *compressor,
- virDomainAsyncJob asyncJob)
+static int
+qemuMigrationSrcToFileAux(virQEMUDriver *driver, virDomainObj *vm,
+ int fd,
+ virCommand *compressor,
+ virDomainAsyncJob asyncJob,
+ const char *sun_path,
+ int nchannels)
{
qemuDomainObjPrivate *priv = vm->privateData;
bool bwParam = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH);
@@ -5913,24 +5914,24 @@ qemuMigrationSrcToFile(virQEMUDriver *driver,
virDomainObj *vm,
char *errbuf = NULL;
virErrorPtr orig_err = NULL;
g_autoptr(qemuMigrationParams) migParams = NULL;
+bool needParams = (bwParam || sun_path);
if (qemuMigrationSetDBusVMState(driver, vm) < 0)
return -1;
+if (sun_path && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_MULTIFD))
+return -1;
+
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
-if (bwParam) {
-if (!(migParams = qemuMigrationParamsNew()))
-return -1;
+if (needParams && !((migParams = qemuMigrationParamsNew(
+return -1;
+if (bwParam) {
if (qemuMigrationParamsSetULL(migParams,
QEMU_MIGRATION_PARAM_MAX_BANDWIDTH,
QEMU_DOMAIN_MIG_BANDWIDTH_MAX * 1024 *
1024) < 0)
return -1;
-
-if (qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0)
-return -1;
-
priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
} else {
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
@@ -5941,6 +5942,17 @@ qemuMigrationSrcToFile(virQEMUDriver *driver,
virDomainObj *vm,
}
}
+if (sun_path) {
+qemuMigrationParamsSetCap(migParams, QEMU_MIGRATION_CAP_MULTIFD);
+if (qemuMigrationParamsSetInt(migParams,
+ QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS,
+ nchannels) < 0)
+return -1;
+}
+
+if (needParams && qemuMigrationParamsApply(driver, vm, asyncJob,
migParams) < 0)
+return -1;
+
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest unexpectedly quit"));
@@ -5948,45 +5960,53 @@ qemuMigrationSrcToFile(virQEMUDriver *driver,
virDomainObj *vm,
return -1;
}
-if (compressor && virPipe(pipeFD) < 0)
+if (!sun_path && compressor && virPipe(pipeFD) < 0)
return -1;
-/* All right! We can use fd migration, which means that qemu
- * doesn't have to open() the file, so while we still have to
- * grant SELinux access, we can do it on fd and avoid cleanup
- * later, as well as skip futzing with cgroup. */
-if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def,
-compressor ? pipeFD[1] : fd) < 0)
-goto cleanup;
-
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
-if (!compressor) {
-rc = qemuMonitorMigrateToFd(priv->mon,
-QEMU_MONITOR_MIGRATE_BACKGROUND,
-fd);
+if (sun_path) {
+rc = qemuMonitorMigrateToSocket(priv->mon,
+QEMU_MONITOR_MIGRATE_BACKGROUND,
+sun_path);
} else {
-virCommandSetInputFD(compressor, pipeFD[0]);
-virCommandSetOutputFD(compressor, &fd);
-virCommandSetErrorBuffer(compressor, &errbuf);
-virCommandDoAsyncIO(compressor);
-if (virSetCloseExec(pipeFD[1]) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to set cloexec flag"));
-qemuDomainObjExitMonitor(vm);
-goto cleanup;
-}
-if (virCommandRunAsync(compressor, N
[libvirt RFCv4 05/20] multifd-helper: new helper for parallel save/restore
For the save direction, this helper listens on a unix socket
which QEMU connects to for multifd migration to files.
For the restore direction, this helper connects to a unix socket
QEMU listens at for multifd migration from files.
The file descriptors are passed as command line parameters.
Signed-off-by: Claudio Fontana
---
po/POTFILES.in| 1 +
src/libvirt_private.syms | 1 +
src/util/meson.build | 13 ++
src/util/multifd-helper.c | 250 ++
src/util/virthread.c | 5 +
src/util/virthread.h | 1 +
6 files changed, 271 insertions(+)
create mode 100644 src/util/multifd-helper.c
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 5b4c00d7ac..493a5c3f80 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -242,6 +242,7 @@
@SRCDIR@src/test/test_driver.c
@SRCDIR@src/util/iohelper.c
@SRCDIR@src/util/runio.c
+@SRCDIR@src/util/multifd-helper.c
@SRCDIR@src/util/viralloc.c
@SRCDIR@src/util/virarptable.c
@SRCDIR@src/util/viraudit.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 97bfca906b..5f2bee985e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3427,6 +3427,7 @@ virThreadCreateFull;
virThreadID;
virThreadIsSelf;
virThreadJoin;
+virThreadJoinRet;
virThreadMaxName;
virThreadSelf;
virThreadSelfID;
diff --git a/src/util/meson.build b/src/util/meson.build
index 58001a1699..8ea74ff9e8 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -179,6 +179,12 @@ io_helper_sources = [
'runio.h',
]
+multifd_helper_sources = [
+ 'multifd-helper.c',
+ 'runio.c',
+ 'runio.h',
+]
+
virt_util_lib = static_library(
'virt_util',
[
@@ -216,6 +222,13 @@ if conf.has('WITH_LIBVIRTD')
dtrace_gen_headers,
],
}
+ virt_helpers += {
+'name': 'libvirt_multifd_helper',
+'sources': [
+ files(multifd_helper_sources),
+ dtrace_gen_headers,
+],
+ }
endif
util_inc_dir = include_directories('.')
diff --git a/src/util/multifd-helper.c b/src/util/multifd-helper.c
new file mode 100644
index 00..b74f9a4c16
--- /dev/null
+++ b/src/util/multifd-helper.c
@@ -0,0 +1,250 @@
+/*
+ * multifd-helper.c: listens on Unix socket to perform I/O to multiple files
+ *
+ * Copyright (C) 2022 SUSE LLC
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *
[libvirt RFCv4 08/20] remote: Add RPC support for the virDomainSaveParametersFlags API
Signed-off-by: Claudio Fontana
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 17 -
src/remote_protocol-structs | 9 +
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7e7a21fcab..1fc5d41971 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */
.domainSave = remoteDomainSave, /* 0.3.0 */
.domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */
+.domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */
.domainRestore = remoteDomainRestore, /* 0.3.0 */
.domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
.domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 4f13cef662..c2ae5c5748 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64;
/* Upper limit on migrate parameters */
const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
+/* Upper limit on save/restore parameters */
+const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64;
+
/* Upper limit on number of job stats */
const REMOTE_DOMAIN_JOB_STATS_MAX = 64;
@@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args {
int cancelled;
};
+struct remote_domain_save_parameters_flags_args {
+remote_nonnull_domain dom;
+remote_typed_param params;
+unsigned int flags;
+};
+
/* The device removed event is the last event where we have to support
* dual forms for back-compat to older clients; all future events can
* use just the modern form with callbackID. */
@@ -6920,5 +6929,11 @@ enum remote_procedure {
* @generate: both
* @acl: domain:write
*/
-REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439
+REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
+
+/**
+ * @generate: both
+ * @acl: domain:hibernate
+ */
+REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index d88176781d..89eadeb644 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -563,6 +563,14 @@ struct remote_domain_save_flags_args {
remote_string dxml;
u_int flags;
};
+struct remote_domain_save_parameters_flags_args {
+remote_nonnull_domain dom;
+struct {
+u_int params_len;
+remote_typed_param * params_val;
+} params;
+u_int flags;
+};
struct remote_domain_restore_args {
remote_nonnull_string from;
};
@@ -3689,4 +3697,5 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437,
REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
+REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
};
--
2.34.1
[libvirt RFCv4 09/20] remote: Add RPC support for the virDomainRestoreParametersFlags API
Signed-off-by: Claudio Fontana
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 14 +-
src/remote_protocol-structs | 8
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 1fc5d41971..c5b644ce49 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */
.domainRestore = remoteDomainRestore, /* 0.3.0 */
.domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
+.domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /*
8.3.0 */
.domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */
.domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */
.domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index c2ae5c5748..7b919ef375 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args {
unsigned int flags;
};
+struct remote_domain_restore_parameters_flags_args {
+remote_typed_param params;
+unsigned int flags;
+};
+
/* The device removed event is the last event where we have to support
* dual forms for back-compat to older clients; all future events can
* use just the modern form with callbackID. */
@@ -6935,5 +6940,12 @@ enum remote_procedure {
* @generate: both
* @acl: domain:hibernate
*/
-REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
+REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
+
+/**
+ * @generate: both
+ * @acl: domain:start
+ * @acl: domain:write
+ */
+REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 89eadeb644..72e92184ca 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args {
remote_string dxml;
u_int flags;
};
+struct remote_domain_restore_parameters_flags_args {
+struct {
+u_int params_len;
+remote_typed_param * params_val;
+} params;
+u_int flags;
+};
struct remote_domain_save_image_get_xml_desc_args {
remote_nonnull_string file;
u_int flags;
@@ -3698,4 +3705,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
+REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441,
};
--
2.34.1
[libvirt RFCv4 11/20] qemu: add a stub for virDomainRestoreParametersFlags API
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_driver.c | 59 --
1 file changed, 51 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c341df5f8e..7a4e64a118 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5811,12 +5811,13 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn,
return 0;
}
-
static int
-qemuDomainRestoreFlags(virConnectPtr conn,
- const char *path,
- const char *dxml,
- unsigned int flags)
+qemuDomainRestoreInternal(virConnectPtr conn,
+ const char *path,
+ const char *dxml,
+ int nchannels G_GNUC_UNUSED,
+ unsigned int flags,
+ int (*ensureACL)(virConnectPtr, virDomainDef *))
{
virQEMUDriver *driver = conn->privateData;
qemuDomainObjPrivate *priv = NULL;
@@ -5834,7 +5835,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED |
- VIR_DOMAIN_SAVE_RESET_NVRAM, -1);
+ VIR_DOMAIN_SAVE_RESET_NVRAM |
+ VIR_DOMAIN_SAVE_PARALLEL, -1);
if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
reset_nvram = true;
@@ -5845,7 +5847,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
if (fd < 0)
goto cleanup;
-if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
+if (ensureACL(conn, def) < 0)
goto cleanup;
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
@@ -5913,11 +5915,51 @@ qemuDomainRestoreFlags(virConnectPtr conn,
return ret;
}
+static int
+qemuDomainRestoreFlags(virConnectPtr conn,
+ const char *path,
+ const char *dxml,
+ unsigned int flags)
+{
+return qemuDomainRestoreInternal(conn, path, dxml, -1, flags,
+ virDomainRestoreFlagsEnsureACL);
+}
+
static int
qemuDomainRestore(virConnectPtr conn,
const char *path)
{
-return qemuDomainRestoreFlags(conn, path, NULL, 0);
+return qemuDomainRestoreInternal(conn, path, NULL, -1, 0,
+ virDomainRestoreEnsureACL);
+}
+
+static int
+qemuDomainRestoreParametersFlags(virConnectPtr conn,
+ virTypedParameterPtr params, int nparams,
+ unsigned int flags)
+{
+const char *path = NULL;
+const char *dxml = NULL;
+int ret = -1;
+int nchannels = 2;
+
+if (virTypedParamsValidate(params, nparams,
+ VIR_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING,
+ VIR_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING,
+ VIR_SAVE_PARAM_PARALLEL_CONNECTIONS,
VIR_TYPED_PARAM_INT,
+ NULL) < 0)
+return -1;
+
+if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_FILE, &path) <
0)
+return -1;
+if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_DXML, &dxml) <
0)
+return -1;
+if (virTypedParamsGetInt(params, nparams,
VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, &nchannels) < 0)
+return -1;
+
+ret = qemuDomainRestoreInternal(conn, path, dxml, nchannels, flags,
+virDomainRestoreParametersFlagsEnsureACL);
+return ret;
}
static char *
@@ -20884,6 +20926,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainSaveParametersFlags = qemuDomainSaveParametersFlags, /* 8.3.0 */
.domainRestore = qemuDomainRestore, /* 0.2.0 */
.domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */
+.domainRestoreParametersFlags = qemuDomainRestoreParametersFlags, /* 8.3.0
*/
.domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */
.domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */
.domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */
--
2.34.1
[libvirt RFCv4 12/20] qemu: saveimage: introduce virQEMUSaveFd
use this data type to encapsulate the pathname,
file descriptor, wrapper, and need to unlink.
Adapt qemuSaveImageCreate and qemuSaveImageOpen to use a
virQEMUSaveFd instead of a plain file descriptor.
This makes management of wrapper, need_unlink etc
much easier.
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_driver.c| 100 +--
src/qemu/qemu_saveimage.c | 257 +++---
src/qemu/qemu_saveimage.h | 27 +++-
3 files changed, 234 insertions(+), 150 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a4e64a118..6851270be3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5825,12 +5825,13 @@ qemuDomainRestoreInternal(virConnectPtr conn,
virDomainObj *vm = NULL;
g_autofree char *xmlout = NULL;
const char *newxml = dxml;
-int fd = -1;
int ret = -1;
virQEMUSaveData *data = NULL;
-virFileWrapperFd *wrapperFd = NULL;
+virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
bool hook_taint = false;
bool reset_nvram = false;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+int oflags = O_RDONLY;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
VIR_DOMAIN_SAVE_RUNNING |
@@ -5841,10 +5842,18 @@ qemuDomainRestoreInternal(virConnectPtr conn,
if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
reset_nvram = true;
-fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
- (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
- &wrapperFd, false, false);
-if (fd < 0)
+if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) {
+if (virFileDirectFdFlag() < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("bypass cache unsupported by this system"));
+return -1;
+}
+oflags |= O_DIRECT;
+}
+if (virQEMUSaveFdInit(&saveFd, path, 0, oflags, cfg) < 0)
+return -1;
+
+if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
goto cleanup;
if (ensureACL(conn, def) < 0)
@@ -5898,16 +5907,13 @@ qemuDomainRestoreInternal(virConnectPtr conn,
flags) < 0)
goto cleanup;
-ret = qemuSaveImageStartVM(conn, driver, vm, &fd, data, path,
+ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
false, reset_nvram, VIR_ASYNC_JOB_START);
qemuProcessEndJob(vm);
cleanup:
-VIR_FORCE_CLOSE(fd);
-if (virFileWrapperFdClose(wrapperFd) < 0)
-ret = -1;
-virFileWrapperFdFree(wrapperFd);
+ret = virQEMUSaveFdFini(&saveFd, vm, ret);
virQEMUSaveDataFree(data);
if (vm && ret < 0)
qemuDomainRemoveInactive(driver, vm);
@@ -5969,15 +5975,16 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const
char *path,
virQEMUDriver *driver = conn->privateData;
char *ret = NULL;
g_autoptr(virDomainDef) def = NULL;
-int fd = -1;
virQEMUSaveData *data = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
-fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
- false, NULL, false, false);
+if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDONLY, cfg) < 0)
+return NULL;
-if (fd < 0)
+if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
goto cleanup;
if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
@@ -5987,7 +5994,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const
char *path,
cleanup:
virQEMUSaveDataFree(data);
-VIR_FORCE_CLOSE(fd);
+if (virQEMUSaveFdFini(&saveFd, NULL, ret ? 0 : -1) < 0)
+ret = NULL;
return ret;
}
@@ -5999,22 +6007,23 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const
char *path,
int ret = -1;
g_autoptr(virDomainDef) def = NULL;
g_autoptr(virDomainDef) newdef = NULL;
-int fd = -1;
virQEMUSaveData *data = NULL;
+virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
int state = -1;
virCheckFlags(VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
+if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDWR, cfg) < 0)
+return -1;
+
if (flags & VIR_DOMAIN_SAVE_RUNNING)
state = 1;
else if (flags & VIR_DOMAIN_SAVE_PAUSED)
state = 0;
-fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
- false, NULL, true, false);
-
-if (fd < 0)
+if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
goto cleanup;
if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
@@ -6041,15 +6050,15 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const
char *path,
[libvirt RFCv4 10/20] qemu: add a stub for virDomainSaveParametersFlags API
Signed-off-by: Claudio Fontana
---
src/qemu/qemu_driver.c| 66 ---
src/qemu/qemu_saveimage.c | 1 +
src/qemu/qemu_saveimage.h | 1 +
src/qemu/qemu_snapshot.c | 2 +-
4 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ee0963c30d..c341df5f8e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2641,7 +2641,7 @@ static int
qemuDomainSaveInternal(virQEMUDriver *driver,
virDomainObj *vm, const char *path,
int compressed, virCommand *compressor,
- const char *xmlin, unsigned int flags)
+ const char *xmlin, int nconn, unsigned int flags)
{
g_autofree char *xml = NULL;
bool was_running = false;
@@ -2722,7 +2722,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
xml = NULL;
ret = qemuSaveImageCreate(driver, vm, path, data, compressor,
- flags, VIR_ASYNC_JOB_SAVE);
+ nconn, flags, VIR_ASYNC_JOB_SAVE);
if (ret < 0)
goto endjob;
@@ -2791,7 +2791,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path,
const char *dxml,
goto cleanup;
ret = qemuDomainSaveInternal(driver, vm, path, compressed,
- compressor, dxml, flags);
+ compressor, dxml, -1, flags);
cleanup:
virDomainObjEndAPI(&vm);
@@ -2804,6 +2804,63 @@ qemuDomainSave(virDomainPtr dom, const char *path)
return qemuDomainSaveFlags(dom, path, NULL, 0);
}
+static int
+qemuDomainSaveParametersFlags(virDomainPtr dom,
+ virTypedParameterPtr params, int nparams,
+ unsigned int flags)
+{
+const char *to = NULL;
+const char *dxml = NULL;
+virQEMUDriver *driver = dom->conn->privateData;
+int compressed;
+g_autoptr(virCommand) compressor = NULL;
+int ret = -1;
+int nconn = 0;
+virDomainObj *vm = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = NULL;
+
+virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
+ VIR_DOMAIN_SAVE_RUNNING |
+ VIR_DOMAIN_SAVE_PAUSED |
+ VIR_DOMAIN_SAVE_PARALLEL, -1);
+
+if (virTypedParamsValidate(params, nparams,
+ VIR_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING,
+ VIR_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING,
+ VIR_SAVE_PARAM_PARALLEL_CONNECTIONS,
VIR_TYPED_PARAM_INT,
+ NULL) < 0)
+return -1;
+
+if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_FILE, &to) < 0)
+return -1;
+if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_DXML, &dxml) <
0)
+return -1;
+if (virTypedParamsGetInt(params, nparams,
VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, &nconn) < 0)
+return -1;
+
+cfg = virQEMUDriverGetConfig(driver);
+if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
+ &compressor,
+ "save", false)) < 0)
+goto cleanup;
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainSaveParametersFlagsEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+ret = qemuDomainSaveInternal(driver, vm, to, compressed,
+ compressor, dxml, nconn, flags);
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
static char *
qemuDomainManagedSavePath(virQEMUDriver *driver, virDomainObj *vm)
{
@@ -2854,7 +2911,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int
flags)
VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, name);
ret = qemuDomainSaveInternal(driver, vm, name, compressed,
- compressor, NULL, flags);
+ compressor, NULL, -1, flags);
if (ret == 0)
vm->hasManagedSave = true;
@@ -20824,6 +20881,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainGetControlInfo = qemuDomainGetControlInfo, /* 0.9.3 */
.domainSave = qemuDomainSave, /* 0.2.0 */
.domainSaveFlags = qemuDomainSaveFlags, /* 0.9.4 */
+.domainSaveParametersFlags = qemuDomainSaveParametersFlags, /* 8.3.0 */
.domainRestore = qemuDomainRestore, /* 0.2.0 */
.domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */
.domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 4fd4c5cfcd..7c76db359e 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -258,6 +258,7 @@ qemuSaveImageCreate(virQEMUDriver *driver,
const char *path,
[libvirt RFCv4 07/20] libvirt: introduce virDomainRestoreParametersFlags public API
add new API in order to be able to extend parameters to the domain
restore operation. We will use it to fit the existing arguments of
VirDomainRestoreFlags, and then add parallel restore functionality.
Signed-off-by: Claudio Fontana
---
include/libvirt/libvirt-domain.h | 4 +++
src/driver-hypervisor.h | 7 +
src/libvirt-domain.c | 48
src/libvirt_public.syms | 1 +
4 files changed, 60 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b870a73b64..ec178a8150 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1501,6 +1501,10 @@ int virDomainRestoreFlags
(virConnectPtr conn,
const char *from,
const char *dxml,
unsigned int flags);
+int virDomainRestoreParametersFlags (virConnectPtr conn,
+ virTypedParameterPtr
params,
+ int nparams,
+ unsigned int flags);
/**
* VIR_SAVE_PARAM_FILE:
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index a4e1d21e76..e62e4c8f74 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -256,6 +256,12 @@ typedef int
const char *dxml,
unsigned int flags);
+typedef int
+(*virDrvDomainRestoreParametersFlags)(virConnectPtr conn,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags);
+
typedef char *
(*virDrvDomainSaveImageGetXMLDesc)(virConnectPtr conn,
const char *file,
@@ -1498,6 +1504,7 @@ struct _virHypervisorDriver {
virDrvDomainSaveParametersFlags domainSaveParametersFlags;
virDrvDomainRestore domainRestore;
virDrvDomainRestoreFlags domainRestoreFlags;
+virDrvDomainRestoreParametersFlags domainRestoreParametersFlags;
virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc;
virDrvDomainSaveImageDefineXML domainSaveImageDefineXML;
virDrvDomainCoreDump domainCoreDump;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9e4fcfd022..f967efa473 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1140,6 +1140,54 @@ virDomainRestoreFlags(virConnectPtr conn, const char
*from, const char *dxml,
}
+/**
+ * virDomainRestoreParametersFlags:
+ * @conn: pointer to the hypervisor connection
+ * @params: restore parameters
+ * @nparams: number of restore parameters
+ * @flags: bitwise-OR of virDomainSaveRestoreFlags
+ *
+ * This method extends virDomainRestoreFlags by adding parameters to Restore.
+ *
+ * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will
+ * attempt to restore from multiple files in parallel,
+ * where the number of extra files is determined by the parameter
+ * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virDomainRestoreParametersFlags(virConnectPtr conn,
+virTypedParameterPtr params, int nparams,
+unsigned int flags)
+{
+VIR_DEBUG("conn=%p, params=%p, nparams=%d, flags=0x%x",
+ conn, params, nparams, flags);
+VIR_TYPED_PARAMS_DEBUG(params, nparams);
+
+virResetLastError();
+
+virCheckConnectReturn(conn, -1);
+virCheckReadOnlyGoto(conn->flags, error);
+
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
+
+if (conn->driver->domainRestoreParametersFlags) {
+if (conn->driver->domainRestoreParametersFlags(conn, params, nparams,
flags) < 0)
+goto error;
+return 0;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(conn);
+return -1;
+}
+
+
/**
* virDomainSaveImageGetXMLDesc:
* @conn: pointer to the hypervisor connection
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index eb3a7afb75..74c1464b38 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -919,6 +919,7 @@ LIBVIRT_8.0.0 {
LIBVIRT_8.3.0 {
global:
virDomainSaveParametersFlags;
+virDomainRestoreParametersFlags;
} LIBVIRT_8.0.0;
# define new API here using predicted next version number
--
2.34.1
[libvirt RFCv4 04/20] runio: add arguments to extend use beyond just stdin and stdout
add arguments to runio to allow read/write from/to arbitrary
file descriptors, as opposed to just stdin and stdout.
Signed-off-by: Claudio Fontana
---
src/util/iohelper.c | 2 +-
src/util/runio.c| 10 +-
src/util/runio.h| 17 -
3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 5a0098542e..93674c1e2f 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -96,7 +96,7 @@ main(int argc, char **argv)
usage(EXIT_FAILURE);
}
-if (fd < 0 || runIO(path, fd, oflags) < 0)
+if (fd < 0 || runIO(path, fd, oflags, STDIN_FILENO, STDOUT_FILENO) < 0)
goto error;
return 0;
diff --git a/src/util/runio.c b/src/util/runio.c
index a7b902af7e..f42acddae9 100644
--- a/src/util/runio.c
+++ b/src/util/runio.c
@@ -134,7 +134,7 @@ runIOCopy(const struct runIOParams p)
off_t
-runIO(const char *path, int fd, int oflags)
+runIO(const char *path, int fd, int oflags, int in_fd, int out_fd)
{
int ret = -1;
off_t total = 0;
@@ -155,13 +155,13 @@ runIO(const char *path, int fd, int oflags)
p.isWrite = false;
p.fdin = fd;
p.fdinname = path;
-p.fdout = STDOUT_FILENO;
-p.fdoutname = "stdout";
+p.fdout = out_fd;
+p.fdoutname = "output";
break;
case O_WRONLY:
p.isWrite = true;
-p.fdin = STDIN_FILENO;
-p.fdinname = "stdin";
+p.fdin = in_fd;
+p.fdinname = "input";
p.fdout = fd;
p.fdoutname = path;
break;
diff --git a/src/util/runio.h b/src/util/runio.h
index beb58606c9..66df51 100644
--- a/src/util/runio.h
+++ b/src/util/runio.h
@@ -20,4 +20,19 @@
#pragma once
-off_t runIO(const char *path, int fd, int oflags);
+/*
+ * runIO: copy unidirectionally all data to/from a file descriptor.
+ *
+ * @path: the pathname corresponding to FD.
+ * @fd: the file descriptor to read from or write to.
+ * @oflags: the file status flags of FD.
+ *
+ * If the oflags indicate O_RDONLY, then the direction will be from FD,
+ * and @out_fd indicates the file to write to.
+ *
+ * If the oflags indicate O_WRONLY, then the direction will be to FD,
+ * and @in_fd indicates the file to read from.
+ *
+ * Returns the number of bytes transferred, or < 0 on error.
+ */
+off_t runIO(const char *path, int fd, int oflags, int in_fd, int out_fd);
--
2.34.1
[libvirt RFCv4 03/20] iohelper: move runIO function to a separate module
where it can be reused by other helpers.
No changes other than the move.
Signed-off-by: Claudio Fontana
---
po/POTFILES.in | 1 +
src/util/iohelper.c | 178 +--
src/util/meson.build | 2 +
src/util/runio.c | 214 +++
src/util/runio.h | 23 +
5 files changed, 241 insertions(+), 177 deletions(-)
create mode 100644 src/util/runio.c
create mode 100644 src/util/runio.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0d9adb0758..5b4c00d7ac 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -241,6 +241,7 @@
@SRCDIR@src/storage_file/storage_source_backingstore.c
@SRCDIR@src/test/test_driver.c
@SRCDIR@src/util/iohelper.c
+@SRCDIR@src/util/runio.c
@SRCDIR@src/util/viralloc.c
@SRCDIR@src/util/virarptable.c
@SRCDIR@src/util/viraudit.c
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 1584321839..5a0098542e 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -38,183 +38,7 @@
#include "virrandom.h"
#include "virstring.h"
#include "virgettext.h"
-
-#define VIR_FROM_THIS VIR_FROM_STORAGE
-
-#ifndef O_DIRECT
-# define O_DIRECT 0
-#endif
-
-struct runIOParams {
-bool isBlockDev;
-bool isDirect;
-bool isWrite;
-int fdin;
-const char *fdinname;
-int fdout;
-const char *fdoutname;
-};
-
-/**
- * runIOCopy: execute the IO copy based on the passed parameters
- * @p: the IO parameters
- *
- * Execute the copy based on the passed parameters.
- *
- * Returns: size transfered, or < 0 on error.
- */
-
-static off_t
-runIOCopy(const struct runIOParams p)
-{
-g_autofree void *base = NULL; /* Location to be freed */
-char *buf = NULL; /* Aligned location within base */
-size_t buflen = 1024*1024;
-intptr_t alignMask = 64*1024 - 1;
-off_t total = 0;
-
-#if WITH_POSIX_MEMALIGN
-if (posix_memalign(&base, alignMask + 1, buflen))
-abort();
-buf = base;
-#else
-buf = g_new0(char, buflen + alignMask);
-base = buf;
-buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
-#endif
-
-while (1) {
-ssize_t got;
-
-/* If we read with O_DIRECT from file we can't use saferead as
- * it can lead to unaligned read after reading last bytes.
- * If we write with O_DIRECT use should use saferead so that
- * writes will be aligned.
- * In other cases using saferead reduces number of syscalls.
- */
-if (!p.isWrite && p.isDirect) {
-if ((got = read(p.fdin, buf, buflen)) < 0 &&
-errno == EINTR)
-continue;
-} else {
-got = saferead(p.fdin, buf, buflen);
-}
-
-if (got < 0) {
-virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
-return -2;
-}
-if (got == 0)
-break;
-
-total += got;
-
-/* handle last write size align in direct case */
-if (got < buflen && p.isDirect && p.isWrite) {
-ssize_t aligned_got = (got + alignMask) & ~alignMask;
-
-memset(buf + got, 0, aligned_got - got);
-
-if (safewrite(p.fdout, buf, aligned_got) < 0) {
-virReportSystemError(errno, _("Unable to write %s"),
p.fdoutname);
-return -3;
-}
-
-if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
-virReportSystemError(errno, _("Unable to truncate %s"),
p.fdoutname);
-return -4;
-}
-
-break;
-}
-
-if (safewrite(p.fdout, buf, got) < 0) {
-virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
-return -3;
-}
-}
-return total;
-}
-
-static int
-runIO(const char *path, int fd, int oflags)
-{
-int ret = -1;
-off_t total = 0;
-struct stat sb;
-struct runIOParams p;
-
-if (fstat(fd, &sb) < 0) {
-virReportSystemError(errno,
- _("Unable to access file descriptor %d path %s"),
- fd, path);
-goto cleanup;
-}
-p.isBlockDev = S_ISBLK(sb.st_mode);
-p.isDirect = O_DIRECT && (oflags & O_DIRECT);
-
-switch (oflags & O_ACCMODE) {
-case O_RDONLY:
-p.isWrite = false;
-p.fdin = fd;
-p.fdinname = path;
-p.fdout = STDOUT_FILENO;
-p.fdoutname = "stdout";
-break;
-case O_WRONLY:
-p.isWrite = true;
-p.fdin = STDIN_FILENO;
-p.fdinname = "stdin";
-p.fdout = fd;
-p.fdoutname = path;
-break;
-
-case O_RDWR:
-default:
-virReportSystemError(EINVAL,
- _("Unable to process file with flags %d"),
- (oflags & O_ACCMODE));
-goto cleanup;
-}
-/* To make the implementation simpler, we give up on any
- * attempt to use O_DIRECT in a non-trivial manner. */
-if (!p.isBlockDev
[libvirt RFCv4 00/20] multifd save restore prototype
This is the multifd save prototype in semi-functional state, with both save and restore minimally functional. There are still quite a few rough edges. KNOWN ISSUES: 1) this applies only to virsh save and virsh restore for now (no managed save etc). 2) the .pl scripts to generate the headers for the new APIs do not reliably work for me, for the Restore case. I get: src/remote/remote_daemon_dispatch_stubs.h:10080:9: error: too few arguments to function ‘virDomainRestoreParametersFlags’ if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0) To work around this I had to fixup the header manually to add the conn parameter like this: ...(conn, params, nparams, args->flags) < 0) 3) error handling is not good yet, especially during resume, errors may leave behind a qemu process and such. May need some help find all of these cases... ... changes from v3: * reordered series to have all helper-related change at the start * solved all reported issues from ninja test, including documentation * fixed most broken migration capabilities code (still imperfect likely) * added G_GNUC_UNUSED as needed * after multifd restore, added what I think were the missing operations: qemuProcessRefreshState(), qemuProcessStartCPUs() - most importantly, virDomainObjSave() The domain now starts running after restore without further encouragement * removed the sleep(10) from the multifd-helper changes from v2: * added ability to restore the VM from disk using multifd * fixed the multifd-helper to work in both directions, assuming the need to listen for save, and connect for restore. * fixed a large number of bugs, and probably introduced some :-) Thanks for your thoughts, Claudio *** BLURB HERE *** Claudio Fontana (20): iohelper: introduce new struct to carry copy operation parameters iohelper: refactor copy operation as a separate function iohelper: move runIO function to a separate module runio: add arguments to extend use beyond just stdin and stdout multifd-helper: new helper for parallel save/restore libvirt: introduce virDomainSaveParametersFlags public API libvirt: introduce virDomainRestoreParametersFlags public API remote: Add RPC support for the virDomainSaveParametersFlags API remote: Add RPC support for the virDomainRestoreParametersFlags API qemu: add a stub for virDomainSaveParametersFlags API qemu: add a stub for virDomainRestoreParametersFlags API qemu: saveimage: introduce virQEMUSaveFd qemu: wire up saveimage code with the multifd helper qemu: capabilities: add multifd to the probed migration capabilities qemu: implement qemuMigrationSrcToFilesMultiFd qemu: add parameter to qemuMigrationDstRun to skip waiting qemu: implement qemuSaveImageLoadMultiFd tools: add parallel parameter to virsh save command tools: add parallel parameter to virsh restore command qemu: add migration parameter multifd-compression docs/manpages/virsh.rst | 34 +- include/libvirt/libvirt-domain.h | 49 ++ po/POTFILES.in| 2 + src/driver-hypervisor.h | 14 + src/libvirt-domain.c | 99 src/libvirt_private.syms | 1 + src/libvirt_public.syms | 6 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_driver.c| 239 +++-- src/qemu/qemu_migration.c | 155 -- src/qemu/qemu_migration.h | 16 +- src/qemu/qemu_migration_params.c | 71 ++- src/qemu/qemu_migration_params.h | 15 + src/qemu/qemu_process.c | 3 +- src/qemu/qemu_process.h | 5 +- src/qemu/qemu_saveimage.c | 496 ++ src/qemu/qemu_saveimage.h | 49 +- src/qemu/qemu_snapshot.c | 6 +- src/remote/remote_driver.c| 2 + src/remote/remote_protocol.x | 29 +- src/remote_protocol-structs | 17 + src/util/iohelper.c | 150 +- src/util/meson.build | 15 + src/util/multifd-helper.c | 250 + src/util/runio.c | 214 src/util/runio.h | 38 ++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + .../caps_4.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml| 1 + .../caps_4.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + ..
Re: [libvirt RFC v3 06/19] remote: Add RPC support for the virDomainRestoreParametersFlags API
On 4/27/22 1:00 AM, Jim Fehlig wrote:
> On 4/26/22 10:47, Claudio Fontana wrote:
>> Signed-off-by: Claudio Fontana
>> ---
>> src/remote/remote_driver.c | 1 +
>> src/remote/remote_protocol.x | 14 +-
>> src/remote_protocol-structs | 8
>> 3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 1fc5d41971..c5b644ce49 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = {
>> .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0
>> */
>> .domainRestore = remoteDomainRestore, /* 0.3.0 */
>> .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
>> +.domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /*
>> 8.3.0 */
>> .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4
>> */
>> .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */
>> .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */
>> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
>> index c2ae5c5748..7b919ef375 100644
>> --- a/src/remote/remote_protocol.x
>> +++ b/src/remote/remote_protocol.x
>> @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args {
>> unsigned int flags;
>> };
>>
>> +struct remote_domain_restore_parameters_flags_args {
>> +remote_typed_param params;
>> +unsigned int flags;
>> +};
>> +
>> /* The device removed event is the last event where we have to support
>>* dual forms for back-compat to older clients; all future events can
>>* use just the modern form with callbackID. */
>> @@ -6935,5 +6940,12 @@ enum remote_procedure {
>>* @generate: both
>>* @acl: domain:hibernate
>>*/
>> -REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
>> +REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
>> +
>> +/**
>> + * @generate: both
>> + * @acl: domain:start
>> + * @acl: domain:write
>> + */
>> +REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
>
> I've stared at this for quite a while but can't figure out why the dispatch
> stub
> does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting
> the
> following build failure
>
> In file included from ../src/remote/remote_daemon_dispatch.c:133:
>
> src/remote/remote_daemon_dispatch_stubs.h: In function
> ‘remoteDispatchDomainRestoreParametersFlags’:
>
> src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1
> of
> ‘virDomainRestoreParametersFlags’ from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>
> 10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags)
> < 0)
>
>| ^~
>
>| |
>
>| virTypedParameterPtr {aka
> struct _virTypedParameter *}
>
> In file included from ../include/libvirt/libvirt.h:36,
>
> from ../src/internal.h:65,
>
> from ../src/util/virerror.h:24,
>
> from ../src/remote/remote_daemon_dispatch.c:23:
>
> ../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’
> {aka
> ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka
> ‘struct _virTypedParameter *’}
>
> 1576 | int virDomainRestoreParametersFlags
> (virConnectPtr
> conn,
>
>
> Perhaps a bug in gendispatch.pl. I'm not familiar with the script or
> debugging
> it, but others here can likely provide help.
>
> Jim
>
Still fighting this one, could not defeat the beast yet..
>> };
>> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
>> index 89eadeb644..72e92184ca 100644
>> --- a/src/remote_protocol-structs
>> +++ b/src/remote_protocol-structs
>> @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args {
>> remote_string dxml;
>> u_int flags;
>> };
>> +struct remote_domain_restore_parameters_flags_args {
>> +struct {
>> +u_int params_len;
>> +remote_typed_param * params_val;
>> +} params;
>> +u_int flags;
>> +};
>> struct remote_domain_save_image_get_xml_desc_args {
>> remote_nonnull_string file;
>> u_int flags;
>> @@ -3698,4 +3705,5 @@ enum remote_procedure {
>> REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
>> REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
>> REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
>> +REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441,
>> };
>
Re: [libvirt RFC v3 05/19] remote: Add RPC support for the virDomainSaveParametersFlags API
On 4/27/22 12:47 AM, Jim Fehlig wrote:
> On 4/26/22 10:47, Claudio Fontana wrote:
>> Signed-off-by: Claudio Fontana
>> ---
>> src/remote/remote_driver.c | 1 +
>> src/remote/remote_protocol.x | 17 -
>> src/remote_protocol-structs | 9 +
>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 7e7a21fcab..1fc5d41971 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = {
>> .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */
>> .domainSave = remoteDomainSave, /* 0.3.0 */
>> .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */
>> +.domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0
>> */
>> .domainRestore = remoteDomainRestore, /* 0.3.0 */
>> .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
>> .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4
>> */
>> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
>> index 4f13cef662..c2ae5c5748 100644
>> --- a/src/remote/remote_protocol.x
>> +++ b/src/remote/remote_protocol.x
>> @@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64;
>> /* Upper limit on migrate parameters */
>> const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
>>
>> +/* Upper limit on save/restore parameters */
>> +const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64;
>> +
>> /* Upper limit on number of job stats */
>> const REMOTE_DOMAIN_JOB_STATS_MAX = 64;
>>
>> @@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args {
>> int cancelled;
>> };
>>
>> +struct remote_domain_save_parameters_flags_args {
>> +remote_nonnull_domain dom;
>> +remote_typed_param params;
>> +unsigned int flags;
>> +};
>> +
>> /* The device removed event is the last event where we have to support
>>* dual forms for back-compat to older clients; all future events can
>>* use just the modern form with callbackID. */
>> @@ -6920,5 +6929,11 @@ enum remote_procedure {
>>* @generate: both
>>* @acl: domain:write
>>*/
>> -REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439
>> +REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
>> +
>> +/**
>> + * @generate: both
>> + * @acl: domain:hibernate
>> + */
>> +REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
>> };
>> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
>> index d88176781d..89eadeb644 100644
>> --- a/src/remote_protocol-structs
>> +++ b/src/remote_protocol-structs
>> @@ -563,6 +563,14 @@ struct remote_domain_save_flags_args {
>> remote_string dxml;
>> u_int flags;
>> };
>> +struct remote_domain_save_parameters_flags_args {
>> +remote_nonnull_domain dom;
>> +struct {
>> +u_int params_len;
>> +remote_typed_param * params_val;
>> +} params;
>> +u_int flags;
>> +};
>
> 'ninja test' fails here, wanting this addition moved later in the file
>
> 41/314 libvirt / check-remote_protocol
> FAIL0.19s exit status 1
> >>> MALLOC_PERTURB_=253 LANG=C LC_ALL='' /usr/bin/python3
> /home/jfehlig/virt/gitlab/libvirt/scripts/check-remote-protocol.py
> remote_protocol virt_remote_driver
> /home/jfehlig/virt/gitlab/libvirt/build/src/remote/libvirt_remote_driver.a
> /usr/bin/pdwtags
> /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs
> --- /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs
> 2022-04-26 15:57:29.515818322 -0600
> +++ - 2022-04-26 15:58:12.933537465 -0600
> @@ -563,14 +563,6 @@
> remote_string dxml;
> u_int flags;
> };
> -struct remote_domain_save_parameters_flags_args {
> -remote_nonnull_domain dom;
> -struct {
> -u_int params_len;
> -remote_typed_param * params_val;
> -} params;
> -u_int flags;
> -};
> struct remote_domain_restore_args {
> remote_nonnull_string from;
> };
> @@ -2609,6 +2601,14 @@
> u_int flags;
> intcancelled;
> };
> +struct remote_domain_save_parameters_flags_args {
> +remote_nonnull_domain dom;
> +struct {
> +u_int params_len;
> +remote_typed_param * params_val;
> +} params;
> +u_int flags;
> +};
> struct remote_domain_event_device_removed_msg {
> remote_nonnull_domain dom;
> remote_nonnull_string devAlias;
> 41/314 libvirt / check-remote_protocol
> FAIL0.19s exit sta
Re: [libvirt RFC v3 03/19] libvirt: introduce virDomainSaveParametersFlags public API
On 4/27/22 12:42 AM, Jim Fehlig wrote:
> On 4/26/22 10:47, Claudio Fontana wrote:
>> add new API in order to be able to extend parameters to the domain
>> save operation. We will use it to fit the existing arguments of
>> VirDomainSaveFlags, and then add parallel saves functionality.
>>
>> Signed-off-by: Claudio Fontana
>> ---
>> include/libvirt/libvirt-domain.h | 9 ++
>> src/driver-hypervisor.h | 7 +
>> src/libvirt-domain.c | 51
>> src/libvirt_public.syms | 5
>> 4 files changed, 72 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h
>> b/include/libvirt/libvirt-domain.h
>> index 9aa214f3df..4beea34f93 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -1481,6 +1481,7 @@ typedef enum {
>> VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */
>> VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */
>> VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from
>> template */
>> +VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to
>> multiple files */
>> } virDomainSaveRestoreFlags;
>>
>> int virDomainSave (virDomainPtr domain,
>> @@ -1489,6 +1490,10 @@ int virDomainSaveFlags
>> (virDomainPtr domain,
>>const char *to,
>>const char *dxml,
>>unsigned int flags);
>> +int virDomainSaveParametersFlags (virDomainPtr domain,
>> + virTypedParameterPtr
>> params,
>> + int nparams,
>> + unsigned int flags);
>> int virDomainRestore(virConnectPtr conn,
>>const char *from);
>> int virDomainRestoreFlags (virConnectPtr conn,
>> @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags
>> (virConnectPtr conn,
>>const char *dxml,
>>unsigned int flags);
>>
>> +# define VIR_SAVE_PARAM_FILE "file"
>> +# define VIR_SAVE_PARAM_DXML "dxml"
>> +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
>
> The common pattern is to use '_' in multi-word parameter names, i.e.
> "parallel_connections". Also, this fails to build the API docs
>
> [499/508] Generating docs/generate-api with a custom command
> FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml
> docs/libvirt-qemu-api.xml
> docs/libvirt-admin-api.xml
> /home/jfehlig/virt/gitlab/libvirt/scripts/meson-python.sh /usr/bin/python3
> /home/jfehlig/virt/gitlab/libvirt/scripts/apibuild.py
> /home/jfehlig/virt/gitlab/libvirt/docs
> /home/jfehlig/virt/gitlab/libvirt/build/docs
> Misformatted macro comment for VIR_SAVE_PARAM_FILE
>Expecting '* VIR_SAVE_PARAM_FILE:' got '* virDomainSaveRestoreFlags:'
> Misformatted macro comment for VIR_SAVE_PARAM_DXML
>Expecting '* VIR_SAVE_PARAM_DXML:' got '* virDomainSaveRestoreFlags:'
> Misformatted macro comment for VIR_SAVE_PARAM_PARALLEL_CONNECTIONS
>Expecting '* VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:' got '*
> virDomainSaveRestoreFlags:'
> Missing 'Since' tag for: VIR_SAVE_PARAM_DXML
> Missing 'Since' tag for: VIR_SAVE_PARAM_FILE
> Missing 'Since' tag for: VIR_SAVE_PARAM_PARALLEL_CONNECTIONS
> Missing 'Since' tag for: VIR_DOMAIN_SAVE_PARALLEL
> Missing 'Since' tag for: virDomainSaveParametersFlags
>
>> +
>> /* See below for virDomainSaveImageXMLFlags */
>> char * virDomainSaveImageGetXMLDesc(virConnectPtr conn,
>>const char *file,
>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index 4423eb0885..a4e1d21e76 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -240,6 +240,12 @@ typedef int
>>const char *dxml,
>>unsigned int flags);
>>
>> +typedef int
>> +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain,
>> + virTypedParameterPtr params,
>> + int nparams,
>> + unsigned int flags);
>> +
>> typedef int
>> (*virDrvDomainRestore)(virConnectPtr conn,
>> const char *from);
>> @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver {
>> virDrvDomainGetControlInfo domainGetControlInfo;
>> virDrvDomainSave domainSave;
>> virDrvDomainSaveFlags domainSaveFlags;
>> +virDrvDomainSaveParametersFlags domainSaveParametersFlag
Re: [PATCH] NEWS: Mention bump of minimum qemu version to qemu-3.1
On a Wednesday in 2022, Peter Krempa wrote: Signed-off-by: Peter Krempa --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH] NEWS: Mention bump of minimum qemu version to qemu-3.1
Signed-off-by: Peter Krempa --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 7ad8b9ec0b..c9b0ad42ed 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -15,6 +15,11 @@ v8.3.0 (unreleased) * **Removed features** + * qemu: Remove support for QEMU < 3.1 + +In accordance with our platform support policy, the oldest supported QEMU +version is now bumped from 2.11 to 3.1. + * **New features** * qemu: Introduce support for virtio-iommu -- 2.35.1
Re: [libvirt RFC v3 06/19] remote: Add RPC support for the virDomainRestoreParametersFlags API
On 4/27/22 1:00 AM, Jim Fehlig wrote:
> On 4/26/22 10:47, Claudio Fontana wrote:
>> Signed-off-by: Claudio Fontana
>> ---
>> src/remote/remote_driver.c | 1 +
>> src/remote/remote_protocol.x | 14 +-
>> src/remote_protocol-structs | 8
>> 3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 1fc5d41971..c5b644ce49 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = {
>> .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0
>> */
>> .domainRestore = remoteDomainRestore, /* 0.3.0 */
>> .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
>> +.domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /*
>> 8.3.0 */
>> .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4
>> */
>> .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */
>> .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */
>> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
>> index c2ae5c5748..7b919ef375 100644
>> --- a/src/remote/remote_protocol.x
>> +++ b/src/remote/remote_protocol.x
>> @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args {
>> unsigned int flags;
>> };
>>
>> +struct remote_domain_restore_parameters_flags_args {
>> +remote_typed_param params;
>> +unsigned int flags;
>> +};
>> +
>> /* The device removed event is the last event where we have to support
>>* dual forms for back-compat to older clients; all future events can
>>* use just the modern form with callbackID. */
>> @@ -6935,5 +6940,12 @@ enum remote_procedure {
>>* @generate: both
>>* @acl: domain:hibernate
>>*/
>> -REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
>> +REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
>> +
>> +/**
>> + * @generate: both
>> + * @acl: domain:start
>> + * @acl: domain:write
>> + */
>> +REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
>
> I've stared at this for quite a while but can't figure out why the dispatch
> stub
> does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting
> the
> following build failure
>
> In file included from ../src/remote/remote_daemon_dispatch.c:133:
>
> src/remote/remote_daemon_dispatch_stubs.h: In function
> ‘remoteDispatchDomainRestoreParametersFlags’:
>
> src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1
> of
> ‘virDomainRestoreParametersFlags’ from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>
> 10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags)
> < 0)
>
>| ^~
>
>| |
>
>| virTypedParameterPtr {aka
> struct _virTypedParameter *}
>
> In file included from ../include/libvirt/libvirt.h:36,
>
> from ../src/internal.h:65,
>
> from ../src/util/virerror.h:24,
>
> from ../src/remote/remote_daemon_dispatch.c:23:
>
> ../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’
> {aka
> ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka
> ‘struct _virTypedParameter *’}
>
> 1576 | int virDomainRestoreParametersFlags
> (virConnectPtr
> conn,
>
>
> Perhaps a bug in gendispatch.pl. I'm not familiar with the script or
> debugging
> it, but others here can likely provide help.
Indeed, same issue I get.
Thanks again,
Claudio
>
> Jim
>
>> };
>> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
>> index 89eadeb644..72e92184ca 100644
>> --- a/src/remote_protocol-structs
>> +++ b/src/remote_protocol-structs
>> @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args {
>> remote_string dxml;
>> u_int flags;
>> };
>> +struct remote_domain_restore_parameters_flags_args {
>> +struct {
>> +u_int params_len;
>> +remote_typed_param * params_val;
>> +} params;
>> +u_int flags;
>> +};
>> struct remote_domain_save_image_get_xml_desc_args {
>> remote_nonnull_string file;
>> u_int flags;
>> @@ -3698,4 +3705,5 @@ enum remote_procedure {
>> REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438,
>> REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439,
>> REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
>> +REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441,
>> };
>
Re: [libvirt RFC v3 03/19] libvirt: introduce virDomainSaveParametersFlags public API
Thanks for looking at this,
On 4/27/22 12:42 AM, Jim Fehlig wrote:
> On 4/26/22 10:47, Claudio Fontana wrote:
>> add new API in order to be able to extend parameters to the domain
>> save operation. We will use it to fit the existing arguments of
>> VirDomainSaveFlags, and then add parallel saves functionality.
>>
>> Signed-off-by: Claudio Fontana
>> ---
>> include/libvirt/libvirt-domain.h | 9 ++
>> src/driver-hypervisor.h | 7 +
>> src/libvirt-domain.c | 51
>> src/libvirt_public.syms | 5
>> 4 files changed, 72 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h
>> b/include/libvirt/libvirt-domain.h
>> index 9aa214f3df..4beea34f93 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -1481,6 +1481,7 @@ typedef enum {
>> VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */
>> VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */
>> VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from
>> template */
>> +VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to
>> multiple files */
>> } virDomainSaveRestoreFlags;
>>
>> int virDomainSave (virDomainPtr domain,
>> @@ -1489,6 +1490,10 @@ int virDomainSaveFlags
>> (virDomainPtr domain,
>>const char *to,
>>const char *dxml,
>>unsigned int flags);
>> +int virDomainSaveParametersFlags (virDomainPtr domain,
>> + virTypedParameterPtr
>> params,
>> + int nparams,
>> + unsigned int flags);
>> int virDomainRestore(virConnectPtr conn,
>>const char *from);
>> int virDomainRestoreFlags (virConnectPtr conn,
>> @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags
>> (virConnectPtr conn,
>>const char *dxml,
>>unsigned int flags);
>>
>> +# define VIR_SAVE_PARAM_FILE "file"
>> +# define VIR_SAVE_PARAM_DXML "dxml"
>> +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
>
> The common pattern is to use '_' in multi-word parameter names, i.e.
I used the same parameter as per:
libvirt/libvirt-domain.h:# define VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS
"parallel.connections"
since it is basically the name parameter and meaning, just now used for save
and restore, maybe it make sense to keep it as-is?
> "parallel_connections". Also, this fails to build the API docs
>
> [499/508] Generating docs/generate-api with a custom command
> FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml
> docs/libvirt-qemu-api.xml
> docs/libvirt-admin-api.xml
> /home/jfehlig/virt/gitlab/libvirt/scripts/meson-python.sh /usr/bin/python3
> /home/jfehlig/virt/gitlab/libvirt/scripts/apibuild.py
> /home/jfehlig/virt/gitlab/libvirt/docs
> /home/jfehlig/virt/gitlab/libvirt/build/docs
> Misformatted macro comment for VIR_SAVE_PARAM_FILE
>Expecting '* VIR_SAVE_PARAM_FILE:' got '* virDomainSaveRestoreFlags:'
> Misformatted macro comment for VIR_SAVE_PARAM_DXML
>Expecting '* VIR_SAVE_PARAM_DXML:' got '* virDomainSaveRestoreFlags:'
> Misformatted macro comment for VIR_SAVE_PARAM_PARALLEL_CONNECTIONS
>Expecting '* VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:' got '*
> virDomainSaveRestoreFlags:'
> Missing 'Since' tag for: VIR_SAVE_PARAM_DXML
> Missing 'Since' tag for: VIR_SAVE_PARAM_FILE
> Missing 'Since' tag for: VIR_SAVE_PARAM_PARALLEL_CONNECTIONS
> Missing 'Since' tag for: VIR_DOMAIN_SAVE_PARALLEL
> Missing 'Since' tag for: virDomainSaveParametersFlags
Thanks, will fix.
>
>> +
>> /* See below for virDomainSaveImageXMLFlags */
>> char * virDomainSaveImageGetXMLDesc(virConnectPtr conn,
>>const char *file,
>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index 4423eb0885..a4e1d21e76 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -240,6 +240,12 @@ typedef int
>>const char *dxml,
>>unsigned int flags);
>>
>> +typedef int
>> +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain,
>> + virTypedParameterPtr params,
>> + int nparams,
>> + unsigned int flags);
>> +
>> typedef int
>> (*virDrvDomainRestore)(virConnectPt
Re: [PATCH 01/18] hw/audio: Remove -soundhw support
On 4/25/22 10:21, Martin Kletzander wrote: One thing I am not sure about is whether to keep the aliases of ac97 and es1370 in the qdev_alias_table. Signed-off-by: Martin Kletzander I agree that -soundhw is a bad option, but I think we should preserve something similarly easy to use. For example: -audio OPTIONS,model=XXX would expand to -audiodev OPTIONS,id=audiodev0 and then call the same init_isa/init_pci functions that already exist; except that those would add an audiodev=audiodev0 property. This way, one can use "-audio pa,model=hda". Later on we can add a hook in the machines for the case when no model is specified, but you don't need to do that now because it's backwards compatible. Let me post a sketch after lunch. Paolo --- docs/about/deprecated.rst | 9 - docs/about/removed-features.rst | 10 + docs/qdev-device-use.txt | 21 +-- docs/replay.txt | 2 +- hw/audio/ac97.c | 3 - hw/audio/adlib.c | 2 - hw/audio/cs4231a.c| 2 - hw/audio/es1370.c | 3 - hw/audio/gus.c| 2 - hw/audio/intel-hda.c | 21 --- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 11 -- hw/audio/sb16.c | 3 - hw/audio/soundhw.c| 177 -- include/hw/audio/soundhw.h| 15 -- qemu-options.hx | 27 --- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c| 2 - softmmu/vl.c | 6 - 19 files changed, 19 insertions(+), 299 deletions(-) delete mode 100644 hw/audio/soundhw.c delete mode 100644 include/hw/audio/soundhw.h diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index cf02ef6821e4..7ba71ebd3435 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -39,15 +39,6 @@ should specify an ``audiodev=`` property. Additionally, when using vnc, you should specify an ``audiodev=`` property if you plan to transmit audio through the VNC protocol. -Creating sound card devices using ``-soundhw`` (since 5.1) -'' - -Sound card devices should be created using ``-device`` instead. The -names are the same for most devices. The exceptions are ``hda`` which -needs two devices (``-device intel-hda -device hda-duplex``) and -``pcspk`` which can be activated using ``-machine -pcspk-audiodev=``. - ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 4b831ea29176..086ba3edb042 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine. The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which should be used instead. +Creating sound card devices using ``-soundhw`` (removed in 7.1) +''' + +Sound card devices should be created using ``-device`` instead. The +names are the same for most devices. The exceptions are ``hda`` which +needs two devices (``-device intel-hda -device hda-duplex``) and +``pcspk`` which can be activated using ``-machine +pcspk-audiodev=``. And ``AC97`` and ``ES1370`` now have to be +specified in uppercase. + QEMU Machine Protocol (QMP) commands diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt index 240888933482..30e7eaa3e66d 100644 --- a/docs/qdev-device-use.txt +++ b/docs/qdev-device-use.txt @@ -311,21 +311,16 @@ constraints. Host and guest part of audio devices have always been separate. -The old way to define guest audio devices is -soundhw C1,... +Host side (backend) is defined using -audiodev with a specific driver: -The new way is to define each guest audio device separately with --device. +spice +pa +none -Map from -soundhw sound card name to -device: - -ac97-device AC97 -cs4231a -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA -es1370 -device ES1370 -gus -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F -hda -device intel-hda,msi=MSI -device hda-duplex -sb16-device sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V -adlib not yet available with -device -pcspk not yet available with -device +And each guest audio device is then defined with -device with +audiodev=AUDIODEV_ID that refers to the audio backend above. E
Re: [PATCH 06/18] ui/vnc: Require audiodev=
On Wed, Apr 27, 2022 at 11:32:41AM +0200, Paolo Bonzini wrote:
> On 4/25/22 10:21, Martin Kletzander wrote:
> > @@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
> > vd->ledstate = 0;
> > audiodev = qemu_opt_get(opts, "audiodev");
> > -if (audiodev) {
> > -vd->audio_state = audio_state_by_name(audiodev);
> > -if (!vd->audio_state) {
> > -error_setg(errp, "Audiodev '%s' not found", audiodev);
> > -goto fail;
> > -}
> > +if (!audiodev) {
> > +error_setg(errp, "Audiodev parameter for vnc required");
> > +goto fail;
> > +}
> > +
>
> Wouldn't this break "-vnc :0"? You can just ignore the audio commands if
> vd->audio_state is NULL.
Yep, that's wha I suggested with skipping advertizing VNC_ENCODING_AUDIO
when audiodev is NULL
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 06/18] ui/vnc: Require audiodev=
On 4/25/22 10:21, Martin Kletzander wrote:
@@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
vd->ledstate = 0;
audiodev = qemu_opt_get(opts, "audiodev");
-if (audiodev) {
-vd->audio_state = audio_state_by_name(audiodev);
-if (!vd->audio_state) {
-error_setg(errp, "Audiodev '%s' not found", audiodev);
-goto fail;
-}
+if (!audiodev) {
+error_setg(errp, "Audiodev parameter for vnc required");
+goto fail;
+}
+
Wouldn't this break "-vnc :0"? You can just ignore the audio commands
if vd->audio_state is NULL.
Paolo
Re: [PATCH v4 00/19] Add 'version' to other exported types
On Tue, Apr 26, 2022 at 06:26:26PM +0200, Victor Toso wrote: > On Tue, Apr 26, 2022 at 04:06:08PM +, Andrea Bolognani wrote: > > Ideas for follow-up work: > > > > * improve the generator so that multi-line comments attached to > > enum values and macros get all newlines stripped before ending up > > in the XML. Right now it's done very inconsistently; > > > > * include version information in the HTML documentation; > > > > * whatever can be done to make apibuild.py at least somewhat sane :) > > Would you mind opening an issue for those and cc me there? I > don't mind working on it later on. Will do. > > One more thing. Right now version tags look like > > > > Since: v1.2.3 > > > > but the "v" part doesn't really need to be there IMO, and in fact it > > has to be stripped when generating the XML. How would you feel about > > not having it in the documentation in the first place? > > IIRC, I used the 'v' because it was easier to write match > patterns with the leading 'v' but was considering removing as > well. I don't mind stripping it. Can you please prepare a patch implementing this change? There are still a few days left before the next release is tagged. > Another suggestion of mine, was adding other metadata tags to the > documentation, for example, @deprecated: , or info > related to APIs that might be hypervisor specific... don't know. The ability to mark symbols as deprecated would be very valuable I think: for example, language bindings could decide not to expose them. Hypervisor-specific APIs should already be relegated to the corresponding sub-libraries (e.g. libvirt-qemu) so I don't think that'd be applicable. If we wanted to get real fancy, we could implement annotations similar to the ones gtk-doc support[1], to mark things such as whether the return value of a function is supposed to be freed by the caller. But I have to wonder if, in the long run, it wouldn't make more sense to evaluate switching from our homegrown API documentation scaffolding to an off the shelf solution such as Doxygen. [1] https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] cpu: Remove pointless check
On Wed, Apr 27, 2022 at 10:12:25AM +0200, Peter Krempa wrote:
On Wed, Apr 27, 2022 at 09:51:34 +0200, Martin Kletzander wrote:
These two pointers can never be NULL since they are initialised to a reference
of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG
which used both pointers because due to the concise condition the compiler saw
that if the "and" part of the condition did short-circuit (and it assumed that
can happen) the second variable would not be initialised, but it is used in the
debugging message, so the build failed with:
In file included from ../src/cpu/cpu_x86.c:27:
../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’:
../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Fix this by just assigning the helper pointers and remove the condition
altogether.
Signed-off-by: Martin Kletzander
---
Pushed under the build-breaker rule, but feel free to review/object.
Oops! Unfortunately neither my complilers:
clang version 13.0.0 (Fedora 13.0.0-3.fc35),
gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)
nor the CI:
https://gitlab.com/pipo.sk/libvirt/-/pipelines/524030525 (besides the
usual macos-11 fail)
complained. Which compiler did you use?
gcc (Gentoo 11.3.0 p4) 11.3.0
src/cpu/cpu_x86.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 7e9d1cea47d1..a5eac7601cad 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a,
return VIR_CPU_COMPARE_INCOMPATIBLE;
}
-if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) {
-VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata);
-return VIR_CPU_COMPARE_ERROR;
-}
+adata = &a->data.x86;
+bdata = &b->data.x86;
Yup, checking whether the result of the '&' operator is non-null doesn't
make sense.
signature.asc
Description: PGP signature
Re: [PATCH] cpu: Remove pointless check
On Wed, Apr 27, 2022 at 09:51:34 +0200, Martin Kletzander wrote:
> These two pointers can never be NULL since they are initialised to a reference
> of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG
> which used both pointers because due to the concise condition the compiler saw
> that if the "and" part of the condition did short-circuit (and it assumed that
> can happen) the second variable would not be initialised, but it is used in
> the
> debugging message, so the build failed with:
>
> In file included from ../src/cpu/cpu_x86.c:27:
> ../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’:
> ../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
>
> Fix this by just assigning the helper pointers and remove the condition
> altogether.
>
> Signed-off-by: Martin Kletzander
> ---
> Pushed under the build-breaker rule, but feel free to review/object.
Oops! Unfortunately neither my complilers:
clang version 13.0.0 (Fedora 13.0.0-3.fc35),
gcc (GCC) 11.2.1 20220127 (Red Hat 11.2.1-9)
nor the CI:
https://gitlab.com/pipo.sk/libvirt/-/pipelines/524030525 (besides the
usual macos-11 fail)
complained. Which compiler did you use?
>
> src/cpu/cpu_x86.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index 7e9d1cea47d1..a5eac7601cad 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a,
> return VIR_CPU_COMPARE_INCOMPATIBLE;
> }
>
> -if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) {
> -VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata);
> -return VIR_CPU_COMPARE_ERROR;
> -}
> +adata = &a->data.x86;
> +bdata = &b->data.x86;
Yup, checking whether the result of the '&' operator is non-null doesn't
make sense.
[PATCH] cpu: Remove pointless check
These two pointers can never be NULL since they are initialised to a reference
of a struct. This became apparent when commit 210a19539447 added a VIR_DEBUG
which used both pointers because due to the concise condition the compiler saw
that if the "and" part of the condition did short-circuit (and it assumed that
can happen) the second variable would not be initialised, but it is used in the
debugging message, so the build failed with:
In file included from ../src/cpu/cpu_x86.c:27:
../src/cpu/cpu_x86.c: In function ‘virCPUx86DataIsIdentical’:
../src/util/virlog.h:79:5: error: ‘bdata’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Fix this by just assigning the helper pointers and remove the condition
altogether.
Signed-off-by: Martin Kletzander
---
Pushed under the build-breaker rule, but feel free to review/object.
src/cpu/cpu_x86.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 7e9d1cea47d1..a5eac7601cad 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3314,10 +3314,8 @@ virCPUx86DataIsIdentical(const virCPUData *a,
return VIR_CPU_COMPARE_INCOMPATIBLE;
}
-if (!((adata = &a->data.x86) && (bdata = &b->data.x86))) {
-VIR_DEBUG("missing x86 data: a:%p b:%p", adata, bdata);
-return VIR_CPU_COMPARE_ERROR;
-}
+adata = &a->data.x86;
+bdata = &b->data.x86;
if (adata->len != bdata->len) {
VIR_DEBUG("unequal length a:%zu b:%zu", adata->len, bdata->len);
--
2.35.1
Re: [libvirt PATCH] util: fix double destroy / missing unref of GSource
On 4/26/22 16:26, Daniel P. Berrangé wrote:
> A leak of the GSource was introduced in
>
> commit 87a43a907f0ad4897a28ad7c216bc70f37270b93
> Author: Michal Prívozník
> Date: Fri Jan 28 18:42:45 2022 +0100
>
> lib: Use g_clear_pointer() more
>
> As it mistakenly replaced the g_vir_source_unref call with a second
> call to g_source_destroy.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/util/vireventglib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
> index fc04d8f712..3cc42597fd 100644
> --- a/src/util/vireventglib.c
> +++ b/src/util/vireventglib.c
> @@ -227,7 +227,7 @@ virEventGLibHandleUpdate(int watch,
> goto cleanup;
>
> VIR_DEBUG("Removed old handle source=%p", data->source);
> -g_source_destroy(data->source);
> +vir_g_source_unref(data->source, NULL);
> g_clear_pointer(&data->source, g_source_destroy);
> data->events = 0;
> }
Ooops,
Reviewed-by: Michal Privoznik
Michal
