[libvirt PATCH] qemu: Enable unprivileged userfaultfd for post-copy migration
Userfaultfd is by default allowed only for privileged processes. Since libvirt runs QEMU unprivileged, we need to enable unprivileged access to userfaultfd before starting post-copy migration. Rather than providing a static sysctl configuration file, we set the sysctl knob in runtime once post-copy migration is requested. This way unprivileged_userfaultfd is only enabled once actually used. https://bugzilla.redhat.com/show_bug.cgi?id=1945420 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index dbc3219826..a9449ed1ff 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -804,6 +804,24 @@ qemuMigrationCapsToJSON(virBitmap *caps, } +static void +qemuMigrationParamsEnableUserfaultfd(virDomainObj *vm) +{ + const char *sysctl = "/proc/sys/vm/unprivileged_userfaultfd"; + + if (!virFileExists(sysctl)) + return; + + VIR_DEBUG("Enabling unprivileged userfaultfd for post-copy migration of " + "domain %s", vm->def->name); + + if (virFileWriteStr(sysctl, "1", 0) < 0) { + virReportSystemError(errno, "%s", + _("failed to enable unprivileged userfaultfd")); + } +} + + /** * qemuMigrationParamsApply * @driver: qemu driver @@ -839,6 +857,13 @@ qemuMigrationParamsApply(virQEMUDriver *driver, goto cleanup; } } else { + /* userfaultfd may only be enabled for privileged processes by default, + * we need to make sure QEMU can use it before enabling post-copy + * migration */ + if (virBitmapIsBitSet(priv->migrationCaps, QEMU_MIGRATION_CAP_POSTCOPY) && + virBitmapIsBitSet(migParams->caps, QEMU_MIGRATION_CAP_POSTCOPY)) + qemuMigrationParamsEnableUserfaultfd(vm); + if (!(caps = qemuMigrationCapsToJSON(priv->migrationCaps, migParams->caps))) goto cleanup; -- 2.34.1
On 12/2/21 16:56, Jiri Denemark wrote:
Userfaultfd is by default allowed only for privileged processes. Since libvirt runs QEMU unprivileged, we need to enable unprivileged access to userfaultfd before starting post-copy migration.
Rather than providing a static sysctl configuration file, we set the sysctl knob in runtime once post-copy migration is requested. This way unprivileged_userfaultfd is only enabled once actually used.
https://bugzilla.redhat.com/show_bug.cgi?id=1945420
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index dbc3219826..a9449ed1ff 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -804,6 +804,24 @@ qemuMigrationCapsToJSON(virBitmap *caps, }
+static void +qemuMigrationParamsEnableUserfaultfd(virDomainObj *vm) +{ + const char *sysctl = "/proc/sys/vm/unprivileged_userfaultfd"; + + if (!virFileExists(sysctl)) + return; + + VIR_DEBUG("Enabling unprivileged userfaultfd for post-copy migration of " + "domain %s", vm->def->name); + + if (virFileWriteStr(sysctl, "1", 0) < 0) { + virReportSystemError(errno, "%s", + _("failed to enable unprivileged userfaultfd")); + }
I can imagine a wild scenario where libvirtd is run inside a restricted container but with the proper value set. How about we check whether the expected value isn't set already? No need to resend, the change is trivial enough to be done before push. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Thu, Dec 02, 2021 at 16:56:49 +0100, Jiri Denemark wrote:
Userfaultfd is by default allowed only for privileged processes. Since libvirt runs QEMU unprivileged, we need to enable unprivileged access to userfaultfd before starting post-copy migration.
Rather than providing a static sysctl configuration file, we set the sysctl knob in runtime once post-copy migration is requested. This way unprivileged_userfaultfd is only enabled once actually used.
https://bugzilla.redhat.com/show_bug.cgi?id=1945420
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index dbc3219826..a9449ed1ff 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -804,6 +804,24 @@ qemuMigrationCapsToJSON(virBitmap *caps, }
+static void +qemuMigrationParamsEnableUserfaultfd(virDomainObj *vm)
passing 'vm'
+{ + const char *sysctl = "/proc/sys/vm/unprivileged_userfaultfd"; + + if (!virFileExists(sysctl)) + return; + + VIR_DEBUG("Enabling unprivileged userfaultfd for post-copy migration of " + "domain %s", vm->def->name);
Just to debug-log the VM name is a bit pointless. If debug logs are enabled you'll be able to figure out the migrated VM by other log entries.
+ + if (virFileWriteStr(sysctl, "1", 0) < 0) { + virReportSystemError(errno, "%s", + _("failed to enable unprivileged userfaultfd"));
Reporting an error and not actually failing is also something we rarely do. Firstly it can set the error object and report a spurious error in cases when we'd forget to set an error, and additionally it can flood system logs with 'error' level messages which is also getting frequently reported.
+ } +} + + /** * qemuMigrationParamsApply * @driver: qemu driver @@ -839,6 +857,13 @@ qemuMigrationParamsApply(virQEMUDriver *driver, goto cleanup; } } else { + /* userfaultfd may only be enabled for privileged processes by default, + * we need to make sure QEMU can use it before enabling post-copy + * migration */ + if (virBitmapIsBitSet(priv->migrationCaps, QEMU_MIGRATION_CAP_POSTCOPY) && + virBitmapIsBitSet(migParams->caps, QEMU_MIGRATION_CAP_POSTCOPY)) + qemuMigrationParamsEnableUserfaultfd(vm); + if (!(caps = qemuMigrationCapsToJSON(priv->migrationCaps, migParams->caps))) goto cleanup;
-- 2.34.1
participants (3)
-
Jiri Denemark -
Michal Prívozník -
Peter Krempa