- Devel - libvirt List Archives

[PATCH] qemu: migration: Fix blockdev config with VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES
by Peter Krempa 14 Oct '24

14 Oct '24

The idea of migration with VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES populated is to sparsify the image. The QEMU NBD client as it was configured in commit 621f879adf98e2c93ac5c8c869733a57f06cd9aa would signal to the destination to do thick allocation of holes which would result in a non-sparse image for any backend except a qcow2 image which I used to test it. Switch to VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP and VIR_DOMAIN_DISK_DISCARD_UNMAP which tells the NBD client (and that in turn the NBD server) to preserve the sparse blocks it detected from the image. Fixes: 621f879adf98e2c93ac5c8c869733a57f06cd9aa Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_migration.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 179e9d7c83..832b2946e0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1092,8 +1092,14 @@ qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDef *disk, copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; copysrc->format = VIR_STORAGE_FILE_RAW; - if (detect_zeroes) - copysrc->detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + if (detect_zeroes) { + /* We need to use both VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP and + * VIR_DOMAIN_DISK_DISCARD_UNMAP as the qemu NBD client otherwise singals + * to the server to fully allocate the zero blocks + */ + copysrc->detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP; + copysrc->discard = VIR_DOMAIN_DISK_DISCARD_UNMAP; + } copysrc->backingStore = virStorageSourceNew(); -- 2.46.2

2 1

Re: [PATCH RFC v4 11/17] qemu: block: Support block disk along with throttle filters
by Peter Krempa 14 Oct '24

14 Oct '24

On Mon, Sep 30, 2024 at 10:14:59 +0000, Edward Arulanadam wrote: > Dear All, > > My sincere apologies on reaching out regarding this change as I know, someone would be reviewing the change and provide review comments as per the community guidelines. Since this change is very critical for us to move forward, may I request for a review and let us know if this is good now. Hi, when this last iteration was posted I got a note (off-list) from the author stating that they might not be able to continue work on that series. Are you going to pick up the work on that series? If so make sure to configure your mail client to avoid breaking threads (as you did with this mail). I'll use the existing convention of using 'reply-all' to the patches so you might not get CC'd for the postings which are already on-list. If that's a problem please re-post the series (obviously keeping authorship intact). I'm keeping it on my to-do list but due to personal reasons I'm really backlogged in reviews so please be patient.

1 1

[PATCH] qemu: snapshot: Remove dead code in 'qemuSnapshotDeleteBlockJobRunning'
by Peter Krempa 14 Oct '24

14 Oct '24

'qemuSnapshotDeleteBlockJobIsRunning' returns only 0 and 1. Convert it to bool and remove the dead code handling -1 return in the caller. Closes: https://gitlab.com/libvirt/libvirt/-/issues/682 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_snapshot.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 77ff842fab..1187ebf276 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3303,7 +3303,7 @@ qemuSnapshotDeleteUpdateDisks(void *payload, /* Deleting external snapshot is started by running qemu block-commit job. * We need to wait for all block-commit jobs to be 'ready' or 'pending' to * continue with external snapshot deletion. */ -static int +static bool qemuSnapshotDeleteBlockJobIsRunning(qemuBlockjobState state) { switch (state) { @@ -3311,7 +3311,7 @@ qemuSnapshotDeleteBlockJobIsRunning(qemuBlockjobState state) case QEMU_BLOCKJOB_STATE_RUNNING: case QEMU_BLOCKJOB_STATE_ABORTING: case QEMU_BLOCKJOB_STATE_PIVOTING: - return 1; + return true; case QEMU_BLOCKJOB_STATE_COMPLETED: case QEMU_BLOCKJOB_STATE_FAILED: @@ -3323,7 +3323,7 @@ qemuSnapshotDeleteBlockJobIsRunning(qemuBlockjobState state) break; } - return 0; + return false; } @@ -3359,18 +3359,14 @@ static int qemuSnapshotDeleteBlockJobRunning(virDomainObj *vm, qemuBlockJobData *job) { - int rc; qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_SNAPSHOT); - while ((rc = qemuSnapshotDeleteBlockJobIsRunning(job->state)) > 0) { + while (qemuSnapshotDeleteBlockJobIsRunning(job->state)) { if (qemuDomainObjWait(vm) < 0) return -1; qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_SNAPSHOT); } - if (rc < 0) - return -1; - return 0; } -- 2.46.2

