- Devel - libvirt List Archives

[PATCH 0/2] qemu: Clarify purpose of image format settings
by Jim Fehlig 17 Aug '24

17 Aug '24

The current documentation of the various foo_image_format settings in qemu.conf subtly implies they are only used for specifying compression. Patch1 of this small series attempts to clarify and improve the description of the settings. It defines image format as a way to specify the desired layout of guest memory blocks on disk. Patch2 changes the name of the 'compressed' field of virQEMUSaveHeader to 'format', removing implication that format == compressed in the code. Jim Fehlig (2): qemu: conf: Improve the foo_image_format setting descriptions qemu: Rename compressed field of save image header src/qemu/qemu.conf.in | 46 +++++++++++++++++++++++++-------------- src/qemu/qemu_saveimage.c | 18 +++++++-------- src/qemu/qemu_saveimage.h | 4 ++-- 3 files changed, 41 insertions(+), 27 deletions(-) -- 2.35.3

1 3

[PATCH RFC 0/9] qemu: Support mapped-ram migration capability
by Jim Fehlig 16 Aug '24

16 Aug '24

This series is a RFC for support of QEMU's mapped-ram migration capability [1] for saving and restoring VMs. It implements the first part of the design approach we discussed for supporting parallel save/restore [2]. In summary, the approach is 1. Add mapped-ram migration capability 2. Steal an element from save header 'unused' for a 'features' variable and bump save version to 3. 3. Add /etc/libvirt/qemu.conf knob for the save format version, defaulting to latest v3 4. Use v3 (aka mapped-ram) by default 5. Use mapped-ram with BYPASS_CACHE for v3, old approach for v2 6. include: Define constants for parallel save/restore 7. qemu: Add support for parallel save. Implies mapped-ram, reject if v2 8. qemu: Add support for parallel restore. Implies mapped-ram. Reject if v2 9. tools: add parallel parameter to virsh save command 10. tools: add parallel parameter to virsh restore command This series implements 1-5, with the BYPASS_CACHE support in patches 8 and 9 being quite hacky. They are included to discuss approaches to make them less hacky. See the patches for details. The QEMU mapped-ram capability currently does not support directio. Fabino is working on that now [3]. This complicates merging support in libvirt. I don't think it's reasonable to enable mapped-ram by default when BYPASS_CACHE cannot be supported. Should we wait until the mapped-ram directio support is merged in QEMU before supporting mapped-ram in libvirt? For the moment, compression is ignored in the new save version. Currently, libvirt connects the output of QEMU's save stream to the specified compression program via a pipe. This approach is incompatible with mapped-ram since the fd provided to QEMU must be seekable. One option is to reopen and compress the saved image after the actual save operation has completed. This has the downside of requiring the iohelper to handle BYPASS_CACHE, which would preclude us from removing it sometime in the future. Other suggestions much welcomed. Note the logical file size of mapped-ram saved images is slightly larger than guest RAM size, so the files are often much larger than the files produced by the existing, sequential format. However, actual blocks written to disk is often lower with mapped-ram saved images. E.g. a saved image from a 30G, freshly booted, idle guest results in the following 'Size' and 'Blocks' values reported by stat(1) Size Blocks sequential 998595770 1950392 mapped-ram 34368584225 1800456 With the same guest running a workload that dirties memory Size Blocks sequential 33173330615 64791672 mapped-ram 34368578210 64706944 Thanks for any comments on this RFC! [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/map… [2] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/K4B… [3] https://mail.gnu.org/archive/html/qemu-devel/2024-05/msg04432.html Jim Fehlig (9): qemu: Enable mapped-ram migration capability qemu_fd: Add function to retrieve fdset ID qemu: Add function to get migration params for save qemu: Add a 'features' element to save image header and bump version qemu: conf: Add setting for save image version qemu: Add support for mapped-ram on save qemu: Enable mapped-ram on restore qemu: Support O_DIRECT with mapped-ram on save qemu: Support O_DIRECT with mapped-ram on restore src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 6 + src/qemu/qemu_conf.c | 8 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 25 ++-- src/qemu/qemu_fd.c | 18 +++ src/qemu/qemu_fd.h | 3 + src/qemu/qemu_migration.c | 99 ++++++++++++++- src/qemu/qemu_migration.h | 11 +- src/qemu/qemu_migration_params.c | 20 +++ src/qemu/qemu_migration_params.h | 4 + src/qemu/qemu_monitor.c | 40 ++++++ src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_process.c | 63 +++++++--- src/qemu/qemu_process.h | 16 ++- src/qemu/qemu_saveimage.c | 187 +++++++++++++++++++++++------ src/qemu/qemu_saveimage.h | 20 ++- src/qemu/qemu_snapshot.c | 12 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + 19 files changed, 455 insertions(+), 85 deletions(-) -- 2.44.0

3 22

[PATCH v3 0/4] multiple memory backend support for CPR Live Updates
by mgalaxy＠akamai.com 16 Aug '24

16 Aug '24

From: Michael Galaxy <mgalaxy(a)akamai.com> CPR-based support for whole-hypervisor kexec-based live updates is now finally merged into QEMU. In support of this, we need NUMA to be supported in these kinds of environments. To do this we use a technology called PMEM (persistent memory) in Linux, which underpins the ability for CPR Live Updates to work so that QEMU memory can remain in RAM and be recovered after a kexec operationg has completed. Our systems are highly NUMA-aware, and so this patch series enables NUMA awareness for live updates. Further, we make a small change that allows live migrations to work between *non* PMEM-based systems and PMEM-based systems (and vice-versa). This allows for seemless upgrades from non-live-compatible systems to live-update-compatible sytems without any downtime. Changes since v2: 1. Corrected invalid privateData casts and re-organized variables and error codes. 2. Re-ran ninja test and corrected formatting 3. Tested that individual patches apply and build cleanly. 4. Separated out NEWS item into separate patch Michael Galaxy (4): qemu.conf changes to support multiple memory backend Support live migration between file-backed memory and anonymous memory. Update configuration file documentation to support multiple memory backends Add NEWS item for multiple memory backend support NEWS.rst | 8 +++ docs/kbase/virtiofs.rst | 2 + src/qemu/qemu.conf.in | 4 ++ src/qemu/qemu_command.c | 6 +- src/qemu/qemu_conf.c | 139 +++++++++++++++++++++++++++++++++++----- src/qemu/qemu_conf.h | 16 +++-- src/qemu/qemu_domain.c | 24 +++++-- src/qemu/qemu_driver.c | 37 +++++++---- src/qemu/qemu_hotplug.c | 6 +- src/qemu/qemu_process.c | 42 ++++++------ src/qemu/qemu_process.h | 6 +- tests/testutilsqemu.c | 5 +- 12 files changed, 224 insertions(+), 71 deletions(-) -- 2.34.1

1 4

[PATCH v2 0/2] conf: Validate QoS values
by Michal Privoznik 13 Aug '24

13 Aug '24

v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/EDR7… diff to v1: - Validate in validation phase of XML parsing, or since network XML doesn't have validation phase, do it during network start (just like other stuff) Michal Prívozník (2): conf: Introduce virNetDevBandwidthValidate() conf: Validate QoS values src/conf/domain_validate.c | 9 +++++++ src/conf/netdev_bandwidth_conf.c | 42 ++++++++++++++++++++++++++++++ src/conf/netdev_bandwidth_conf.h | 2 ++ src/conf/schemas/networkcommon.rng | 3 ++- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 4 +++ 6 files changed, 60 insertions(+), 1 deletion(-) -- 2.44.2

2 3

[PATCH] conf: Check for bandwidth limits during parsing
by Michal Privoznik 13 Aug '24

13 Aug '24

The 'tc' program stores speeds in 64bit integers (unit is bytes per second) and sizes in uints (unit is bytes). We use different units: kilobytes per second and kibibytes and therefore we can parse values larger than 'tc' can handle. Reject those values right away. And while at it, fix the schema which assumed speed values fit into uint. Resolves: https://issues.redhat.com/browse/RHEL-45200 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/netdev_bandwidth_conf.c | 17 +++++++++++++++++ src/conf/schemas/networkcommon.rng | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 9faa46a27f..f3f0b2209a 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -24,6 +24,16 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define CHECK_LIMIT(val, limit, name) \ + do { \ + if ((val) > (limit)) { \ + virReportError(VIR_ERR_OVERFLOW, \ + _("value '%1$llu' is too big for '%2$s' parameter, maximum is '%3$llu'"), \ + val, name, (unsigned long long) limit); \ + return -1; \ + } \ + } while (0) + static int virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRate *rate, @@ -50,6 +60,11 @@ virNetDevBandwidthParseRate(xmlNodePtr node, &rate->floor)) < 0) return -1; + CHECK_LIMIT(rate->average, 1ULL << 54, "average"); + CHECK_LIMIT(rate->peak, 1ULL << 54, "peak"); + CHECK_LIMIT(rate->burst, UINT_MAX >> 10, "burst"); + CHECK_LIMIT(rate->floor, 1ULL << 54, "floor"); + if (!rc_average && !rc_floor) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Missing mandatory average or floor attributes")); @@ -71,6 +86,8 @@ virNetDevBandwidthParseRate(xmlNodePtr node, return 0; } +#undef CHECK_LIMIT + /** * virNetDevBandwidthParse: * @bandwidth: parsed bandwidth diff --git a/src/conf/schemas/networkcommon.rng b/src/conf/schemas/networkcommon.rng index 6df6d43f54..2b3f902ffe 100644 --- a/src/conf/schemas/networkcommon.rng +++ b/src/conf/schemas/networkcommon.rng @@ -180,7 +180,7 @@ </define> <define name="speed"> - <data type="unsignedInt"> + <data type="unsignedLong"> <param name="pattern">[0-9]+</param> <param name="minInclusive">1</param> </data> -- 2.44.2

3 2

[PATCH] qemu: hotplug: Rollback FD passthrough for 'slirpfd' and 'vdpafd' on hotplug failure
by Peter Krempa 13 Aug '24

13 Aug '24

On failure to plug the device the cleanup path didn't roll back the FD passing to qemu thus qemu would hold the FDs indefinitely. Resolves: https://issues.redhat.com/browse/RHEL-53964 Fixes: b79abf9c3cdab8bcecfa8769629a4cdf4bf0b6c3 (vdpafd) Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7cb1800504..75b97cf736 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1456,6 +1456,9 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); + qemuFDPassTransferMonitorRollback(netpriv->vdpafd, priv->mon); + qemuFDPassDirectTransferMonitorRollback(netpriv->slirpfd, priv->mon); + for (n = netpriv->tapfds; n; n = n->next) qemuFDPassDirectTransferMonitorRollback(n->data, priv->mon); -- 2.46.0

2 1

[PATCH] virnetlibsshsession: Reflect API change in libssh
by Michal Privoznik 12 Aug '24

12 Aug '24

As of libssh commit of libssh-0.11.0~70 [1] the ssh_channel_get_exit_status() function is deprecated and a new one is introduced instead: ssh_channel_get_exit_state(). It's not a drop-in replacement, but it's simple enough. Adapt our libssh handling code to this change. 1: https://git.libssh.org/projects/libssh.git/commit/?id=04d86aeeae73c78af8b3d… Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- meson.build | 3 +++ src/rpc/virnetlibsshsession.c | 39 +++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index 06d88ad1f3..f31485c395 100644 --- a/meson.build +++ b/meson.build @@ -1096,6 +1096,9 @@ if conf.has('WITH_REMOTE') libssh_dep = dependency('libssh', version: '>=' + libssh_version, required: get_option('libssh')) if libssh_dep.found() conf.set('WITH_LIBSSH', 1) + if cc.has_function('ssh_channel_get_exit_state', dependencies: libssh_dep) + conf.set('WITH_SSH_CHANNEL_GET_EXIT_STATE', 1) + endif endif else libssh_dep = dependency('', required: false) diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 6632e4a9ef..e496d3334e 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -170,6 +170,25 @@ virNetLibsshSessionOnceInit(void) } VIR_ONCE_GLOBAL_INIT(virNetLibsshSession); + +static int virNetLibsshChannelGetExitStatus(ssh_channel channel, + uint32_t *exit_status) +{ +#ifdef WITH_SSH_CHANNEL_GET_EXIT_STATE + return ssh_channel_get_exit_state(channel, exit_status, NULL, NULL); +#else + int rc; + + rc = *exit_status = ssh_channel_get_exit_status(channel); + + if (rc != SSH_OK) + return SSH_ERROR; + + return *exit_status; +#endif +} + + static virNetLibsshAuthMethod * virNetLibsshSessionAuthMethodNew(virNetLibsshSession *sess) { @@ -1179,12 +1198,16 @@ virNetLibsshChannelRead(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { + uint32_t exit_status; + int rc; eof: - if (ssh_channel_get_exit_status(sess->channel)) { + + rc = virNetLibsshChannelGetExitStatus(sess->channel, &exit_status); + if (rc != SSH_OK || exit_status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote command terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + exit_status); + sess->channelCommandReturnValue = exit_status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; virObjectUnlock(sess); return -1; @@ -1227,12 +1250,16 @@ virNetLibsshChannelWrite(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { - if (ssh_channel_get_exit_status(sess->channel)) { + uint32_t exit_status; + int rc; + + rc = virNetLibsshChannelGetExitStatus(sess->channel, &exit_status); + if (rc != SSH_OK || exit_status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote program terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); + exit_status); sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + sess->channelCommandReturnValue = exit_status; ret = -1; goto cleanup; -- 2.44.2

2 1

[PATCH] rpc: adapt for deprecation of ssh_channel_get_exit_status
by Daniel P. Berrangé 12 Aug '24

12 Aug '24

This method is deprecated in libssh 0.11.0 in favour of the more flexible ssh_channel_get_exit_state. Rewrite our code to call the latter, and provide a compat shim that only fills out exit status. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetlibsshsession.c | 50 +++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 6632e4a9ef..faf7307725 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -48,6 +48,22 @@ VIR_LOG_INIT("rpc.netlibsshsession"); */ #define TRACE_LIBSSH 0 +#if LIBSSH_VERSION_INT < SSH_VERSION_INT(0, 11, 0) +static int ssh_channel_get_exit_state(ssh_channel channel, + uint32_t *pexit_code, + char **pexit_signal, + int *pcore_dumped) +{ + if (pexit_code) + *pexit_code = ssh_channel_get_exit_status(channel); + if (pexit_signal) + *pexit_signal = 0; + if (pcore_dumped) + *pcore_dumped = 0; + return 0; +} +#endif + typedef enum { VIR_NET_LIBSSH_STATE_NEW, VIR_NET_LIBSSH_STATE_HANDSHAKE_COMPLETE, @@ -1179,12 +1195,21 @@ virNetLibsshChannelRead(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { + uint32_t status; eof: - if (ssh_channel_get_exit_status(sess->channel)) { + if (ssh_channel_get_exit_state(sess->channel, &status, NULL, NULL) != 0) { + virReportError(VIR_ERR_LIBSSH, "%s", + _("Unable to fetch remote command exit status")); + sess->channelCommandReturnValue = 0; + sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; + virObjectUnlock(sess); + return -1; + } + if (status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote command terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + status); + sess->channelCommandReturnValue = status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; virObjectUnlock(sess); return -1; @@ -1227,15 +1252,24 @@ virNetLibsshChannelWrite(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { - if (ssh_channel_get_exit_status(sess->channel)) { + uint32_t status; + if (ssh_channel_get_exit_state(sess->channel, &status, NULL, NULL) != 0) { + virReportError(VIR_ERR_LIBSSH, "%s", + _("Unable to fetch remote command exit status")); + sess->channelCommandReturnValue = 0; + sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; + ret = -1; + goto cleanup; + } + if (status != 0) { virReportError(VIR_ERR_LIBSSH, - _("Remote program terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); + _("Remote command terminated with non-zero code: %1$d"), + status); + sess->channelCommandReturnValue = status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); - ret = -1; goto cleanup; + } sess->state = VIR_NET_LIBSSH_STATE_CLOSED; -- 2.45.2

2 1

[PATCH] glibcompat: remove obsolete clang workaround
by Daniel P. Berrangé 12 Aug '24

12 Aug '24

This mostly reverts commit 65491a2dfe00bfcf9f09a8d6eab60234b56c8cc4. There was a bug introduced in glib 2.67.0 which impacted libvirt with clang causing -Wincompatible-pointer-types-discards-qualifiers warnings. This was actually fixed quite quickly in 2.67.1 with https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 Our workaround was then broken with glib 2.81.1 due to commit 14b3d5da9019150d821f6178a075d85044b4c255 changing the signature of the (private) macro we were overriding. Since odd-number glib releases are development snapshots, and the original problem was only present in 2.67.0 and no other releases, just drop the workaround entirely. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/util/glibcompat.h | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 3518023a41..474ff95bc5 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -22,33 +22,7 @@ #include <glib/gstdio.h> #include <glib-object.h> -#if GLIB_CHECK_VERSION(2, 67, 0) - -# if defined(__clang__) - -/* - * Clang detects (valid) issue in G_DEFINE_TYPE and derivatives starting with - * glib >= 2.67.0. See https://gitlab.gnome.org/GNOME/glib/-/issues/600 - * - * For that we need to disable the one check that produces an error in our - * builds when using any G_DEFINE_TYPE* macro. Thankfully all those macros end - * up using _G_DEFINE_TYPE_EXTENDED_BEGIN. Because with that we can redefine - * this one macro to cover all use cases. The macro is defined the same way it - * is defined in glib (with a very low probability of being changed thanks to a - * comment above it). - */ -# undef _G_DEFINE_TYPE_EXTENDED_BEGIN - -# define _G_DEFINE_TYPE_EXTENDED_BEGIN(TypeName, type_name, TYPE_PARENT, flags) \ - _Pragma("GCC diagnostic push") \ - _Pragma("GCC diagnostic ignored \"-Wincompatible-pointer-types-discards-qualifiers\"") \ - _G_DEFINE_TYPE_EXTENDED_BEGIN_PRE(TypeName, type_name, TYPE_PARENT) \ - _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ - _Pragma("GCC diagnostic pop") - -# endif /* __clang__ */ - -#else /* GLib < 2.67.0 */ +#if !GLIB_CHECK_VERSION(2, 67, 0) /* * ...meanwhile GCC >= 11 has started issuing warnings about volatile -- 2.45.2

2 1

[PATCH] add support for xml about cipher info
by luzhipeng 12 Aug '24

12 Aug '24

The destination disk's xml contains the ciper info and it check fail, when snaphot,blockcopy and blockpull vm's disk. So it removes the check about cipher info. Signed-off-by: luzhipeng <luzhipeng(a)cestc.cn> --- src/conf/domain_validate.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 39b8d67928..b70edcaaa0 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -529,18 +529,6 @@ virDomainDiskDefValidateSourceChainOne(const virStorageSource *src) } } - if (src->encryption) { - virStorageEncryption *encryption = src->encryption; - - if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && - encryption->encinfo.cipher_name) { - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("supplying <cipher> for domain disk definition is unnecessary")); - return -1; - } - } - /* internal snapshots and config files are currently supported only with rbd: */ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK && src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { -- 2.39.3

3 2