May 2022 - Devel - libvirt List Archives

[libvirt PATCH] qemu: Do not return NULL when qemuMigrationSrcBegin succeeds
by Jiri Denemark 23 May '22

23 May '22

My recent commit v8.3.0-201-gc500955e95 tried to fix a regression which would cause the function to return success even if virCloseCallbacksSet failed. But due to a strange code flow in the function introduced an opposite regression. The function would return NULL on success when called without VIR_MIGRATE_CHANGE_PROTECTION flag. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_migration.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 38596fa4de..6cc68a567a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2534,23 +2534,23 @@ qemuMigrationSrcBegin(virConnectPtr conn, if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, qemuMigrationSrcCleanup) < 0) goto endjob; - qemuMigrationJobContinue(vm); - } else { - goto endjob; } ret = g_steal_pointer(&xml); + endjob: + if (flags & VIR_MIGRATE_CHANGE_PROTECTION) { + if (ret) + qemuMigrationJobContinue(vm); + else + qemuMigrationJobFinish(vm); + } else { + qemuDomainObjEndJob(vm); + } + cleanup: virDomainObjEndAPI(&vm); return ret; - - endjob: - if (flags & VIR_MIGRATE_CHANGE_PROTECTION) - qemuMigrationJobFinish(vm); - else - qemuDomainObjEndJob(vm); - goto cleanup; } -- 2.35.1

2 1

[PATCH 0/2] Fix missing 'manual' mode for default snapshot disk definition and clean up docs
by Peter Krempa 23 May '22

23 May '22

Peter Krempa (2): docs: domain: Remove extraneous quotes schemas: Allow 'manual' snapshot mode in domain definition docs/formatdomain.rst | 20 ++++++++++---------- src/conf/schemas/domaincommon.rng | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) -- 2.35.3

2 3

[libvirt PATCH v2 0/7] po: Various fixes and cleanups
by Andrea Bolognani 23 May '22

23 May '22

Changes from [v1] * instead of checking that the potfile doesn't contain unwanted comments at syntax-check time, prevent them from being added by passing all xgettext options explicitly ourselves. [v1] https://listman.redhat.com/archives/libvir-list/2022-May/231526.html Andrea Bolognani (7): po: Drop unwanted comments from potfile po: Stop using 'glib' preset for i18n.gettext() po: Drop prefixes from POTFILES.in po: Don't generate POTFILES syntax-check: Don't exclude src/false.c from sc_po_check po: Sort LINGUAS syntax-check: Introduce sc_linguas_sorting build-aux/syntax-check.mk | 21 ++- po/LINGUAS | 8 +- po/POTFILES | 384 ++++++++++++++++++++++++++++++++++++++ po/POTFILES.in | 384 -------------------------------------- po/libvirt.pot | 216 --------------------- po/meson.build | 17 +- 6 files changed, 406 insertions(+), 624 deletions(-) create mode 100644 po/POTFILES delete mode 100644 po/POTFILES.in -- 2.35.3

2 8

[PATCH v2] remote_daemon: Don't run virStateCleanup() if virStateReload() is still running
by Michal Privoznik 23 May '22

23 May '22