2 1

[PATCH] docs: Document watchdog action=dump slightly more
by Martin Kletzander 12 Oct '24

12 Oct '24

With watchdog action=dump the actual watchdog action is set to pause and the daemon then proceeds to dump the process. After that the domain is resumed. That was the case since the feature was added. However the resuming of the domain might be unexpected, especially when compared to HW watchdog, which will never run the guest from the point where it got interrupted. Document the pre-existing behaviour, since any change might be unexpected as well. Change of behaviour would require new options like dump+reset, dump+pause, etc. That option is still possible, but orthogonal to this change. Resolves: https://issues.redhat.com/browse/RHEL-753 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/formatdomain.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4336cff3ac05..e6f09a728f0f 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7914,7 +7914,8 @@ southbridge, which is used with the q35 machine type. :since:`Since 9.1.0` - 'poweroff' - forcefully power off the guest - 'pause' - pause the guest - 'none' - do nothing - - 'dump' - automatically dump the guest :since:`Since 0.8.7` + - 'dump' - automatically dump the guest, beware that after the + dump the guest will be resumed :since:`Since 0.8.7` - 'inject-nmi' - inject a non-maskable interrupt into the guest :since:`Since 1.2.17` -- 2.46.2

3 2

[PATCH] ch: Enable a logfile for ch guests
by Praveen K Paladugu 11 Oct '24

11 Oct '24

Create a logfile to capture output from cloud-hypervisor whenever a guest is started. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com> --- src/ch/ch_monitor.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 3e49902791..7421a550b6 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -541,6 +541,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg) g_autoptr(virCHMonitor) mon = NULL; g_autoptr(virCommand) cmd = NULL; const char *socketdir = cfg->stateDir; + const char *logdir = cfg->logDir; + g_autofree char *logfile = NULL; + int socket_fd = 0; if (virCHMonitorInitialize() < 0) @@ -564,6 +567,14 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg) return NULL; } + logfile = g_strdup_printf("%s/%s.log", logdir, vm->def->name); + if (g_mkdir_with_parents(logdir, 0777) < 0) { + virReportSystemError(errno, + _("Cannot create log directory '%1$s'"), + logdir); + return NULL; + } + if (g_mkdir_with_parents(cfg->saveDir, 0777) < 0) { virReportSystemError(errno, _("Cannot create save directory '%1$s'"), @@ -583,6 +594,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg) virCommandAddArg(cmd, "--api-socket"); virCommandAddArgFormat(cmd, "fd=%d", socket_fd); + virCommandAddArg(cmd, "-v"); + virCommandAddArg(cmd, "--log-file"); + virCommandAddArgFormat(cmd, "%s", logfile); virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); /* launch Cloud-Hypervisor socket */ -- 2.44.0

1 0

[PATCH] util: Rename variable "major" in virIsDevMapperDevice
by Jiri Denemark 11 Oct '24

11 Oct '24

major() is a macro defined in sys/sysmacros.h so luckily the code works, but it's very confusing. Let's rename the local variable to make the difference between it and the macro more obvious. And while touching the line we can also initialize it to make sure "clever" analyzers do not think it may be used uninitialized. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/util/virdevmapper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c index 70ce0f0c23..d0eae671ab 100644 --- a/src/util/virdevmapper.c +++ b/src/util/virdevmapper.c @@ -321,14 +321,14 @@ bool virIsDevMapperDevice(const char *dev_name) { struct stat buf; - unsigned int major; + unsigned int maj = 0; - if (virDevMapperGetMajor(&major) < 0) + if (virDevMapperGetMajor(&maj) < 0) return false; if (!stat(dev_name, &buf) && S_ISBLK(buf.st_mode) && - major(buf.st_rdev) == major) + major(buf.st_rdev) == maj) return true; return false; -- 2.47.0

