[PATCH] libxl: Fix domain shutdown

2021-02-19 Thread Jim Fehlig
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

2021-02-19 Thread Jiri Denemark
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

2021-02-19 Thread Jiri Denemark
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

2021-02-19 Thread Jiri Denemark
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

2021-02-19 Thread Jiri Denemark
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

2021-02-19 Thread Pavel Hrdina
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

2021-02-19 Thread Pavel Hrdina
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

2021-02-19 Thread Pavel Hrdina
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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()

2021-02-19 Thread Laine Stump

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()

2021-02-19 Thread Laine Stump

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

2021-02-19 Thread Jonathon Jongsma
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

2021-02-19 Thread Michal Privoznik

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

2021-02-19 Thread Stefan Berger
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Stefan Berger
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

2021-02-19 Thread Ján Tomko

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

2021-02-19 Thread Kristina Hanicova
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
'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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Peter Krempa
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

2021-02-19 Thread Ján Tomko

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

2021-02-19 Thread Tim Wiederhake
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

2021-02-19 Thread Tim Wiederhake
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