When a SIGHUP is received a thread is spawned that runs virStateReload(). However, if SIGINT is received while the former thread is still running then we may get into problematic situation: the cleanup code in main() sees drivers initialized and thus calls virStateCleanup(). So now we have two threads, one running virStateReload() the other virStateCleanup(). In this situation it's very likely that a race condition occurs and either of threads causes SIGSEGV. To fix this, unmark drivers as initialized in the virStateReload() thread for the time the function runs. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2075837 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://listman.redhat.com/archives/libvir-list/2022-April/230415.html diff to v1: - reworked how int is set (instead of inc/dec I'm using set(0)/set(1)) so that reload can be attempted again and again if previous attempt failed. src/remote/remote_daemon.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 26469e0d9f..b8ecc51758 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -77,7 +77,7 @@ virNetSASLContext *saslCtxt = NULL; virNetServerProgram *remoteProgram = NULL; virNetServerProgram *qemuProgram = NULL; -volatile bool driversInitialized = false; +volatile gint driversInitialized = 0; static void daemonErrorHandler(void *opaque G_GNUC_UNUSED, virErrorPtr err G_GNUC_UNUSED) @@ -453,8 +453,13 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED) VIR_INFO("Reloading configuration on SIGHUP"); virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL); - if (virStateReload() < 0) + + if (virStateReload() < 0) { VIR_WARN("Error while reloading drivers"); + } + + /* Drivers are initialized again. */ + g_atomic_int_set(&driversInitialized, 1); } static void daemonReloadHandler(virNetDaemon *dmn G_GNUC_UNUSED, @@ -463,7 +468,7 @@ static void daemonReloadHandler(virNetDaemon *dmn G_GNUC_UNUSED, { virThread thr; - if (!driversInitialized) { + if (!g_atomic_int_compare_and_exchange(&driversInitialized, 1, 0)) { VIR_WARN("Drivers are not initialized, reload ignored"); return; } @@ -474,6 +479,10 @@ static void daemonReloadHandler(virNetDaemon *dmn G_GNUC_UNUSED, * Not much we can do on error here except log it. */ VIR_ERROR(_("Failed to create thread to handle daemon restart")); + + /* Drivers were initialized at the beginning, otherwise we wouldn't + * even get here. */ + g_atomic_int_set(&driversInitialized, 1); } } @@ -607,7 +616,7 @@ static void daemonRunStateInit(void *opaque) goto cleanup; } - driversInitialized = true; + g_atomic_int_set(&driversInitialized, 1); virNetDaemonSetShutdownCallbacks(dmn, virStateShutdownPrepare, @@ -1212,10 +1221,9 @@ int main(int argc, char **argv) { cleanup: virNetlinkEventServiceStopAll(); - if (driversInitialized) { + if (g_atomic_int_compare_and_exchange(&driversInitialized, 1, 0)) { /* NB: Possible issue with timing window between driversInitialized * setting if virNetlinkEventServerStart fails */ - driversInitialized = false; virStateCleanup(); } -- 2.35.1

2 1

[libvirt PATCH] qemu: Fix error propagation in qemuMigrationBegin
by Jiri Denemark 23 May '22

23 May '22

Commit v8.3.0-152-g49ef0f95c6 removed explicit VIR_FREE from qemuMigrationBegin, effectively reverting v1.2.14-57-g77ddd0bba2 The xml variable was used to hold the return value and thus had to be unset when an error happened after xml was already non-NULL. Such code may be quite confusing though and we usually avoid it by not storing anything to a return variable until everything succeeded. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_migration.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 438f2bc999..38596fa4de 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2487,6 +2487,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, virQEMUDriver *driver = conn->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *xml = NULL; + char *ret = NULL; virDomainAsyncJob asyncJob; if (cfg->migrateTLSForce && @@ -2538,9 +2539,11 @@ qemuMigrationSrcBegin(virConnectPtr conn, goto endjob; } + ret = g_steal_pointer(&xml); + cleanup: virDomainObjEndAPI(&vm); - return g_steal_pointer(&xml); + return ret; endjob: if (flags & VIR_MIGRATE_CHANGE_PROTECTION) -- 2.35.1

2 1

[libvirt PATCH] apparmor: Enable locking AAVMF firmware
by Andrea Bolognani 23 May '22

23 May '22

We already allow this for OVMF. Closes: https://gitlab.com/libvirt/libvirt/-/issues/312 Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- src/security/apparmor/libvirt-qemu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index c29168da27..02ee273e7e 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -78,7 +78,7 @@ /var/lib/dbus/machine-id r, # access to firmware's etc - /usr/share/AAVMF/** r, + /usr/share/AAVMF/** rk, /usr/share/bochs/** r, /usr/share/edk2-ovmf/** rk, /usr/share/kvm/** r, -- 2.35.3

2 1

[PATCH] Allow VM to read sysfs PCI config, revision files
by Max Goodhart 20 May '22

20 May '22

From: Max Goodhart <gitlab(a)chromakode.com> This fixes a blank screen when viewing a VM with virtio graphics and gl-accelerated Spice display on Ubuntu 22.04 / libvirt 8.0.0 / qemu 6.2. Without these AppArmor permissions, the libvirt error log contains repetitions of: qemu_spice_gl_scanout_texture: failed to get fd for texture This appears to be similar to this GNOME Boxes issue: https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/586 Signed-off-by: Max Goodhart <c(a)chromakode.com> --- src/security/virt-aa-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 1f1cce8b3d..b314d2a059 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1316,7 +1316,7 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n"); virBufferAddLit(&buf, " # Probe DRI device attributes\n"); virBufferAddLit(&buf, " \"/dev/dri/\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); + virBufferAddLit(&buf, " \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device,config,revision}\" r,\n"); virBufferAddLit(&buf, " # dri libs will trigger that, but t is not requited and DAC would deny it anyway\n"); virBufferAddLit(&buf, " deny \"/var/lib/libvirt/.cache/\" w,\n"); } -- 2.34.1

3 5

[libvirt PATCH 00/80] Add support for post-copy recovery
by Jiri Denemark 19 May '22

19 May '22

This series implements a new VIR_MIGRATE_POSTCOPY_RESUME flag (virsh migrate --resume) for recovering from a failed post-copy migration. You can also fetch the series from my gitlab fork: git fetch https://gitlab.com/jirkade/libvirt.git post-copy-recovery Jiri Denemark (80): qemu: Add debug messages to job recovery code qemumonitorjsontest: Test more migration capabilities qemu: Return state from qemuMonitorGetMigrationCapabilities qemu: Enable migration events only when disabled Introduce VIR_DOMAIN_RUNNING_POSTCOPY_FAILED qemu: Keep domain running on dst on failed post-copy migration qemu: Explicitly emit events on post-copy failure qemu: Make qemuDomainCleanupAdd return void conf: Introduce virDomainObjIsFailedPostcopy helper conf: Introduce virDomainObjIsPostcopy helper qemu: Introduce qemuProcessCleanupMigrationJob qemu: Rename qemuDomainObjRestoreJob as qemuDomainObjPreserveJob qemu: Add qemuDomainObjRestoreAsyncJob qemu: Keep migration job active after failed post-copy qemu: Abort failed post-copy when we haven't called Finish yet qemu: Restore failed migration job on reconnect qemu: Restore async job start timestamp on reconnect qemu: Drop forward declarations in migration code qemu: Don't wait for migration job when migration is running qemu: Use switch in qemuDomainGetJobInfoMigrationStats qemu: Fetch paused migration stats qemu: Handle 'postcopy-paused' migration state qemu: Add support for postcopy-recover QEMU migration state qemu: Create domain object at the end of qemuMigrationDstFinish qemu: Move success-only code out of endjob in qemuMigrationDstFinish qemu: Separate success and failure path in qemuMigrationDstFinish qemu: Rename "endjob" label in qemuMigrationDstFinish qemu: Generate migration cookie in Finish phase earlier qemu: Make final part of migration Finish phase reusable qemu: Drop obsolete comment in qemuMigrationDstFinish qemu: Preserve error in qemuMigrationDstFinish qemu: Introduce qemuMigrationDstFinishFresh qemu: Introduce qemuMigrationDstFinishOffline qemu: Separate cookie parsing for qemuMigrationDstFinishOffline qemu: Introduce qemuMigrationDstFinishActive qemu: Handle migration job in qemuMigrationDstFinish qemu: Make final part of migration Confirm phase reusable qemu: Make sure migrationPort is released even in callbacks qemu: Pass qemuDomainJobObj to qemuMigrationDstComplete qemu: Finish completed unattended migration qemu: Ignore missing memory statistics in query-migrate qemu: Improve post-copy migration handling on reconnect qemu: Check flags incompatible with offline migration earlier qemu: Introduce qemuMigrationSrcBeginXML helper qemu: Add new migration phases for post-copy recovery qemu: Separate protocol checks from qemuMigrationJobSetPhase qemu: Make qemuMigrationCheckPhase failure fatal qemu: Refactor qemuDomainObjSetJobPhase qemu: Do not set job owner in qemuMigrationJobSetPhase qemu: Use QEMU_MIGRATION_PHASE_POSTCOPY_FAILED Introduce VIR_MIGRATE_POSTCOPY_RESUME flag virsh: Add --postcopy-resume option for migrate command qemu: Don't set VIR_MIGRATE_PAUSED for post-copy resume qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Begin phase qmeu: Refactor qemuMigrationSrcPerformPhase qemu: Separate starting migration from qemuMigrationSrcRun qemu: Add support for 'resume' parameter of migrate QMP command qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Perform phase qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Confirm phase qemu: Introduce qemuMigrationDstPrepareFresh qemu: Refactor qemuMigrationDstPrepareFresh qemu: Simplify cleanup in qemuMigrationDstPrepareFresh qemu: Add support for migrate-recover QMP command qemu: Rename qemuMigrationSrcCleanup qemu: Refactor qemuMigrationAnyConnectionClosed qemu: Handle incoming migration in qemuMigrationAnyConnectionClosed qemu: Start a migration phase in qemuMigrationAnyConnectionClosed qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Prepare phase qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Finish phase qemu: Create completed jobData in qemuMigrationSrcComplete qemu: Register qemuProcessCleanupMigrationJob after Begin phase qemu: Call qemuDomainCleanupAdd from qemuMigrationJobContinue qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for peer-to-peer migration qemu: Enable support for VIR_MIGRATE_POSTCOPY_RESUME Add virDomainAbortJobFlags public API qemu: Implement virDomainAbortJobFlags Add VIR_DOMAIN_ABORT_JOB_POSTCOPY flag for virDomainAbortJobFlags qemu: Implement VIR_DOMAIN_ABORT_JOB_POSTCOPY flag virsh: Add --postcopy option for domjobabort command NEWS: Add support for post-copy recovery NEWS.rst | 5 + docs/manpages/virsh.rst | 17 +- examples/c/misc/event-test.c | 3 + include/libvirt/libvirt-domain.h | 26 + src/conf/domain_conf.c | 33 + src/conf/domain_conf.h | 8 + src/driver-hypervisor.h | 5 + src/hypervisor/domain_job.c | 1 + src/hypervisor/domain_job.h | 1 + src/libvirt-domain.c | 72 +- src/libvirt_private.syms | 2 + src/libvirt_public.syms | 1 + src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_domain.c | 9 +- src/qemu/qemu_domain.h | 5 +- src/qemu/qemu_domainjob.c | 103 +- src/qemu/qemu_domainjob.h | 16 +- src/qemu/qemu_driver.c | 83 +- src/qemu/qemu_migration.c | 2383 +++++++++++------ src/qemu/qemu_migration.h | 37 +- src/qemu/qemu_migration_params.c | 19 +- src/qemu/qemu_monitor.c | 27 +- src/qemu/qemu_monitor.h | 13 +- src/qemu/qemu_monitor_json.c | 151 +- src/qemu/qemu_monitor_json.h | 10 +- src/qemu/qemu_process.c | 355 ++- src/qemu/qemu_process.h | 3 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +- src/remote_protocol-structs | 5 + tests/qemumonitorjsontest.c | 42 +- .../migration-in-params-in.xml | 2 +- .../migration-out-nbd-bitmaps-in.xml | 2 +- .../migration-out-nbd-out.xml | 2 +- .../migration-out-nbd-tls-out.xml | 2 +- .../migration-out-params-in.xml | 2 +- tools/virsh-domain-event.c | 3 +- tools/virsh-domain-monitor.c | 1 + tools/virsh-domain.c | 24 +- 39 files changed, 2556 insertions(+), 934 deletions(-) -- 2.35.1

5 195

[PATCH] remote_daemon: Don't run virStateCleanup() if virStateReload() is still running
by Michal Privoznik 19 May '22

19 May '22

When a SIGHUP is received a thread is spawned that runs virStateReload(). However, if SIGINT is received while the former thread is still running then we may get into problematic situation: the cleanup code in main() sees drivers initialized and thus calls virStateCleanup(). So now we have two threads, one running virStateReload() the other virStateCleanup(). In this situation it's very likely that a race condition occurs and either of threads causes SIGSEGV. To fix this, unmark drivers as initialized in the virStateReload() thread for the time the function runs. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2075837 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/remote/remote_daemon.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 26469e0d9f..37d27f93f4 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -77,7 +77,7 @@ virNetSASLContext *saslCtxt = NULL; virNetServerProgram *remoteProgram = NULL; virNetServerProgram *qemuProgram = NULL; -volatile bool driversInitialized = false; +volatile gint driversInitialized = 0; static void daemonErrorHandler(void *opaque G_GNUC_UNUSED, virErrorPtr err G_GNUC_UNUSED) @@ -453,8 +453,13 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED) VIR_INFO("Reloading configuration on SIGHUP"); virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL); - if (virStateReload() < 0) + + g_atomic_int_set(&driversInitialized, 0); + if (virStateReload() < 0) { VIR_WARN("Error while reloading drivers"); + } else { + g_atomic_int_inc(&driversInitialized); + } } static void daemonReloadHandler(virNetDaemon *dmn G_GNUC_UNUSED, @@ -463,7 +468,7 @@ static void daemonReloadHandler(virNetDaemon *dmn G_GNUC_UNUSED, { virThread thr; - if (!driversInitialized) { + if (g_atomic_int_get(&driversInitialized) == 0) { VIR_WARN("Drivers are not initialized, reload ignored"); return; } @@ -607,7 +612,7 @@ static void daemonRunStateInit(void *opaque) goto cleanup; } - driversInitialized = true; + g_atomic_int_inc(&driversInitialized); virNetDaemonSetShutdownCallbacks(dmn, virStateShutdownPrepare, @@ -1212,10 +1217,10 @@ int main(int argc, char **argv) { cleanup: virNetlinkEventServiceStopAll(); - if (driversInitialized) { + if (g_atomic_int_get(&driversInitialized) != 0) { /* NB: Possible issue with timing window between driversInitialized * setting if virNetlinkEventServerStart fails */ - driversInitialized = false; + g_atomic_int_set(&driversInitialized, 0); virStateCleanup(); } -- 2.35.1

3 4

[libvirt PATCH 0/5] Description Here
by Ján Tomko 19 May '22

19 May '22

Ján Tomko (5): Do not check if unsigned vars are less than zero src: QemuMonitorCommandWithFiles: report error when fd passing is unsupported apparmor: report error when removing profile failed vbox: SnapshotConfAllChildren: reduce scope of tempSize storagefile: set size field of ploop to 8 src/esx/esx_stream.c | 2 +- src/libvirt-qemu.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage_file/storage_file_probe.c | 2 +- src/vbox/vbox_snapshot_conf.c | 3 +-- 5 files changed, 6 insertions(+), 7 deletions(-) -- 2.34.1

3 7