[PATCH] libxl: Fix domain shutdown
Commit fa30ee04a2 caused a regression in normal domain shutown. Initiating a shutdown from within the domain or via 'virsh shutdown' does cause the guest OS running in the domain to shutdown, but libvirt never reaps the domain so it is always shown in a running state until calling 'virsh destroy'. The shutdown thread is also an internal user of the driver shutdown machinery and eventually calls libxlDomainDestroyInternal where the ignoreDeathEvent inhibitor is set, but running in a thread introduces the possibility of racing with the death event from libxl. This can be prevented by setting ignoreDeathEvent before running the shutdown thread. An additional improvement is to handle the destroy event synchronously instead of spawning a thread. The time consuming aspects of destroying a domain have been completed when the destroy event is delivered. Signed-off-by: Jim Fehlig --- src/libxl/libxl_domain.c | 115 ++- 1 file changed, 54 insertions(+), 61 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index d59153fffa..32dc503089 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -476,6 +476,7 @@ libxlDomainShutdownHandleRestart(libxlDriverPrivatePtr driver, struct libxlShutdownThreadInfo { libxlDriverPrivatePtr driver; +virDomainObjPtr vm; libxl_event *event; }; @@ -484,7 +485,7 @@ static void libxlDomainShutdownThread(void *opaque) { struct libxlShutdownThreadInfo *shutdown_info = opaque; -virDomainObjPtr vm = NULL; +virDomainObjPtr vm = shutdown_info->vm; libxl_event *ev = shutdown_info->event; libxlDriverPrivatePtr driver = shutdown_info->driver; virObjectEventPtr dom_event = NULL; @@ -494,12 +495,6 @@ libxlDomainShutdownThread(void *opaque) libxl_domain_config_init(_config); -vm = virDomainObjListFindByID(driver->domains, ev->domid); -if (!vm) { -VIR_INFO("Received event for unknown domain ID %d", ev->domid); -goto cleanup; -} - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) goto cleanup; @@ -616,32 +611,18 @@ libxlDomainShutdownThread(void *opaque) } static void -libxlDomainDeathThread(void *opaque) +libxlDomainHandleDeath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { -struct libxlShutdownThreadInfo *shutdown_info = opaque; -virDomainObjPtr vm = NULL; -libxl_event *ev = shutdown_info->event; -libxlDriverPrivatePtr driver = shutdown_info->driver; virObjectEventPtr dom_event = NULL; -g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); -libxlDomainObjPrivatePtr priv; - -vm = virDomainObjListFindByID(driver->domains, ev->domid); -if (!vm) { -/* vm->def->id already cleared, means the death was handled by the - * driver already */ -goto cleanup; -} - -priv = vm->privateData; +libxlDomainObjPrivatePtr priv = vm->privateData; if (priv->ignoreDeathEvent) { priv->ignoreDeathEvent = false; -goto cleanup; +return; } if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) -goto cleanup; +return; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_DESTROYED); dom_event = virDomainEventLifecycleNewFromObj(vm, @@ -651,12 +632,7 @@ libxlDomainDeathThread(void *opaque) if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); libxlDomainObjEndJob(driver, vm); - - cleanup: -virDomainObjEndAPI(); virObjectEventStateQueue(driver->domainEventState, dom_event); -libxl_event_free(cfg->ctx, ev); -VIR_FREE(shutdown_info); } @@ -668,16 +644,13 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) { libxlDriverPrivatePtr driver = data; libxl_shutdown_reason xl_reason = event->u.domain_shutdown.shutdown_reason; -struct libxlShutdownThreadInfo *shutdown_info = NULL; -virThread thread; +virDomainObjPtr vm = NULL; g_autoptr(libxlDriverConfig) cfg = NULL; -int ret = -1; -g_autofree char *name = NULL; if (event->type != LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN && event->type != LIBXL_EVENT_TYPE_DOMAIN_DEATH) { VIR_INFO("Unhandled event type %d", event->type); -goto error; +goto cleanup; } /* @@ -685,42 +658,62 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) * after calling libxl_domain_suspend() are handled by its callers. */ if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND) -goto error; +goto cleanup; -/* - * Start a thread to handle shutdown. We don't want to be tying up - * libxl's event machinery by doing a potentially lengthy shutdown. - */ -shutdown_info = g_new0(struct libxlShutdownThreadInfo, 1); +vm = virDomainObjListFindByID(driver->domains, event->domid); +if (!vm) { +/* Nothing
Re: [PATCH v2 14/15] qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints
On Fri, Feb 19, 2021 at 12:58:26 +0100, Peter Krempa wrote: > Preserve block dirty bitmaps after migration with > QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC). > > This patch implements functions which offer the bitmaps to the > destination, check for eligibility on destination and then configure > source for the migration. > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_migration.c | 333 +- > 1 file changed, 331 insertions(+), 2 deletions(-) Reviewed-by: Jiri Denemark
Re: [PATCH v2 07/15] qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature
On Fri, Feb 19, 2021 at 12:58:19 +0100, Peter Krempa wrote: > Add the migration capability flag and the propagation of the > corresponding mapping configuration. The mapping will be produced from > the bitmaps on disk depending on both sides of the migration and the > necessity to perform merges. > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_migration_params.c | 29 + > src/qemu/qemu_migration_params.h | 5 + > 2 files changed, 34 insertions(+) Reviewed-by: Jiri Denemark
Re: [PATCH v2 04/15] qemu: migration: Create qcow2 v3 images for VIR_MIGRATE_NON_SHARED_DISK
On Fri, Feb 19, 2021 at 12:58:16 +0100, Peter Krempa wrote: > Use the new format when pre-creating the image for the user. Users > wishing to use the legacy format can always provide their own images or > use hared storage. s/hared/shared/ > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_migration.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index e8e35c1c7c..94b9b34ca0 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -180,6 +180,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, > char *volStr = NULL; > g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; > const char *format = NULL; > +const char *compat = NULL; > unsigned int flags = 0; > > VIR_DEBUG("Precreate disk type=%s", > virStorageTypeToString(disk->src->type)); > @@ -212,8 +213,11 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, > if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath))) > goto cleanup; > format = virStorageFileFormatTypeToString(disk->src->format); > -if (disk->src->format == VIR_STORAGE_FILE_QCOW2) > +if (disk->src->format == VIR_STORAGE_FILE_QCOW2) { > flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; > +/* format qcow2v3 image */ > +compat = "1.1"; > +} > break; > > case VIR_STORAGE_TYPE_VOLUME: And how about creating v3 only when the source offered bitmaps for migration? Although automatic creation of disk images during migration has always been magic and anyone caring about the exact image format on the destination needs to precreate the images manually. So I guess this should good enough. Reviewed-by: Jiri Denemark
Re: [PATCH v2 01/15] qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64
On Fri, Feb 19, 2021 at 12:58:13 +0100, Peter Krempa wrote: > Include the 'transform' member of 'block-bitmap-mapping'. This is based > on qemu commit v5.2.0-2208-gc79f01c945 > > Signed-off-by: Peter Krempa > --- > .../caps_6.0.0.x86_64.replies | 741 ++ > .../caps_6.0.0.x86_64.xml | 22 +- > 2 files changed, 441 insertions(+), 322 deletions(-) Reviewed-by: Jiri Denemark
[libvirt PATCH 1/2] domain_validate: use defines for cpu period and quota limits
Commints and <98a09ca48ed4fc011abf2aa290e02ce1b8f1bb5f> fixed the code to use defines instead of magic numbers but missed this place. Following commit changed the cpu quota limit to reflect what kernel actually allows so using the defines fixes XML validations as well. Signed-off-by: Pavel Hrdina --- src/conf/domain_validate.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index b47ecba86b..b4e09e21fe 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -23,6 +23,7 @@ #include "domain_validate.h" #include "domain_conf.h" #include "snapshot_conf.h" +#include "vircgroup.h" #include "virconftypes.h" #include "virlog.h" #include "virutil.h" @@ -1200,10 +1201,13 @@ virDomainDefOSValidate(const virDomainDef *def, #define CPUTUNE_VALIDATE_PERIOD(name) \ do { \ if (def->cputune.name > 0 && \ -(def->cputune.name < 1000 || def->cputune.name > 100)) { \ +(def->cputune.name < VIR_CGROUP_CPU_PERIOD_MIN || \ + def->cputune.name > VIR_CGROUP_CPU_PERIOD_MAX)) { \ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ - _("Value of cputune '%s' must be in range " \ - "[1000, 100]"), #name); \ + _("Value of cputune '%s' must be in range [%llu, %llu]"), \ + #name, \ + VIR_CGROUP_CPU_PERIOD_MIN, \ + VIR_CGROUP_CPU_PERIOD_MAX); \ return -1; \ } \ } while (0) @@ -1211,11 +1215,13 @@ virDomainDefOSValidate(const virDomainDef *def, #define CPUTUNE_VALIDATE_QUOTA(name) \ do { \ if (def->cputune.name > 0 && \ -(def->cputune.name < 1000 || \ -def->cputune.name > 18446744073709551LL)) { \ +(def->cputune.name < VIR_CGROUP_CPU_QUOTA_MIN || \ + def->cputune.name > VIR_CGROUP_CPU_QUOTA_MAX)) { \ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ - _("Value of cputune '%s' must be in range " \ - "[1000, 18446744073709551]"), #name); \ + _("Value of cputune '%s' must be in range [%llu, %llu]"), \ + #name, \ + VIR_CGROUP_CPU_QUOTA_MIN, \ + VIR_CGROUP_CPU_QUOTA_MAX); \ return -1; \ } \ } while (0) -- 2.29.2
[libvirt PATCH 2/2] docs: use proper cpu quota value in our documentation
Commit changed the cpu quota value that reflects what kernel allows but did not update our documentation. Signed-off-by: Pavel Hrdina --- docs/formatdomain.rst | 8 docs/manpages/virsh.rst | 2 +- docs/schemas/domaincommon.rng | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2587106191..e23bcc3e5a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -754,7 +754,7 @@ CPU Tuning microseconds). A domain with ``quota`` as any negative value indicates that the domain has infinite bandwidth for vCPU threads, which means that it is not bandwidth controlled. The value should be in range [1000, - 18446744073709551] or less than 0. A quota with value 0 means no value. You + 17592186044415] or less than 0. A quota with value 0 means no value. You can use this feature to ensure that all vCPUs run at the same speed. :since:`Only QEMU driver support since 0.9.4, LXC since 0.9.10` ``global_period`` @@ -768,7 +768,7 @@ CPU Tuning (unit: microseconds) within a period for the whole domain. A domain with ``global_quota`` as any negative value indicates that the domain has infinite bandwidth, which means that it is not bandwidth controlled. The value should - be in range [1000, 18446744073709551] or less than 0. A ``global_quota`` with + be in range [1000, 17592186044415] or less than 0. A ``global_quota`` with value 0 means no value. :since:`Only QEMU driver support since 1.3.3` ``emulator_period`` The optional ``emulator_period`` element specifies the enforcement interval @@ -783,7 +783,7 @@ CPU Tuning vCPUs). A domain with ``emulator_quota`` as any negative value indicates that the domain has infinite bandwidth for emulator threads (those excluding vCPUs), which means that it is not bandwidth controlled. The value should be - in range [1000, 18446744073709551] or less than 0. A quota with value 0 means + in range [1000, 17592186044415] or less than 0. A quota with value 0 means no value. :since:`Only QEMU driver support since 0.10.0` ``iothread_period`` The optional ``iothread_period`` element specifies the enforcement interval @@ -797,7 +797,7 @@ CPU Tuning bandwidth (unit: microseconds) for IOThreads. A domain with ``iothread_quota`` as any negative value indicates that the domain IOThreads have infinite bandwidth, which means that it is not bandwidth controlled. The - value should be in range [1000, 18446744073709551] or less than 0. An + value should be in range [1000, 17592186044415] or less than 0. An ``iothread_quota`` with value 0 means no value. You can use this feature to ensure that all IOThreads run at the same speed. :since:`Only QEMU driver support since 2.1.0` diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e3afa48f7b..8a4328faa0 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3818,7 +3818,7 @@ XEN_CREDIT scheduler. ``Note``: The vcpu_period, emulator_period, and iothread_period parameters have a valid value range of 1000-100 or 0, and the vcpu_quota, emulator_quota, and iothread_quota parameters have a valid value range of -1000-18446744073709551 or less than 0. The value 0 for +1000-17592186044415 or less than 0. The value 0 for either parameter is the same as not specifying that parameter. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e6de934456..d73db65742 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -7010,7 +7010,7 @@ -?[0-9]+ - 18446744073709551 + 17592186044415 -1 -- 2.29.2
[libvirt PATCH 0/2] followup fix for cpu quota limits
Pavel Hrdina (2): domain_validate: use defines for cpu period and quota limits docs: use proper cpu quota value in our documentation docs/formatdomain.rst | 8 docs/manpages/virsh.rst | 2 +- docs/schemas/domaincommon.rng | 2 +- src/conf/domain_validate.c| 20 +--- 4 files changed, 19 insertions(+), 13 deletions(-) -- 2.29.2
[PATCH 4/4] ui, monitor: remove deprecated VNC ACL option and HMP commands
The VNC ACL concept has been replaced by the pluggable "authz" framework which does not use monitor commands. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 16 --- docs/system/removed-features.rst | 13 +++ hmp-commands.hx | 76 - monitor/misc.c | 187 --- ui/vnc.c | 38 --- 5 files changed, 13 insertions(+), 317 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 57ff9f47cc..beed4b4f02 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -37,12 +37,6 @@ The 'file' driver for drives is no longer appropriate for character or host devices and will only accept regular files (S_IFREG). The correct driver for these file types is 'host_cdrom' or 'host_device' as appropriate. -``-vnc acl`` (since 4.0.0) -'' - -The ``acl`` option to the ``-vnc`` argument has been replaced -by the ``tls-authz`` and ``sasl-authz`` options. - ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0) ' @@ -262,16 +256,6 @@ Use the more generic commands ``block-export-add`` and ``block-export-del`` instead. As part of this deprecation, where ``nbd-server-add`` used a single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``. -Human Monitor Protocol (HMP) commands -- - -``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (since 4.0.0) -'' - -The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and -``acl_remove`` commands are deprecated with no replacement. Authorization -for VNC should be performed using the pluggable QAuthZ objects. - System emulator CPUS diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst index c8481cafbd..0424b9a89d 100644 --- a/docs/system/removed-features.rst +++ b/docs/system/removed-features.rst @@ -38,6 +38,12 @@ or ``-display default,show-cursor=on`` instead. QEMU 5.0 introduced an alternative syntax to specify the size of the translation block cache, ``-accel tcg,tb-size=``. +``-vnc acl`` (removed in 6.0) +' + +The ``acl`` option to the ``-vnc`` argument has been replaced +by the ``tls-authz`` and ``sasl-authz`` options. + QEMU Machine Protocol (QMP) commands @@ -79,6 +85,13 @@ documentation of ``query-hotpluggable-cpus`` for additional details. No replacement. The ``change vnc password`` and ``change DEVICE MEDIUM`` commands are not affected. +``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (removed in 6.0) +' + +The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and +``acl_remove`` commands were removed with no replacement. Authorization +for VNC should be performed using the pluggable QAuthZ objects. + Guest Emulator ISAs --- diff --git a/hmp-commands.hx b/hmp-commands.hx index d4001f9c5d..b500b8526d 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1433,82 +1433,6 @@ SRST Change watchdog action. ERST -{ -.name = "acl_show", -.args_type = "aclname:s", -.params = "aclname", -.help = "list rules in the access control list", -.cmd= hmp_acl_show, -}, - -SRST -``acl_show`` *aclname* - List all the matching rules in the access control list, and the default - policy. There are currently two named access control lists, - *vnc.x509dname* and *vnc.username* matching on the x509 client - certificate distinguished name, and SASL username respectively. -ERST - -{ -.name = "acl_policy", -.args_type = "aclname:s,policy:s", -.params = "aclname allow|deny", -.help = "set default access control list policy", -.cmd= hmp_acl_policy, -}, - -SRST -``acl_policy`` *aclname* ``allow|deny`` - Set the default access control list policy, used in the event that - none of the explicit rules match. The default policy at startup is - always ``deny``. -ERST - -{ -.name = "acl_add", -.args_type = "aclname:s,match:s,policy:s,index:i?", -.params = "aclname match allow|deny [index]", -.help = "add a match rule to the access control list", -.cmd= hmp_acl_add, -}, - -SRST -``acl_add`` *aclname* *match* ``allow|deny`` [*index*] - Add a match rule to the access control list, allowing or denying access. - The match will normally be an exact username or x509 distinguished name, - but can optionally include wildcard globs. eg ``*@EXAMPLE.COM`` to - allow all users
[PATCH 3/4] ui: deprecate "password" option for SPICE server
With the new "password-secret" option, there is no reason to use the old inecure "password" option with -spice, so it can be deprecated. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 8 qemu-options.hx| 4 ui/spice-core.c| 4 3 files changed, 16 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 2fcac7861e..57ff9f47cc 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts. +``-spice password=string`` (since 6.0) +'' + +This option is insecure because the SPICE password remains visible in +the process listing. This is replaced by the new ``password-secret`` +option which lets the password be securely provided on the command +line using a ``secret`` object instance. + QEMU Machine Protocol (QMP) commands diff --git a/qemu-options.hx b/qemu-options.hx index ff4ef3b708..4833bd59cf 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1926,6 +1926,10 @@ SRST ``password=`` Set the password you need to authenticate. +This option is deprecated and insecure because it leaves the +password visible in the process listing. Use ``password-secret`` +instead. + ``password-secret=`` Set the ID of the ``secret`` object containing the password you need to authenticate. diff --git a/ui/spice-core.c b/ui/spice-core.c index 353848b244..5e00e31457 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -685,6 +685,10 @@ static void qemu_spice_init(void) } } else { str = qemu_opt_get(opts, "password"); +if (str) { +warn_report("'password' option is deprecated and insecure, " +"use 'password-secret' instead"); +} password = g_strdup(str); } -- 2.29.2
[PATCH 2/4] ui: introduce "password-secret" option for SPICE server
Currently when using SPICE the "password" option provides the password in plain text on the command line. This is insecure as it is visible to all processes on the host. As an alternative, the password can be provided separately via the monitor. This introduces a "password-secret" option which lets the password be provided up front. $QEMU --object secret,id=vncsec0,file=passwd.txt \ --spice port=5901,password-secret=vncsec0 Signed-off-by: Daniel P. Berrangé --- qemu-options.hx | 8 ++-- ui/spice-core.c | 28 ++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 893d0f500b..ff4ef3b708 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1898,7 +1898,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice, " [,tls-ciphers=]\n" " [,tls-channel=[main|display|cursor|inputs|record|playback]]\n" " [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n" -" [,sasl][,password=][,disable-ticketing]\n" +" [,sasl][,password=][,password-secret=][,disable-ticketing]\n" " [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n" " [,jpeg-wan-compression=[auto|never|always]]\n" " [,zlib-glz-wan-compression=[auto|never|always]]\n" @@ -1923,9 +1923,13 @@ SRST ``ipv4``; \ ``ipv6``; \ ``unix`` Force using the specified IP version. -``password=`` +``password=`` Set the password you need to authenticate. +``password-secret=`` +Set the ID of the ``secret`` object containing the password +you need to authenticate. + ``sasl`` Require that the client use SASL to authenticate with the spice. The exact choice of authentication method used is controlled diff --git a/ui/spice-core.c b/ui/spice-core.c index beee932f55..353848b244 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -34,6 +34,7 @@ #include "qapi/qapi-events-ui.h" #include "qemu/notify.h" #include "qemu/option.h" +#include "crypto/secret_common.h" #include "migration/misc.h" #include "hw/pci/pci_bus.h" #include "ui/spice-display.h" @@ -415,6 +416,9 @@ static QemuOptsList qemu_spice_opts = { },{ .name = "password", .type = QEMU_OPT_STRING, +},{ +.name = "password-secret", +.type = QEMU_OPT_STRING, },{ .name = "disable-ticketing", .type = QEMU_OPT_BOOL, @@ -636,7 +640,9 @@ void qemu_spice_display_init_done(void) static void qemu_spice_init(void) { QemuOpts *opts = QTAILQ_FIRST(_spice_opts.head); -const char *password, *str, *x509_dir, *addr, +char *password = NULL; +const char *passwordSecret; +const char *str, *x509_dir, *addr, *x509_key_password = NULL, *x509_dh_file = NULL, *tls_ciphers = NULL; @@ -663,7 +669,24 @@ static void qemu_spice_init(void) error_report("spice tls-port is out of range"); exit(1); } -password = qemu_opt_get(opts, "password"); +passwordSecret = qemu_opt_get(opts, "password-secret"); +if (passwordSecret) { +Error *local_err = NULL; +if (qemu_opt_get(opts, "password")) { +error_report("'password' option is mutually exclusive with " + "'password-secret'"); +exit(1); +} +password = qcrypto_secret_lookup_as_utf8(passwordSecret, + _err); +if (!password) { +error_report_err(local_err); +exit(1); +} +} else { +str = qemu_opt_get(opts, "password"); +password = g_strdup(str); +} if (tls_port) { x509_dir = qemu_opt_get(opts, "x509-dir"); @@ -809,6 +832,7 @@ static void qemu_spice_init(void) g_free(x509_key_file); g_free(x509_cert_file); g_free(x509_cacert_file); +g_free(password); #ifdef HAVE_SPICE_GL if (qemu_opt_get_bool(opts, "gl", 0)) { -- 2.29.2
[PATCH 1/4] ui: introduce "password-secret" option for VNC servers
Currently when using VNC the "password" flag turns on password based authentication. The actual password has to be provided separately via the monitor. This introduces a "password-secret" option which lets the password be provided up front. $QEMU --object secret,id=vncsec0,file=passwd.txt \ --vnc localhost:0,password-secret=vncsec0 Signed-off-by: Daniel P. Berrangé --- qemu-options.hx | 5 + ui/vnc.c| 23 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 6c34c7050f..893d0f500b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2164,6 +2164,11 @@ SRST time to allow password to expire immediately or never expire. +``password-secret=`` +Require that password based authentication is used for client +connections, using the password provided by the ``secret`` +object identified by ``secret-id``. + ``tls-creds=ID`` Provides the ID of a set of TLS credentials to use to secure the VNC server. They will apply to both the normal VNC server socket diff --git a/ui/vnc.c b/ui/vnc.c index 16bb3be770..77e07ac351 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -48,6 +48,7 @@ #include "crypto/tlscredsanon.h" #include "crypto/tlscredsx509.h" #include "crypto/random.h" +#include "crypto/secret_common.h" #include "qom/object_interfaces.h" #include "qemu/cutils.h" #include "qemu/help_option.h" @@ -3469,6 +3470,9 @@ static QemuOptsList qemu_vnc_opts = { },{ .name = "password", .type = QEMU_OPT_BOOL, +},{ +.name = "password-secret", +.type = QEMU_OPT_STRING, },{ .name = "reverse", .type = QEMU_OPT_BOOL, @@ -3941,6 +3945,7 @@ void vnc_display_open(const char *id, Error **errp) int lock_key_sync = 1; int key_delay_ms; const char *audiodev; +const char *passwordSecret; if (!vd) { error_setg(errp, "VNC display not active"); @@ -3958,7 +3963,23 @@ void vnc_display_open(const char *id, Error **errp) goto fail; } -password = qemu_opt_get_bool(opts, "password", false); + +passwordSecret = qemu_opt_get(opts, "password-secret"); +if (passwordSecret) { +if (qemu_opt_get(opts, "password")) { +error_setg(errp, + "'password' flag is redundant with 'password-secret'"); +goto fail; +} +vd->password = qcrypto_secret_lookup_as_utf8(passwordSecret, + errp); +if (!vd->password) { +goto fail; +} +password = true; +} else { +password = qemu_opt_get_bool(opts, "password", false); +} if (password) { if (fips_get_state()) { error_setg(errp, -- 2.29.2
[PATCH 0/4] ui: add support for 'secret' object to provide VNC/SPICE passwords
This fixes a long standing limitation of the VNC/SPICE code which was unable to securely accept passswords on the CLI, instead requiring use of separate monitor commands after startup. This takes the opportunity to also remove previously deprecated ACL functionality from VNC. Daniel P. Berrangé (4): ui: introduce "password-secret" option for VNC servers ui: introduce "password-secret" option for SPICE server ui: deprecate "password" option for SPICE server ui, monitor: remove deprecated VNC ACL option and HMP commands docs/system/deprecated.rst | 24 ++-- docs/system/removed-features.rst | 13 +++ hmp-commands.hx | 76 - monitor/misc.c | 187 --- qemu-options.hx | 17 ++- ui/spice-core.c | 32 +- ui/vnc.c | 61 -- 7 files changed, 88 insertions(+), 322 deletions(-) -- 2.29.2
Re: [PATCH RESEND 12/20] libxl_driver.c: modernize libxlNodeDeviceReAttach()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: Use g_auto* wherever we can and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza This patch is also obsoleted by 23cdab6a3de0f6336505adcb446f77a6e0628e6b --- src/libxl/libxl_driver.c | 37 ++--- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3eaf106006..fbb67e9ed6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5852,18 +5852,17 @@ libxlNodeDeviceDettach(virNodeDevicePtr dev) static int libxlNodeDeviceReAttach(virNodeDevicePtr dev) { -virPCIDevicePtr pci = NULL; +g_autoptr(virPCIDevice) pci = NULL; virPCIDeviceAddress devAddr; -int ret = -1; -virNodeDeviceDefPtr def = NULL; -char *xml = NULL; +g_autoptr(virNodeDeviceDef) def = NULL; +g_autofree char *xml = NULL; libxlDriverPrivatePtr driver = dev->conn->privateData; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; -virConnectPtr nodeconn = NULL; -virNodeDevicePtr nodedev = NULL; +g_autoptr(virConnect) nodeconn = NULL; +g_autoptr(virNodeDevice) nodedev = NULL; if (!(nodeconn = virGetConnectNodeDev())) -goto cleanup; +return -1; /* 'dev' is associated with the QEMU virConnectPtr, * so for split daemons, we need to get a copy that @@ -5871,40 +5870,32 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev) */ if (!(nodedev = virNodeDeviceLookupByName( nodeconn, virNodeDeviceGetName(dev -goto cleanup; +return -1; xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) -goto cleanup; +return -1; def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); if (!def) -goto cleanup; +return -1; /* ACL check must happen against original 'dev', * not the new 'nodedev' we acquired */ if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0) -goto cleanup; +return -1; if (virDomainDriverNodeDeviceGetPCIInfo(def, ) < 0) -goto cleanup; +return -1; pci = virPCIDeviceNew(); if (!pci) -goto cleanup; +return -1; if (virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci) < 0) -goto cleanup; - -ret = 0; +return -1; - cleanup: -virPCIDeviceFree(pci); -virNodeDeviceDefFree(def); -virObjectUnref(nodedev); -virObjectUnref(nodeconn); -VIR_FREE(xml); -return ret; +return 0; } static int
Re: [PATCH RESEND 11/20] qemu_driver.c: modernize qemuNodeDeviceReAttach()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: Add virObjectUnref an autoptr cleanup func for virNodeDevice, then remove all unref and free calls from qemuNodeDeviceReAttach(). Signed-off-by: Daniel Henrique Barboza This patch is obsoleted by your own commit 23cdab6a3de0f6336505adcb446f77a6e0628e6b --- src/datatypes.h| 2 ++ src/qemu/qemu_driver.c | 32 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index ade3779e43..7a88aba0df 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -707,6 +707,8 @@ struct _virNodeDevice { char *parentName; /* parent device name */ }; +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDevice, virObjectUnref); + /** * _virSecret: * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7581e3c8cb..f9aa93fa3e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12045,17 +12045,16 @@ static int qemuNodeDeviceReAttach(virNodeDevicePtr dev) { virQEMUDriverPtr driver = dev->conn->privateData; -virPCIDevicePtr pci = NULL; +g_autoptr(virPCIDevice) pci = NULL; virPCIDeviceAddress devAddr; -int ret = -1; -virNodeDeviceDefPtr def = NULL; +g_autoptr(virNodeDeviceDef) def = NULL; g_autofree char *xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; -virConnectPtr nodeconn = NULL; -virNodeDevicePtr nodedev = NULL; +g_autoptr(virConnect) nodeconn = NULL; +g_autoptr(virNodeDevice) nodedev = NULL; if (!(nodeconn = virGetConnectNodeDev())) -goto cleanup; +return -1; /* 'dev' is associated with the QEMU virConnectPtr, * so for split daemons, we need to get a copy that @@ -12063,36 +12062,29 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) */ if (!(nodedev = virNodeDeviceLookupByName( nodeconn, virNodeDeviceGetName(dev -goto cleanup; +return -1; xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) -goto cleanup; +return -1; def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); if (!def) -goto cleanup; +return -1; /* ACL check must happen against original 'dev', * not the new 'nodedev' we acquired */ if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0) -goto cleanup; +return -1; if (virDomainDriverNodeDeviceGetPCIInfo(def, ) < 0) -goto cleanup; +return -1; pci = virPCIDeviceNew(); if (!pci) -goto cleanup; - -ret = virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci); +return -1; -virPCIDeviceFree(pci); - cleanup: -virNodeDeviceDefFree(def); -virObjectUnref(nodedev); -virObjectUnref(nodeconn); -return ret; +return virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci); } static int
Re: [PATCH 4/4] qemu*xml2*test: Cache capabilities between tests
On Fri, 19 Feb 2021 16:53:21 +0100 Peter Krempa wrote: > Invoking the XML parser every time is quite expensive. Since we have a > deep copy function for 'virQEMUCapsPtr' object, we can cache the > parsed results lazily. > > This brings significant speedup to qemuxml2argvtest: > > real 0m2.234s > user 0m2.140s > sys 0m0.089s > > vs. > > real 0m1.161s > user 0m1.087s > sys 0m0.072s > > qemuxml2xmltest benefits too: > > real 0m0.879s > user 0m0.801s > sys 0m0.071s > > vs. > > real 0m0.466s > user 0m0.424s > sys 0m0.040s > > Signed-off-by: Peter Krempa > --- > tests/qemustatusxml2xmltest.c | 3 ++- > tests/qemuxml2argvtest.c | 3 ++- > tests/qemuxml2xmltest.c | 3 ++- > tests/testutilsqemu.c | 18 +++--- > tests/testutilsqemu.h | 1 + > 5 files changed, 22 insertions(+), 6 deletions(-) > > diff --git a/tests/qemustatusxml2xmltest.c > b/tests/qemustatusxml2xmltest.c index 67a070c986..2b2c409cba 100644 > --- a/tests/qemustatusxml2xmltest.c > +++ b/tests/qemustatusxml2xmltest.c > @@ -73,6 +73,7 @@ mymain(void) > g_autofree char *fakerootdir = NULL; > g_autoptr(virQEMUDriverConfig) cfg = NULL; > g_autoptr(GHashTable) capslatest = NULL; > +g_autoptr(GHashTable) capscache = > virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; > > capslatest = testQemuGetLatestCaps(); > @@ -109,7 +110,7 @@ mymain(void) > static struct testQemuInfo info = { \ > .name = _name, \ > }; \ > -if (testQemuInfoSetArgs(, capslatest, \ > +if (testQemuInfoSetArgs(, capscache, capslatest, \ > ARG_QEMU_CAPS, QEMU_CAPS_LAST, \ > ARG_END) < 0 || \ > qemuTestCapsCacheInsert(driver.qemuCapsCache, > info.qemuCaps) < 0) { \ diff --git a/tests/qemuxml2argvtest.c > b/tests/qemuxml2argvtest.c index 2d538bee9c..d6d707cd24 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -789,6 +789,7 @@ mymain(void) > g_autofree char *fakerootdir = NULL; > g_autoptr(GHashTable) capslatest = NULL; > g_autoptr(GHashTable) qapiSchemaCache = > virHashNew((GDestroyNotify) virHashFree); > +g_autoptr(GHashTable) capscache = > virHashNew(virObjectFreeHashData); > > fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); > > @@ -883,7 +884,7 @@ mymain(void) > .name = _name, \ > }; \ > info.qapiSchemaCache = qapiSchemaCache; \ > -if (testQemuInfoSetArgs(, capslatest, \ > +if (testQemuInfoSetArgs(, capscache, capslatest, \ > __VA_ARGS__, ARG_END) < 0) \ > return EXIT_FAILURE; \ > testInfoSetPaths(, _suffix); \ > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 5cd945f28f..01ac5886f2 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -85,6 +85,7 @@ mymain(void) > g_autofree char *fakerootdir = NULL; > g_autoptr(virQEMUDriverConfig) cfg = NULL; > g_autoptr(GHashTable) capslatest = NULL; > +g_autoptr(GHashTable) capscache = > virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; > > capslatest = testQemuGetLatestCaps(); > @@ -130,7 +131,7 @@ mymain(void) > static struct testQemuInfo info = { \ > .name = _name, \ > }; \ > -if (testQemuInfoSetArgs(, capslatest, \ > +if (testQemuInfoSetArgs(, capscache, capslatest, \ > __VA_ARGS__, \ > ARG_END) < 0 || \ > qemuTestCapsCacheInsert(driver.qemuCapsCache, > info.qemuCaps) < 0) { \ diff --git a/tests/testutilsqemu.c > b/tests/testutilsqemu.c index a96c9d487a..1dcf5caf6a 100644 > --- a/tests/testutilsqemu.c > +++ b/tests/testutilsqemu.c > @@ -681,6 +681,7 @@ testQemuCapsIterate(const char *suffix, > > int > testQemuInfoSetArgs(struct testQemuInfo *info, > +GHashTable *capscache, > GHashTable *capslatest, ...) > { > va_list argptr; > @@ -773,6 +774,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info, > > if (!qemuCaps && capsarch && capsver) { > bool stripmachinealiases = false; > +virQEMUCapsPtr cachecaps = NULL; > > info->arch = virArchFromString(capsarch); > > @@ -784,11 +786,21 @@ testQemuInfoSetArgs(struct testQemuInfo *info, > TEST_QEMU_CAPS_PATH, capsver, > capsarch); } > > -if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, > capsfile))) > +if (!g_hash_table_lookup_extended(capscache, capsfile, NULL, > (void **) )) { > +if (!(qemuCaps = > qemuTestParseCapabilitiesArch(info->arch, capsfile))) > +goto cleanup; > + > +if (stripmachinealiases) > +virQEMUCapsStripMachineAliases(qemuCaps); > + > +cachecaps = qemuCaps; > + > +
Re: [PATCH v2] qemu: Fix libvirt hang due to early TPM device stop
On 2/19/21 4:57 PM, Stefan Berger wrote: This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop() was moved to a location before the QEMU process is stopped. It may be alright to tear down some devices before QEMU is stopped, but it doesn't work for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop it before libvirt may try to clean it up. So this patch moves the virFileDeleteTree() calls after the call to qemuExtDevicesStop() so that the pid file of virtiofsd is not deleted before that call. Afftected libvirt versions are 6.10 and 7.0. Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2 Cc: Masayoshi Mizuma Signed-off-by: Stefan Berger --- src/qemu/qemu_process.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
[PATCH v2] qemu: Fix libvirt hang due to early TPM device stop
This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop() was moved to a location before the QEMU process is stopped. It may be alright to tear down some devices before QEMU is stopped, but it doesn't work for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop it before libvirt may try to clean it up. So this patch moves the virFileDeleteTree() calls after the call to qemuExtDevicesStop() so that the pid file of virtiofsd is not deleted before that call. Afftected libvirt versions are 6.10 and 7.0. Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2 Cc: Masayoshi Mizuma Signed-off-by: Stefan Berger --- src/qemu/qemu_process.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7feb35e609..d930ff9a74 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7659,11 +7659,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* Do this before we delete the tree and remove pidfile. */ qemuProcessKillManagedPRDaemon(vm); -qemuExtDevicesStop(driver, vm); - -virFileDeleteTree(priv->libDir); -virFileDeleteTree(priv->channelTargetDir); - ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, @@ -7677,10 +7672,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainCleanupRun(driver, vm); +qemuExtDevicesStop(driver, vm); + qemuDBusStop(driver, vm); vm->def->id = -1; +virFileDeleteTree(priv->libDir); +virFileDeleteTree(priv->channelTargetDir); + /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); -- 2.26.2
[PATCH 3/4] testQemuInfoSetArgs: Use curly braces in else section
Signed-off-by: Peter Krempa --- tests/testutilsqemu.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index c2cf3be9ac..a96c9d487a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -779,8 +779,10 @@ testQemuInfoSetArgs(struct testQemuInfo *info, if (STREQ(capsver, "latest")) { capsfile = g_strdup(virHashLookup(capslatest, capsarch)); stripmachinealiases = true; -} else capsfile = g_strdup_printf("%s/caps_%s.%s.xml", - TEST_QEMU_CAPS_PATH, capsver, capsarch); +} else { +capsfile = g_strdup_printf("%s/caps_%s.%s.xml", + TEST_QEMU_CAPS_PATH, capsver, capsarch); +} if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) goto cleanup; -- 2.29.2
[PATCH 1/4] qemuxml2argvtest: Cache QAPI schema between tests
It's quite wasteful to reparse the QAPI schema for each _CAPS_ test. Add a simple cache filled lazily by encountered schemas. The time saving on my box is quite significant: real0m3.318s user0m3.203s sys 0m0.107s vs real0m2.223s user0m2.134s sys 0m0.084s Signed-off-by: Peter Krempa --- tests/qemuxml2argvtest.c | 17 ++--- tests/testutilsqemu.h| 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db438c5466..647187404c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -523,13 +523,22 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, qemuDomainObjPrivatePtr priv = NULL; size_t nargs = 0; size_t i; -g_autoptr(GHashTable) schema = NULL; +GHashTable *schema = NULL; g_autoptr(virCommand) cmd = NULL; unsigned int parseFlags = info->parseFlags; bool netdevQAPIfied = false; -if (info->schemafile) -schema = testQEMUSchemaLoad(info->schemafile); +if (info->schemafile) { +/* lookup and insert into cache if not found */ +if (!g_hash_table_lookup_extended(info->qapiSchemaCache, + info->schemafile, + NULL, (void **) )) { +schema = testQEMUSchemaLoad(info->schemafile); +g_hash_table_insert(info->qapiSchemaCache, +g_strdup(info->schemafile), +schema); +} +} /* comment out with line comment to enable schema checking for non _CAPS tests if (!schema) @@ -779,6 +788,7 @@ mymain(void) int ret = 0; g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = NULL; +g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -872,6 +882,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ +info.qapiSchemaCache = qapiSchemaCache; \ if (testQemuInfoSetArgs(, capslatest, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 0c6f0ea378..86ba69e96b 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -66,6 +66,7 @@ struct testQemuInfo { unsigned int parseFlags; virArch arch; char *schemafile; +GHashTable *qapiSchemaCache; }; virCapsPtr testQemuCapsInit(void); -- 2.29.2
[PATCH 4/4] qemu*xml2*test: Cache capabilities between tests
Invoking the XML parser every time is quite expensive. Since we have a deep copy function for 'virQEMUCapsPtr' object, we can cache the parsed results lazily. This brings significant speedup to qemuxml2argvtest: real0m2.234s user0m2.140s sys 0m0.089s vs. real0m1.161s user0m1.087s sys 0m0.072s qemuxml2xmltest benefits too: real0m0.879s user0m0.801s sys 0m0.071s vs. real0m0.466s user0m0.424s sys 0m0.040s Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmltest.c | 3 ++- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 3 ++- tests/testutilsqemu.c | 18 +++--- tests/testutilsqemu.h | 1 + 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c index 67a070c986..2b2c409cba 100644 --- a/tests/qemustatusxml2xmltest.c +++ b/tests/qemustatusxml2xmltest.c @@ -73,6 +73,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; +g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; capslatest = testQemuGetLatestCaps(); @@ -109,7 +110,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ -if (testQemuInfoSetArgs(, capslatest, \ +if (testQemuInfoSetArgs(, capscache, capslatest, \ ARG_QEMU_CAPS, QEMU_CAPS_LAST, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d538bee9c..d6d707cd24 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -789,6 +789,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = NULL; g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); +g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -883,7 +884,7 @@ mymain(void) .name = _name, \ }; \ info.qapiSchemaCache = qapiSchemaCache; \ -if (testQemuInfoSetArgs(, capslatest, \ +if (testQemuInfoSetArgs(, capscache, capslatest, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ testInfoSetPaths(, _suffix); \ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5cd945f28f..01ac5886f2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -85,6 +85,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; +g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; capslatest = testQemuGetLatestCaps(); @@ -130,7 +131,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ -if (testQemuInfoSetArgs(, capslatest, \ +if (testQemuInfoSetArgs(, capscache, capslatest, \ __VA_ARGS__, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index a96c9d487a..1dcf5caf6a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -681,6 +681,7 @@ testQemuCapsIterate(const char *suffix, int testQemuInfoSetArgs(struct testQemuInfo *info, +GHashTable *capscache, GHashTable *capslatest, ...) { va_list argptr; @@ -773,6 +774,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info, if (!qemuCaps && capsarch && capsver) { bool stripmachinealiases = false; +virQEMUCapsPtr cachecaps = NULL; info->arch = virArchFromString(capsarch); @@ -784,11 +786,21 @@ testQemuInfoSetArgs(struct testQemuInfo *info, TEST_QEMU_CAPS_PATH, capsver, capsarch); } -if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) +if (!g_hash_table_lookup_extended(capscache, capsfile, NULL, (void **) )) { +if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) +goto cleanup; + +if (stripmachinealiases) +virQEMUCapsStripMachineAliases(qemuCaps); + +cachecaps = qemuCaps; + +g_hash_table_insert(capscache, g_strdup(capsfile), g_steal_pointer()); +} + +if (!(qemuCaps = virQEMUCapsNewCopy(cachecaps))) goto cleanup; -if (stripmachinealiases) -virQEMUCapsStripMachineAliases(qemuCaps); info->flags |=
[PATCH 2/4] testCompareXMLToArgvValidateSchema: Improve and fix helper for testing everything
The schema validator has a comment which allows checking all xml2argv input files for schema validity by forcing the latest schema onto files which don't have any schema. Fix it so that it works properly with the caching introduced in previous commit. Signed-off-by: Peter Krempa --- tests/qemuxml2argvtest.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 647187404c..2d538bee9c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -528,6 +528,11 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, unsigned int parseFlags = info->parseFlags; bool netdevQAPIfied = false; +/* comment out with line comment to enable schema checking for non _CAPS tests +if (!info->schemafile) +info->schemafile = testQemuGetLatestCapsForArch(virArchToString(info->arch), "replies"); +// */ + if (info->schemafile) { /* lookup and insert into cache if not found */ if (!g_hash_table_lookup_extended(info->qapiSchemaCache, @@ -540,11 +545,6 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, } } -/* comment out with line comment to enable schema checking for non _CAPS tests -if (!schema) -schema = testQEMUSchemaLoadLatest(virArchToString(info->arch)); -// */ - if (!schema) return 0; -- 2.29.2
[PATCH 0/4] qemuxml2*test: Significantly improve performance by caching qemu capabilities and qapi schema
This results in 1/3 of the original execution time. Peter Krempa (4): qemuxml2argvtest: Cache QAPI schema between tests testCompareXMLToArgvValidateSchema: Improve and fix helper for testing everything testQemuInfoSetArgs: Use curly braces in else section qemu*xml2*test: Cache capabilities between tests tests/qemustatusxml2xmltest.c | 3 ++- tests/qemuxml2argvtest.c | 26 +++--- tests/qemuxml2xmltest.c | 3 ++- tests/testutilsqemu.c | 24 +++- tests/testutilsqemu.h | 2 ++ 5 files changed, 44 insertions(+), 14 deletions(-) -- 2.29.2
Re: [PATCH] qemu: Fix libvirt hang due to early TPM device stop
On Fri, Feb 19, 2021 at 09:49:36AM -0500, Stefan Berger wrote: > This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop() > was moved to a location before the QEMU process is stopped. It may be > alright to tear down some devices before QEMU is stopped, but it doesn't work > for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop > it before libvirt may try to clean it up. So this patch introduces a function > qemuExtDevicesStopEarly() to accommodate those devices that can/must > be torn down early, which seems to be the case with virtio and its virtiofsd, > and recreates the location of the call to qemuExtDevicesStop() from before the > regression for all other devices. I don't think splitting the cleanup like this is a good idea. Also I think 5cde9dee was reall wrong, because we shouldn't tear down /any/ external processes while QEMU itself still exists. The comit message for 5cde9dee talks aboutneeding to run before virFileDeleteTree(priv->libDir) which makes sense. The solution was wrong though IMHO. We shouldn't be calling virFileDeleteTree() until all the processes are stopped. So we should revert 5cde9dee entirely, and then move the virFileDeleteTree calls further to the end of qemuProcessStop > > Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2 > Cc: Masayoshi Mizuma > Signed-off-by: Stefan Berger > --- > src/qemu/qemu_extdevice.c | 27 --- > src/qemu/qemu_extdevice.h | 4 > src/qemu/qemu_process.c | 5 - > 3 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c > index 8fe7ceaa10..96fdc9a589 100644 > --- a/src/qemu/qemu_extdevice.c > +++ b/src/qemu/qemu_extdevice.c > @@ -205,7 +205,27 @@ qemuExtDevicesStart(virQEMUDriverPtr driver, > return 0; > } > > +/* qemuExtDevicesStopEarly stops devices that may be stopped > + * before QEMU terminates > + */ > +void > +qemuExtDevicesStopEarly(virQEMUDriverPtr driver, > +virDomainObjPtr vm) > +{ > +virDomainDefPtr def = vm->def; > +size_t i; > > +for (i = 0; i < def->nfss; i++) { > +virDomainFSDefPtr fs = def->fss[i]; > + > +if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) > +qemuVirtioFSStop(driver, vm, fs); > +} > +} > + > +/* qemuExtDevicesStop stops devices that may only be stopped > + * after QEMU terminated > + */ > void > qemuExtDevicesStop(virQEMUDriverPtr driver, > virDomainObjPtr vm) > @@ -236,13 +256,6 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, > if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) > virNetDevRunEthernetScript(net->ifname, net->downscript); > } > - > -for (i = 0; i < def->nfss; i++) { > -virDomainFSDefPtr fs = def->fss[i]; > - > -if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) > -qemuVirtioFSStop(driver, vm, fs); > -} > } > > > diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h > index 49373a15a1..df0cbd6993 100644 > --- a/src/qemu/qemu_extdevice.h > +++ b/src/qemu/qemu_extdevice.h > @@ -51,6 +51,10 @@ int qemuExtDevicesStart(virQEMUDriverPtr driver, > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) > G_GNUC_WARN_UNUSED_RESULT; > > +void qemuExtDevicesStopEarly(virQEMUDriverPtr driver, > + virDomainObjPtr vm) > +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); > + > void qemuExtDevicesStop(virQEMUDriverPtr driver, > virDomainObjPtr vm) > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 7feb35e609..b5b2ddef4c 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -7659,7 +7659,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, > /* Do this before we delete the tree and remove pidfile. */ > qemuProcessKillManagedPRDaemon(vm); > > -qemuExtDevicesStop(driver, vm); > +qemuExtDevicesStopEarly(driver, vm); > > virFileDeleteTree(priv->libDir); > virFileDeleteTree(priv->channelTargetDir); > @@ -7677,6 +7677,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, > > qemuDomainCleanupRun(driver, vm); > > +/* tear down external devices after QEMU is gone */ > +qemuExtDevicesStop(driver, vm); > + > qemuDBusStop(driver, vm); > > vm->def->id = -1; > -- > 2.26.2 > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] qemu: Fix libvirt hang due to early TPM device stop
This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop() was moved to a location before the QEMU process is stopped. It may be alright to tear down some devices before QEMU is stopped, but it doesn't work for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop it before libvirt may try to clean it up. So this patch introduces a function qemuExtDevicesStopEarly() to accommodate those devices that can/must be torn down early, which seems to be the case with virtio and its virtiofsd, and recreates the location of the call to qemuExtDevicesStop() from before the regression for all other devices. Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2 Cc: Masayoshi Mizuma Signed-off-by: Stefan Berger --- src/qemu/qemu_extdevice.c | 27 --- src/qemu/qemu_extdevice.h | 4 src/qemu/qemu_process.c | 5 - 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 8fe7ceaa10..96fdc9a589 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -205,7 +205,27 @@ qemuExtDevicesStart(virQEMUDriverPtr driver, return 0; } +/* qemuExtDevicesStopEarly stops devices that may be stopped + * before QEMU terminates + */ +void +qemuExtDevicesStopEarly(virQEMUDriverPtr driver, +virDomainObjPtr vm) +{ +virDomainDefPtr def = vm->def; +size_t i; +for (i = 0; i < def->nfss; i++) { +virDomainFSDefPtr fs = def->fss[i]; + +if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) +qemuVirtioFSStop(driver, vm, fs); +} +} + +/* qemuExtDevicesStop stops devices that may only be stopped + * after QEMU terminated + */ void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -236,13 +256,6 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) virNetDevRunEthernetScript(net->ifname, net->downscript); } - -for (i = 0; i < def->nfss; i++) { -virDomainFSDefPtr fs = def->fss[i]; - -if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) -qemuVirtioFSStop(driver, vm, fs); -} } diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 49373a15a1..df0cbd6993 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -51,6 +51,10 @@ int qemuExtDevicesStart(virQEMUDriverPtr driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +void qemuExtDevicesStopEarly(virQEMUDriverPtr driver, + virDomainObjPtr vm) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainObjPtr vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7feb35e609..b5b2ddef4c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7659,7 +7659,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* Do this before we delete the tree and remove pidfile. */ qemuProcessKillManagedPRDaemon(vm); -qemuExtDevicesStop(driver, vm); +qemuExtDevicesStopEarly(driver, vm); virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); @@ -7677,6 +7677,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainCleanupRun(driver, vm); +/* tear down external devices after QEMU is gone */ +qemuExtDevicesStop(driver, vm); + qemuDBusStop(driver, vm); vm->def->id = -1; -- 2.26.2
Re: [PATCH] event-test: Properly terminate strings printed from callbacks
On a Friday in 2021, Kristina Hanicova wrote: Stdio was buffering strings in functions: myDomainEventBlockJobCallback, myDomainEventBlockThresholdCallback, myDomainEventMemoryFailureCallback. It caused flushing the printed strings from callbacks at the end of a run, not gradually. The solution is to add \n at the end of each string. Signed-off-by: Kristina Hanicova --- examples/c/misc/event-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko and pushed. Jano signature.asc Description: PGP signature
[PATCH] event-test: Properly terminate strings printed from callbacks
Stdio was buffering strings in functions: myDomainEventBlockJobCallback, myDomainEventBlockThresholdCallback, myDomainEventMemoryFailureCallback. It caused flushing the printed strings from callbacks at the end of a run, not gradually. The solution is to add \n at the end of each string. Signed-off-by: Kristina Hanicova --- examples/c/misc/event-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/c/misc/event-test.c b/examples/c/misc/event-test.c index f164e825e1..76d4f3f6e8 100644 --- a/examples/c/misc/event-test.c +++ b/examples/c/misc/event-test.c @@ -938,7 +938,7 @@ myDomainEventBlockJobCallback(virConnectPtr conn G_GNUC_UNUSED, const char *eventName = opaque; printf("%s EVENT: Domain %s(%d) block job callback '%s' disk '%s', " - "type '%s' status '%s'", + "type '%s' status '%s'\n", __func__, virDomainGetName(dom), virDomainGetID(dom), eventName, disk, blockJobTypeToStr(type), blockJobStatusToStr(status)); return 0; @@ -956,7 +956,7 @@ myDomainEventBlockThresholdCallback(virConnectPtr conn G_GNUC_UNUSED, { /* Casts to uint64_t to work around mingw not knowing %lld */ printf("%s EVENT: Domain %s(%d) block threshold callback dev '%s'(%s), " - "threshold: '%" PRIu64 "', excess: '%" PRIu64 "'", + "threshold: '%" PRIu64 "', excess: '%" PRIu64 "'\n", __func__, virDomainGetName(dom), virDomainGetID(dom), dev, NULLSTR(path), (uint64_t)threshold, (uint64_t)excess); return 0; @@ -972,7 +972,7 @@ myDomainEventMemoryFailureCallback(virConnectPtr conn G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED) { printf("%s EVENT: Domain %s(%d) memory failure: recipient '%d', " - "aciont '%d', flags '%d'", __func__, virDomainGetName(dom), + "aciont '%d', flags '%d'\n", __func__, virDomainGetName(dom), virDomainGetID(dom), recipient, action, flags); return 0; } -- 2.29.2
[PATCH v2 14/15] qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints
Preserve block dirty bitmaps after migration with QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC). This patch implements functions which offer the bitmaps to the destination, check for eligibility on destination and then configure source for the migration. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration.c | 333 +- 1 file changed, 331 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4e69fab384..08f60c6db3 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2205,6 +2205,91 @@ qemuMigrationSrcCleanup(virDomainObjPtr vm, } +/** + * qemuMigrationSrcBeginPhaseBlockDirtyBitmaps: + * @mig: migration cookie struct + * @vm: domain object + * @migrate_disks: disks which are being migrated + * @nmigrage_disks: number of @migrate_disks + * + * Enumerates block dirty bitmaps on disks which will undergo storage migration + * and fills them into @mig to be offered to the destination. + */ +static int +qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookiePtr mig, +virDomainObjPtr vm, +const char **migrate_disks, +size_t nmigrate_disks) + +{ +GSList *disks = NULL; +qemuDomainObjPrivatePtr priv = vm->privateData; +size_t i; + +g_autoptr(GHashTable) blockNamedNodeData = NULL; + +if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, priv->job.asyncJob))) +return -1; + +for (i = 0; i < vm->def->ndisks; i++) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk; +GSList *bitmaps = NULL; +virDomainDiskDefPtr diskdef = vm->def->disks[i]; +qemuBlockNamedNodeDataPtr nodedata = virHashLookup(blockNamedNodeData, diskdef->src->nodeformat); +size_t j; + +if (!nodedata) +continue; + +if (migrate_disks) { +bool migrating = false; + +for (j = 0; j < nmigrate_disks; j++) { +if (STREQ(migrate_disks[j], diskdef->dst)) { +migrating = true; +break; +} +} + +if (!migrating) +continue; +} + +for (j = 0; j < nodedata->nbitmaps; j++) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap; + +if (!qemuBlockBitmapChainIsValid(diskdef->src, + nodedata->bitmaps[j]->name, + blockNamedNodeData)) +continue; + +bitmap = g_new0(qemuMigrationBlockDirtyBitmapsDiskBitmap, 1); +bitmap->bitmapname = g_strdup(nodedata->bitmaps[j]->name); +bitmap->alias = g_strdup_printf("libvirt-%s-%s", +diskdef->dst, +nodedata->bitmaps[j]->name); +bitmaps = g_slist_prepend(bitmaps, bitmap); +} + +if (!bitmaps) +continue; + +disk = g_new0(qemuMigrationBlockDirtyBitmapsDisk, 1); +disk->target = g_strdup(diskdef->dst); +disk->bitmaps = bitmaps; +disks = g_slist_prepend(disks, disk); +} + +if (!disks) +return 0; + +mig->blockDirtyBitmaps = disks; +mig->flags |= QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS; + +return 0; +} + + /* The caller is supposed to lock the vm and start a migration job. */ static char * qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, @@ -2317,6 +2402,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, if (!(mig = qemuMigrationCookieNew(vm->def, priv->origname))) return NULL; +if (cookieFlags & QEMU_MIGRATION_COOKIE_NBD && +virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING) && +qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(mig, vm, migrate_disks, +nmigrate_disks) < 0) +return NULL; + if (qemuMigrationCookieFormat(mig, driver, vm, QEMU_MIGRATION_SOURCE, cookieout, cookieoutlen, @@ -2530,6 +2621,92 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, migrateFrom, fd, NULL); } + +/** + * qemuMigrationDstPrepareAnyBlockDirtyBitmaps: + * @vm: domain object + * @mig: migration cookie + * @migParams: migration parameters + * @flags: migration flags + * + * Checks whether block dirty bitmaps offered by the migration source are + * to be migrated (e.g. they don't exist, the destination is compatible etc) + * and sets up destination qemu for migrating the bitmaps as well as updates the + * list of eligible bitmaps in the migration cookie to be sent back to the + * source. + */ +static int +qemuMigrationDstPrepareAnyBlockDirtyBitmaps(virDomainObjPtr vm, +
[PATCH v2 08/15] qemu: migration_cookie: Add XML handling for setting up bitmap migration
In cases where we are copying the storage we need to ensure that also bitmaps are copied properly. This patch adds migration cookie XML infrastructure which will allow the migration sides reach consensus on which bitmaps to migrate. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_migration_cookie.c | 146 +++ src/qemu/qemu_migration_cookie.h | 34 +++ 2 files changed, 180 insertions(+) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 6f2b1b2f57..0f8555cbb0 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -51,6 +51,7 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag, "cpu", "allowReboot", "capabilities", + "block-dirty-bitmaps", ); @@ -116,6 +117,39 @@ qemuMigrationCookieCapsFree(qemuMigrationCookieCapsPtr caps) G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieCaps, qemuMigrationCookieCapsFree); +static void +qemuMigrationBlockDirtyBitmapsDiskBitmapFree(qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bmp) +{ +if (!bmp) +return; + +g_free(bmp->bitmapname); +g_free(bmp->alias); +g_free(bmp->sourcebitmap); +g_free(bmp); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationBlockDirtyBitmapsDiskBitmap, + qemuMigrationBlockDirtyBitmapsDiskBitmapFree); + + +static void +qemuMigrationBlockDirtyBitmapsDiskFree(qemuMigrationBlockDirtyBitmapsDiskPtr dsk) +{ +if (!dsk) +return; + +g_free(dsk->target); +if (dsk->bitmaps) +g_slist_free_full(dsk->bitmaps, + (GDestroyNotify) qemuMigrationBlockDirtyBitmapsDiskBitmapFree); +g_free(dsk); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationBlockDirtyBitmapsDisk, + qemuMigrationBlockDirtyBitmapsDiskFree); + + void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) { @@ -135,6 +169,9 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig) g_clear_pointer(>jobInfo, qemuDomainJobInfoFree); virCPUDefFree(mig->cpu); qemuMigrationCookieCapsFree(mig->caps); +if (mig->blockDirtyBitmaps) +g_slist_free_full(mig->blockDirtyBitmaps, + (GDestroyNotify) qemuMigrationBlockDirtyBitmapsDiskFree); g_free(mig); } @@ -758,6 +795,48 @@ qemuMigrationCookieNBDXMLFormat(qemuMigrationCookieNBDPtr nbd, } +static void +qemuMigrationCookieBlockDirtyBitmapsFormat(virBufferPtr buf, + GSList *bitmaps) +{ +g_auto(virBuffer) disksBuf = VIR_BUFFER_INIT_CHILD(buf); +GSList *nextdisk; + +for (nextdisk = bitmaps; nextdisk; nextdisk = nextdisk->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data; +g_auto(virBuffer) diskAttrBuf = VIR_BUFFER_INITIALIZER; +g_auto(virBuffer) diskChildBuf = VIR_BUFFER_INIT_CHILD(); +bool hasBitmaps = false; +GSList *nextbitmap; + +if (disk->skip || !disk->bitmaps) +continue; + +for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data; + +if (bitmap->skip) +continue; + +virBufferAsprintf(, + "\n", + bitmap->bitmapname, bitmap->alias); + +hasBitmaps = true; +} + +if (!hasBitmaps) +continue; + +virBufferAsprintf(, " target='%s'", disk->target); +virXMLFormatElement(, "disk", , ); +} + + +virXMLFormatElement(buf, "blockDirtyBitmaps", NULL, ); +} + + int qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps, @@ -829,6 +908,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, if (mig->flags & QEMU_MIGRATION_COOKIE_CAPS) qemuMigrationCookieCapsXMLFormat(buf, mig->caps); +if (mig->flags & QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS) +qemuMigrationCookieBlockDirtyBitmapsFormat(buf, mig->blockDirtyBitmaps); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); return 0; @@ -1132,6 +1214,65 @@ qemuMigrationCookieXMLParseMandatoryFeatures(xmlXPathContextPtr ctxt, } +static int +qemuMigrationCookieBlockDirtyBitmapsParse(xmlXPathContextPtr ctxt, + qemuMigrationCookiePtr mig) +{ +g_autoslist(qemuMigrationBlockDirtyBitmapsDisk) disks = NULL; +g_autofree xmlNodePtr *disknodes = NULL; +int ndisknodes; +size_t i; +VIR_XPATH_NODE_AUTORESTORE(ctxt) + +if ((ndisknodes = virXPathNodeSet("./blockDirtyBitmaps/disk", ctxt, )) < 0) +return -1; + +for (i = 0; i < ndisknodes; i++) { +g_autoslist(qemuMigrationBlockDirtyBitmapsDiskBitmap) bitmaps = NULL; +qemuMigrationBlockDirtyBitmapsDiskPtr
[PATCH v2 15/15] qemu: capabilities: Enable QEMU_CAPS_INCREMENTAL_BACKUP
For incremental backup we need QEMU_CAPS_BLOCKDEV, QEMU_CAPS_BLOCKDEV_REOPEN, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_capabilities.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 600952a53a..f40d6d77be 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5167,8 +5167,10 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) void virQEMUCapsInitProcessCapsInterlock(virQEMUCapsPtr qemuCaps) { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) -virQEMUCapsClear(qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP); +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && +virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN) && +virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_WERROR)) { -- 2.29.2
[PATCH v2 13/15] qemu: migration: Clean up temporary bitmaps when cancelling a migration
In case when the block migration job required temporary bitmaps for merging the appropriate checkpoints we need to clean them up when cancelling the job. On success we don't need to do that though as the bitmaps are just temporary thus are not written to disk. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_migration.c | 28 1 file changed, 28 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 94b9b34ca0..4e69fab384 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -840,6 +840,29 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, } +static int +qemuMigrationSrcCancelRemoveTempBitmaps(virDomainObjPtr vm, +qemuDomainAsyncJob asyncJob) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +virQEMUDriverPtr driver = priv->driver; +qemuDomainJobPrivatePtr jobPriv = priv->job.privateData; +GSList *next; + +for (next = jobPriv->migTempBitmaps; next; next = next->next) { +qemuDomainJobPrivateMigrateTempBitmapPtr t = next->data; + +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) +return -1; +qemuMonitorBitmapRemove(priv->mon, t->nodename, t->bitmapname); +if (qemuDomainObjExitMonitor(driver, vm) < 0) +return -1; +} + +return 0; +} + + static virStorageSourcePtr qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk, const char *host, @@ -4003,6 +4026,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT, dconn); +qemuMigrationSrcCancelRemoveTempBitmaps(vm, QEMU_ASYNC_JOB_MIGRATION_OUT); + if (priv->job.current->status != QEMU_DOMAIN_JOB_STATUS_CANCELED) priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED; } @@ -5705,6 +5730,9 @@ qemuMigrationSrcCancel(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_NONE, NULL) < 0) return -1; +if (qemuMigrationSrcCancelRemoveTempBitmaps(vm, QEMU_ASYNC_JOB_NONE) < 0) +return -1; + return 0; } -- 2.29.2
[PATCH v2 12/15] tests: qemumigrationcookie: Add testing for block dirty bitmap migration
Test the XML infrastructure for migration cookie element as well as the conversion to migration parameters for QMP schema validation. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- tests/meson.build | 2 +- .../nbd-bitmaps-xml2xml-in.xml| 52 ++ .../nbd-bitmaps-xml2xml-migparams.json| 25 +++ .../nbd-bitmaps-xml2xml-out.xml | 51 ++ tests/qemumigrationcookiexmltest.c| 166 +++--- 5 files changed, 269 insertions(+), 27 deletions(-) create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml diff --git a/tests/meson.build b/tests/meson.build index 0de0783839..b9b2255666 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -452,7 +452,7 @@ if conf.has('WITH_QEMU') { 'name': 'qemuhotplugtest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemumemlocktest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemumigparamstest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, -{ 'name': 'qemumigrationcookiexmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, +{ 'name': 'qemumigrationcookiexmltest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, { 'name': 'qemumonitorjsontest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemusecuritytest', 'sources': [ 'qemusecuritytest.c', 'qemusecuritymock.c' ], 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemustatusxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml new file mode 100644 index 00..b219c25f27 --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml @@ -0,0 +1,52 @@ + + migr + 10b01607-0323-486b-afe2-3014a8a5b98b + sourcehost + 1f5e0da0-fecf-413f-9bf1-1aa9c21e71e4 + + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json new file mode 100644 index 00..100da7c270 --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json @@ -0,0 +1,25 @@ +{ + "block-bitmap-mapping": [ +{ + "node-name": "libvirt-1-format", + "alias": "vda", + "bitmaps": [ +{ + "name": "a", + "alias": "libvirt-vda-a", + "transform": { +"persistent": true + } +}, +{ + "name": "b", + "alias": "libvirt-vda-b" +}, +{ + "name": "c", + "alias": "libvirt-vda-c" +} + ] +} + ] +} diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml new file mode 100644 index 00..09b6fa291c --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml @@ -0,0 +1,51 @@ + + migr + 10b01607-0323-486b-afe2-3014a8a5b98b + hostname + 4a802f00-4cba-5df6-9679-a08c4c5b577f + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemumigrationcookiexmltest.c b/tests/qemumigrationcookiexmltest.c index 5fe0ba8a8a..7f2437a7fe 100644 --- a/tests/qemumigrationcookiexmltest.c +++ b/tests/qemumigrationcookiexmltest.c @@ -25,9 +25,13 @@ #include "internal.h" #include "testutilsqemu.h" +#include "testutilsqemuschema.h" #include "configmake.h" +#define LIBVIRT_QEMU_MIGRATION_PARAMSPRIV_H_ALLOW + #include "qemu/qemu_migration_cookie.h" +#include "qemu/qemu_migration_paramspriv.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -61,13 +65,33 @@ struct testQemuMigrationCookieData { qemuMigrationParty cookiePopulateParty; +qemuMigrationCookiePtr cookie; + char *xmlstr; int xmlstrlen; char *infile;
[PATCH v2 11/15] tests: qemustatusxml2xml: Add status XML from migration with bitmaps
The XML sample shows the status XML when migrating with bitmaps including the element added in previous commit. It will also be used for the migration cookie test. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- .../migration-out-nbd-bitmaps-in.xml | 574 ++ .../migration-out-nbd-bitmaps-out.xml | 1 + tests/qemustatusxml2xmltest.c | 1 + 3 files changed, 576 insertions(+) create mode 100644 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml create mode 12 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-out.xml diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml new file mode 100644 index 00..b3d24eda98 --- /dev/null +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml @@ -0,0 +1,574 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + -2 + +migr +10b01607-0323-486b-afe2-3014a8a5b98b +1024000 +1024000 +1 + + /machine + + + hvm + + + + + + + + + EPYC-Rome + AMD + + + + + + + + + + + + + + + + + + + + + + + + +destroy +restart +restart + + + + + + /home/pipo/git/qemu.git/build/x86_64-softmmu/qemu-system-x86_64 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
[PATCH v2 10/15] qemu: domain: Store list of temporary bitmaps for migration in status XML
Add status XML infrastructure for storing a list of block dirty bitmaps which are temporarily used when migrating a VM with VIR_MIGRATE_NON_SHARED_DISK for cleanup after a libvirtd restart during migration. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_domain.c | 87 -- src/qemu/qemu_domain.h | 15 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 541d592bbe..bb14fe2e33 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -86,6 +86,18 @@ qemuJobAllocPrivate(void) } +void +qemuDomainJobPrivateMigrateTempBitmapFree(qemuDomainJobPrivateMigrateTempBitmapPtr bmp) +{ +if (!bmp) +return; + +g_free(bmp->nodename); +g_free(bmp->bitmapname); +g_free(bmp); +} + + static void qemuJobFreePrivate(void *opaque) { @@ -95,6 +107,9 @@ qemuJobFreePrivate(void *opaque) return; qemuMigrationParamsFree(priv->migParams); +if (priv->migTempBitmaps) +g_slist_free_full(priv->migTempBitmaps, + (GDestroyNotify) qemuDomainJobPrivateMigrateTempBitmapFree); g_free(priv); } @@ -165,6 +180,28 @@ qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, return 0; } + +static void +qemuDomainObjPrivateXMLFormatMigrateTempBitmap(virBufferPtr buf, + GSList *bitmaps) +{ +g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); +GSList *next; + +for (next = bitmaps; next; next = next->next) { +qemuDomainJobPrivateMigrateTempBitmapPtr t = next->data; +g_auto(virBuffer) bitmapBuf = VIR_BUFFER_INITIALIZER; + +virBufferAsprintf(, " name='%s'", t->bitmapname); +virBufferAsprintf(, " nodename='%s'", t->nodename); + +virXMLFormatElement(, "bitmap", , NULL); +} + +virXMLFormatElement(buf, "tempBlockDirtyBitmaps", NULL, ); +} + + static int qemuDomainFormatJobPrivate(virBufferPtr buf, qemuDomainJobObjPtr job, @@ -172,9 +209,12 @@ qemuDomainFormatJobPrivate(virBufferPtr buf, { qemuDomainJobPrivatePtr priv = job->privateData; -if (job->asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT && -qemuDomainObjPrivateXMLFormatNBDMigration(buf, vm) < 0) -return -1; +if (job->asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { +if (qemuDomainObjPrivateXMLFormatNBDMigration(buf, vm) < 0) +return -1; + +qemuDomainObjPrivateXMLFormatMigrateTempBitmap(buf, priv->migTempBitmaps); +} if (priv->migParams) qemuMigrationParamsFormat(buf, priv->migParams); @@ -267,6 +307,44 @@ qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm, return 0; } + +static int +qemuDomainObjPrivateXMLParseMigrateTempBitmap(qemuDomainJobPrivatePtr jobPriv, + xmlXPathContextPtr ctxt) +{ +g_autoslist(qemuDomainJobPrivateMigrateTempBitmap) bitmaps = NULL; +g_autofree xmlNodePtr *nodes = NULL; +size_t i; +int n; + +if ((n = virXPathNodeSet("./tempBlockDirtyBitmaps/bitmap", ctxt, )) < 0) +return -1; + +if (n == 0) +return 0; + +for (i = 0; i < n; i++) { +qemuDomainJobPrivateMigrateTempBitmapPtr bmp; + +bmp = g_new0(qemuDomainJobPrivateMigrateTempBitmap, 1); +bmp->nodename = virXMLPropString(nodes[i], "nodename"); +bmp->bitmapname = virXMLPropString(nodes[i], "name"); + +bitmaps = g_slist_prepend(bitmaps, bmp); + +if (!bmp->bitmapname || !bmp->nodename) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed in status XML")); +return -1; +} + +} + +jobPriv->migTempBitmaps = g_slist_reverse(g_steal_pointer()); +return 0; +} + + static int qemuDomainParseJobPrivate(xmlXPathContextPtr ctxt, qemuDomainJobObjPtr job, @@ -277,6 +355,9 @@ qemuDomainParseJobPrivate(xmlXPathContextPtr ctxt, if (qemuDomainObjPrivateXMLParseJobNBD(vm, ctxt) < 0) return -1; +if (qemuDomainObjPrivateXMLParseMigrateTempBitmap(priv, ctxt) < 0) +return -1; + if (qemuMigrationParamsParse(ctxt, >migParams) < 0) return -1; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9a7d997d65..949307229b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -487,6 +487,20 @@ struct _qemuDomainXmlNsDef { char **capsdel; }; + +typedef struct _qemuDomainJobPrivateMigrateTempBitmap qemuDomainJobPrivateMigrateTempBitmap; +typedef qemuDomainJobPrivateMigrateTempBitmap *qemuDomainJobPrivateMigrateTempBitmapPtr; + +struct _qemuDomainJobPrivateMigrateTempBitmap { +char *nodename; +char *bitmapname; +}; + +void +qemuDomainJobPrivateMigrateTempBitmapFree(qemuDomainJobPrivateMigrateTempBitmapPtr bmp); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobPrivateMigrateTempBitmap,
[PATCH v2 09/15] qemu: migration_cookie: Add helpers for transforming the cookie into migration params
'qemuMigrationCookieBlockDirtyBitmapsMatchDisks' maps the bitmaps from the migration cookie to actual disk objects definition pointers. 'qemuMigrationCookieBlockDirtyBitmapsToParams' converts the bitmap definitions from the migration cookie into parameters for the 'block-bitmap-mapping' migration parameter. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_migration_cookie.c | 115 +++ src/qemu/qemu_migration_cookie.h | 8 +++ 2 files changed, 123 insertions(+) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 0f8555cbb0..186fe7bc9e 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1580,3 +1580,118 @@ qemuMigrationCookieParse(virQEMUDriverPtr driver, return g_steal_pointer(); } + + +/** + * qemuMigrationCookieBlockDirtyBitmapsMatchDisks: + * @def: domain definition + * @disks: list of qemuMigrationBlockDirtyBitmapsDiskPtr + * + * Matches all of the @disks to the actual domain disk definition objects + * by looking up the target. + */ +int +qemuMigrationCookieBlockDirtyBitmapsMatchDisks(virDomainDefPtr def, + GSList *disks) +{ +GSList *next; + +for (next = disks; next; next = next->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = next->data; + +if (!(disk->disk = virDomainDiskByTarget(def, disk->target))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Can't find disk '%s' in domain definition"), + disk->target); +return -1; +} + +disk->nodename = disk->disk->src->nodeformat; +} + +return 0; +} + + +/** + * qemuMigrationCookieBlockDirtyBitmapsToParams: + * @disks: list of qemuMigrationBlockDirtyBitmapsDisk + * @mapping: filled with resulting mapping + * + * Converts @disks into the arguments for 'block-bitmap-mapping' migration + * parameter. + */ +int +qemuMigrationCookieBlockDirtyBitmapsToParams(GSList *disks, + virJSONValuePtr *mapping) +{ +g_autoptr(virJSONValue) map = virJSONValueNewArray(); +bool hasDisks = false; +GSList *nextdisk; + +for (nextdisk = disks; nextdisk; nextdisk = nextdisk->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data; +g_autoptr(virJSONValue) jsondisk = NULL; +g_autoptr(virJSONValue) jsonbitmaps = virJSONValueNewArray(); +bool hasBitmaps = false; +GSList *nextbitmap; + +if (disk->skip || !disk->bitmaps) +continue; + +for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data; +g_autoptr(virJSONValue) jsonbitmap = NULL; +g_autoptr(virJSONValue) transform = NULL; +const char *bitmapname = bitmap->sourcebitmap; + +if (bitmap->skip) +continue; + +/* if there isn't an override, use the real name */ +if (!bitmapname) +bitmapname = bitmap->bitmapname; + +if (bitmap->persistent == VIR_TRISTATE_BOOL_YES) { +if (virJSONValueObjectCreate(, + "b:persistent", true, NULL) < 0) +return -1; +} + +if (virJSONValueObjectCreate(, + "s:name", bitmapname, + "s:alias", bitmap->alias, + "A:transform", , + NULL) < 0) +return -1; + +if (virJSONValueArrayAppend(jsonbitmaps, jsonbitmap) < 0) +return -1; + +jsonbitmap = NULL; +hasBitmaps = true; +} + +if (!hasBitmaps) +continue; + +if (virJSONValueObjectCreate(, + "s:node-name", disk->nodename, + "s:alias", disk->target, + "a:bitmaps", , + NULL) < 0) +return -1; + +if (virJSONValueArrayAppend(map, jsondisk) < 0) +return -1; + +jsondisk = NULL; +hasDisks = true; +} + +if (!hasDisks) +return 0; + +*mapping = g_steal_pointer(); +return 0; +} diff --git a/src/qemu/qemu_migration_cookie.h b/src/qemu/qemu_migration_cookie.h index 8636f955da..e50dee7ba7 100644 --- a/src/qemu/qemu_migration_cookie.h +++ b/src/qemu/qemu_migration_cookie.h @@ -226,3 +226,11 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps, virBufferPtr buf, qemuMigrationCookiePtr mig); + +int
[PATCH v2 06/15] qemu: blockjob: Use qemuMonitorBitmapRemove for single bitmap removal
There's no need in the cleanup steps to invoke a transaction to delete a single bitmap. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_blockjob.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 582fe45c66..aa065b1319 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1420,7 +1420,6 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; -g_autoptr(virJSONValue) actions = virJSONValueNewArray(); virDomainDiskDefPtr disk = job->disk; VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name, vm->def->name); @@ -1428,13 +1427,12 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, if (!disk) return; -ignore_value(qemuMonitorTransactionBitmapRemove(actions, disk->mirror->nodeformat, - "libvirt-tmp-activewrite")); - if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) return; -qemuMonitorTransaction(priv->mon, ); +qemuMonitorBitmapRemove(priv->mon, +disk->mirror->nodeformat, +"libvirt-tmp-activewrite"); if (qemuDomainObjExitMonitor(priv->driver, vm) < 0) return; @@ -1502,7 +1500,6 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, unsigned long long progressTotal) { g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; -g_autoptr(virJSONValue) actions = NULL; qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate, job->errmsg, progressCurrent, progressTotal, asyncJob); @@ -1511,23 +1508,16 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, !(backend = qemuBlockStorageSourceDetachPrepare(job->data.backup.store, NULL))) return; -if (job->data.backup.bitmap) { -actions = virJSONValueNewArray(); - -if (qemuMonitorTransactionBitmapRemove(actions, - job->disk->src->nodeformat, - job->data.backup.bitmap) < 0) -return; -} - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return; if (backend) qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), backend); -if (actions) -qemuMonitorTransaction(qemuDomainGetMonitor(vm), ); +if (job->data.backup.bitmap) +qemuMonitorBitmapRemove(qemuDomainGetMonitor(vm), +job->disk->src->nodeformat, +job->data.backup.bitmap); if (qemuDomainObjExitMonitor(driver, vm) < 0) return; -- 2.29.2
[PATCH v2 04/15] qemu: migration: Create qcow2 v3 images for VIR_MIGRATE_NON_SHARED_DISK
Use the new format when pre-creating the image for the user. Users wishing to use the legacy format can always provide their own images or use hared storage. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e8e35c1c7c..94b9b34ca0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -180,6 +180,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, char *volStr = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const char *format = NULL; +const char *compat = NULL; unsigned int flags = 0; VIR_DEBUG("Precreate disk type=%s", virStorageTypeToString(disk->src->type)); @@ -212,8 +213,11 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath))) goto cleanup; format = virStorageFileFormatTypeToString(disk->src->format); -if (disk->src->format == VIR_STORAGE_FILE_QCOW2) +if (disk->src->format == VIR_STORAGE_FILE_QCOW2) { flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; +/* format qcow2v3 image */ +compat = "1.1"; +} break; case VIR_STORAGE_TYPE_VOLUME: @@ -261,6 +265,8 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, virBufferAddLit(, "\n"); virBufferAdjustIndent(, 2); virBufferAsprintf(, "\n", format); +if (compat) +virBufferAsprintf(, "%s\n", compat); virBufferAdjustIndent(, -2); virBufferAddLit(, "\n"); virBufferAdjustIndent(, -2); -- 2.29.2
[PATCH v2 07/15] qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature
Add the migration capability flag and the propagation of the corresponding mapping configuration. The mapping will be produced from the bitmaps on disk depending on both sides of the migration and the necessity to perform merges. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration_params.c | 29 + src/qemu/qemu_migration_params.h | 5 + 2 files changed, 34 insertions(+) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 510dad783a..e9b4601510 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -63,6 +63,7 @@ struct _qemuMigrationParams { unsigned long long compMethods; /* bit-wise OR of qemuMigrationCompressMethod */ virBitmapPtr caps; qemuMigrationParamValue params[QEMU_MIGRATION_PARAM_LAST]; +virJSONValuePtr blockDirtyBitmapMapping; }; typedef enum { @@ -89,6 +90,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "pause-before-switchover", "late-block-activate", "multifd", + "dirty-bitmaps", ); @@ -265,6 +267,7 @@ qemuMigrationParamsFree(qemuMigrationParamsPtr migParams) } virBitmapFree(migParams->caps); +virJSONValueFree(migParams->blockDirtyBitmapMapping); g_free(migParams); } @@ -524,6 +527,20 @@ qemuMigrationParamsSetCompression(virTypedParameterPtr params, } +void +qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParamsPtr migParams, + virJSONValuePtr *params) +{ +virJSONValueFree(migParams->blockDirtyBitmapMapping); +migParams->blockDirtyBitmapMapping = g_steal_pointer(params); + +if (migParams->blockDirtyBitmapMapping) +ignore_value(virBitmapSetBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS)); +else +ignore_value(virBitmapClearBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS)); +} + + qemuMigrationParamsPtr qemuMigrationParamsFromFlags(virTypedParameterPtr params, int nparams, @@ -747,6 +764,17 @@ qemuMigrationParamsToJSON(qemuMigrationParamsPtr migParams) return NULL; } +if (migParams->blockDirtyBitmapMapping) { +g_autoptr(virJSONValue) mapping = virJSONValueCopy(migParams->blockDirtyBitmapMapping); + +if (!mapping) +return NULL; + +if (virJSONValueObjectAppend(params, "block-bitmap-mapping", mapping) < 0) +return NULL; +mapping = NULL; +} + return g_steal_pointer(); } @@ -1202,6 +1230,7 @@ qemuMigrationParamsReset(virQEMUDriverPtr driver, goto cleanup; qemuMigrationParamsResetTLS(driver, vm, asyncJob, origParams, apiFlags); +/* We don't reset 'block-bitmap-mapping' as it can't be unset */ cleanup: virErrorRestore(); diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 9876101bfc..f1db42ce94 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -39,6 +39,7 @@ typedef enum { QEMU_MIGRATION_CAP_PAUSE_BEFORE_SWITCHOVER, QEMU_MIGRATION_CAP_LATE_BLOCK_ACTIVATE, QEMU_MIGRATION_CAP_MULTIFD, +QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS, QEMU_MIGRATION_CAP_LAST } qemuMigrationCapability; @@ -132,6 +133,10 @@ qemuMigrationParamsGetULL(qemuMigrationParamsPtr migParams, qemuMigrationParam param, unsigned long long *value); +void +qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParamsPtr migParams, + virJSONValuePtr *params); + int qemuMigrationParamsCheck(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.29.2
[PATCH v2 05/15] qemu: monitor: Introduce qemuMonitorBitmapRemove
The non-transaction wrapper is useful for code paths which want to delete individual bitmaps or for cleanup after a failed job where we want to attempt to delete every bitmap individually to prevent a failure from cleaning up the rest. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_monitor_json.c | 24 src/qemu/qemu_monitor_json.h | 6 ++ tests/qemumonitorjsontest.c | 2 ++ 5 files changed, 50 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 151f69acef..ed35da17e1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4653,6 +4653,19 @@ qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions, } +int +qemuMonitorBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name) +{ +VIR_DEBUG("node='%s', name='%s'", node, name); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBitmapRemove(mon, node, name); +} + + int qemuMonitorTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0108703a33..d25c26343a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1478,6 +1478,11 @@ int qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions, const char *node, const char *name); + +int +qemuMonitorBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name); int qemuMonitorTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a07306365..03224d4af2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9252,6 +9252,30 @@ qemuMonitorJSONTransactionBitmapRemove(virJSONValuePtr actions, } +int +qemuMonitorJSONBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name) +{ +g_autoptr(virJSONValue) cmd = NULL; +g_autoptr(virJSONValue) reply = NULL; + +if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-remove", + "s:node", node, + "s:name", name, + NULL))) +return -1; + +if (qemuMonitorJSONCommand(mon, cmd, ) < 0) +return -1; + +if (qemuMonitorJSONCheckError(cmd, reply) < 0) +return -1; + +return 0; +} + + int qemuMonitorJSONTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 31652d4207..3dd1eb24c7 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -659,6 +659,12 @@ int qemuMonitorJSONTransactionBitmapRemove(virJSONValuePtr actions, const char *node, const char *name); + +int +qemuMonitorJSONBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name); + int qemuMonitorJSONTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 5a5976dbe4..82c74e2ef9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1215,6 +1215,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode") +GEN_TEST_FUNC(qemuMonitorJSONBitmapRemove, "foodev", "newnode") GEN_TEST_FUNC(qemuMonitorJSONJobDismiss, "jobname") GEN_TEST_FUNC(qemuMonitorJSONJobCancel, "jobname", false) GEN_TEST_FUNC(qemuMonitorJSONJobComplete, "jobname") @@ -3132,6 +3133,7 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONBlockdevTrayClose); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumRemove); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert); +DO_TEST_GEN(qemuMonitorJSONBitmapRemove); DO_TEST_GEN(qemuMonitorJSONJobDismiss); DO_TEST_GEN(qemuMonitorJSONJobCancel); DO_TEST_GEN(qemuMonitorJSONJobComplete); -- 2.29.2
[PATCH v2 02/15] qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING
The capability represents qemu's ability to setup mappings for migrating block dirty bitmaps and is based on presence of the 'transform' property of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP command. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + 3 files changed, 5 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3f8593a9e5..600952a53a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -617,6 +617,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "cpu-max", "memory-backend-file.x-use-canonical-path-for-ramblock-id", "vnc-opts", + "migration-param.block-bitmap-mapping", ); @@ -1550,6 +1551,8 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "migrate-set-parameters/arg-type/xbzrle-cache-size", QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE }, { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA }, +{ "migrate-set-parameters/arg-type/block-bitmap-mapping/bitmaps/transform", + QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 38574eef16..a5b6c7f104 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -597,6 +597,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_CPU_MAX, /* -cpu max */ QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object memory-backend-file,x-use-canonical-path-for-ramblock-id= */ QEMU_CAPS_VNC_OPTS, /* -vnc uses QemuOpts parser instead of custom code */ +QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING, /* block-bitmap-mapping in migrate-set-parameters */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index e68df54071..e7e6254293 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -256,6 +256,7 @@ + 5002050 0 43100242 -- 2.29.2
[PATCH v2 00/15] qemu: migrate block bitmaps when migrating storage
When we are copying storage we should also preserve the block dirty bitmaps. This series implements their migration. For standard migration with shared storage we let qemu flush the bitmaps to disk and reload them on destination. There is possibility to migrate them using the migration stream too but it's not implemented in this series. v2: Patches from v1: - patches 4-7 from v1 were pushed already - patch 8 from v1 was dropped This version: - patch 1 was updated to capture the latest capabilities - patch 4 is new, replaces patch 8 from v1 - all other patches apply review feedback from Jirka - patch 14 has one additional change (thus I didn't carry Jirka's R-b): The migration of bitmaps is enabled based on QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING instead of QEMU_CAPS_INCREMENTAL_BACKUP. Peter Krempa (15): qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64 qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes qemu: migration: Create qcow2 v3 images for VIR_MIGRATE_NON_SHARED_DISK qemu: monitor: Introduce qemuMonitorBitmapRemove qemu: blockjob: Use qemuMonitorBitmapRemove for single bitmap removal qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature qemu: migration_cookie: Add XML handling for setting up bitmap migration qemu: migration_cookie: Add helpers for transforming the cookie into migration params qemu: domain: Store list of temporary bitmaps for migration in status XML tests: qemustatusxml2xml: Add status XML from migration with bitmaps tests: qemumigrationcookie: Add testing for block dirty bitmap migration qemu: migration: Clean up temporary bitmaps when cancelling a migration qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints qemu: capabilities: Enable QEMU_CAPS_INCREMENTAL_BACKUP src/qemu/qemu_blockjob.c | 24 +- src/qemu/qemu_capabilities.c | 9 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c| 87 +- src/qemu/qemu_domain.h| 15 + src/qemu/qemu_migration.c | 369 - src/qemu/qemu_migration_cookie.c | 261 ++ src/qemu/qemu_migration_cookie.h | 42 + src/qemu/qemu_migration_params.c | 29 + src/qemu/qemu_migration_params.h | 5 + src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 8 + src/qemu/qemu_monitor_json.c | 35 + src/qemu/qemu_monitor_json.h | 6 + tests/meson.build | 2 +- tests/qemublocktest.c | 2 + tests/qemublocktestdata/bitmap/synthetic.json | 2 +- tests/qemublocktestdata/bitmap/synthetic.out | 1 + .../caps_6.0.0.x86_64.replies | 741 ++ .../caps_6.0.0.x86_64.xml | 23 +- .../nbd-bitmaps-xml2xml-in.xml| 52 ++ .../nbd-bitmaps-xml2xml-migparams.json| 25 + .../nbd-bitmaps-xml2xml-out.xml | 51 ++ tests/qemumigrationcookiexmltest.c| 166 +++- tests/qemumonitorjsontest.c | 2 + .../migration-out-nbd-bitmaps-in.xml | 574 ++ .../migration-out-nbd-bitmaps-out.xml | 1 + tests/qemustatusxml2xmltest.c | 1 + 28 files changed, 2172 insertions(+), 375 deletions(-) create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml create mode 100644 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml create mode 12 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-out.xml -- 2.29.2
[PATCH v2 03/15] qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes
Such images don't support stuff like dirty bitmaps. Note that the synthetic test for detecting bitmaps is used as an example to prevent adding additional test cases. Signed-off-by: Peter Krempa Reviewed-by: Jiri Denemark --- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 11 +++ tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/bitmap/synthetic.json | 2 +- tests/qemublocktestdata/bitmap/synthetic.out | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 32dc96ee82..0108703a33 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -732,6 +732,9 @@ struct _qemuBlockNamedNodeData { /* the cluster size of the image is valid only when > 0 */ unsigned long long clusterSize; + +/* image version */ +bool qcow2v2; }; GHashTable * diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f8c78d9093..3a07306365 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2978,6 +2978,7 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, GHashTable *nodes = opaque; virJSONValuePtr img; virJSONValuePtr bitmaps; +virJSONValuePtr format_specific; const char *nodename; g_autoptr(qemuBlockNamedNodeData) ent = NULL; @@ -3000,6 +3001,16 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps"))) qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent); +/* query qcow2 format specific props */ +if ((format_specific = virJSONValueObjectGetObject(img, "format-specific")) && +STREQ_NULLABLE(virJSONValueObjectGetString(format_specific, "type"), "qcow2")) { +virJSONValuePtr qcow2props = virJSONValueObjectGetObject(format_specific, "data"); + +if (qcow2props && +STREQ_NULLABLE(virJSONValueObjectGetString(qcow2props, "compat"), "0.10")) +ent->qcow2v2 = true; +} + if (virHashAddEntry(nodes, nodename, ent) < 0) return -1; diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index ddaf73359d..bbfcfee92d 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -599,6 +599,8 @@ testQemuDetectBitmapsWorker(GHashTable *nodedata, return; virBufferAsprintf(buf, "%s:\n", nodename); +if (data->qcow2v2) +virBufferAddLit(buf, " qcow2 v2\n"); virBufferAdjustIndent(buf, 1); for (i = 0; i < data->nbitmaps; i++) { diff --git a/tests/qemublocktestdata/bitmap/synthetic.json b/tests/qemublocktestdata/bitmap/synthetic.json index 3712c8e5fc..cd468a42a2 100644 --- a/tests/qemublocktestdata/bitmap/synthetic.json +++ b/tests/qemublocktestdata/bitmap/synthetic.json @@ -12,7 +12,7 @@ "format-specific": { "type": "qcow2", "data": { - "compat": "1.1", + "compat": "0.10", "compression-type": "zlib", "lazy-refcounts": false, "bitmaps": [ diff --git a/tests/qemublocktestdata/bitmap/synthetic.out b/tests/qemublocktestdata/bitmap/synthetic.out index cde7228e01..2d9545fc9b 100644 --- a/tests/qemublocktestdata/bitmap/synthetic.out +++ b/tests/qemublocktestdata/bitmap/synthetic.out @@ -1,4 +1,5 @@ libvirt-1-format: + qcow2 v2 current: record:1 busy:0 persist:1 inconsist:1 gran:65536 dirty:0 top-ok: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 top-inactive: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 -- 2.29.2
[PATCH v2 01/15] qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64
Include the 'transform' member of 'block-bitmap-mapping'. This is based on qemu commit v5.2.0-2208-gc79f01c945 Signed-off-by: Peter Krempa --- .../caps_6.0.0.x86_64.replies | 741 ++ .../caps_6.0.0.x86_64.xml | 22 +- 2 files changed, 441 insertions(+), 322 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies index a1e3850b59..04ebd04583 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies @@ -21,7 +21,7 @@ "minor": 2, "major": 5 }, -"package": "v5.2.0-1810-g2436651b26" +"package": "v5.2.0-2208-gc79f01c945" }, "id": "libvirt-2" } @@ -811,6 +811,10 @@ "name": "usb-hub", "parent": "usb-device" }, +{ + "name": "chardev-serial", + "parent": "chardev-fd" +}, { "name": "virtio-blk-device", "parent": "virtio-device" @@ -852,8 +856,8 @@ "parent": "accel" }, { - "name": "chardev-serial", - "parent": "chardev-fd" + "name": "Cooperlake-x86_64-cpu", + "parent": "x86_64-cpu" }, { "name": "vhost-user-vsock-pci", @@ -907,10 +911,6 @@ "name": "pxb-host", "parent": "pci-host-bridge" }, -{ - "name": "Cooperlake-x86_64-cpu", - "parent": "x86_64-cpu" -}, { "name": "scsi-disk", "parent": "scsi-disk-base" @@ -1143,6 +1143,10 @@ "name": "i82557c", "parent": "pci-device" }, +{ + "name": "i82557b", + "parent": "pci-device" +}, { "name": "virtio-scsi-device", "parent": "virtio-scsi-common" @@ -1151,10 +1155,6 @@ "name": "pxb-pcie", "parent": "pci-device" }, -{ - "name": "i82557b", - "parent": "pci-device" -}, { "name": "Haswell-IBRS-x86_64-cpu", "parent": "x86_64-cpu" @@ -1172,12 +1172,8 @@ "parent": "sys-bus-device" }, { - "name": "chardev-memory", - "parent": "chardev-ringbuf" -}, -{ - "name": "pc-q35-2.8-machine", - "parent": "generic-pc-machine" + "name": "i82557a", + "parent": "pci-device" }, { "name": "vhost-user-scsi-pci-non-transitional", @@ -1188,12 +1184,16 @@ "parent": "isa-device" }, { - "name": "generic-sdhci", - "parent": "sys-bus-device" + "name": "chardev-udp", + "parent": "chardev" }, { - "name": "i82557a", - "parent": "pci-device" + "name": "pc-q35-2.8-machine", + "parent": "generic-pc-machine" +}, +{ + "name": "generic-sdhci", + "parent": "sys-bus-device" }, { "name": "virtio-scsi-pci-non-transitional", @@ -1212,8 +1212,8 @@ "parent": "x86_64-cpu" }, { - "name": "chardev-udp", - "parent": "chardev" + "name": "chardev-memory", + "parent": "chardev-ringbuf" }, { "name": "EPYC-Rome-v1-x86_64-cpu", @@ -1311,14 +1311,14 @@ "name": "isabus-bridge", "parent": "sys-bus-device" }, -{ - "name": "ne2k_pci", - "parent": "pci-device" -}, { "name": "IvyBridge-v2-x86_64-cpu", "parent": "x86_64-cpu" }, +{ + "name": "ne2k_pci", + "parent": "pci-device" +}, { "name": "usb-bus", "parent": "bus" @@ -1328,29 +1328,25 @@ "parent": "ide-device" }, { - "name": "tcg-accel", - "parent": "accel" + "name": "qemu:memory-region", + "parent": "object" }, { - "name": "piix4-ide", - "parent": "pci-ide" + "name": "tcg-accel", + "parent": "accel" }, { "name": "virtio-balloon-pci", "parent": "virtio-balloon-pci-base" }, -{ - "name": "Cascadelake-Server-v2-x86_64-cpu", - "parent": "x86_64-cpu" -}, -{ - "name": "qemu:memory-region", - "parent": "object" -}, { "name": "virtio-gpu-device", "parent": "virtio-gpu-base" }, +{ + "name": "Cascadelake-Server-v2-x86_64-cpu", + "parent": "x86_64-cpu" +}, { "name": "e1000", "parent": "e1000-base" @@ -1367,6 +1363,10 @@ "name": "ES1370", "parent": "pci-device" }, +{ + "name": "pc-i440fx-2.6-machine", + "parent": "generic-pc-machine" +}, { "name": "i82551", "parent": "pci-device" @@ -1384,8 +1384,8 @@ "parent": "x86_64-cpu" }, { - "name": "pc-i440fx-2.6-machine", - "parent": "generic-pc-machine" + "name": "piix4-ide", + "parent": "pci-ide" }, { "name": "SandyBridge-x86_64-cpu", @@ -1403,14 +1403,14 @@ "name": "pc-q35-6.0-machine", "parent": "generic-pc-machine" }, -{ - "name": "vhost-user-backend", - "parent": "object" -}, { "name":
Re: [libvirt PATCH v2 1/1] qemu_validate: Allow kvm hint-dedicated on non-passthrough VMs
On a Friday in 2021, Tim Wiederhake wrote: A VM defined similar to: ... ... is currently invalid, as hint-dedicated is only allowed if cpu mode is host-passthrough or maximum. This restriction is unnecessary, see https://bugzilla.redhat.com/show_bug.cgi?id=1857671 Signed-off-by: Tim Wiederhake --- src/qemu/qemu_validate.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[libvirt PATCH v2 1/1] qemu_validate: Allow kvm hint-dedicated on non-passthrough VMs
A VM defined similar to: ... ... is currently invalid, as hint-dedicated is only allowed if cpu mode is host-passthrough or maximum. This restriction is unnecessary, see https://bugzilla.redhat.com/show_bug.cgi?id=1857671 Signed-off-by: Tim Wiederhake --- src/qemu/qemu_validate.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 2541ae856a..b9971b66bb 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -253,17 +253,6 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; -case VIR_DOMAIN_FEATURE_KVM: -if (def->kvm_features[VIR_DOMAIN_KVM_DEDICATED] == VIR_TRISTATE_SWITCH_ON && -(!def->cpu || (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && - def->cpu->mode != VIR_CPU_MODE_MAXIMUM))) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("kvm-hint-dedicated=on is only applicable " - "for cpu host-passthrough / maximum")); -return -1; -} -break; - case VIR_DOMAIN_FEATURE_VMPORT: if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && !virQEMUCapsSupportsVmport(qemuCaps, def)) { @@ -336,6 +325,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; +case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_XEN: case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: -- 2.26.2
[libvirt PATCH v2 0/1] Allow kvm hint-dedicated on non-passthrough VMs
V1: https://listman.redhat.com/archives/libvir-list/2021-January/msg00756.html Changes since V1: * rebased * updated commit message Tim Wiederhake (1): qemu_validate: Allow kvm hint-dedicated on non-passthrough VMs src/qemu/qemu_validate.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) -- 2.26.2