Request to move my GitLab custom executor repo under the libvirt umbrella
by Erik Skultety
So, we successfully enabled functional testing utilizing GitLab's custom
executor feature. The code responsible for creating template images and
provisioning and tearing down the VM workers currently lives under my GitLab
account [1] and I think it would be better if we hosted it under the libvirt
umbrella, especially since the project should serve as a uniform way of setting
up custom runners for anyone in the libvirt community.
If you think this is not a good idea, then please speak up and raise your
concerns.
Note there's still quite some work to be done with the project in general, but
I'd be happy for any feedback in terms of what needs to be done prior moving
the project to the libvirt namespace.
Regards,
Erik
[1] https://gitlab.com/eskultety/libvirt-gitlab-executor
2 years, 7 months
[libvirt RFCv4] virfile: set pipe size in virFileWrapperFdNew to improve throughput
by Claudio Fontana
currently the only user of virFileWrapperFdNew is the qemu driver;
virsh save is very slow with a default pipe size.
This change improves throughput by ~400% on fast nvme or ramdisk.
Best value currently measured is 1MB, which happens to be also
the kernel default for the pipe-max-size.
Signed-off-by: Claudio Fontana <cfontana(a)suse.de>
---
src/util/virfile.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
see v2 at
https://listman.redhat.com/archives/libvir-list/2022-March/229423.html
Changes v3 -> v4:
* changed INFO and WARN messages to DEBUG (Daniel)
Changes v2 -> v3:
* removed reading of max-pipe-size from procfs,
instead make multiple attempts on EPERM with smaller sizes.
In the regular case, this should succeed on the first try.
(Daniel)
Changes v1 -> v2:
* removed VIR_FILE_WRAPPER_BIG_PIPE, made the new pipe resizing
unconditional (Michal)
* moved code to separate functions (Michal)
* removed ternary op, disliked in libvirt (Michal)
* added #ifdef __linux__ (Ani Sinha)
* try smallest value between currently best measured value (1MB)
and the pipe-max-size setting. If pipe-max-size cannot be read,
try kernel default max (1MB). (Daniel)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a04f888e06..87539be0b9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -201,6 +201,50 @@ struct _virFileWrapperFd {
};
#ifndef WIN32
+
+#ifdef __linux__
+
+/**
+ * virFileWrapperSetPipeSize:
+ * @fd: the fd of the pipe
+ *
+ * Set best pipe size on the passed file descriptor for bulk transfers of data.
+ *
+ * default pipe size (usually 64K) is generally not suited for large transfers
+ * to fast devices. A value of 1MB has been measured to improve virsh save
+ * by 400% in ideal conditions. We retry multiple times with smaller sizes
+ * on EPERM to account for possible small values of /proc/sys/fs/pipe-max-size.
+ *
+ * OS note: only for linux, on other OS this is a no-op.
+ */
+static void
+virFileWrapperSetPipeSize(int fd)
+{
+ int sz;
+
+ for (sz = 1024 * 1024; sz >= 64 * 1024; sz /= 2) {
+ int rv = fcntl(fd, F_SETPIPE_SZ, sz);
+ if (rv < 0 && errno == EPERM) {
+ VIR_DEBUG("EPERM trying to set fd %d pipe size to %d", fd, sz);
+ continue; /* retry with half the size */
+ }
+ if (rv < 0) {
+ break;
+ }
+ VIR_DEBUG("fd %d pipe size adjusted to %d", fd, sz);
+ return;
+ }
+ virReportSystemError(errno, "%s", _("unable to set pipe size, data transfer might be slow"));
+}
+
+#else /* !__linux__ */
+static void virFileWrapperSetPipeSize(int fd)
+{
+ return;
+}
+#endif /* !__linux__ */
+
+
/**
* virFileWrapperFdNew:
* @fd: pointer to fd to wrap
@@ -282,6 +326,8 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
ret->cmd = virCommandNewArgList(iohelper_path, name, NULL);
+ virFileWrapperSetPipeSize(pipefd[!output]);
+
if (output) {
virCommandSetInputFD(ret->cmd, pipefd[0]);
virCommandSetOutputFD(ret->cmd, fd);
--
2.35.1
2 years, 7 months
[build-fix PATCH] util: virfile: Fix indentation of preprocessor directives
by Peter Krempa
stderr:
cppi: /home/pipo/libvirt/src/util/virfile.c: line 205: not properly indented
cppi: /home/pipo/libvirt/src/util/virfile.c: line 243: not properly indented
cppi: /home/pipo/libvirt/src/util/virfile.c: line 249: not properly indented
build-aux/syntax-check.mk: incorrect preprocessor indentation
make: *** [/home/pipo/libvirt/build-aux/syntax-check.mk:565: sc_preprocessor_indentation] Error 1
Fixes: c61d1e9ba0a0bec18fdb0bdd485060dc27a4e5cc
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
Pushed.
src/util/virfile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6f2af35201..12b359d550 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -202,7 +202,7 @@ struct _virFileWrapperFd {
#ifndef WIN32
-#ifdef __linux__
+# ifdef __linux__
/**
* virFileWrapperSetPipeSize:
@@ -240,13 +240,13 @@ virFileWrapperSetPipeSize(int fd)
g_strerror(errno));
}
-#else /* !__linux__ */
+# else /* !__linux__ */
static void
virFileWrapperSetPipeSize(int fd G_GNUC_UNUSED)
{
return;
}
-#endif /* !__linux__ */
+# endif /* !__linux__ */
/**
--
2.35.1
2 years, 7 months
[PATCH v2 0/6] qemu: Add qemu-namespace XML to override device properties (-set replacement)
by Peter Krempa
v2:
- patch 1/7 from last time was pushed
- XML changes:
- top level element renamed to "qemu:override" to accomodate
possible extension in the future
- wrapped 'qemu:props' meant to override the frontend under a
'qemu:frontend' element
- code changes:
- renamed few bits (dropped 'Device') from code that can theoretically be reused
- split out some functions for enhanced extensibility in the future
Peter Krempa (6):
docs: drvqemu: Document overriding of device properties
qemu: domain: Add XML namespace code for overriding device config
conf: Introduce VIR_DOMAIN_TAINT_CUSTOM_DEVICE and use it in qemu
qemuBuildDeviceCommandlineFromJSON: Pass 'virDomainDef' into the
function
qemu: command: Override device definition according to the namespace
config
NEWS: Mention the qemu device property override feature
NEWS.rst | 7 +
docs/drvqemu.rst | 57 +++++
src/conf/domain_conf.c | 2 +
src/conf/domain_conf.h | 1 +
src/conf/schemas/domaincommon.rng | 42 ++++
src/qemu/qemu_command.c | 133 +++++++----
src/qemu/qemu_domain.c | 221 ++++++++++++++++++
src/qemu/qemu_domain.h | 33 +++
.../qemu-ns.x86_64-4.0.0.args | 2 +-
.../qemu-ns.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/qemu-ns.xml | 14 ++
.../qemu-ns.x86_64-latest.xml | 14 ++
12 files changed, 478 insertions(+), 50 deletions(-)
--
2.35.1
2 years, 7 months
[PATCH 0/2] Revert two patches I've pushed mistakenly
by Michal Privoznik
Firstly, QEMU part is not merged which alone should have prevented me
from pushing these patches.
Secondly, in the end QEMU is fixing the bug in a different manner that
does not require any libvirt work.
https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06077.html
https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06188.html
Michal Prívozník (2):
Revert "domain: add tsc.on_reboot element"
Revert "qemu: add support for tsc.on_reboot element"
docs/formatdomain.rst | 4 --
src/conf/domain_conf.c | 17 -------
src/conf/domain_conf.h | 10 -----
src/conf/schemas/domaincommon.rng | 8 ----
src/qemu/qemu_capabilities.c | 2 -
src/qemu/qemu_capabilities.h | 1 -
src/qemu/qemu_command.c | 11 -----
src/qemu/qemu_validate.c | 45 ++++++++-----------
.../caps_7.0.0.x86_64.replies | 4 --
.../caps_7.0.0.x86_64.xml | 1 -
.../cpu-tsc-clear-on-reset.args | 32 -------------
.../cpu-tsc-clear-on-reset.x86_64-7.0.0.args | 34 --------------
.../cpu-tsc-clear-on-reset.xml | 35 ---------------
tests/qemuxml2argvtest.c | 2 -
14 files changed, 19 insertions(+), 187 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args
delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args
delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml
--
2.34.1
2 years, 7 months
[PATCH v2 0/7] Do async IO when starting swtpm emulator
by Michal Privoznik
v2 of:
https://listman.redhat.com/archives/libvir-list/2022-March/229433.html
diff to v1:
- More fixes
- New test case, yay!
Michal Prívozník (7):
qemu_tpm: Do async IO when starting swtpm emulator
vircommand: Document virCommandSetSendBuffer() behaviour wrt daemonize
vircommand: Don't set nonblocking FDs in virCommandDoAsyncIO()
commandtest: Use unsigned char in test27()
virCommandSetSendBuffer: Take double pointer of @buffer
commandtest: Test virCommandSetSendBuffer() with virCommandDoAsyncIO()
commandtest: Use virTestCompareToFile() in checkoutput()
src/qemu/qemu_tpm.c | 3 +-
src/util/vircommand.c | 12 ++--
src/util/vircommand.h | 2 +-
tests/commanddata/test29.log | 19 ++++++
tests/commandtest.c | 110 +++++++++++++++++++++++++++++------
5 files changed, 121 insertions(+), 25 deletions(-)
create mode 100644 tests/commanddata/test29.log
--
2.34.1
2 years, 7 months
[PATCH 0/2] domain, qemu: add support for clearing TSC on reset
by Paolo Bonzini
Some versions of Windows hang on reboot if their TSC value is greater
than 2^54. The workaround is to reset the TSC to a small value, and
is implemented by QEMU and ESXi. This series adds a domain attribute
for this, and implements it in QEMU.
Paolo
Paolo Bonzini (2):
domain: add tsc.on_reboot element
qemu: add support for tsc.on_reboot element
docs/formatdomain.rst | 4 +++
src/conf/domain_conf.c | 22 ++++++++++++
src/conf/domain_conf.h | 10 ++++++
src/conf/schemas/domaincommon.rng | 9 +++++
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 4 +++
src/qemu/qemu_validate.c | 36 +++++++++++++------
.../caps_7.0.0.x86_64.replies | 4 +++
.../caps_7.0.0.x86_64.xml | 1 +
...uency.args => cpu-tsc-clear-on-reset.args} | 2 +-
... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++--
...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +-
tests/qemuxml2argvtest.c | 2 ++
14 files changed, 89 insertions(+), 16 deletions(-)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%)
copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%)
--
2.35.1
2 years, 7 months
[libvirt PATCH] iohelper: some refactoring
by Claudio Fontana
while doing research with alternative implementations of runIO,
it seemed necessary to do some refactoring, in order to separate
parameter setting from the actual copy, so that alternative
copy methods can be researched and hopefully eventually implemented.
No functional changes are expected.
Signed-off-by: Claudio Fontana <cfontana(a)suse.de>
---
src/util/iohelper.c | 193 +++++++++++++++++++++++++++-----------------
1 file changed, 118 insertions(+), 75 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 2c91bf4f93..c04c97af27 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -45,21 +45,38 @@
# define O_DIRECT 0
#endif
-static int
-runIO(const char *path, int fd, int oflags)
+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 */
+/**
+ * runIOCopy:
+ * @p: the IO parameters
+ *
+ * Execute the copy based on the passed parameters.
+ *
+ * Returns: size transfered, or < 0 on error.
+ * Errors: -1 = read/write error
+ * -2 = read error
+ * -3 = write error
+ * -4 = truncate error
+ */
+
+static ssize_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;
- int fdin, fdout;
- const char *fdinname, *fdoutname;
- unsigned long long total = 0;
- bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0);
- off_t end = 0;
- struct stat sb;
- bool isBlockDev = false;
+ ssize_t total = 0;
#if WITH_POSIX_MEMALIGN
if (posix_memalign(&base, alignMask + 1, buflen))
@@ -71,50 +88,6 @@ runIO(const char *path, int fd, int oflags)
buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
#endif
- if (fstat(fd, &sb) < 0) {
- virReportSystemError(errno,
- _("Unable to access file descriptor %d path %s"),
- fd, path);
- goto cleanup;
- }
- isBlockDev = S_ISBLK(sb.st_mode);
-
- 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;
- }
- 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;
- }
- break;
-
- case O_RDWR:
- default:
- virReportSystemError(EINVAL,
- _("Unable to process file with flags %d"),
- (oflags & O_ACCMODE));
- goto cleanup;
- }
-
while (1) {
ssize_t got;
@@ -124,53 +97,123 @@ 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);
- }
-
- if (got < 0) {
- virReportSystemError(errno, _("Unable to read %s"), fdinname);
- goto cleanup;
+ got = saferead(p.fdin, buf, buflen);
}
+ if (got < 0)
+ return -2;
if (got == 0)
break;
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 (safewrite(fdout, buf, aligned_got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), fdoutname);
- goto cleanup;
+ if (safewrite(p.fdout, buf, aligned_got) < 0) {
+ return -3;
}
-
- if (!isBlockDev && ftruncate(fd, total) < 0) {
- virReportSystemError(errno, _("Unable to truncate %s"), fdoutname);
- goto cleanup;
+ if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) {
+ return -4;
}
-
break;
}
+ }
+ return total;
+}
- if (safewrite(fdout, buf, got) < 0) {
- virReportSystemError(errno, _("Unable to write %s"), fdoutname);
+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 && p.isDirect) {
+ off_t end;
+ if (p.isWrite) {
+ if ((end = lseek(fd, 0, SEEK_END)) != 0) {
+ virReportSystemError(end < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT write needs empty seekable file"));
+ goto cleanup;
+ }
+ } else if ((end = lseek(fd, 0, SEEK_CUR)) != 0) {
+ virReportSystemError(end < 0 ? errno : EINVAL, "%s",
+ _("O_DIRECT read needs entire seekable file"));
goto cleanup;
}
}
+ total = runIOCopy(p);
+
+ if (total < 0) {
+ switch (total) {
+ case -1:
+ virReportSystemError(errno, _("Unable to move data from %s to %s"),
+ p.fdinname, p.fdoutname);
+ break;
+ case -2:
+ virReportSystemError(errno, _("Unable to read %s"), p.fdinname);
+ break;
+ case -3:
+ virReportSystemError(errno, _("Unable to write %s"), p.fdoutname);
+ break;
+ case -4:
+ virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname);
+ break;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown runIOCopy error %ld"), total);
+ break;
+ }
+ goto cleanup;
+ }
+
/* Ensure all data is written */
- if (virFileDataSync(fdout) < 0) {
+ if (virFileDataSync(p.fdout) < 0) {
if (errno != EINVAL && errno != EROFS) {
/* fdatasync() may fail on some special FDs, e.g. pipes */
- virReportSystemError(errno, _("unable to fsync %s"), fdoutname);
+ virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname);
goto cleanup;
}
}
--
2.35.1
2 years, 8 months
[PATCH v2 00/19] Refactoring conf to use XPath
by Kristina Hanicova
This is v2 from
https://listman.redhat.com/archives/libvir-list/2021-April/msg00617.html
Changes since v1:
- rebase to the lastest git master
This series reworks the outdated way of parsing XML to parsing by XPath,
which is more obvious and saves a few lines of code.
Kristina Hanicova (19):
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysUSBDefParseXML()
Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into virDomainBlkioDeviceParseXML()
Refactoring virDomainBlkioDeviceParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysPCIDefParseXML()
Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
Refactoring virDomainLeaseDefParseXML() to use XPath
Refactoring virDomainFSDefParseXML() to use XPath
Refactoring virDomainNetDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainChrDefParseTargetXML()
Refactoring virDomainChrDefParseTargetXML() to use XPath
Refactoring virDomainChrSourceDefParseXML() to use XPath
Refactoring virDomainChrDefParseXML() to use XPath
Refactoring virDomainSmartcardDefParseXML() to use XPath
Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainVideoDriverDefParseXML()
Refactoring virDomainVideoDriverDefParseXML() to use XPath
Refactoring virDomainVideoDefParseXML() to use XPath
src/conf/domain_conf.c | 1641 +++++++++++++++++-----------------------
1 file changed, 707 insertions(+), 934 deletions(-)
--
2.30.2
2 years, 8 months
[PATCH v2] meson: do not look for libparted if not necessary
by Paolo Bonzini
libparted_dep is not used if -Dstorage_disk=disabled. Do
not bother looking for it if the disk storage backend was not
requested.
Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com>
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index eb13c7efa4..27c21d1d67 100644
--- a/meson.build
+++ b/meson.build
@@ -1013,7 +1013,7 @@ else
endif
libparted_version = '1.8.0'
-libparted_dep = dependency('libparted', version: '>=' + libparted_version, required: false)
+libparted_dep = dependency('libparted', version: '>=' + libparted_version, required: get_option('storage_disk'))
libpcap_version = '1.5.0'
if not get_option('libpcap').disabled()
--
2.35.1
2 years, 8 months
