[libvirt PATCH] qemu: Do not return NULL when qemuMigrationSrcBegin succeeds
by Jiri Denemark
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
3 years
[libvirt PATCH v2 0/7] po: Various fixes and cleanups
by Andrea Bolognani
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
3 years
[PATCH v2] remote_daemon: Don't run virStateCleanup() if virStateReload() is still running
by Michal Privoznik
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
3 years
[libvirt PATCH] qemu: Fix error propagation in qemuMigrationBegin
by Jiri Denemark
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
3 years
[libvirt PATCH] apparmor: Enable locking AAVMF firmware
by Andrea Bolognani
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
3 years
[PATCH] Allow VM to read sysfs PCI config, revision files
by Max Goodhart
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 years
[libvirt PATCH 00/80] Add support for post-copy recovery
by Jiri Denemark
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
3 years
[PATCH] remote_daemon: Don't run virStateCleanup() if virStateReload() is still running
by Michal Privoznik
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 years
[libvirt PATCH 0/5] Description Here
by Ján Tomko
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 years