2 1

[libvirt PATCH] Remove pointless bool conversions
by Ján Tomko 11 Oct '24

11 Oct '24

Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_process.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 8 ++++---- src/qemu/qemu_process.c | 2 +- tools/virsh-snapshot.c | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0f129ec69c..c0122eb295 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -855,7 +855,7 @@ libxlDomainCleanup(libxlDriverPrivate *driver, priv->deathW = NULL; } - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); /* Release auto-allocated graphics ports */ diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 205ab96ebb..dc6f4fc03c 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -203,7 +203,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver, vm->pid = 0; vm->def->id = -1; - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); virLXCDomainReAttachHostDevices(driver, vm->def); diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 26072ec26e..e65dbb93d9 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -588,7 +588,7 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReq *req) if (!req) return; - if (!!g_atomic_int_dec_and_test(&req->refctr)) { + if (g_atomic_int_dec_and_test(&req->refctr)) { /* * delete the request: * - if we don't find req on the global list anymore @@ -743,7 +743,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReq *req, skip_instantiate: g_free(ipl); - ignore_value(!!g_atomic_int_dec_and_test(&virNWFilterSnoopState.nLeases)); + ignore_value(g_atomic_int_dec_and_test(&virNWFilterSnoopState.nLeases)); return ret; } @@ -1004,7 +1004,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) _("Instantiation of rules failed on interface '%1$s'"), req->binding->portdevname); } - ignore_value(!!g_atomic_int_dec_and_test(job->qCtr)); + ignore_value(g_atomic_int_dec_and_test(job->qCtr)); } /* @@ -1388,7 +1388,7 @@ virNWFilterDHCPSnoopThread(void *req0) pcap_close(pcapConf[i].handle); } - ignore_value(!!g_atomic_int_dec_and_test(&virNWFilterSnoopState.nThreads)); + ignore_value(g_atomic_int_dec_and_test(&virNWFilterSnoopState.nThreads)); return; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a00066e88e..f85e164c10 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8743,7 +8743,7 @@ void qemuProcessStop(virQEMUDriver *driver, if (priv->eventThread) g_object_unref(g_steal_pointer(&priv->eventThread)); - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + if (g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); /* Clear network bandwidth */ diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 8b6a950a01..e0be8e7582 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -780,8 +780,8 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, return -1; } if (STREQ(state, "disk-snapshot")) { - return !!((flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && - (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); + return (flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && + (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL); } if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) -- 2.47.0

2 1

[PATCH] network: inhibit idle timeout of daemon if there are any active networks
by Laine Stump 11 Oct '24

11 Oct '24

When the daemons were split out from the monolithic libvirtd, the network driver didn't implement "inhibit idle timeout if there are any active objects" as was done for other drivers, so virtnetworkd would always exit after 120 seconds of no incoming connections. This didn't every cause any visible problem, although it did mean that anytime a network API was called after an idle time > 120 seconds, that the restarting virtnetworkd would flush and reload all the iptables/nftables rules for any active networks. This patch replicates what is done in the QEMU driver - an nactive is added to the network driver object, along with an inhibitCallback; the latter is passed into networkStateInitialize when the driver is loaded, and the former is incremented for each already-active network, then incremented/decremented each time a network is started or stopped. If nactive transitions from 0 to 1 or 1 to 0, inhibitCallback is called, and it "does the right stuff" to prevent/enable the idle timeout. Signed-off-by: Laine Stump <laine(a)redhat.com> --- I had made this patch as a part of a larger series that will require it, but haven't sent that yet and keep being annoyed when virtnetworkd exits out from under a gdb session, so I decided it has enough general usefulness to send by itself. src/network/bridge_driver.c | 20 +++++++++++++++++--- src/network/bridge_driver_conf.h | 9 ++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 74ba59b4e9..6a48516fdf 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -501,9 +501,13 @@ networkUpdateState(virNetworkObj *obj, return -1; } - if (virNetworkObjIsActive(obj)) + if (virNetworkObjIsActive(obj)) { virNetworkObjPortForEach(obj, networkUpdatePort, obj); + if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + } + /* Try and read dnsmasq pids of both active and inactive networks, just in * case a network became inactive and we need to clean up. */ if (def->ips && (def->nips > 0)) { @@ -617,8 +621,8 @@ static virDrvStateInitResult networkStateInitialize(bool privileged, const char *root, bool monolithic G_GNUC_UNUSED, - virStateInhibitCallback callback G_GNUC_UNUSED, - void *opaque G_GNUC_UNUSED) + virStateInhibitCallback callback, + void *opaque) { virNetworkDriverConfig *cfg; bool autostart = true; @@ -640,6 +644,9 @@ networkStateInitialize(bool privileged, goto error; } + network_driver->inhibitCallback = callback; + network_driver->inhibitOpaque = opaque; + network_driver->privileged = privileged; if (!(network_driver->xmlopt = networkDnsmasqCreateXMLConf())) @@ -2427,6 +2434,9 @@ networkStartNetwork(virNetworkDriverState *driver, obj, network_driver->xmlopt) < 0) goto cleanup; + if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback) + driver->inhibitCallback(true, driver->inhibitOpaque); + virNetworkObjSetActive(obj, true); VIR_INFO("Network '%s' started up", def->name); ret = 0; @@ -2500,6 +2510,10 @@ networkShutdownNetwork(virNetworkDriverState *driver, VIR_HOOK_SUBOP_END); virNetworkObjSetActive(obj, false); + + if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) + driver->inhibitCallback(false, driver->inhibitOpaque); + virNetworkObjUnsetDefTransient(obj); return ret; } diff --git a/src/network/bridge_driver_conf.h b/src/network/bridge_driver_conf.h index 8f221f391e..1beed01efb 100644 --- a/src/network/bridge_driver_conf.h +++ b/src/network/bridge_driver_conf.h @@ -21,7 +21,7 @@ #pragma once -#include "internal.h" +#include "libvirt_internal.h" #include "virthread.h" #include "virdnsmasq.h" #include "virnetworkobj.h" @@ -49,6 +49,13 @@ typedef struct _virNetworkDriverState virNetworkDriverState; struct _virNetworkDriverState { virMutex lock; + /* Atomic inc/dec only */ + unsigned int nactive; + + /* Immutable pointers. Caller must provide locking */ + virStateInhibitCallback inhibitCallback; + void *inhibitOpaque; + /* Read-only */ bool privileged; -- 2.46.1

4 4

[PATCH v2] chardev: introduce 'reconnect-ms' and deprecate 'reconnect'
by Daniil Tatianin 10 Oct '24

10 Oct '24

The 'reconnect' option only allows to specify the time in seconds, which is way too long for certain workflows. We have a lightweight disk backend server, which takes about 20ms to live update, but due to this limitation in QEMU, previously the guest disk controller would hang for one second because it would take this long for QEMU to reinitialize the socket connection. Introduce a new option called 'reconnect-ms', which is the same as 'reconnect', except the value is treated as milliseconds. These are mutually exclusive and specifying both results in an error. 'reconnect' is also deprecated by this commit to make it possible to remove it in the future as to not keep two options that control the same thing. Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)yandex-team.ru> Acked-by: Peter Krempa <pkrempa(a)redhat.com> Signed-off-by: Daniil Tatianin <d-tatianin(a)yandex-team.ru> --- Changes since v0: - Mention the deprecation in docs (Paolo) Changes since v1: - Move option validation to qmp_chardev_validate_socket as qemu_chr_parse_socket is only called for the command line and not QMP. (thanks to Markus Armbruster for spotting) --- chardev/char-socket.c | 33 ++++++++++++++++++++++++--------- chardev/char.c | 3 +++ docs/about/deprecated.rst | 6 ++++++ include/chardev/char-socket.h | 2 +- qapi/char.json | 17 +++++++++++++++-- 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 1ca9441b1b..91496ceda9 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -74,7 +74,7 @@ static void qemu_chr_socket_restart_timer(Chardev *chr) assert(!s->reconnect_timer); name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label); s->reconnect_timer = qemu_chr_timeout_add_ms(chr, - s->reconnect_time * 1000, + s->reconnect_time_ms, socket_reconnect_timeout, chr); g_source_set_name(s->reconnect_timer, name); @@ -481,7 +481,7 @@ static void tcp_chr_disconnect_locked(Chardev *chr) if (emit_close) { qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } - if (s->reconnect_time && !s->reconnect_timer) { + if (s->reconnect_time_ms && !s->reconnect_timer) { qemu_chr_socket_restart_timer(chr); } } @@ -1080,9 +1080,9 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) } else { Error *err = NULL; if (tcp_chr_connect_client_sync(chr, &err) < 0) { - if (s->reconnect_time) { + if (s->reconnect_time_ms) { error_free(err); - g_usleep(s->reconnect_time * 1000ULL * 1000ULL); + g_usleep(s->reconnect_time_ms * 1000ULL); } else { error_propagate(errp, err); return -1; @@ -1267,13 +1267,13 @@ skip_listen: static int qmp_chardev_open_socket_client(Chardev *chr, - int64_t reconnect, + int64_t reconnect_ms, Error **errp) { SocketChardev *s = SOCKET_CHARDEV(chr); - if (reconnect > 0) { - s->reconnect_time = reconnect; + if (reconnect_ms > 0) { + s->reconnect_time_ms = reconnect_ms; tcp_chr_connect_client_async(chr); return 0; } else { @@ -1354,6 +1354,12 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock, } } + if (sock->has_reconnect_ms && sock->has_reconnect) { + error_setg(errp, + "'reconnect' and 'reconnect-ms' are mutually exclusive"); + return false; + } + return true; } @@ -1371,7 +1377,7 @@ static void qmp_chardev_open_socket(Chardev *chr, bool is_tn3270 = sock->has_tn3270 ? sock->tn3270 : false; bool is_waitconnect = sock->has_wait ? sock->wait : false; bool is_websock = sock->has_websocket ? sock->websocket : false; - int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; + int64_t reconnect_ms = 0; SocketAddress *addr; s->is_listen = is_listen; @@ -1443,7 +1449,13 @@ static void qmp_chardev_open_socket(Chardev *chr, return; } } else { - if (qmp_chardev_open_socket_client(chr, reconnect, errp) < 0) { + if (sock->has_reconnect) { + reconnect_ms = sock->reconnect * 1000ULL; + } else if (sock->has_reconnect_ms) { + reconnect_ms = sock->reconnect_ms; + } + + if (qmp_chardev_open_socket_client(chr, reconnect_ms, errp) < 0) { return; } } @@ -1509,6 +1521,9 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, sock->wait = qemu_opt_get_bool(opts, "wait", true); sock->has_reconnect = qemu_opt_find(opts, "reconnect"); sock->reconnect = qemu_opt_get_number(opts, "reconnect", 0); + sock->has_reconnect_ms = qemu_opt_find(opts, "reconnect-ms"); + sock->reconnect_ms = qemu_opt_get_number(opts, "reconnect-ms", 0); + sock->tls_creds = g_strdup(qemu_opt_get(opts, "tls-creds")); sock->tls_authz = g_strdup(qemu_opt_get(opts, "tls-authz")); diff --git a/chardev/char.c b/chardev/char.c index ba847b6e9e..35623c78a3 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -888,6 +888,9 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "reconnect", .type = QEMU_OPT_NUMBER, + },{ + .name = "reconnect-ms", + .type = QEMU_OPT_NUMBER, },{ .name = "telnet", .type = QEMU_OPT_BOOL, diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 88f0f03786..e5db9bc6e9 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -430,6 +430,12 @@ Backend ``memory`` (since 9.0) ``memory`` is a deprecated synonym for ``ringbuf``. +``reconnect`` (since 9.2) +^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``reconnect`` option only allows specifiying second granularity timeouts, +which is not enough for all types of use cases, use ``reconnect-ms`` instead. + CPU device properties ''''''''''''''''''''' diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h index 0708ca6fa9..d6d13ad37f 100644 --- a/include/chardev/char-socket.h +++ b/include/chardev/char-socket.h @@ -74,7 +74,7 @@ struct SocketChardev { bool is_websock; GSource *reconnect_timer; - int64_t reconnect_time; + int64_t reconnect_time_ms; bool connect_err_reported; QIOTask *connect_task; diff --git a/qapi/char.json b/qapi/char.json index ef58445cee..7f117438c6 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -273,7 +273,19 @@ # # @reconnect: For a client socket, if a socket is disconnected, then # attempt a reconnect after the given number of seconds. Setting -# this to zero disables this function. (default: 0) (Since: 2.2) +# this to zero disables this function. The use of this member is +# deprecated, use @reconnect-ms instead. (default: 0) (Since: 2.2) +# +# @reconnect-ms: For a client socket, if a socket is disconnected, +# then attempt a reconnect after the given number of milliseconds. +# Setting this to zero disables this function. This member is +# mutually exclusive with @reconnect. +# (default: 0) (Since: 9.2) +# +# Features: +# +# @deprecated: Member @reconnect is deprecated. Use @reconnect-ms +# instead. # # Since: 1.4 ## @@ -287,7 +299,8 @@ '*telnet': 'bool', '*tn3270': 'bool', '*websocket': 'bool', - '*reconnect': 'int' }, + '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] }, + '*reconnect-ms': 'int' }, 'base': 'ChardevCommon' } ## -- 2.34.1

4 4

[PATCH V3] libxl: Reject VM config referencing nwfilters
by Jim Fehlig 10 Oct '24

10 Oct '24

The Xen libxl driver does not support nwfilter. Introduce a deviceValidateCallback function with a check for nwfilters, returning VIR_ERR_CONFIG_UNSUPPORTED if any are found. Also fail to start any existing VMs referencing nwfilters. Drivers generally ignore unrecognized XML configuration, but ignoring a user's request to filter VM network traffic can be viewed as a security issue. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- V2: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/UHG… Changes in V3: When starting a VM, use virDomainDefValidate to validate the def instead of duplicating the checks. Note: With the change in V3, the validation is only done when starting a new VM. Validation is skipped if restoring a managed saved VM. src/libxl/libxl_domain.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0f129ec69c..029b12fa3b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -356,12 +356,30 @@ libxlDomainDefValidate(const virDomainDef *def, return 0; } +static int +libxlDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + void *opaque G_GNUC_UNUSED, + void *parseOpaque G_GNUC_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->filter) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filterref is not supported in %1$s"), + virDomainVirtTypeToString(def->virtType)); + return -1; + } + + return 0; +} + + virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .netPrefix = LIBXL_GENERATED_PREFIX_XEN, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, .domainValidateCallback = libxlDomainDefValidate, + .deviceValidateCallback = libxlDomainDeviceDefValidate, .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | @@ -1460,6 +1478,10 @@ libxlDomainStartNew(libxlDriverPrivate *driver, managed_save_path); vm->hasManagedSave = false; + } else { + /* Validate configuration if starting a new VM */ + if (virDomainDefValidate(vm->def, 0, driver->xmlopt, NULL) < 0) + goto cleanup; } ret = libxlDomainStart(driver, vm, start_paused, restore_fd, restore_ver); -- 2.35.3

2 1