Re: [PATCH] Share Dup Daemon Function *SetupLogging

[Ján Tomko]

On a Wednesday in 2020, Lan wrote:

Create a new function virDaemonSetupLogging (src/util/virdaemon.c)
for shared code in
virLockDaemonSetupLogging (src/locking/lock_daemon.c)
virLogDaemonSetupLogging (src/logging/log_daemon.c)
daemonSetupLogging (src/remote/remote_daemon.c)



As mentioned in our contributor guidelines (number 6. in general tips):
https://libvirt.org/hacking.html
we require a sign-off line with your legal name and e-mail address
that tells us that we can legally use the patch you send us:
https://developercertificate.org/


One of the BiteSizedTasks
---
src/libvirt_private.syms   |  2 +
src/locking/lock_daemon.c  | 58 ++---
src/logging/log_daemon.c   | 50 ++---
src/remote/remote_daemon.c | 57 ++---
src/util/Makefile.inc.am   |  4 +-
src/util/virdaemon.c   | 75 ++
src/util/virdaemon.h   | 35 ++
7 files changed, 126 insertions(+), 155 deletions(-)
create mode 100644 src/util/virdaemon.c
create mode 100644 src/util/virdaemon.h

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index de0c7a3133..50cbd6d7af 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1906,6 +1906,8 @@ virCryptoHashBuf;
virCryptoHashString;
virCryptoHaveCipher;

+# util/virdaemon.h
+virDaemonSetupLogging;

# util/virdbus.h
virDBusCallMethod;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 5e5a0c1089..5ba851cb55 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -46,6 +46,7 @@
#include "virstring.h"
#include "virgettext.h"
#include "virenum.h"
+#include "virdaemon.h"

#include "locking/lock_daemon_dispatch.h"
#include "locking/lock_protocol.h"
@@ -477,59 +478,6 @@ virLockDaemonErrorHandler(void *opaque G_GNUC_UNUSED,
}


-/*
- * Set up the logging environment
- * By default if daemonized all errors go to the logfile libvirtd.log,
- * but if verbose or error debugging is asked for then also output
- * informational and debug messages. Default size if 64 kB.
- */
-static int
-virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
-  bool privileged,
-  bool verbose,
-  bool godaemon)
-{
-virLogReset();
-
-/*
- * Libvirtd's order of precedence is:
- * cmdline > environment > config
- *
- * Given the precedence, we must process the variables in the opposite
- * order, each one overriding the previous.
- */
-if (config->log_level != 0)
-virLogSetDefaultPriority(config->log_level);
-
-/* In case the config is empty, both filters and outputs will become empty,
- * however we can't start with empty outputs, thus we'll need to define and
- * setup a default one.
- */
-ignore_value(virLogSetFilters(config->log_filters));
-ignore_value(virLogSetOutputs(config->log_outputs));
-
-/* If there are some environment variables defined, use those instead */
-virLogSetFromEnv();
-
-/*
- * Command line override for --verbose
- */
-if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
-virLogSetDefaultPriority(VIR_LOG_INFO);
-
-/* Define the default output. This is only applied if there was no setting
- * from either the config or the environment.
- */
-virLogSetDefaultOutput("virtlockd", godaemon, privileged);
-
-if (virLogGetNbOutputs() == 0)
-virLogSetOutputs(virLogGetDefaultOutput());
-
-return 0;
-}
-
-
-
/* Display version information. */
static void
virLockDaemonVersion(const char *argv0)
@@ -1186,7 +1134,9 @@ int main(int argc, char **argv) {
}
VIR_FREE(remote_config_file);

-if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) {
+if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
+  "virtlockd", privileged,
+  verbose, godaemon)< 0) {


This does not compile for me:
../../src/logging/log_daemon.c:916:27: error: cast from 'unsigned int *' to 
'virDaemonLogConfigPtr'
  (aka 'struct _virDaemonLogConfig *') increases required alignment from 4 
to 8
  [-Werror,-Wcast-align]
virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
  ^
config->log_level is an unsigned int
virDaemonLogConfig is a structure

While this might possibly work if the structs and stars are properly
aligned, the clean, readable, and maintainable solution is to change
all of the individual DaemonConfig structures to store the
log_* variables in a virDaemonLogConfig structure. And only then
change all the daemons to use the new virDaemonSetupLogging function.

So this change will require a few separate commits:
* Introduce virDaemonLogConfig (and the virdaemon files)
* Use virDaemonLogConfig instead of the separate config->log_* variables
* Pass 

Re: [PATCH v4] admin: use g_autofree

[Ján Tomko]

On a Wednesday in 2020, Daniel Henrique Barboza wrote:



On 3/4/20 3:06 PM, Gaurav Agrawal wrote:

Signed-off-by: Gaurav Agrawal 
---
 src/admin/libvirt-admin.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..709e009467 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
 virURIParamPtr param = &uri->params[i];
 if (STREQ(param->name, "socket")) {
-VIR_FREE(sock_path);
+g_free(sock_path);
 sock_path = g_strdup(param->value);
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
 virAdmConnectPtr
 virAdmConnectOpen(const char *name, unsigned int flags)
 {
-char *sock_path = NULL;
+g_autofree char *sock_path = NULL;
 char *alias = NULL;
 virAdmConnectPtr conn = NULL;
 g_autoptr(virConf) conf = NULL;
-char *uristr = NULL;
+g_autofree char *uristr = NULL;
 if (virAdmInitialize() < 0)
 goto error;
@@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
 goto error;
 if (alias) {
-VIR_FREE(uristr);
+g_free(uristr);
 uristr = alias;
 }
@@ -251,16 +251,13 @@ virAdmConnectOpen(const char *name, unsigned int flags)
 if (remoteAdminConnectOpen(conn, flags) < 0)
 goto error;
- cleanup:
-VIR_FREE(sock_path);
-VIR_FREE(uristr);
 return conn;
  error:
 virDispatchError(NULL);
 virObjectUnref(conn);
 conn = NULL;



virObjectUnref(conn) will end up turning conn to NULL via 'VIR_FREE()', given
that 'conn' is guaranteed to have only one reference at this point (it was
just created via virAdmConnectNew() in this function).


This is only partially correct.

The virAdmConnect structure at the address 'conn' does have only one
reference and the virObjectUnref call will call free() on that address
after it sets the reference count to zero.

Our 'VIR_FREE' wrapper over the 'free' function does set the pointer it
frees to NULL. But it does so by taking a pointer to the pointer to the
freed data as a parameter.

virObjectUnref only takes a pointer to the data. So the VIR_FREE call
can only set the 'obj' pointer inside the virObjectUnref to NULL. The
original 'conn' variable here stays unchanged.



This 'conn = NULL' assignment was probably being used here to make it clearer
that the 'goto cleanup' jump would return NULL in this situation. Since you're
removing the need for the jump and the label, this 'conn = NULL' assignment is
now unneeded.



All that being said, the assignment is not needed anymore so I've
removed it before pushing the patch.




The rest of the code LGTM. If the maintainer does not mind removing the 'conn = 
NULL'
when pushing the patch, here's my r-b:


Reviewed-by: Daniel Henrique Barboza 


Reviewed-by: Ján Tomko 
and pushed now.

Jano


signature.asc
Description: PGP signature

Re: [PATCH v4] admin: use g_autofree

[Daniel Henrique Barboza]

On 3/4/20 3:06 PM, Gaurav Agrawal wrote:

Signed-off-by: Gaurav Agrawal 
---
  src/admin/libvirt-admin.c | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..709e009467 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
  virURIParamPtr param = &uri->params[i];
  
  if (STREQ(param->name, "socket")) {

-VIR_FREE(sock_path);
+g_free(sock_path);
  sock_path = g_strdup(param->value);
  } else {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
  virAdmConnectPtr
  virAdmConnectOpen(const char *name, unsigned int flags)
  {
-char *sock_path = NULL;
+g_autofree char *sock_path = NULL;
  char *alias = NULL;
  virAdmConnectPtr conn = NULL;
  g_autoptr(virConf) conf = NULL;
-char *uristr = NULL;
+g_autofree char *uristr = NULL;
  
  if (virAdmInitialize() < 0)

  goto error;
@@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
  goto error;
  
  if (alias) {

-VIR_FREE(uristr);
+g_free(uristr);
  uristr = alias;
  }
  
@@ -251,16 +251,13 @@ virAdmConnectOpen(const char *name, unsigned int flags)

  if (remoteAdminConnectOpen(conn, flags) < 0)
  goto error;
  
- cleanup:

-VIR_FREE(sock_path);
-VIR_FREE(uristr);
  return conn;
  
   error:

  virDispatchError(NULL);
  virObjectUnref(conn);
  conn = NULL;



virObjectUnref(conn) will end up turning conn to NULL via 'VIR_FREE()', given
that 'conn' is guaranteed to have only one reference at this point (it was
just created via virAdmConnectNew() in this function).

This 'conn = NULL' assignment was probably being used here to make it clearer
that the 'goto cleanup' jump would return NULL in this situation. Since you're
removing the need for the jump and the label, this 'conn = NULL' assignment is
now unneeded.



The rest of the code LGTM. If the maintainer does not mind removing the 'conn = 
NULL'
when pushing the patch, here's my r-b:


Reviewed-by: Daniel Henrique Barboza 





-goto cleanup;
+return NULL;
  }
  
  /**

[PATCH v4] admin: use g_autofree

[Gaurav Agrawal]

Signed-off-by: Gaurav Agrawal 
---
 src/admin/libvirt-admin.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..709e009467 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
 virURIParamPtr param = &uri->params[i];
 
 if (STREQ(param->name, "socket")) {
-VIR_FREE(sock_path);
+g_free(sock_path);
 sock_path = g_strdup(param->value);
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
 virAdmConnectPtr
 virAdmConnectOpen(const char *name, unsigned int flags)
 {
-char *sock_path = NULL;
+g_autofree char *sock_path = NULL;
 char *alias = NULL;
 virAdmConnectPtr conn = NULL;
 g_autoptr(virConf) conf = NULL;
-char *uristr = NULL;
+g_autofree char *uristr = NULL;
 
 if (virAdmInitialize() < 0)
 goto error;
@@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
 goto error;
 
 if (alias) {
-VIR_FREE(uristr);
+g_free(uristr);
 uristr = alias;
 }
 
@@ -251,16 +251,13 @@ virAdmConnectOpen(const char *name, unsigned int flags)
 if (remoteAdminConnectOpen(conn, flags) < 0)
 goto error;
 
- cleanup:
-VIR_FREE(sock_path);
-VIR_FREE(uristr);
 return conn;
 
  error:
 virDispatchError(NULL);
 virObjectUnref(conn);
 conn = NULL;
-goto cleanup;
+return NULL;
 }
 
 /**
-- 
2.24.1

[PATCH 12/13] qemu: blockjob: Handle bitmaps after finish of normal block-commit

[Peter Krempa]

Merge the bitmaps into base of the block commit after the job finishes.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 50 
 1 file changed, 50 insertions(+)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 44c0dc2c16..904c4c3f46 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1049,6 +1049,53 @@ qemuBlockJobDeleteImages(virQEMUDriverPtr driver,
 }
 }

+
+/**
+ * qemuBlockJobProcessEventCompletedCommitBitmaps:
+ *
+ * Handles the bitmap changes after commit. This function shall return -1 on
+ * monitor failures.
+ */
+static int
+qemuBlockJobProcessEventCompletedCommitBitmaps(virDomainObjPtr vm,
+   qemuBlockJobDataPtr job,
+   qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+g_autoptr(virJSONValue) actions = NULL;
+
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))
+return -1;
+
+if (qemuBlockBitmapsHandleCommitFinish(job->data.commit.top,
+   job->data.commit.base,
+   blockNamedNodeData,
+   &actions,
+   
job->data.commit.disabledBitmapsBase) < 0)
+return 0;
+
+if (!actions)
+return 0;
+
+if (qemuBlockReopenReadWrite(vm, job->data.commit.base, asyncJob) < 0)
+return -1;
+
+if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
+return -1;
+
+qemuMonitorTransaction(priv->mon, &actions);
+
+if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)
+return -1;
+
+if (qemuBlockReopenReadOnly(vm, job->data.commit.base, asyncJob) < 0)
+return -1;
+
+return 0;
+}
+
+
 /**
  * qemuBlockJobProcessEventCompletedCommit:
  * @driver: qemu driver object
@@ -1106,6 +1153,9 @@ qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr 
driver,
 if (!n)
 return;

+if (qemuBlockJobProcessEventCompletedCommitBitmaps(vm, job, asyncJob) < 0)
+return;
+
 /* revert access to images */
 qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base, 
true, false);
 if (job->data.commit.topparent != job->disk->src)
-- 
2.24.1

[PATCH 02/13] qemu: domain: Extract parsing of 'commit' blockjob data into a function

[Peter Krempa]

I'll be adding more fields to care about so splitting the code out will
be better long-term.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 57 ++
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 71d0a400cc..0a478d2080 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3153,6 +3153,40 @@ 
qemuDomainObjPrivateXMLParseBlockjobNodename(qemuBlockJobDataPtr job,
 }


+static int
+qemuDomainObjPrivateXMLParseBlockjobDataCommit(qemuBlockJobDataPtr job,
+   xmlXPathContextPtr ctxt)
+{
+if (job->type == QEMU_BLOCKJOB_TYPE_COMMIT) {
+qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+ 
"string(./topparent/@node)",
+ 
&job->data.commit.topparent,
+ ctxt);
+
+if (!job->data.commit.topparent)
+return -1;
+}
+
+qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+ "string(./top/@node)",
+ &job->data.commit.top,
+ ctxt);
+qemuDomainObjPrivateXMLParseBlockjobNodename(job,
+ "string(./base/@node)",
+ &job->data.commit.base,
+ ctxt);
+
+if (virXPathNode("./deleteCommittedImages", ctxt))
+job->data.commit.deleteCommittedImages = true;
+
+if (!job->data.commit.top ||
+!job->data.commit.base)
+return -1;
+
+return 0;
+}
+
+
 static void
 qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job,
  xmlXPathContextPtr ctxt,
@@ -3172,29 +3206,10 @@ 
qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job,
 break;

 case QEMU_BLOCKJOB_TYPE_COMMIT:
-qemuDomainObjPrivateXMLParseBlockjobNodename(job,
- 
"string(./topparent/@node)",
- 
&job->data.commit.topparent,
- ctxt);
-
-if (!job->data.commit.topparent)
-goto broken;
-
-G_GNUC_FALLTHROUGH;
 case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-qemuDomainObjPrivateXMLParseBlockjobNodename(job,
- "string(./top/@node)",
- &job->data.commit.top,
- ctxt);
-qemuDomainObjPrivateXMLParseBlockjobNodename(job,
- 
"string(./base/@node)",
- 
&job->data.commit.base,
- ctxt);
-if (virXPathNode("./deleteCommittedImages", ctxt))
-job->data.commit.deleteCommittedImages = true;
-if (!job->data.commit.top ||
-!job->data.commit.base)
+if (qemuDomainObjPrivateXMLParseBlockjobDataCommit(job, ctxt) < 0)
 goto broken;
+
 break;

 case QEMU_BLOCKJOB_TYPE_CREATE:
-- 
2.24.1

[PATCH 13/13] qemu: blockjob: Re-enable bitmaps after failed block-copy

[Peter Krempa]

If a block-copy fails we should at least re-enable the bitmaps so that
the operation can be re-tried.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 42 ++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 904c4c3f46..5ac1cd8856 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1341,6 +1341,40 @@ 
qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
 }


+static void
+qemuBlockJobProcessEventFailedCommitCommon(virDomainObjPtr vm,
+   qemuBlockJobDataPtr job,
+   qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+g_autoptr(virJSONValue) actions = virJSONValueNewArray();
+char **disabledBitmaps = job->data.commit.disabledBitmapsBase;
+
+if (!disabledBitmaps || !*disabledBitmaps)
+return;
+
+for (; *disabledBitmaps; disabledBitmaps++) {
+qemuMonitorTransactionBitmapEnable(actions,
+   job->data.commit.base->nodeformat,
+   *disabledBitmaps);
+}
+
+if (qemuBlockReopenReadWrite(vm, job->data.commit.base, asyncJob) < 0)
+return;
+
+if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
+return;
+
+qemuMonitorTransaction(priv->mon, &actions);
+
+if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)
+return;
+
+if (qemuBlockReopenReadOnly(vm, job->data.commit.base, asyncJob) < 0)
+return;
+}
+
+
 static void
 qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
@@ -1448,13 +1482,17 @@ 
qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
 case QEMU_BLOCKJOB_TYPE_COMMIT:
 if (success)
 qemuBlockJobProcessEventCompletedCommit(driver, vm, job, asyncJob);
+else
+qemuBlockJobProcessEventFailedCommitCommon(vm, job, asyncJob);
 break;

 case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-if (success)
+if (success) {
 qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, 
asyncJob);
-else
+} else {
 qemuBlockJobProcessEventFailedActiveCommit(driver, vm, job);
+qemuBlockJobProcessEventFailedCommitCommon(vm, job, asyncJob);
+}
 break;

 case QEMU_BLOCKJOB_TYPE_CREATE:
-- 
2.24.1

[PATCH 11/13] qemuDomainBlockPivot: Handle merging of bitmaps when pivoting an active block-commit

[Peter Krempa]

Active layer block commit makes the 'base' image the new top image of
the disk after it finishes. This means that all bitmap operations need
to be handled prior to this happening as we'd lose writes otherwise.

The ideal place is to handle it when pivoting to the new image as only
guest-writes would be happening after this point.

Use qemuBlockBitmapsHandleCommitFinish to calculate the merging
transaction.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85612f3281..bc3e495064 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17288,6 +17288,21 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 break;

 case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+if (blockdev &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) {
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, 
QEMU_ASYNC_JOB_NONE)))
+return -1;
+
+if (qemuBlockBitmapsHandleCommitFinish(job->data.commit.top,
+   job->data.commit.base,
+   blockNamedNodeData,
+   &actions,
+   
job->data.commit.disabledBitmapsBase) < 0)
+return -1;
+}
+
 break;
 }

-- 
2.24.1

[PATCH 09/13] qemuBlockJobDiskNewCommit: Propagate 'disabledBitmapsBase'

[Peter Krempa]

Add an argument to qemuBlockJobDiskNewCommit to propagate the list of
disabled bitmaps into the job data structure.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 2 ++
 src/qemu/qemu_blockjob.h | 1 +
 src/qemu/qemu_driver.c   | 2 +-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 71df0d1ab2..44c0dc2c16 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -285,6 +285,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
   virStorageSourcePtr topparent,
   virStorageSourcePtr top,
   virStorageSourcePtr base,
+  char ***disabledBitmapsBase,
   bool delete_imgs,
   unsigned int jobflags)
 {
@@ -310,6 +311,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
 job->data.commit.top = top;
 job->data.commit.base = base;
 job->data.commit.deleteCommittedImages = delete_imgs;
+job->data.commit.disabledBitmapsBase = 
g_steal_pointer(disabledBitmapsBase);
 job->jobflags = jobflags;

 if (qemuBlockJobRegister(job, vm, disk, true) < 0)
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index e2e28ca4d3..9264c70217 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -187,6 +187,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
   virStorageSourcePtr topparent,
   virStorageSourcePtr top,
   virStorageSourcePtr base,
+  char ***disabledBitmapsBase,
   bool delete_imgs,
   unsigned int jobflags);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 79ce518c5b..ef1314835e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18544,7 +18544,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
 goto endjob;

 if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
-  baseSource,
+  baseSource, NULL,
   flags & 
VIR_DOMAIN_BLOCK_COMMIT_DELETE,
   flags)))
 goto endjob;
-- 
2.24.1

[PATCH 10/13] qemuDomainBlockCommit: Handle bitmaps on start of commit

[Peter Krempa]

On start of the commit job, we need to disable any active bitmap in the
base. Use qemuBlockBitmapsHandleCommitStart to calculate which and call
the appropriate QMP APIs. We use blockdev-reopen to make the 'base'
writable to disable the bitmaps.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 44 +-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ef1314835e..85612f3281 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18395,6 +18395,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
 const char *nodebase = NULL;
 bool persistjob = false;
 bool blockdev = false;
+g_autoptr(virJSONValue) bitmapDisableActions = NULL;
+VIR_AUTOSTRINGLIST bitmapDisableList = NULL;

 virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
   VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
@@ -18543,8 +18545,30 @@ qemuDomainBlockCommit(virDomainPtr dom,
  qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, 
false) < 0))
 goto endjob;

+if (blockdev &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP)) {
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, 
QEMU_ASYNC_JOB_NONE)))
+goto endjob;
+
+if (qemuBlockBitmapsHandleCommitStart(topSource, baseSource,
+  blockNamedNodeData,
+  &bitmapDisableActions,
+  &bitmapDisableList) < 0)
+goto endjob;
+
+/* if we don't have terminator on 'base' we can't reopen it */
+if (bitmapDisableActions && !baseSource->backingStore) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("can't handle bitmaps on unterminated backing 
image '%s'"),
+   base);
+goto endjob;
+}
+}
+
 if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
-  baseSource, NULL,
+  baseSource, &bitmapDisableList,
   flags & 
VIR_DOMAIN_BLOCK_COMMIT_DELETE,
   flags)))
 goto endjob;
@@ -18566,6 +18590,24 @@ qemuDomainBlockCommit(virDomainPtr dom,
 if (!backingPath && top_parent &&
 !(backingPath = qemuBlockGetBackingStoreString(baseSource)))
 goto endjob;
+
+if (bitmapDisableActions) {
+int rc;
+
+if (qemuBlockReopenReadWrite(vm, baseSource, QEMU_ASYNC_JOB_NONE) 
< 0)
+goto endjob;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorTransaction(priv->mon, &bitmapDisableActions);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto endjob;
+
+if (qemuBlockReopenReadOnly(vm, baseSource, QEMU_ASYNC_JOB_NONE) < 
0)
+goto endjob;
+
+if (rc < 0)
+goto endjob;
+}
 } else {
 device = job->name;
 }
-- 
2.24.1

[PATCH 08/13] qemublocktest: Add tests of broken bitmap chain handling during block-commit

[Peter Krempa]

Use the 'snapshots-synthetic-broken' test data for block-commit.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c | 13 
 .../snapshots-synthetic-broken-1-2| 30 
 .../snapshots-synthetic-broken-1-3| 66 
 .../snapshots-synthetic-broken-1-4| 76 +++
 .../snapshots-synthetic-broken-1-5| 76 +++
 .../snapshots-synthetic-broken-2-3| 43 +++
 .../snapshots-synthetic-broken-2-4| 53 +
 .../snapshots-synthetic-broken-2-5| 53 +
 .../snapshots-synthetic-broken-3-4| 30 
 .../snapshots-synthetic-broken-3-5| 30 
 .../snapshots-synthetic-broken-4-5| 20 +
 11 files changed, 490 insertions(+)
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-3
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-4
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-5
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-3
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-4
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-2-5
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-3-4
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-3-5
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-4-5

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 5eb38c3981..b782e7969d 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1305,6 +1305,19 @@ mymain(void)

 TEST_BITMAP_BLOCKCOMMIT("snapshots-4-5", 4, 5, "snapshots");

+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-1-2", 1, 2, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-1-3", 1, 3, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-1-4", 1, 4, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-1-5", 1, 5, 
"snapshots-synthetic-broken");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-2-3", 2, 3, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-2-4", 2, 4, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-2-5", 2, 5, 
"snapshots-synthetic-broken");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-3-4", 3, 4, 
"snapshots-synthetic-broken");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-3-5", 3, 5, 
"snapshots-synthetic-broken");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-synthetic-broken-4-5", 4, 5, 
"snapshots-synthetic-broken");

  cleanup:
 virHashFree(diskxmljsondata.schema);
diff --git 
a/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2 
b/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2
new file mode 100644
index 00..ab269d9256
--- /dev/null
+++ b/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2
@@ -0,0 +1,30 @@
+pre job bitmap disable:
+[
+
+]
+merge bitmpas:
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "current",
+  "persistent": true,
+  "disabled": false,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "current",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-3 
b/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-3
new file mode 100644
index 00..6eb14f927a
--- /dev/null
+++ b/tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-3
@@ -0,0 +1,66 @@
+pre job bitmap disable:
+[
+  {
+"type": "block-dirty-bitmap-disable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  }
+]
+merge bitmpas:
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  },
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "current",
+  "persistent": true,
+  "disabled": false,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "current",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+   

[PATCH 07/13] qemublocktest: Add more tests for block-commit bitmap handling with snapshots

[Peter Krempa]

Test handling of more complex cases of merging bitmaps accross
snapshots.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  15 ++
 .../bitmapblockcommit/snapshots-1-2   |  49 +++
 .../bitmapblockcommit/snapshots-1-3   |  76 ++
 .../bitmapblockcommit/snapshots-1-4   | 126 +
 .../bitmapblockcommit/snapshots-1-5   | 130 ++
 .../bitmapblockcommit/snapshots-2-3   |  49 +++
 .../bitmapblockcommit/snapshots-2-4   |  99 +
 .../bitmapblockcommit/snapshots-2-5   | 103 ++
 .../bitmapblockcommit/snapshots-3-4   |  72 ++
 .../bitmapblockcommit/snapshots-3-5   |  76 ++
 .../bitmapblockcommit/snapshots-4-4   |  11 ++
 .../bitmapblockcommit/snapshots-4-5   |  33 +
 12 files changed, 839 insertions(+)
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-5

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3662fee42a..5eb38c3981 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1291,6 +1291,21 @@ mymain(void)
 TEST_BITMAP_BLOCKCOMMIT("basic-1-3", 1, 3, "basic");
 TEST_BITMAP_BLOCKCOMMIT("basic-2-3", 2, 3, "basic");

+TEST_BITMAP_BLOCKCOMMIT("snapshots-1-2", 1, 2, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-1-3", 1, 3, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-1-4", 1, 4, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-1-5", 1, 5, "snapshots");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-2-3", 2, 3, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-2-4", 2, 4, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-2-5", 2, 5, "snapshots");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-3-4", 3, 4, "snapshots");
+TEST_BITMAP_BLOCKCOMMIT("snapshots-3-5", 3, 5, "snapshots");
+
+TEST_BITMAP_BLOCKCOMMIT("snapshots-4-5", 4, 5, "snapshots");
+
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree(&driver);
diff --git a/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2 
b/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2
new file mode 100644
index 00..0015b9ceb3
--- /dev/null
+++ b/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2
@@ -0,0 +1,49 @@
+pre job bitmap disable:
+[
+  {
+"type": "block-dirty-bitmap-disable",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "d"
+}
+  }
+]
+merge bitmpas:
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "current",
+  "persistent": true,
+  "disabled": false,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "current",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  ]
+}
+  }
+]
diff --git a/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3 
b/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3
new file mode 100644
index 00..5691b408aa
--- /dev/null
+++ b/tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3
@@ -0,0 +1,76 @@
+pre job bitmap disable:
+[
+  {
+"type": "block-dirty-bitmap-disable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "c"
+}
+  }
+]
+merge bitmpas:
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "current",
+  "persistent": true,
+  "disabled": false,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "current",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "tar

[PATCH 06/13] qemublocktest: Add tests for handling of bitmaps during block-commit

[Peter Krempa]

Add code for testing the two necessary steps of handling bitmaps during
block commit and excercise the code on the test data which we have for
bitmap handling.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  95 ++
 .../bitmapblockcommit/basic-1-2   | 119 ++
 .../bitmapblockcommit/basic-1-3   | 119 ++
 .../bitmapblockcommit/basic-2-3   |   2 +
 4 files changed, 335 insertions(+)
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-2
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-2-3

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index a6b6376c7d..3662fee42a 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -599,6 +599,21 @@ testQemuBackupIncrementalBitmapCalculateGetFakeChain(void)
 }


+static virStorageSourcePtr
+testQemuBitmapGetFakeChainEntry(virStorageSourcePtr src,
+size_t idx)
+{
+virStorageSourcePtr n;
+
+for (n = src; n; n = n->backingStore) {
+if (n->id == idx)
+return n;
+}
+
+return NULL;
+}
+
+
 typedef virDomainMomentDefPtr testMomentList;

 static void
@@ -853,6 +868,68 @@ testQemuBlockBitmapBlockcopy(const void *opaque)
 return virTestCompareToFile(actual, expectpath);
 }

+static const char *blockcommitPrefix = "qemublocktestdata/bitmapblockcommit/";
+
+struct testQemuBlockBitmapBlockcommitData {
+const char *name;
+virStorageSourcePtr top;
+virStorageSourcePtr base;
+virStorageSourcePtr chain;
+const char *nodedatafile;
+};
+
+
+static int
+testQemuBlockBitmapBlockcommit(const void *opaque)
+{
+const struct testQemuBlockBitmapBlockcommitData *data = opaque;
+
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_autoptr(virJSONValue) actionsDisable = NULL;
+g_autoptr(virJSONValue) actionsMerge = NULL;
+g_autoptr(virJSONValue) nodedatajson = NULL;
+g_autoptr(virHashTable) nodedata = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+VIR_AUTOSTRINGLIST bitmapsDisable = NULL;
+
+expectpath = g_strdup_printf("%s/%s%s", abs_srcdir,
+ blockcommitPrefix, data->name);
+
+if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, 
data->nodedatafile,
+ ".json", NULL)))
+return -1;
+
+if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {
+VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
+return -1;
+}
+
+if (qemuBlockBitmapsHandleCommitStart(data->top, data->base, nodedata,
+  &actionsDisable, &bitmapsDisable) < 
0)
+return -1;
+
+virBufferAddLit(&buf, "pre job bitmap disable:\n");
+
+if (actionsDisable &&
+virJSONValueToBuffer(actionsDisable, &buf, true) < 0)
+return -1;
+
+virBufferAddLit(&buf, "merge bitmpas:\n");
+
+if (qemuBlockBitmapsHandleCommitFinish(data->top, data->base, nodedata,
+   &actionsMerge, bitmapsDisable) < 0)
+return -1;
+
+if (actionsMerge &&
+virJSONValueToBuffer(actionsMerge, &buf, true) < 0)
+return -1;
+
+actual = virBufferContentAndReset(&buf);
+
+return virTestCompareToFile(actual, expectpath);
+}
+

 static int
 mymain(void)
@@ -866,6 +943,7 @@ mymain(void)
 struct testQemuCheckpointDeleteMergeData checkpointdeletedata;
 struct testQemuBlockBitmapValidateData blockbitmapvalidatedata;
 struct testQemuBlockBitmapBlockcopyData blockbitmapblockcopydata;
+struct testQemuBlockBitmapBlockcommitData blockbitmapblockcommitdata;
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 g_autoptr(virStorageSource) bitmapSourceChain = NULL;
@@ -1196,6 +1274,23 @@ mymain(void)
 TEST_BITMAP_BLOCKCOPY("snapshots-shallow", true, "snapshots");
 TEST_BITMAP_BLOCKCOPY("snapshots-deep", false, "snapshots");

+
+#define TEST_BITMAP_BLOCKCOMMIT(testname, topimg, baseimg, ndf) \
+do {\
+blockbitmapblockcommitdata.name = testname; \
+blockbitmapblockcommitdata.top = 
testQemuBitmapGetFakeChainEntry(bitmapSourceChain, topimg); \
+blockbitmapblockcommitdata.base = 
testQemuBitmapGetFakeChainEntry(bitmapSourceChain, baseimg); \
+blockbitmapblockcommitdata.nodedatafile = ndf; \
+if (virTestRun("bitmap block commit " testname, \
+   testQemuBlockBitmapBlockcommit, \
+   &blockbitmapblockcommitdata) < 0) \
+ret = -1; \
+} while (0)
+
+TEST_BITMAP_BLOCKCOMMIT("basic-1-2", 1, 2, "basic");
+TEST_BITMAP_BLOCKCOMMIT("basic-1-3", 1, 3, "basic");
+TEST_BITMAP_BLOCKCOMMIT("basic-2-3", 2, 3, "basic");
+
  cleanup:
 virHashFree(diskxmljsondata.schema);

[PATCH 01/13] qemu: domain: Extract formatting of 'commit' blockjob data into a function

[Peter Krempa]

I'll be adding more fields to care about so splitting the code out will
be better long-term.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 33c2158eb5..71d0a400cc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2530,6 +2530,24 @@ 
qemuDomainObjPrivateXMLFormatBlockjobFormatSource(virBufferPtr buf,
 }


+static void
+qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job,
+  virBufferPtr buf)
+{
+if (job->data.commit.base)
+virBufferAsprintf(buf, "\n", 
job->data.commit.base->nodeformat);
+
+if (job->data.commit.top)
+virBufferAsprintf(buf, "\n", 
job->data.commit.top->nodeformat);
+
+if (job->data.commit.topparent)
+virBufferAsprintf(buf, "\n", 
job->data.commit.topparent->nodeformat);
+
+if (job->data.commit.deleteCommittedImages)
+virBufferAddLit(buf, "\n");
+}
+
+
 static int
 qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload,
   const void *name G_GNUC_UNUSED,
@@ -2589,14 +2607,7 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void 
*payload,

 case QEMU_BLOCKJOB_TYPE_COMMIT:
 case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-if (job->data.commit.base)
-virBufferAsprintf(&childBuf, "\n", 
job->data.commit.base->nodeformat);
-if (job->data.commit.top)
-virBufferAsprintf(&childBuf, "\n", 
job->data.commit.top->nodeformat);
-if (job->data.commit.topparent)
-virBufferAsprintf(&childBuf, "\n", 
job->data.commit.topparent->nodeformat);
-if (job->data.commit.deleteCommittedImages)
-virBufferAddLit(&childBuf, "\n");
+qemuDomainPrivateBlockJobFormatCommit(job, &childBuf);
 break;

 case QEMU_BLOCKJOB_TYPE_CREATE:
-- 
2.24.1

[PATCH 04/13] qemublocktest: Fix and optimize fake image chain

[Peter Krempa]

Set the 'id' field of the backing chain properly so that we can look
up images and initialize 6 images instead of 10 as we don't use more
currently.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 7b7948d4c6..a6b6376c7d 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -570,6 +570,7 @@ testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t 
idx)
if (!(ret = virStorageSourceNew()))
abort();

+   ret->id = idx;
ret->type = VIR_STORAGE_TYPE_FILE;
ret->format = VIR_STORAGE_FILE_QCOW2;
ret->path = g_strdup_printf("/image%zu", idx);
@@ -589,7 +590,7 @@ testQemuBackupIncrementalBitmapCalculateGetFakeChain(void)

 n = ret = testQemuBackupIncrementalBitmapCalculateGetFakeImage(1);

-for (i = 2; i < 10; i++) {
+for (i = 2; i < 6; i++) {
 n->backingStore = 
testQemuBackupIncrementalBitmapCalculateGetFakeImage(i);
 n = n->backingStore;
 }
-- 
2.24.1

[PATCH 00/13] incremental backup: Handle bitmaps during block-commit

[Peter Krempa]

This implements handling of bitmaps during block-commit so that
incremental backups are not broken after commit.

Note that this applies on top of

https://www.redhat.com/archives/libvir-list/2020-February/msg01125.html

or can be fetched at

 git fetch https://gitlab.com/pipo.sk/libvirt.git reopen-impl

 https://gitlab.com/pipo.sk/libvirt/-/commits/reopen-impl

It uses blockdev-reopen so it's required to use that series. As noted
the branch mentioned above the series contains patches to enable
incremental backup and blockdev-reopen and depends on the following qemu
patches:

https://lists.gnu.org/archive/html/qemu-block/2020-02/msg01423.html
https://lists.gnu.org/archive/html/qemu-block/2020-02/msg01467.html

 git fetch https://gitlab.com/pipo.sk/qemu.git bitmap-reopen

 https://gitlab.com/pipo.sk/qemu/-/commits/bitmap-reopen

Peter Krempa (13):
  qemu: domain: Extract formatting of 'commit' blockjob data into a
function
  qemu: domain: Extract parsing of 'commit' blockjob data into a
function
  qemu: blockjob: Store list of bitmaps disabled prior to commit
  qemublocktest: Fix and optimize fake image chain
  qemu: block: Implement helpers for dealing with bitmaps during block
commit
  qemublocktest: Add tests for handling of bitmaps during block-commit
  qemublocktest: Add more tests for block-commit bitmap handling with
snapshots
  qemublocktest: Add tests of broken bitmap chain handling during
block-commit
  qemuBlockJobDiskNewCommit: Propagate 'disabledBitmapsBase'
  qemuDomainBlockCommit: Handle bitmaps on start of commit
  qemuDomainBlockPivot: Handle merging of bitmaps when pivoting an
active block-commit
  qemu: blockjob: Handle bitmaps after finish of normal block-commit
  qemu: blockjob: Re-enable bitmaps after failed block-copy

 src/qemu/qemu_block.c | 217 ++
 src/qemu/qemu_block.h |  14 ++
 src/qemu/qemu_blockjob.c  |  94 +++-
 src/qemu/qemu_blockjob.h  |   3 +
 src/qemu/qemu_domain.c| 110 ++---
 src/qemu/qemu_driver.c|  59 -
 tests/qemublocktest.c | 126 +-
 .../bitmapblockcommit/basic-1-2   | 119 ++
 .../bitmapblockcommit/basic-1-3   | 119 ++
 .../bitmapblockcommit/basic-2-3   |   2 +
 .../bitmapblockcommit/snapshots-1-2   |  49 
 .../bitmapblockcommit/snapshots-1-3   |  76 ++
 .../bitmapblockcommit/snapshots-1-4   | 126 ++
 .../bitmapblockcommit/snapshots-1-5   | 130 +++
 .../bitmapblockcommit/snapshots-2-3   |  49 
 .../bitmapblockcommit/snapshots-2-4   |  99 
 .../bitmapblockcommit/snapshots-2-5   | 103 +
 .../bitmapblockcommit/snapshots-3-4   |  72 ++
 .../bitmapblockcommit/snapshots-3-5   |  76 ++
 .../bitmapblockcommit/snapshots-4-4   |  11 +
 .../bitmapblockcommit/snapshots-4-5   |  33 +++
 .../snapshots-synthetic-broken-1-2|  30 +++
 .../snapshots-synthetic-broken-1-3|  66 ++
 .../snapshots-synthetic-broken-1-4|  76 ++
 .../snapshots-synthetic-broken-1-5|  76 ++
 .../snapshots-synthetic-broken-2-3|  43 
 .../snapshots-synthetic-broken-2-4|  53 +
 .../snapshots-synthetic-broken-2-5|  53 +
 .../snapshots-synthetic-broken-3-4|  30 +++
 .../snapshots-synthetic-broken-3-5|  30 +++
 .../snapshots-synthetic-broken-4-5|  20 ++
 .../blockjob-blockdev-in.xml  |   4 +
 32 files changed, 2135 insertions(+), 33 deletions(-)
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-2
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-1-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/basic-2-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-2
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-1-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-3
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-2-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-3-5
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-4
 create mode 100644 tests/qemublocktestdata/bitmapblockcommit/snapshots-4-5
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken-1-2
 create mode 100644 
tests/qemublocktestdata/bitmapblockcommit/snapshots-synthetic-broken

[PATCH 03/13] qemu: blockjob: Store list of bitmaps disabled prior to commit

[Peter Krempa]

Starting a commit job will require disabling bitmaps in the base image
so that they are not dirtied by the commit job. We need to store a list
of the bitmaps so that we can later re-enable them.

Add a field and status XML handling code as well as a test.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.h  |  2 ++
 src/qemu/qemu_domain.c| 26 +++
 .../blockjob-blockdev-in.xml  |  4 +++
 3 files changed, 32 insertions(+)

diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 72c7fa053e..e2e28ca4d3 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -88,6 +88,8 @@ struct _qemuBlockJobCommitData {
 virStorageSourcePtr top;
 virStorageSourcePtr base;
 bool deleteCommittedImages;
+char **disabledBitmapsBase; /* a NULL-terminated list of bitmap names which
+   were disabled in @base for the commit job */
 };


diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0a478d2080..c5f4b0ae7f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2534,6 +2534,9 @@ static void
 qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job,
   virBufferPtr buf)
 {
+g_auto(virBuffer) disabledBitmapsBuf = VIR_BUFFER_INIT_CHILD(buf);
+char **bitmaps = job->data.commit.disabledBitmapsBase;
+
 if (job->data.commit.base)
 virBufferAsprintf(buf, "\n", 
job->data.commit.base->nodeformat);

@@ -2545,6 +2548,11 @@ 
qemuDomainPrivateBlockJobFormatCommit(qemuBlockJobDataPtr job,

 if (job->data.commit.deleteCommittedImages)
 virBufferAddLit(buf, "\n");
+
+while (bitmaps && *bitmaps)
+virBufferEscapeString(&disabledBitmapsBuf, "\n", 
*(bitmaps++));
+
+virXMLFormatElement(buf, "disabledBaseBitmaps", NULL, &disabledBitmapsBuf);
 }


@@ -3157,6 +3165,9 @@ static int
 qemuDomainObjPrivateXMLParseBlockjobDataCommit(qemuBlockJobDataPtr job,
xmlXPathContextPtr ctxt)
 {
+g_autofree xmlNodePtr *nodes = NULL;
+ssize_t nnodes;
+
 if (job->type == QEMU_BLOCKJOB_TYPE_COMMIT) {
 qemuDomainObjPrivateXMLParseBlockjobNodename(job,
  
"string(./topparent/@node)",
@@ -3183,6 +3194,21 @@ 
qemuDomainObjPrivateXMLParseBlockjobDataCommit(qemuBlockJobDataPtr job,
 !job->data.commit.base)
 return -1;

+if ((nnodes = virXPathNodeSet("./disabledBaseBitmaps/bitmap", ctxt, 
&nodes)) > 0) {
+size_t i;
+
+job->data.commit.disabledBitmapsBase = g_new0(char *, nnodes + 1);
+
+for (i = 0; i < nnodes; i++) {
+char *tmp;
+
+if (!(tmp = virXMLPropString(nodes[i], "name")))
+return -1;
+
+job->data.commit.disabledBitmapsBase[i] = g_steal_pointer(&tmp);
+}
+}
+
 return 0;
 }

diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml 
b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
index ca6d110179..cc17a17ff4 100644
--- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
+++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
@@ -243,6 +243,10 @@
   
   
   
+  
+
+
+  
 
 
   
-- 
2.24.1

[PATCH 05/13] qemu: block: Implement helpers for dealing with bitmaps during block commit

[Peter Krempa]

qemuBlockBitmapsHandleCommitStart prepares for disabling the bitmaps in
the 'base' of the commit job so that the bitmaps are not dirtied by the
commit job. This needs to be done prior to start of the commit job.

qemuBlockBitmapsHandleCommitFinish then calculates the necessary merges
that agregate all the bitmaps between the commited images and write them
into the base bitmap.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 217 ++
 src/qemu/qemu_block.h |  14 +++
 2 files changed, 231 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 11df8eedd0..2467315563 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2962,6 +2962,223 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src,
 }


+/**
+ * @topsrc: virStorageSource representing 'top' of the job
+ * @basesrc: virStorageSource representing 'base' of the job
+ * @blockNamedNodeData: hash table containing data about bitmaps
+ * @actions: filled with arguments for a 'transaction' command
+ * @disabledBitmapsBase: filled with a list of bitmap names which must be 
disabled
+ *
+ * Prepares data for correctly hanlding bitmaps during the start of a commit
+ * job. The bitmaps in the 'base' image must be disabled, so that the writes
+ * done by the blockjob don't dirty the enabled bitmaps.
+ *
+ * @actions and @disabledBitmapsBase are untouched if no bitmaps need
+ * to be disabled.
+ */
+int
+qemuBlockBitmapsHandleCommitStart(virStorageSourcePtr top,
+  virStorageSourcePtr basesrc,
+  virHashTablePtr blockNamedNodeData,
+  virJSONValuePtr *actions,
+  char ***disabledBitmapsBase)
+{
+g_autoptr(virJSONValue) act = virJSONValueNewArray();
+VIR_AUTOSTRINGLIST bitmaplist = NULL;
+size_t curbitmapstr = 0;
+qemuBlockNamedNodeDataPtr entry;
+bool disable_bitmaps = true;
+size_t i;
+
+if (!(entry = virHashLookup(blockNamedNodeData, basesrc->nodeformat)))
+return 0;
+
+bitmaplist = g_new0(char *, entry->nbitmaps);
+
+for (i = 0; i < entry->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
+
+if (!bitmap->recording || bitmap->inconsistent ||
+!qemuBlockBitmapChainIsValid(top, bitmap->name, 
blockNamedNodeData))
+continue;
+
+disable_bitmaps = true;
+
+if (qemuMonitorTransactionBitmapDisable(act, basesrc->nodeformat,
+bitmap->name) < 0)
+return -1;
+
+bitmaplist[curbitmapstr++] = g_strdup(bitmap->name);
+}
+
+if (disable_bitmaps) {
+*actions = g_steal_pointer(&act);
+*disabledBitmapsBase = g_steal_pointer(&bitmaplist);
+}
+
+return 0;
+}
+
+
+struct qemuBlockBitmapsHandleCommitData {
+bool skip;
+bool create;
+bool enable;
+const char *basenode;
+virJSONValuePtr merge;
+unsigned long long granularity;
+bool persistent;
+};
+
+
+static void
+qemuBlockBitmapsHandleCommitDataFree(void *opaque)
+{
+struct qemuBlockBitmapsHandleCommitData *data = opaque;
+
+virJSONValueFree(data->merge);
+g_free(data);
+}
+
+
+static int
+qemuBlockBitmapsHandleCommitFinishIterate(void *payload,
+  const void *entryname,
+  void *opaque)
+{
+struct qemuBlockBitmapsHandleCommitData *data = payload;
+const char *bitmapname = entryname;
+virJSONValuePtr actions = opaque;
+
+if (data->skip)
+return 0;
+
+if (data->create) {
+if (qemuMonitorTransactionBitmapAdd(actions, data->basenode, 
bitmapname,
+data->persistent, !data-> enable,
+data->granularity) < 0)
+return -1;
+} else {
+if (data->enable &&
+qemuMonitorTransactionBitmapEnable(actions, data->basenode, 
bitmapname) < 0)
+return -1;
+}
+
+if (data->merge &&
+qemuMonitorTransactionBitmapMerge(actions, data->basenode, bitmapname,
+  &data->merge) < 0)
+return -1;
+
+return 0;
+}
+
+
+/**
+ * @topsrc: virStorageSource representing 'top' of the job
+ * @basesrc: virStorageSource representing 'base' of the job
+ * @blockNamedNodeData: hash table containing data about bitmaps
+ * @actions: filled with arguments for a 'transaction' command
+ * @disabledBitmapsBase: bitmap names which were disabled
+ *
+ * Calculates the necessary bitmap merges/additions/enablements to properly
+ * handle commit of images from 'top' into 'base'. The necessary operations
+ * in form of argumets of the 'transaction' command are filled into 'actions'
+ * if there is anything to do. Otherwise NULL is returned.
+ */
+int
+qemuBlockBitmapsHandleCommitFinish(virStorageSourceP

Re: Release of libvirt-6.1.0

[Daniel Veillard]

On Wed, Mar 04, 2020 at 09:42:38AM -0500, Cole Robinson wrote:
> I see the 6.1.0 commit in libvirt-python, but it seems the release
> .tar.gz wasn't uploaded:
> 
> https://libvirt.org/sources/python/

Dohhh, way too much multitasking going on I'm afraid :-\

  hopefully fixed,

   thanks for raising this up !

Daniel

> Thanks,
> Cole
> 
> On 3/4/20 4:40 AM, Daniel Veillard wrote:
> >   It's out, one day late, though I did the build and pushed the tag
> > in git yesterday, but I had forgotten to push the commit, thanks
> > Boris for raising this to me. So it's now available as signed tarball
> > and rpm source package at the usual place:
> > 
> >https://libvirt.org/sources/
> > 

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

Re: [libvirt PATCH] tests: do not include skipped tests in failedTests

[Peter Krempa]

On Wed, Mar 04, 2020 at 16:56:02 +0100, Ján Tomko wrote:
> We recognize three return values from tests:
> * OK-> 0
> * SKIP  -> EXIT_AM_SKIP
> * ERROR -> anything else
> 
> Also check for EXIT_AM_SKIP when building a bitmap of failed tests,
> otherwise the skipped tests would be printed in the suggested range
> of tests that shoud be re-run.
> 
> Reported-by: Peter Krempa 
> Signed-off-by: Ján Tomko 
> Fixes: cebb468ef5e82b8d4253e27ef70c67812cf93c5a

Reviewed-by: Peter Krempa 

[libvirt PATCH] tests: do not include skipped tests in failedTests

[Ján Tomko]

We recognize three return values from tests:
* OK-> 0
* SKIP  -> EXIT_AM_SKIP
* ERROR -> anything else

Also check for EXIT_AM_SKIP when building a bitmap of failed tests,
otherwise the skipped tests would be printed in the suggested range
of tests that shoud be re-run.

Reported-by: Peter Krempa 
Signed-off-by: Ján Tomko 
Fixes: cebb468ef5e82b8d4253e27ef70c67812cf93c5a
---
 tests/testutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index 2f6d65364e..5fd81b70a2 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -172,7 +172,7 @@ virTestRun(const char *title,
 fprintf(stderr, "!");
 }
 
-if (ret != 0)
+if (ret != 0 && ret != EXIT_AM_SKIP)
 ignore_value(virBitmapSetBitExpand(failedTests, testCounter));
 
 g_unsetenv("VIR_TEST_MOCK_TESTNAME");
-- 
2.24.1

Re: [PATCH 0/3] cleanups inspired by QMP-deprecation-introspection

[Daniel Henrique Barboza]

On 3/4/20 11:16 AM, Peter Krempa wrote:

I take a look at Markus' QMP deprecation-introspection series. While
that requires a lot more work I've fixed a few things along.

Peter Krempa (3):
   util: json: Convert virJSONValueNewObject() to g_new0
   qemuMonitorJSONSetMigrationParams: Refactor command construction and
 cleanup
   qemuhotplugtestcpus: Always use 'query-cpus-fast'



Reviewed-by: Daniel Henrique Barboza 

Re: Release of libvirt-6.1.0

[Cole Robinson]

I see the 6.1.0 commit in libvirt-python, but it seems the release
.tar.gz wasn't uploaded:

https://libvirt.org/sources/python/

Thanks,
Cole

On 3/4/20 4:40 AM, Daniel Veillard wrote:
>   It's out, one day late, though I did the build and pushed the tag
> in git yesterday, but I had forgotten to push the commit, thanks
> Boris for raising this to me. So it's now available as signed tarball
> and rpm source package at the usual place:
> 
>https://libvirt.org/sources/
> 

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

On Wed, Mar 04, 2020 at 03:13:41PM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2020 at 15:01:07 +0100, Pavel Mores wrote:
> > On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote:
> > > On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> > > > On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > > > > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > > > > A new command-line option --top was added to virsh's blockpull 
> > > > > > command.
> > > > > > Similar to how --base is handled, in presence of --top the 
> > > > > > operation is
> > > > > > implemented internally as a rebase.  To that end, a corresponding 
> > > > > > new 'top'
> > > > > > parameter was added to virDomainBlockRebase().
> > > > > > 
> > > > > > Signed-off-by: Pavel Mores 
> > > > > > ---
> > > > > >  include/libvirt/libvirt-domain.h |  4 ++--
> > > > > >  src/libvirt-domain.c |  5 +++--
> > > > > >  tools/virsh-domain.c | 14 +++---
> > > > > >  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> [...]
> 
> > > This is obviously a lot of work, thus we need to decide whether adding
> > > an old-school API is worth it in the inerim. Are there any real users
> > > who would benefit from the new pull semantics? blockpull is around for a
> > > long time already, but it seems that commit is favoured.
> > > 
> > > If there is no real demand though I'd probably prefer if we don't add
> > > any more block job APIs any more.
> > 
> > I'm not aware of any real demand for this, however as I stated in the cover
> > letter I believe I need full blockpull to deal with the bug I'm actually
> > working on, which is full support for external snapshots in snapshot-delete.
> 
> Deleting/reverting external snapshots needs to be done internally under
> the hood of virDomainRevertToSnapshot/virDomainSnapshotDelete so if you
> require use of the 'block-stream' command to an intermediate layer you
> don't actually need to expose it via virDomainBlockPull/Rebase to take
> advantage of it.

That's true.  I think this basically means I should drop patches 3 a 4 from
this series (I'll keep them locally as they give me a reasonably easy way to
test my changes), right?

> For reversion of external snapshots you'll probably need a new API
> anyways as you'll need to be able to specify a new set of disk images to
> hold the writes.

Okay, I'll deal with that in due time, I guess when I'm back to working on
snapshot-delete.

pvl

[PATCH 3/3] qemuhotplugtestcpus: Always use 'query-cpus-fast'

[Peter Krempa]

Use the new command in the test suite by asserting the capability
and adjusting test data to the correct field names as they changed
compared to 'query-cpus'.

Signed-off-by: Peter Krempa 
---
 tests/qemuhotplugtest.c   |   2 +
 .../ppc64-modern-bulk-monitor.json| 390 +-
 .../ppc64-modern-individual-monitor.json  | 196 -
 .../x86-modern-bulk-monitor.json  | 150 +++
 .../x86-modern-individual-add-monitor.json|  92 ++---
 .../x86-old-bulk-monitor.json | 150 +++
 6 files changed, 491 insertions(+), 489 deletions(-)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 9dd42664cd..e008c1bf0d 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -437,6 +437,8 @@ testQemuHotplugCpuPrepare(const char *test,

 priv = data->vm->privateData;

+virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST);
+
 if (data->modern)
 virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);

diff --git a/tests/qemuhotplugtestcpus/ppc64-modern-bulk-monitor.json 
b/tests/qemuhotplugtestcpus/ppc64-modern-bulk-monitor.json
index c139426b7b..068b45e3c0 100644
--- a/tests/qemuhotplugtestcpus/ppc64-modern-bulk-monitor.json
+++ b/tests/qemuhotplugtestcpus/ppc64-modern-bulk-monitor.json
@@ -35,81 +35,81 @@
   "id": "libvirt-15"
 }

-{"execute":"query-cpus","id":"libvirt-2"}
+{"execute":"query-cpus-fast","id":"libvirt-2"}

 {
   "return": [
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": true,
-  "CPU": 0,
+  "cpu-index": 0,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[0]",
+  "qom-path": "/machine/unattached/device[1]/thread[0]",
   "halted": false,
-  "thread_id": 21925
+  "thread-id": 21925
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 1,
+  "cpu-index": 1,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[1]",
+  "qom-path": "/machine/unattached/device[1]/thread[1]",
   "halted": false,
-  "thread_id": 21926
+  "thread-id": 21926
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 2,
+  "cpu-index": 2,
   "nip": -4611686018422360608,
-  "qom_path": "/machine/unattached/device[1]/thread[2]",
+  "qom-path": "/machine/unattached/device[1]/thread[2]",
   "halted": false,
-  "thread_id": 21927
+  "thread-id": 21927
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 3,
+  "cpu-index": 3,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[3]",
+  "qom-path": "/machine/unattached/device[1]/thread[3]",
   "halted": false,
-  "thread_id": 21928
+  "thread-id": 21928
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 4,
+  "cpu-index": 4,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[4]",
+  "qom-path": "/machine/unattached/device[1]/thread[4]",
   "halted": false,
-  "thread_id": 21930
+  "thread-id": 21930
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 5,
+  "cpu-index": 5,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[5]",
+  "qom-path": "/machine/unattached/device[1]/thread[5]",
   "halted": false,
-  "thread_id": 21931
+  "thread-id": 21931
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 6,
+  "cpu-index": 6,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[6]",
+  "qom-path": "/machine/unattached/device[1]/thread[6]",
   "halted": false,
-  "thread_id": 21932
+  "thread-id": 21932
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": false,
-  "CPU": 7,
+  "cpu-index": 7,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[7]",
+  "qom-path": "/machine/unattached/device[1]/thread[7]",
   "halted": false,
-  "thread_id": 21933
+  "thread-id": 21933
 }
   ],
   "id": "libvirt-12"
@@ -165,153 +165,153 @@
   "id": "libvirt-15"
 }

-{"execute":"query-cpus","id":"libvirt-5"}
+{"execute":"query-cpus-fast","id":"libvirt-5"}

 {
   "return": [
 {
-  "arch": "ppc",
+  "target": "ppc",
   "current": true,
-  "CPU": 0,
+  "cpu-index": 0,
   "nip": -4611686018426772172,
-  "qom_path": "/machine/unattached/device[1]/thread[0]",
+  "qom-path": "/machine/unattached/device[1]/thread[0]",
   "halted": false,
-  "thread_id": 21925
+  "thread-id": 21925
 },
 {
-  "arch": "ppc",
+  "target": "ppc",
   "cu

[PATCH 1/3] util: json: Convert virJSONValueNewObject() to g_new0

[Peter Krempa]

Make it obvious that the function always returns a valid pointer and fix
all callers.

Signed-off-by: Peter Krempa 
---
 src/admin/admin_server_dispatch.c |  3 ---
 src/locking/lock_daemon.c |  8 +---
 src/logging/log_daemon.c  |  8 +---
 src/logging/log_handler.c |  5 -
 src/qemu/qemu_agent.c |  8 ++--
 src/qemu/qemu_command.c   |  3 +--
 src/qemu/qemu_firmware.c  | 20 
 src/qemu/qemu_migration_params.c  |  8 ++--
 src/qemu/qemu_monitor_json.c  | 30 +++---
 src/qemu/qemu_vhost_user.c|  3 +--
 src/rpc/virnetdaemon.c| 10 ++
 src/rpc/virnetserver.c|  5 +
 src/rpc/virnetserverclient.c  |  3 ---
 src/rpc/virnetserverservice.c |  3 ---
 src/rpc/virnetsocket.c|  3 +--
 src/util/virjson.c| 21 -
 src/util/virlease.c   |  6 +-
 src/util/virlockspace.c   |  6 --
 src/util/virmacmap.c  |  5 +
 src/util/virnetdev.c  |  3 +--
 tests/qemumonitortestutils.c  |  8 ++--
 21 files changed, 32 insertions(+), 137 deletions(-)

diff --git a/src/admin/admin_server_dispatch.c 
b/src/admin/admin_server_dispatch.c
index 7b3bd697f3..b3da577995 100644
--- a/src/admin/admin_server_dispatch.c
+++ b/src/admin/admin_server_dispatch.c
@@ -119,9 +119,6 @@ virJSONValuePtr 
remoteAdmClientPreExecRestart(virNetServerClientPtr client G_GNU
 {
 virJSONValuePtr object = virJSONValueNewObject();

-if (!object)
-return NULL;
-
 /* No content to add at this time - just need empty object */

 return object;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 5e5a0c1089..245155206a 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -774,9 +774,6 @@ virLockDaemonClientPreExecRestart(virNetServerClientPtr 
client G_GNUC_UNUSED,
 virJSONValuePtr object = virJSONValueNewObject();
 char uuidstr[VIR_UUID_STRING_BUFLEN];

-if (!object)
-return NULL;
-
 if (virJSONValueObjectAppendBoolean(object, "restricted", 
priv->restricted) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot set restricted data in JSON document"));
@@ -923,16 +920,13 @@ virLockDaemonPreExecRestart(const char *state_file,
 virJSONValuePtr child;
 char *state = NULL;
 int ret = -1;
-virJSONValuePtr object;
+virJSONValuePtr object = virJSONValueNewObject();
 char *magic;
 virHashKeyValuePairPtr pairs = NULL, tmp;
 virJSONValuePtr lockspaces;

 VIR_DEBUG("Running pre-restart exec");

-if (!(object = virJSONValueNewObject()))
-goto cleanup;
-
 if (!(child = virNetDaemonPreExecRestart(dmn)))
 goto cleanup;

diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 772bbb805b..47377deb4f 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -597,9 +597,6 @@ virLogDaemonClientPreExecRestart(virNetServerClientPtr 
client G_GNUC_UNUSED,
 {
 virJSONValuePtr object = virJSONValueNewObject();

-if (!object)
-return NULL;
-
 return object;
 }

@@ -718,15 +715,12 @@ virLogDaemonPreExecRestart(const char *state_file,
 virJSONValuePtr child;
 char *state = NULL;
 int ret = -1;
-virJSONValuePtr object;
+virJSONValuePtr object = virJSONValueNewObject();
 char *magic;
 virHashKeyValuePairPtr pairs = NULL;

 VIR_DEBUG("Running pre-restart exec");

-if (!(object = virJSONValueNewObject()))
-goto cleanup;
-
 if (!(child = virNetDaemonPreExecRestart(dmn)))
 goto cleanup;

diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index 576d0d6921..87748d96d1 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -615,9 +615,6 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler)
 size_t i;
 char domuuid[VIR_UUID_STRING_BUFLEN];

-if (!ret)
-return NULL;
-
 files = virJSONValueNewArray();

 if (virJSONValueObjectAppend(ret, "files", files) < 0) {
@@ -627,8 +624,6 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler)

 for (i = 0; i < handler->nfiles; i++) {
 virJSONValuePtr file = virJSONValueNewObject();
-if (!file)
-goto error;

 if (virJSONValueArrayAppend(files, file) < 0) {
 virJSONValueFree(file);
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 23a775034f..cd25ef6cd3 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1166,15 +1166,12 @@ static virJSONValuePtr G_GNUC_NULL_TERMINATED
 qemuAgentMakeCommand(const char *cmdname,
  ...)
 {
-virJSONValuePtr obj;
+virJSONValuePtr obj = virJSONValueNewObject();
 virJSONValuePtr jargs = NULL;
 va_list args;

 va_start(args, cmdname);

-if (!(obj = virJSONValueNewObject()))
-goto error;
-
 if

[PATCH 2/3] qemuMonitorJSONSetMigrationParams: Refactor command construction and cleanup

[Peter Krempa]

qemuMonitorJSONMakeCommandInternal does the full command construction if
you pass in what would become the value of the 'arguments' key. Refactor
the open-coded implementation to use the helper and use modern cleanup
helpers at the same time.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor_json.c | 25 +++--
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5f0185d10b..3eac80c060 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3440,30 +3440,19 @@ int
 qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon,
   virJSONValuePtr params)
 {
-int ret = -1;
-virJSONValuePtr cmd = virJSONValueNewObject();
-virJSONValuePtr reply = NULL;
-
-if (virJSONValueObjectAppendString(cmd, "execute",
-   "migrate-set-parameters") < 0)
-goto cleanup;
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;

-if (virJSONValueObjectAppend(cmd, "arguments", params) < 0)
-goto cleanup;
-params = NULL;
+if (!(cmd = qemuMonitorJSONMakeCommandInternal("migrate-set-parameters", 
params)))
+return -1;

 if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
-goto cleanup;
+return -1;

 if (qemuMonitorJSONCheckError(cmd, reply) < 0)
-goto cleanup;
+return -1;

-ret = 0;
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(params);
-virJSONValueFree(reply);
-return ret;
+return 0;
 }


-- 
2.24.1

[PATCH 0/3] cleanups inspired by QMP-deprecation-introspection

[Peter Krempa]

I take a look at Markus' QMP deprecation-introspection series. While
that requires a lot more work I've fixed a few things along.

Peter Krempa (3):
  util: json: Convert virJSONValueNewObject() to g_new0
  qemuMonitorJSONSetMigrationParams: Refactor command construction and
cleanup
  qemuhotplugtestcpus: Always use 'query-cpus-fast'

 src/admin/admin_server_dispatch.c |   3 -
 src/locking/lock_daemon.c |   8 +-
 src/logging/log_daemon.c  |   8 +-
 src/logging/log_handler.c |   5 -
 src/qemu/qemu_agent.c |   8 +-
 src/qemu/qemu_command.c   |   3 +-
 src/qemu/qemu_firmware.c  |  20 +-
 src/qemu/qemu_migration_params.c  |   8 +-
 src/qemu/qemu_monitor_json.c  |  53 +--
 src/qemu/qemu_vhost_user.c|   3 +-
 src/rpc/virnetdaemon.c|  10 +-
 src/rpc/virnetserver.c|   5 +-
 src/rpc/virnetserverclient.c  |   3 -
 src/rpc/virnetserverservice.c |   3 -
 src/rpc/virnetsocket.c|   3 +-
 src/util/virjson.c|  21 +-
 src/util/virlease.c   |   6 +-
 src/util/virlockspace.c   |   6 -
 src/util/virmacmap.c  |   5 +-
 src/util/virnetdev.c  |   3 +-
 tests/qemuhotplugtest.c   |   2 +
 .../ppc64-modern-bulk-monitor.json| 390 +-
 .../ppc64-modern-individual-monitor.json  | 196 -
 .../x86-modern-bulk-monitor.json  | 150 +++
 .../x86-modern-individual-add-monitor.json|  92 ++---
 .../x86-old-bulk-monitor.json | 150 +++
 tests/qemumonitortestutils.c  |   8 +-
 27 files changed, 529 insertions(+), 643 deletions(-)

-- 
2.24.1

Re: Redfish API implementation for GSoC 2020.

[Cedric Bosdonnat]

Hi Rohan,

Thank you for your interest in the RedFish project idea. I'll will
answer you back the same thing I already said to the other students
that reached me.

In order to prepare a good application, throughly read the RedFish
specification and come up with a paper describing how you would map
this to the libvirt API. You can work on a few uses cases and describe
them.

The difficult part of the project is not to write the REST APIs, but
rather to come up with a meaningful binding between redfish and lbvirt.

--
Cedric

On Wed, 2020-03-04 at 18:30 +0530, ROHAN Gupta wrote:
> Hi everyone, I am Rohan Gupta a CSE undergraduate student at Shri Mata 
> Vaishno Devi University currently in the 4th semester. I would like to work 
> on the idea: Redfish API implementation for GSoC 2020. I find this 
> interesting as I have worked with REST apis in the past and have sound 
> knowledge about it. Here's the link to my GitHub profile: 
> https://github.com/Rohan-cod

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Peter Krempa]

On Wed, Mar 04, 2020 at 15:01:07 +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote:
> > On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> > > On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > > > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > > > A new command-line option --top was added to virsh's blockpull 
> > > > > command.
> > > > > Similar to how --base is handled, in presence of --top the operation 
> > > > > is
> > > > > implemented internally as a rebase.  To that end, a corresponding new 
> > > > > 'top'
> > > > > parameter was added to virDomainBlockRebase().
> > > > > 
> > > > > Signed-off-by: Pavel Mores 
> > > > > ---
> > > > >  include/libvirt/libvirt-domain.h |  4 ++--
> > > > >  src/libvirt-domain.c |  5 +++--
> > > > >  tools/virsh-domain.c | 14 +++---
> > > > >  3 files changed, 16 insertions(+), 7 deletions(-)

[...]

> > This is obviously a lot of work, thus we need to decide whether adding
> > an old-school API is worth it in the inerim. Are there any real users
> > who would benefit from the new pull semantics? blockpull is around for a
> > long time already, but it seems that commit is favoured.
> > 
> > If there is no real demand though I'd probably prefer if we don't add
> > any more block job APIs any more.
> 
> I'm not aware of any real demand for this, however as I stated in the cover
> letter I believe I need full blockpull to deal with the bug I'm actually
> working on, which is full support for external snapshots in snapshot-delete.

Deleting/reverting external snapshots needs to be done internally under
the hood of virDomainRevertToSnapshot/virDomainSnapshotDelete so if you
require use of the 'block-stream' command to an intermediate layer you
don't actually need to expose it via virDomainBlockPull/Rebase to take
advantage of it.

For reversion of external snapshots you'll probably need a new API
anyways as you'll need to be able to specify a new set of disk images to
hold the writes.

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> > On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > > A new command-line option --top was added to virsh's blockpull command.
> > > > Similar to how --base is handled, in presence of --top the operation is
> > > > implemented internally as a rebase.  To that end, a corresponding new 
> > > > 'top'
> > > > parameter was added to virDomainBlockRebase().
> > > > 
> > > > Signed-off-by: Pavel Mores 
> > > > ---
> > > >  include/libvirt/libvirt-domain.h |  4 ++--
> > > >  src/libvirt-domain.c |  5 +++--
> > > >  tools/virsh-domain.c | 14 +++---
> > > >  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> [...]
> 
> > > > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > > *disk,
> > > >   */
> > > >  int
> > > >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > > - const char *base, unsigned long bandwidth,
> > > > - unsigned int flags)
> > > > + const char *base, const char *top,
> > > > + unsigned long bandwidth, unsigned int flags)
> > > >  {
> > > >  virConnectPtr conn;
> > > 
> > > So this is one thing we can't do. This modifies the public API of
> > > libvirt and would cause software which is already compiled to pass wrong
> > > arguments to this function.
> > > 
> > > If the semantics can't be mapped to existing arguments of this function
> > > we'll need to add a new function in the first place.
> > 
> > Yes.  Seeing as the function takes just a bunch of primitive data types and 
> > a
> > virDomainPtr, I'm afraid I don't see much room for not changing the 
> > signature,
> > apart from arguably dubious tricks like stuffing both base and top to the
> > existing 'base' parameter and parsing the individual values out of it in the
> > function body.
> > 
> > Any convention or suggestion to help pick a good name for the new function?
> 
> If we are going to implement new APIs for blockjobs I'd prefer we take a
> step back and re-design the (block)-job APIs rather than add yet another
> oldschool one.
> 
> Similarly to our criticisms of qemu's apis which didn't preserve the job
> state after the job finished, which is now fixed I've heard exactly the
> same requests from at least oVirt developers requesting we do the same
> thing.
> 
> The new API(s) should thus return a job object (virDomainJob?) similarly
> to how the domain lookup APIs return them. This new object then can be
> used to refer to the job even after it has finished. Similarly to qemu
> we should also introduce provisions to auto-dismiss the job if the
> management APP doesn't wish to have to deal with it (and that probably
> should be default behaviour given how we approached it previously).
> 
> This will then obviously require support APIs for looking up/listing
> jobs, getting the state or configuration etc.
> 
> This is obviously a lot of work, thus we need to decide whether adding
> an old-school API is worth it in the inerim. Are there any real users
> who would benefit from the new pull semantics? blockpull is around for a
> long time already, but it seems that commit is favoured.
> 
> If there is no real demand though I'd probably prefer if we don't add
> any more block job APIs any more.

I'm not aware of any real demand for this, however as I stated in the cover
letter I believe I need full blockpull to deal with the bug I'm actually
working on, which is full support for external snapshots in snapshot-delete.

Considering what you said, the "dubious trick" I mentioned in the other branch
of this thread might be a bearable interim solution...

pvl

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Peter Krempa]

On Wed, Mar 04, 2020 at 14:55:30 +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 02:41:09PM +0100, Pavel Mores wrote:
> > On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > > A new command-line option --top was added to virsh's blockpull command.
> > > > Similar to how --base is handled, in presence of --top the operation is
> > > > implemented internally as a rebase.  To that end, a corresponding new 
> > > > 'top'
> > > > parameter was added to virDomainBlockRebase().
> > > > 
> > > > Signed-off-by: Pavel Mores 
> > > > ---
> > > >  include/libvirt/libvirt-domain.h |  4 ++--
> > > >  src/libvirt-domain.c |  5 +++--
> > > >  tools/virsh-domain.c | 14 +++---
> > > >  3 files changed, 16 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/include/libvirt/libvirt-domain.h 
> > > > b/include/libvirt/libvirt-domain.h
> > > > index b440818ec2..069d7f98ec 100644
> > > > --- a/include/libvirt/libvirt-domain.h
> > > > +++ b/include/libvirt/libvirt-domain.h
> > > > @@ -2560,8 +2560,8 @@ typedef enum {
> > > >  } virDomainBlockRebaseFlags;
> > > >  
> > > >  int virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > > - const char *base, unsigned long bandwidth,
> > > > - unsigned int flags);
> > > > + const char *base, const char *top,
> > > > + unsigned long bandwidth, unsigned int flags);
> > > >  
> > > >  /**
> > > >   * virDomainBlockCopyFlags:
> > > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > > > index 65813b68cc..1f9d1b5b84 100644
> > > > --- a/src/libvirt-domain.c
> > > > +++ b/src/libvirt-domain.c
> > > > @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > > *disk,
> > > >   * @disk: path to the block device, or device shorthand
> > > >   * @base: path to backing file to keep, or device shorthand,
> > > >   *or NULL for no backing file
> > > > + * @top: path to top file, or device shorthand, or NULL for no top
> > > >   * @bandwidth: (optional) specify bandwidth limit; flags determine the 
> > > > unit
> > > >   * @flags: bitwise-OR of virDomainBlockRebaseFlags
> > > >   *
> > > > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > > *disk,
> > > >   */
> > > >  int
> > > >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > > - const char *base, unsigned long bandwidth,
> > > > - unsigned int flags)
> > > > + const char *base, const char *top,
> > > > + unsigned long bandwidth, unsigned int flags)
> > > >  {
> > > >  virConnectPtr conn;
> > > 
> > > So this is one thing we can't do. This modifies the public API of
> > > libvirt and would cause software which is already compiled to pass wrong
> > > arguments to this function.
> > > 
> > > If the semantics can't be mapped to existing arguments of this function
> > > we'll need to add a new function in the first place.
> > 
> > Yes.  Seeing as the function takes just a bunch of primitive data types and 
> > a
> > virDomainPtr, I'm afraid I don't see much room for not changing the 
> > signature,
> > apart from arguably dubious tricks like stuffing both base and top to the
> > existing 'base' parameter and parsing the individual values out of it in the
> > function body.
> 
> Actually, on second thought, it might not be as dubious as I first thought.
> Certainly not as clean as just adding a parameter in general but depending on
> what the cost of adding a new API would be, reusing the existing 'base' param
> might be workable.

That's gross. Please don't do that.

> 
> Also, how about the RPC change, is that acceptable?

No, that's on same par as API.

> 
> Thanks,
> 
>   pvl
> 

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

On Wed, Mar 04, 2020 at 02:41:09PM +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > A new command-line option --top was added to virsh's blockpull command.
> > > Similar to how --base is handled, in presence of --top the operation is
> > > implemented internally as a rebase.  To that end, a corresponding new 
> > > 'top'
> > > parameter was added to virDomainBlockRebase().
> > > 
> > > Signed-off-by: Pavel Mores 
> > > ---
> > >  include/libvirt/libvirt-domain.h |  4 ++--
> > >  src/libvirt-domain.c |  5 +++--
> > >  tools/virsh-domain.c | 14 +++---
> > >  3 files changed, 16 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/include/libvirt/libvirt-domain.h 
> > > b/include/libvirt/libvirt-domain.h
> > > index b440818ec2..069d7f98ec 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -2560,8 +2560,8 @@ typedef enum {
> > >  } virDomainBlockRebaseFlags;
> > >  
> > >  int virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > - const char *base, unsigned long bandwidth,
> > > - unsigned int flags);
> > > + const char *base, const char *top,
> > > + unsigned long bandwidth, unsigned int flags);
> > >  
> > >  /**
> > >   * virDomainBlockCopyFlags:
> > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > > index 65813b68cc..1f9d1b5b84 100644
> > > --- a/src/libvirt-domain.c
> > > +++ b/src/libvirt-domain.c
> > > @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > *disk,
> > >   * @disk: path to the block device, or device shorthand
> > >   * @base: path to backing file to keep, or device shorthand,
> > >   *or NULL for no backing file
> > > + * @top: path to top file, or device shorthand, or NULL for no top
> > >   * @bandwidth: (optional) specify bandwidth limit; flags determine the 
> > > unit
> > >   * @flags: bitwise-OR of virDomainBlockRebaseFlags
> > >   *
> > > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > *disk,
> > >   */
> > >  int
> > >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > - const char *base, unsigned long bandwidth,
> > > - unsigned int flags)
> > > + const char *base, const char *top,
> > > + unsigned long bandwidth, unsigned int flags)
> > >  {
> > >  virConnectPtr conn;
> > 
> > So this is one thing we can't do. This modifies the public API of
> > libvirt and would cause software which is already compiled to pass wrong
> > arguments to this function.
> > 
> > If the semantics can't be mapped to existing arguments of this function
> > we'll need to add a new function in the first place.
> 
> Yes.  Seeing as the function takes just a bunch of primitive data types and a
> virDomainPtr, I'm afraid I don't see much room for not changing the signature,
> apart from arguably dubious tricks like stuffing both base and top to the
> existing 'base' parameter and parsing the individual values out of it in the
> function body.

Actually, on second thought, it might not be as dubious as I first thought.
Certainly not as clean as just adding a parameter in general but depending on
what the cost of adding a new API would be, reusing the existing 'base' param
might be workable.

Also, how about the RPC change, is that acceptable?

Thanks,

pvl

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Peter Krempa]

On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> > On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > > A new command-line option --top was added to virsh's blockpull command.
> > > Similar to how --base is handled, in presence of --top the operation is
> > > implemented internally as a rebase.  To that end, a corresponding new 
> > > 'top'
> > > parameter was added to virDomainBlockRebase().
> > > 
> > > Signed-off-by: Pavel Mores 
> > > ---
> > >  include/libvirt/libvirt-domain.h |  4 ++--
> > >  src/libvirt-domain.c |  5 +++--
> > >  tools/virsh-domain.c | 14 +++---
> > >  3 files changed, 16 insertions(+), 7 deletions(-)

[...]

> > > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > > *disk,
> > >   */
> > >  int
> > >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > > - const char *base, unsigned long bandwidth,
> > > - unsigned int flags)
> > > + const char *base, const char *top,
> > > + unsigned long bandwidth, unsigned int flags)
> > >  {
> > >  virConnectPtr conn;
> > 
> > So this is one thing we can't do. This modifies the public API of
> > libvirt and would cause software which is already compiled to pass wrong
> > arguments to this function.
> > 
> > If the semantics can't be mapped to existing arguments of this function
> > we'll need to add a new function in the first place.
> 
> Yes.  Seeing as the function takes just a bunch of primitive data types and a
> virDomainPtr, I'm afraid I don't see much room for not changing the signature,
> apart from arguably dubious tricks like stuffing both base and top to the
> existing 'base' parameter and parsing the individual values out of it in the
> function body.
> 
> Any convention or suggestion to help pick a good name for the new function?

If we are going to implement new APIs for blockjobs I'd prefer we take a
step back and re-design the (block)-job APIs rather than add yet another
oldschool one.

Similarly to our criticisms of qemu's apis which didn't preserve the job
state after the job finished, which is now fixed I've heard exactly the
same requests from at least oVirt developers requesting we do the same
thing.

The new API(s) should thus return a job object (virDomainJob?) similarly
to how the domain lookup APIs return them. This new object then can be
used to refer to the job even after it has finished. Similarly to qemu
we should also introduce provisions to auto-dismiss the job if the
management APP doesn't wish to have to deal with it (and that probably
should be default behaviour given how we approached it previously).

This will then obviously require support APIs for looking up/listing
jobs, getting the state or configuration etc.

This is obviously a lot of work, thus we need to decide whether adding
an old-school API is worth it in the inerim. Are there any real users
who would benefit from the new pull semantics? blockpull is around for a
long time already, but it seems that commit is favoured.

If there is no real demand though I'd probably prefer if we don't add
any more block job APIs any more.

Re: [PATCH v2 00/30] Configurable policy for handling deprecated interfaces

[Markus Armbruster]

Markus Armbruster  writes:

> Based-on: <20200227144531.24309-1-arm...@redhat.com>
>
> This series extends QMP introspection to cover deprecation.
> Additionally, new option -compat lets you configure what to do when
> deprecated interfaces get used.  This is intended for testing users of
> the management interfaces.  It is experimental.
>
> -compat deprecated-input= configures what to do when
> deprecated input is received.  Available policies:
>
> * accept: Accept deprecated commands and arguments (default)
> * reject: Reject them
> * crash: Crash
>
> -compat deprecated-output= configures what to do when
> deprecated output is sent.  Available output policies:
>
> * accept: Emit deprecated command results and events (default)
> * hide: Suppress them

Missing: hide in output of query-qmp-schema.  I can look into that.

> For now, -compat covers only deprecated syntactic aspects of QMP.  We
> may want to extend it to cover semantic aspects, CLI, and experimental
> features.
>
> PATCH 01-04: Documentation fixes
> PATCH 05-10: Test improvements
> PATCH 11-24: Add feature flags to remaining user-defined types and to
>struct members
> PATCH 25-26: New special feature 'deprecated', visible in
>introspection
> PATCH 27-30: New -compat to set policy for handling stuff marked with
>feature 'deprecated'
>
> Comparison to RFC (24 Oct 2019):
> * Cover arguments and results in addition to commands and events
> * Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the
>   response to a deprecated command" dropped
>
> See also last item of
> Subject: Minutes of KVM Forum BoF on deprecating stuff
> Date: Fri, 26 Oct 2018 16:03:51 +0200
> Message-ID: <87mur0ls8o@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > A new command-line option --top was added to virsh's blockpull command.
> > Similar to how --base is handled, in presence of --top the operation is
> > implemented internally as a rebase.  To that end, a corresponding new 'top'
> > parameter was added to virDomainBlockRebase().
> > 
> > Signed-off-by: Pavel Mores 
> > ---
> >  include/libvirt/libvirt-domain.h |  4 ++--
> >  src/libvirt-domain.c |  5 +++--
> >  tools/virsh-domain.c | 14 +++---
> >  3 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index b440818ec2..069d7f98ec 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -2560,8 +2560,8 @@ typedef enum {
> >  } virDomainBlockRebaseFlags;
> >  
> >  int virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > - const char *base, unsigned long bandwidth,
> > - unsigned int flags);
> > + const char *base, const char *top,
> > + unsigned long bandwidth, unsigned int flags);
> >  
> >  /**
> >   * virDomainBlockCopyFlags:
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 65813b68cc..1f9d1b5b84 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > *disk,
> >   * @disk: path to the block device, or device shorthand
> >   * @base: path to backing file to keep, or device shorthand,
> >   *or NULL for no backing file
> > + * @top: path to top file, or device shorthand, or NULL for no top
> >   * @bandwidth: (optional) specify bandwidth limit; flags determine the unit
> >   * @flags: bitwise-OR of virDomainBlockRebaseFlags
> >   *
> > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char 
> > *disk,
> >   */
> >  int
> >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > - const char *base, unsigned long bandwidth,
> > - unsigned int flags)
> > + const char *base, const char *top,
> > + unsigned long bandwidth, unsigned int flags)
> >  {
> >  virConnectPtr conn;
> 
> So this is one thing we can't do. This modifies the public API of
> libvirt and would cause software which is already compiled to pass wrong
> arguments to this function.
> 
> If the semantics can't be mapped to existing arguments of this function
> we'll need to add a new function in the first place.

Yes.  Seeing as the function takes just a bunch of primitive data types and a
virDomainPtr, I'm afraid I don't see much room for not changing the signature,
apart from arguably dubious tricks like stuffing both base and top to the
existing 'base' parameter and parsing the individual values out of it in the
function body.

Any convention or suggestion to help pick a good name for the new function?

Thanks,

pvl

Re: [PATCH V4 0/5] Introduce Advanced Watch Dog module

[Paolo Bonzini]

On 04/03/20 09:06, Zhang, Chen wrote:
>> Hi Eric and Paolo, Can you give some comments about this series?
>>
>>
> No news for a while...
> We already have some users(Cloud Service Provider) try to use is module in 
> their product.
> But they also need to follow the Qemu upstream code.

My main comment about this series is that it's not clear why it is
needed and how to use it.  The documentation includes a demo, but no
description of what is an awd_node, a notification_node and an
opt_script.  I can more or less understand the notification_node and
opt_script role from the documentation, but not entirely because, for
example, the two-host demo has hardcoded IP addresses without saying
which host is which IP address.

The documentation does not describe the protocol, which is absolutely
necessary, and does not describe _why_ the protocol was designed like
that.  Without such documentation it's not clear if, for example, the
watchdog protocol could be implemented as QMP commands (e.g.
start-watchdog, stop-watchdog, notify-watchdog).  Another possibility
could be to use the systemd watchdog protocol, which consists of
essentially three commands (WATCHDOG=1, WATCHDOG=trigger,
WATCHDOG_USEC=...) which are transmitted as datagrams.  Documentation is
important for reviewers to judge the merits of the protocol without (or
before) diving into the code.

In the demo, the opt_script mechanism is currently using the "human"
monitor as opposed to QMP.  The human monitor interface is not stable
and not meant for consumption by management interface.  It is not clear
if this is just a sample usage, and in practice the notification_node
would be outside of QEMU, or not.  In general I would prefer to have the
script as an optional feature, and report the triggering of the watchdog
via QMP events.

Paolo

Redfish API implementation for GSoC 2020.

[ROHAN Gupta]

Hi everyone, I am Rohan Gupta a CSE undergraduate student at Shri Mata
Vaishno Devi University currently in the 4th semester. I would like to work
on the idea: Redfish API implementation for GSoC 2020. I find this
interesting as I have worked with REST apis in the past and have sound
knowledge about it. Here's the link to my GitHub profile:
https://github.com/Rohan-cod

libvirt-devaddr: a new library for device address assignment

[Daniel P . Berrangé]

We've been doing alot of refactoring of code in recent times, and also
have plans for significant infrastructure changes. We still need to
spend time delivering interesting features to users / applications.
This mail is to introduce an idea for a solution to an specific
area applications have had long term pain with libvirt's current
"mechanism, not policy" approach - device addressing. This is a way
for us to show brand new ideas & approaches for what the libvirt
project can deliver in terms of management APIs.

To set expectations straight: I have written no code for this yet,
merely identified the gap & conceptual solution.


The device addressing problem
=

One of the key jobs libvirt does when processing a new domain XML
configuration is to assign addresses to all devices that are present.
This involves adding various device controllers (PCI bridges, PCI root
ports, IDE/SCSI buses, USB controllers, etc) if they are not already
present, and then assigning PCI, USB, IDE, SCSI, etc, addresses to each
device so they are associated with controllers. When libvirt spawns a
QEMU guest, it will pass full address information to QEMU.

Libvirt, as a general rule, aims to avoid defining and implementing
policy around expansion of guest configuration / defaults, however, it
is inescapable in the case of device addressing due to the need to
guarantee a stable hardware ABI to make live migration and save/restore
to disk work.  The policy that libvirt has implemented for device
addressing is, as much as possible, the same as the addressing scheme
QEMU would apply itself.

While libvirt succeeds in its goal of providing a stable hardware API,
the addressing scheme used is not well suited to all deployment
scenarios of QEMU. This is an inevitable result of having a specific
assignment policy implemented in libvirt which has to trade off mutually
incompatible use cases/goals.

When the libvirt addressing policy is not been sufficient, management
applications are forced to take on address assignment themselves,
which is a massive non-trivial job with many subtle problems to
consider.

Places where libvirt's addressing is insufficient for PCI include

 * Setting up multiple guest NUMA nodes and associating devices to
   specific nodes
 * Pre-emptive creation of extra PCIe root ports, to allow for later
   device hotplug on PCIe topologies
 * Determining whether to place a device on a PCI or PCIe bridge
 * Controlling whether a device is placed into a hotpluggable slot
 * Controlling whether a PCIe root port supports hotplug or not
 * Determining whether to places all devices on distinct slots or
   buses, vs grouping them all into functions on the same slot
 * Ability to expand the device addressing without being on the
   hypervisor host

Libvirt wishes to avoid implementing many different address assignment
policies. It also wishes to keep the domain XML as a representation
of the virtual hardware, not add a bunch of properties to it which
merely serve as tunable input parameters for device addressing
algorithms.

There is thus a dilemma here. Management applications increasingly
need fine grained control over device addressing, while libvirt
doesn't want to expose fine grained policy controls via the XML.


The new libvirt-devaddr API
===

The way out of this is to define a brand new virt management API
which tackles this specific problem in a way that addresses all the
problems mgmt apps have with device addressing and explicitly
provides a variety of policy impls with tunable behaviour.

By "new API", I actually mean an entirely new library, completely
distinct from libvirt.so, or anything else we've delivered so
far. The closest we've come to delivering something at this kind
of conceptual level, would be the abortive attempt we made with
"libvirt-builder" to deliver a policy-driven API instead of mechanism
based. This proposal is still quite different from that attempt.

At a high level

 * The new API is "libvirt-devaddr" - short for "libvirt device addressing"

 * As input it will take

   1. The guest CPU architecture and machine type
   2. A list of global tunables specifying desired behaviour of the
  address assignment policy
   3. A minimal list of devices needed in the virtual machine, with
  optional addresses and optional per-device tunables to override
  the global tunables

 * As output it will emit

   1. fully expanded list of devices needed in the virtual machine,
  with addressing information sufficient to ensure stable hardware ABI

Initially the API would implement something that behaves the same
way as libvirt's current address assignment API.

The intended usage would be

 * Mgmt application makes a minimal list of devices they want in
   their guest
 * List of devices is fed into libvirt-devaddr API
 * Mgmt application gets back a full list of devices & addresses
 * Mgmt application writes a libvirt XML doc using this full list

Re: Propose to extend the capability of arm cpu driver

[Andrea Bolognani]

On Wed, 2020-03-04 at 15:15 +0800, Zhenyu Zheng wrote:
> Hello Libvirt,
> 
> With more and more ARM servers on the market and increasing amount of
> users, we got reported alot that Libvirt cannot provide as much
> function on ARM platform as on other platforms(mainly x86), some basic
> founctions are missing, such as showing cpu model details and features
> using ``virsh capabilities``, comparing cpus etc.
> 
> So I would like to propose to extend the capability of ARM cpu driver.
> Since I'm quite new here, I searched through the mailing list and did
> not find much information on this, so I would like to know that will
> this be something that I could work on?

In general terms, any gaps between what is possible on x86 and what
is possible on ARM are considered bugs and anyone interested in
addressing them is very much welcome!

On the topic of CPUs specifically, the limitations have been known
for a long time but there are significant obstacles to solving them:

  a) QEMU does not have named models for ARM CPUs at this point,
 which makes comparing different guest CPUs basically impossible;

  b) the Linux kernel does not report information about eg. CPU
 frequencly consistently;

  c) historically, ARM server vendors have included incorrect or
 incomplete information in their products' ACPI and DMI tables.

b) is, I believe, mostly a consequence of c); the latter is also the
reason why libvirt can't really trust this information even if they
happen to be available on a particular machine.

Addressing a) is a long-term to do item for QEMU developers, and it
has seen some progress lately; b) can only be addressed by hardware
vendors.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Peter Krempa]

On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> A new command-line option --top was added to virsh's blockpull command.
> Similar to how --base is handled, in presence of --top the operation is
> implemented internally as a rebase.  To that end, a corresponding new 'top'
> parameter was added to virDomainBlockRebase().
> 
> Signed-off-by: Pavel Mores 
> ---
>  include/libvirt/libvirt-domain.h |  4 ++--
>  src/libvirt-domain.c |  5 +++--
>  tools/virsh-domain.c | 14 +++---
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index b440818ec2..069d7f98ec 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -2560,8 +2560,8 @@ typedef enum {
>  } virDomainBlockRebaseFlags;
>  
>  int virDomainBlockRebase(virDomainPtr dom, const char *disk,
> - const char *base, unsigned long bandwidth,
> - unsigned int flags);
> + const char *base, const char *top,
> + unsigned long bandwidth, unsigned int flags);
>  
>  /**
>   * virDomainBlockCopyFlags:
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 65813b68cc..1f9d1b5b84 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
>   * @disk: path to the block device, or device shorthand
>   * @base: path to backing file to keep, or device shorthand,
>   *or NULL for no backing file
> + * @top: path to top file, or device shorthand, or NULL for no top
>   * @bandwidth: (optional) specify bandwidth limit; flags determine the unit
>   * @flags: bitwise-OR of virDomainBlockRebaseFlags
>   *
> @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
>   */
>  int
>  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> - const char *base, unsigned long bandwidth,
> - unsigned int flags)
> + const char *base, const char *top,
> + unsigned long bandwidth, unsigned int flags)
>  {
>  virConnectPtr conn;

So this is one thing we can't do. This modifies the public API of
libvirt and would cause software which is already compiled to pass wrong
arguments to this function.

If the semantics can't be mapped to existing arguments of this function
we'll need to add a new function in the first place.

Re: [PATCH v2 00/30] Configurable policy for handling deprecated interfaces

[Peter Krempa]

On Tue, Mar 03, 2020 at 17:34:35 +0100, Markus Armbruster wrote:
> Based-on: <20200227144531.24309-1-arm...@redhat.com>
> 
> This series extends QMP introspection to cover deprecation.
> Additionally, new option -compat lets you configure what to do when
> deprecated interfaces get used.  This is intended for testing users of
> the management interfaces.  It is experimental.

I've quickly hacked up support for the 'deprecated' feature in libvirt's
QMP validator. I've found a few uses of deprecated commands but they
might very well be in code paths that are no longer invoked in modern
qemus:

Offenders from qemumonitorjsontest:

44) qemuMonitorJSONSetCPU ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'cpu-add': failed to validate arguments of 'cpu-add' against QAPI schema: 
ERROR: 'cpu-add' is deprecated
FAILED
46) qemuMonitorJSONChangeMedia... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'change': failed to validate arguments of 'change' against QAPI schema: ERROR: 
'change' is deprecated
FAILED
49) qemuMonitorJSONSetMigrationSpeed  ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'migrate_set_speed': failed to validate arguments of 'migrate_set_speed' 
against QAPI schema: ERROR: 'migrate_set_speed' is deprecated
FAILED
50) qemuMonitorJSONSetMigrationDowntime   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'migrate_set_downtime': failed to validate arguments of 'migrate_set_downtime' 
against QAPI schema: ERROR: 'migrate_set_downtime' is deprecated
FAILED
77) qemuMonitorJSONGetMigrationCacheSize  ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-migrate-cache-size': failed to validate arguments of 
'query-migrate-cache-size' against QAPI schema: ERROR: 
'query-migrate-cache-size' is deprecated
FAILED
83) qemuMonitorJSONQueryCPUs  ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
93) GetCPUInfo(x86-basic-pluggable)   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
94) GetCPUInfo(x86-full)  ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
95) GetCPUInfo(x86-node-full) ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
97) GetCPUInfo(ppc64-basic)   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
98) GetCPUInfo(ppc64-hotplug-1)   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
99) GetCPUInfo(ppc64-hotplug-2)   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
100) GetCPUInfo(ppc64-hotplug-4)   ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED
101) GetCPUInfo(ppc64-no-threads)  ... 
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 
'query-cpus': failed to validate arguments of 'query-cpus' against QAPI schema: 
ERROR: 'query-cpus' is deprecated
FAILED

Here all commands are tested just for legacy reasons. query-cpus-fast is
used on any live codebase, cpu-add is no longer used, change is not used
with -blockdev. I'm not sure about the migration parameter APIs but I
didn't check either.

The above shows that we can't enable this without thinking even in our
test-suite.

I'll try to come up with a solution where we can enable the reporting of
use of deprecated commands through /etc/qemu.conf so tha

[libvirt PATCH 4/6] qemu: block: blockpull param 'top' support in driver and RPC

[Pavel Mores]

Rebase implementation in QEMU driver extended with a new 'top' parameter.
Support for 'top' added to RPC's remote_domain_block_rebase_args structure.

Signed-off-by: Pavel Mores 
---
 src/driver-hypervisor.h  | 1 +
 src/libvirt-domain.c | 2 +-
 src/qemu/qemu_driver.c   | 3 ++-
 src/remote/remote_protocol.x | 1 +
 src/remote_protocol-structs  | 1 +
 5 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index bce023017d..51a25c41e3 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1053,6 +1053,7 @@ typedef int
 (*virDrvDomainBlockRebase)(virDomainPtr dom,
const char *path,
const char *base,
+   const char *top,
unsigned long bandwidth,
unsigned int flags);
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 1f9d1b5b84..33e307cb26 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -10287,7 +10287,7 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
 
 if (conn->driver->domainBlockRebase) {
 int ret;
-ret = conn->driver->domainBlockRebase(dom, disk, base, bandwidth,
+ret = conn->driver->domainBlockRebase(dom, disk, base, top, bandwidth,
   flags);
 if (ret < 0)
 goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7970c913f3..b3f97bf593 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18143,7 +18143,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 
 static int
 qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
-  unsigned long bandwidth, unsigned int flags)
+  const char *top, unsigned long bandwidth,
+  unsigned int flags)
 {
 virDomainObjPtr vm;
 int ret = -1;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d4393680e9..5f5a921057 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1414,6 +1414,7 @@ struct remote_domain_block_rebase_args {
 remote_nonnull_domain dom;
 remote_nonnull_string path;
 remote_string base;
+remote_string top;
 unsigned hyper bandwidth;
 unsigned int flags;
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index bae0f0b545..b317ec9102 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -992,6 +992,7 @@ struct remote_domain_block_rebase_args {
 remote_nonnull_domain  dom;
 remote_nonnull_string  path;
 remote_string  base;
+remote_string  top;
 uint64_t   bandwidth;
 u_int  flags;
 };
-- 
2.24.1

[libvirt PATCH 6/6] qemu: block: extend pull job completion with 'top' handling

[Pavel Mores]

'top' handling basically requires two changes: besides the obvious
adjustments to the code that fixes our backing chains (for both
active and inactive configuration) after a pull, we need to make
the top image read-only again if it's not identical with the
active layer (it had to be made writable before submitting QMP
block-stream to allow QEMU to fix the backing chain in top's
qcow2).

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_blockjob.c | 33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index e19a2ad76b..2e53821d43 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -962,6 +962,7 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr 
driver,
 virDomainDiskDefPtr cfgdisk = NULL;
 virStorageSourcePtr cfgbase = NULL;
 virStorageSourcePtr cfgbaseparent = NULL;
+virStorageSourcePtr cfgtop = NULL;
 virStorageSourcePtr n;
 virStorageSourcePtr tmp;
 
@@ -994,16 +995,40 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr 
driver,
 }
 }
 
-tmp = job->disk->src->backingStore;
-job->disk->src->backingStore = job->data.pull.base;
+if (job->data.pull.top) {
+cfgtop = cfgdisk->src->backingStore;
+for (n = job->disk->src->backingStore; n && n != job->data.pull.top; n 
= n->backingStore) {
+if (cfgtop)
+cfgtop = cfgtop->backingStore;
+}
+}
+
+if (job->data.pull.top && job->data.pull.top != job->disk->src) {
+if (qemuDomainStorageSourceAccessAllow(driver, vm, job->data.pull.top, 
true, false) < 0) {
+virReportError(VIR_ERR_ACCESS_DENIED, "can't reset 'top' to 
read-only");
+}
+}
+
+if (job->data.pull.top) {
+tmp = job->data.pull.top->backingStore;
+job->data.pull.top->backingStore = job->data.pull.base;
+} else {
+tmp = job->disk->src->backingStore;
+job->disk->src->backingStore = job->data.pull.base;
+}
 if (baseparent)
 baseparent->backingStore = NULL;
 qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, tmp);
 virObjectUnref(tmp);
 
 if (cfgdisk) {
-tmp = cfgdisk->src->backingStore;
-cfgdisk->src->backingStore = cfgbase;
+if (job->data.pull.top) {
+tmp = cfgtop->backingStore;
+cfgtop->backingStore = cfgbase;
+} else {
+tmp = cfgdisk->src->backingStore;
+cfgdisk->src->backingStore = cfgbase;
+}
 if (cfgbaseparent)
 cfgbaseparent->backingStore = NULL;
 virObjectUnref(tmp);
-- 
2.24.1

[libvirt PATCH 1/6] qemu: block: add 'top' parameter to qemuDomainBlockPullCommon()

[Pavel Mores]

qemuDomainBlockPullCommon() is one of the functions in block pull
implementation that have to support 'top' to make block pull as
a whole to support it.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9a62684f0..81cca360e0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17311,6 +17311,7 @@ static int
 qemuDomainBlockPullCommon(virDomainObjPtr vm,
   const char *path,
   const char *base,
+  const char *top,
   unsigned long bandwidth,
   unsigned int flags)
 {
@@ -17322,6 +17323,9 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 virStorageSourcePtr baseSource = NULL;
 unsigned int baseIndex = 0;
 g_autofree char *basePath = NULL;
+virStorageSourcePtr topSource = NULL;
+unsigned int topIndex = 0;
+g_autofree char *topPath = NULL;
 g_autofree char *backingPath = NULL;
 unsigned long long speed = bandwidth;
 qemuBlockJobDataPtr job = NULL;
@@ -17355,6 +17359,12 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
   base, baseIndex, NULL
 goto endjob;
 
+if (top &&
+(virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 ||
+ !(topSource = virStorageFileChainLookup(disk->src, disk->src,
+  top, topIndex, NULL
+goto endjob;
+
 if (baseSource) {
 if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
 if (!virQEMUCapsGet(priv->qemuCaps, 
QEMU_CAPS_CHANGE_BACKING_FILE)) {
@@ -17388,7 +17398,7 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 speed <<= 20;
 }
 
-if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, flags)))
+if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, /*topSource, 
*/flags)))
 goto endjob;
 
 if (blockdev) {
@@ -18160,7 +18170,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char 
*path, const char *base,
 /* For normal rebase (enhanced blockpull), the common code handles
  * everything, including vm cleanup. */
 if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
-return qemuDomainBlockPullCommon(vm, path, base, bandwidth, flags);
+return qemuDomainBlockPullCommon(vm, path, base, NULL, bandwidth, 
flags);
 
 /* If we got here, we are doing a block copy rebase. */
 if (!(dest = virStorageSourceNew()))
@@ -18311,7 +18321,7 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, 
unsigned long bandwidth,
 }
 
 /* qemuDomainBlockPullCommon consumes the reference on @vm */
-return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
+return qemuDomainBlockPullCommon(vm, path, NULL, NULL, bandwidth, flags);
 }
 
 
-- 
2.24.1

[libvirt PATCH 5/6] qemu: block: 'top' support in construction of QMP block-stream

[Pavel Mores]

This commit only handles submission of block-stream command.  Since
block-stream is a job, we'll need to handle its completion as well
in an upcoming commit.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f97bf593..b1ffc68e6b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17410,7 +17410,15 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 !(backingPath = qemuBlockGetBackingStoreString(baseSource)))
 goto endjob;
 }
-device = disk->src->nodeformat;
+if (topSource) {
+device = topSource->nodeformat;
+if (qemuDomainStorageSourceAccessAllow(driver, vm, topSource, 
false, false) < 0) {
+virReportError(VIR_ERR_ACCESS_DENIED, "can't make 'top' 
writable");
+goto endjob;
+}
+} else {
+device = disk->src->nodeformat;
+}
 } else {
 device = job->name;
 }
@@ -18171,7 +18179,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char 
*path, const char *base,
 /* For normal rebase (enhanced blockpull), the common code handles
  * everything, including vm cleanup. */
 if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
-return qemuDomainBlockPullCommon(vm, path, base, NULL, bandwidth, 
flags);
+return qemuDomainBlockPullCommon(vm, path, base, top, bandwidth, 
flags);
 
 /* If we got here, we are doing a block copy rebase. */
 if (!(dest = virStorageSourceNew()))
@@ -18302,8 +18310,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, 
const char *destxml,
 
 
 static int
-qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long 
bandwidth,
-unsigned int flags)
+qemuDomainBlockPull(virDomainPtr dom, const char *path,
+unsigned long bandwidth, unsigned int flags)
 {
 virDomainObjPtr vm;
 virCheckFlags(VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES, -1);
-- 
2.24.1

[libvirt PATCH 2/6] qemu: block: pull job extended with 'top' parameter

[Pavel Mores]

'top' member added to qemuBlockJobPullData and the job's "constructor" function
qemuBlockJobDiskNewPull() and its invocation changed to support the new member.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_blockjob.c | 2 ++
 src/qemu/qemu_blockjob.h | 2 ++
 src/qemu/qemu_driver.c   | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 71df0d1ab2..e19a2ad76b 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -253,6 +253,7 @@ qemuBlockJobDataPtr
 qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 virStorageSourcePtr base,
+virStorageSourcePtr top,
 unsigned int jobflags)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -270,6 +271,7 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 return NULL;
 
 job->data.pull.base = base;
+job->data.pull.top = top;
 job->jobflags = jobflags;
 
 if (qemuBlockJobRegister(job, vm, disk, true) < 0)
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 72c7fa053e..7223b4abb8 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -77,6 +77,7 @@ typedef qemuBlockJobPullData *qemuBlockJobDataPullPtr;
 
 struct _qemuBlockJobPullData {
 virStorageSourcePtr base;
+virStorageSourcePtr top;
 };
 
 
@@ -177,6 +178,7 @@ qemuBlockJobDataPtr
 qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 virStorageSourcePtr base,
+virStorageSourcePtr top,
 unsigned int jobflags);
 
 qemuBlockJobDataPtr
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 81cca360e0..7970c913f3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17398,7 +17398,7 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 speed <<= 20;
 }
 
-if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, /*topSource, 
*/flags)))
+if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, topSource, 
flags)))
 goto endjob;
 
 if (blockdev) {
-- 
2.24.1

[libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

A new command-line option --top was added to virsh's blockpull command.
Similar to how --base is handled, in presence of --top the operation is
implemented internally as a rebase.  To that end, a corresponding new 'top'
parameter was added to virDomainBlockRebase().

Signed-off-by: Pavel Mores 
---
 include/libvirt/libvirt-domain.h |  4 ++--
 src/libvirt-domain.c |  5 +++--
 tools/virsh-domain.c | 14 +++---
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b440818ec2..069d7f98ec 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2560,8 +2560,8 @@ typedef enum {
 } virDomainBlockRebaseFlags;
 
 int virDomainBlockRebase(virDomainPtr dom, const char *disk,
- const char *base, unsigned long bandwidth,
- unsigned int flags);
+ const char *base, const char *top,
+ unsigned long bandwidth, unsigned int flags);
 
 /**
  * virDomainBlockCopyFlags:
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 65813b68cc..1f9d1b5b84 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  * @disk: path to the block device, or device shorthand
  * @base: path to backing file to keep, or device shorthand,
  *or NULL for no backing file
+ * @top: path to top file, or device shorthand, or NULL for no top
  * @bandwidth: (optional) specify bandwidth limit; flags determine the unit
  * @flags: bitwise-OR of virDomainBlockRebaseFlags
  *
@@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  */
 int
 virDomainBlockRebase(virDomainPtr dom, const char *disk,
- const char *base, unsigned long bandwidth,
- unsigned int flags)
+ const char *base, const char *top,
+ unsigned long bandwidth, unsigned int flags)
 {
 virConnectPtr conn;
 
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9d0f7d68d2..47ccdd1b94 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2436,7 +2436,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd)
 if (bytes)
 flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
 
-if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0)
+if (virDomainBlockRebase(dom, path, dest, NULL, bandwidth, flags) < 0)
 goto cleanup;
 }
 
@@ -2765,6 +2765,10 @@ static const vshCmdOptDef opts_blockpull[] = {
  .type = VSH_OT_STRING,
  .help = N_("path of backing file in chain for a partial pull")
 },
+{.name = "top",
+ .type = VSH_OT_STRING,
+ .help = N_("path of top backing file in chain for a partial pull")
+},
 {.name = "wait",
  .type = VSH_OT_BOOL,
  .help = N_("wait for job to finish")
@@ -2804,6 +2808,7 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
 int timeout = 0;
 const char *path = NULL;
 const char *base = NULL;
+const char *top = NULL;
 unsigned long bandwidth = 0;
 unsigned int flags = 0;
 virshBlockJobWaitDataPtr bjWait = NULL;
@@ -2817,6 +2822,9 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0)
 return false;
 
+if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0)
+return false;
+
 if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0)
 return false;
 
@@ -2834,11 +2842,11 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
  verbose, timeout, async)))
 goto cleanup;
 
-if (base || flags) {
+if (base || top || flags) {
 if (bytes)
 flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
 
-if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0)
+if (virDomainBlockRebase(dom, path, base, top, bandwidth, flags) < 0)
 goto cleanup;
 } else {
 if (bytes)
-- 
2.24.1

[libvirt PATCH 0/6] [RFC] qemu: implement support for pulling into non-active layers

[Pavel Mores]

QEMU's block-stream command supports pulling ("streaming" in QMP lingo) into
arbitrary layer, however libvirt can only pull into an active layer.  This
series is a proof-of-concept work to extend libvirt's pull to support pulling
into non-active layers as well.

While this doesn't seem to be a much demanded feature by itself, it can be seen
as a prerequisite to implementing full support for external snapshots in
snapshot-delete(*) as that appears to be implementable in terms of a
full-featured pull.

As stated above, this is a proof-of-concept work so admittedly not much effort
went into fully polishing the series.  The expectation is that the series will
have to change heavily anyway before it's good enough to be pushed.

(*) https://bugzilla.redhat.com/show_bug.cgi?id=1519001

Pavel Mores (6):
  qemu: block: add 'top' parameter to qemuDomainBlockPullCommon()
  qemu: block: pull job extended with 'top' parameter
  qemu: block: blockpull param 'top' support in virsh proper
  qemu: block: blockpull param 'top' support in driver and RPC
  qemu: block: 'top' support in construction of QMP block-stream
  qemu: block: extend pull job completion with 'top' handling

 include/libvirt/libvirt-domain.h |  4 ++--
 src/driver-hypervisor.h  |  1 +
 src/libvirt-domain.c |  7 ---
 src/qemu/qemu_blockjob.c | 35 
 src/qemu/qemu_blockjob.h |  2 ++
 src/qemu/qemu_driver.c   | 33 +++---
 src/remote/remote_protocol.x |  1 +
 src/remote_protocol-structs  |  1 +
 tools/virsh-domain.c | 14 ++---
 9 files changed, 79 insertions(+), 19 deletions(-)

-- 
2.24.1

Release of libvirt-6.1.0

[Daniel Veillard]

  It's out, one day late, though I did the build and pushed the tag
in git yesterday, but I had forgotten to push the commit, thanks
Boris for raising this to me. So it's now available as signed tarball
and rpm source package at the usual place:

   https://libvirt.org/sources/

 This release come with a number of new features as listed below
but also make the packaging change of not relying on gnulib anymore for
platform compatibility, so there are some serious changes in this version:


New features:

- qemu: new rng backend type: builtin
It implements qemu builtin rng backend. That uses getrandom syscall to
generate random, no external rng source needed. Available since QEMU
4.2.

- support for virtio+hostdev NIC 
QEMU 4.2.0 and later, combined with a sufficiently recent guest
virtio-net driver (e.g. the driver included in Linux kernel 4.18 and
later), supports setting up a simple network bond device comprised of
one virtio emulated NIC and one hostdev NIC (which must be an SRIOV
VF). (in QEMU, this is known as the "virtio failover" feature). The
allure of this setup is that the bond will always favor the hostdev
device, providing better performance, until the guest is migrated - at
that time QEMU will automatically unplug the hostdev NIC and the bond
will send all traffic via the virtio NIC until migration is completed,
then QEMU on the destination side will hotplug a new hostdev NIC and
the bond will switch back to using the hostdev for network traffic. The
result is that guests desiring the extra performance of a hostdev NIC
are now migratable without network downtime (performance is just
degraded during migration) and without requiring a complicated bonding
configuration in the guest OS network config and complicated
unplug/replug logic in the management application on the host - it can
instead all be accomplished in libvirt with the interface 
subelement "type" and "persistent" attributes.

- support BR_ISOLATED flag for guest interfaces attached to a Linux host
bridge
Since Linux kernel 4.18, the Linux host bridge has had a flag
BR_ISOLATED that can be applied to individual ports. When this flag is
set for a port, traffic is blocked between that port and any other port
that also has the BR_ISOLATED flag set. libvirt domain interface config
now supports setting this flag via the  setting.
It can also be set for all connections to a particular libvirt network
by setting the same option in the network config - since the port for
the host itself does not have BR_ISOLATED set, the guests can
communicate with the host and the outside world, but guests on that
network can't communicate with each other. This feature works for QEMU
and LXC guests with interfaces attached to a Linux host bridge.

- qemu: Introduce the 'armvtimer' timer type
QEMU 5.0 introduces the ability to control the behavior of the virtual
timer for KVM ARM/virt guests, and this new timer type exposes the same
capability to libvirt users.

- qemu: Storage configuration improvements
Libvirt now accepts  and allows specifying
the offset and size of the image format container inside the storage
source via the  subelement.

- qemu: Introduce the 'tpm-spapr' TPM model
This device, available starting from QEMU 5.0, is limited to pSeries
guests.

Improvements:

- qemu: Image format probing is allowed in certain cases
To resolve regressions when users didn't specify the backing image
format in the overlay, libvirt now probes the format in certain secure
scenarios which fixes a few common existing cases. Additionally the
knowledge base was extended to provide more information on how to
rectify the problem.

- qemu: Support "dies" in CPU topology
This CPU topology concept, new in QEMU 4.1.0, sits between the existing
"socket" and "core".

- libxl: Add support for Credit2 scheduler parameters

- lxc: Add support LXC 3 network configuration format

Bug fix:

- conf: Do not generate machine names ending with a dash
Recent systemd versions do not allow them.

Packaging changes:

- use of gnulib has been completely eliminated
Historically libvirt has embedded gnulib to provide fixes for various
platform portability problems. This usage has now been eliminated and
alternative approaches for platform portability problems adopted where
required. This has been validated on the set of platforms covered by
automated CI build testing. Other modern Linux distros using glibc are
expected to work. Linux distros using non-glibc packages, and other
non-Linux platforms may encounter regressions when building this
release. Please report any build problems encountered back to the
project maintainers for evaluation.

  Thanks everybody for your help with this release, and sorry for the delay,

Daniel

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

[PATCH] maint: Post-release version bump to 6.2.0

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---

Pushed.

 configure.ac  | 2 +-
 docs/news.xml | 8 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index ac4052afb6..e565437062 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General 
Public
 dnl License along with this library.  If not, see
 dnl .
 
-AC_INIT([libvirt], [6.1.0], [libvir-list@redhat.com], [], 
[https://libvirt.org])
+AC_INIT([libvirt], [6.2.0], [libvir-list@redhat.com], [], 
[https://libvirt.org])
 
 if test $srcdir = "."
 then
diff --git a/docs/news.xml b/docs/news.xml
index 54afd3e286..7fd88f9998 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -42,6 +42,14 @@
  -->
 
 
+  
+
+
+
+
+
+
+  
   
 
   
-- 
2.24.1

Re: [PATCH v2 00/30] Configurable policy for handling deprecated interfaces

[Markus Armbruster]

Peter Maydell  writes:

> On Wed, 4 Mar 2020 at 08:18, Markus Armbruster  wrote:
>> Peter Maydell  writes:
>> > How much do you think this is likely to affect the
>> > generate-rst-from-qapi-docs series? I'd really like
>> > that to go in for this release, but this looks like
>> > it's shaping up to be a big conflict :-(
>>
>> I paused reviewing your series to post this one, because "I'd really
>> like that to go in for this release" :)
>>
>> My series touches 21 existing commented definitions in qapi/, six more
>> in tests/qapi-schema/doc-good.json, and adds new module
>> qapi/compat.json.  Consolidated diff appended.
>>
>> Rule of thumb for reducing conflict resolution labor: the bigger manual
>> change goes first.  Yours is bigger, but I don't remember how manual it
>> is.
>
> We got pretty much all the manual changes I needed into
> master already, so all that's left really is the script parts.
> The conflicts would basically be where this series affects
> the generate-docs parts of the scripts -- any changes here
> to doc.py are basically dead-code-walking and would need
> to be done over again in the equivalent code for rust generation.
> But looking at the diffstat
>  scripts/qapi/doc.py   |  16 +-
> so hopefully it won't be too bad.

Not bad at all: two patches, both trivial for this file:

$ git-log --patch master..posted/qapi-features scripts/qapi/doc.py
commit 63daae3c1da9a7d8a189a9dfc80804c812b3f6af
Author: Markus Armbruster 
Date:   Fri Oct 18 11:23:40 2019 +0200

qapi: Consistently put @features parameter right after @ifcond

Signed-off-by: Markus Armbruster 

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 36e823338b..92f584edcf 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -249,8 +249,8 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
 texi_members(doc, 'Values',
  member_func=texi_enum_value)))
 
-def visit_object_type(self, name, info, ifcond, base, members, variants,
-  features):
+def visit_object_type(self, name, info, ifcond, features,
+  base, members, variants):
 doc = self.cur_doc
 if base and base.is_implicit():
 base = None
@@ -262,9 +262,9 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
 self._gen.add(texi_type('Alternate', doc, ifcond,
 texi_members(doc, 'Members')))
 
-def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
-  success_response, boxed, allow_oob, allow_preconfig,
-  features):
+def visit_command(self, name, info, ifcond, features,
+  arg_type, ret_type, gen, success_response, boxed,
+  allow_oob, allow_preconfig):
 doc = self.cur_doc
 self._gen.add(texi_msg('Command', doc, ifcond,
texi_arguments(doc,

commit 9e101e2b1803f326df46e42d88bae9f281da9fe4
Author: Markus Armbruster 
Date:   Tue Oct 15 14:33:24 2019 +0200

qapi: Add feature flags to remaining definitions

In v4.1.0, we added feature flags just to struct types (commit
6a8c0b5102^..f3ed93d545), to satisfy an immediate need (commit
c9d4070991 "file-posix: Add dynamic-auto-read-only QAPI feature").  In
v4.2.0, we added them to commands (commit 23394b4c39 "qapi: Add
feature flags to commands") to satisfy another immediate need (commit
d76744e65e "qapi: Allow introspecting fix for savevm's cooperation
with blockdev").

Add them to the remaining definitions: enumeration types, union types,
alternate types, and events.

Signed-off-by: Markus Armbruster 

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 1787a53d91..36e823338b 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -243,7 +243,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
 def write(self, output_dir):
 self._gen.write(output_dir)
 
-def visit_enum_type(self, name, info, ifcond, members, prefix):
+def visit_enum_type(self, name, info, ifcond, features, members, prefix):
 doc = self.cur_doc
 self._gen.add(texi_type('Enum', doc, ifcond,
 texi_members(doc, 'Values',
@@ -257,7 +257,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
 self._gen.add(texi_type('Object', doc, ifcond,
 texi_members(doc, 'Members', base, variants)))
 
-def visit_alternate_type(self, name, info, ifcond, variants):
+def visit_alternate_type(self, name, info, ifcond, features, variants):
 doc = self.cur_doc
 self._gen.add(texi_type('Alternate', doc, ifcond,
 texi_members(doc, 'Members')))
@@ -270,7 +270,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
texi_arguments(doc,

Re: [PATCH v2 00/30] Configurable policy for handling deprecated interfaces

[Peter Maydell]

On Wed, 4 Mar 2020 at 08:18, Markus Armbruster  wrote:
> Peter Maydell  writes:
> > How much do you think this is likely to affect the
> > generate-rst-from-qapi-docs series? I'd really like
> > that to go in for this release, but this looks like
> > it's shaping up to be a big conflict :-(
>
> I paused reviewing your series to post this one, because "I'd really
> like that to go in for this release" :)
>
> My series touches 21 existing commented definitions in qapi/, six more
> in tests/qapi-schema/doc-good.json, and adds new module
> qapi/compat.json.  Consolidated diff appended.
>
> Rule of thumb for reducing conflict resolution labor: the bigger manual
> change goes first.  Yours is bigger, but I don't remember how manual it
> is.

We got pretty much all the manual changes I needed into
master already, so all that's left really is the script parts.
The conflicts would basically be where this series affects
the generate-docs parts of the scripts -- any changes here
to doc.py are basically dead-code-walking and would need
to be done over again in the equivalent code for rust generation.
But looking at the diffstat
 scripts/qapi/doc.py   |  16 +-
so hopefully it won't be too bad.

thanks
-- PMM

[PATCH] Share Dup Daemon Function *SetupLogging

[Lan]

Create a new function virDaemonSetupLogging (src/util/virdaemon.c)
for shared code in
virLockDaemonSetupLogging (src/locking/lock_daemon.c)
virLogDaemonSetupLogging (src/logging/log_daemon.c)
daemonSetupLogging (src/remote/remote_daemon.c)

One of the BiteSizedTasks
---
 src/libvirt_private.syms   |  2 +
 src/locking/lock_daemon.c  | 58 ++---
 src/logging/log_daemon.c   | 50 ++---
 src/remote/remote_daemon.c | 57 ++---
 src/util/Makefile.inc.am   |  4 +-
 src/util/virdaemon.c   | 75 ++
 src/util/virdaemon.h   | 35 ++
 7 files changed, 126 insertions(+), 155 deletions(-)
 create mode 100644 src/util/virdaemon.c
 create mode 100644 src/util/virdaemon.h

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index de0c7a3133..50cbd6d7af 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1906,6 +1906,8 @@ virCryptoHashBuf;
 virCryptoHashString;
 virCryptoHaveCipher;
 
+# util/virdaemon.h
+virDaemonSetupLogging;
 
 # util/virdbus.h
 virDBusCallMethod;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 5e5a0c1089..5ba851cb55 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -46,6 +46,7 @@
 #include "virstring.h"
 #include "virgettext.h"
 #include "virenum.h"
+#include "virdaemon.h"
 
 #include "locking/lock_daemon_dispatch.h"
 #include "locking/lock_protocol.h"
@@ -477,59 +478,6 @@ virLockDaemonErrorHandler(void *opaque G_GNUC_UNUSED,
 }
 
 
-/*
- * Set up the logging environment
- * By default if daemonized all errors go to the logfile libvirtd.log,
- * but if verbose or error debugging is asked for then also output
- * informational and debug messages. Default size if 64 kB.
- */
-static int
-virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
-  bool privileged,
-  bool verbose,
-  bool godaemon)
-{
-virLogReset();
-
-/*
- * Libvirtd's order of precedence is:
- * cmdline > environment > config
- *
- * Given the precedence, we must process the variables in the opposite
- * order, each one overriding the previous.
- */
-if (config->log_level != 0)
-virLogSetDefaultPriority(config->log_level);
-
-/* In case the config is empty, both filters and outputs will become empty,
- * however we can't start with empty outputs, thus we'll need to define and
- * setup a default one.
- */
-ignore_value(virLogSetFilters(config->log_filters));
-ignore_value(virLogSetOutputs(config->log_outputs));
-
-/* If there are some environment variables defined, use those instead */
-virLogSetFromEnv();
-
-/*
- * Command line override for --verbose
- */
-if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
-virLogSetDefaultPriority(VIR_LOG_INFO);
-
-/* Define the default output. This is only applied if there was no setting
- * from either the config or the environment.
- */
-virLogSetDefaultOutput("virtlockd", godaemon, privileged);
-
-if (virLogGetNbOutputs() == 0)
-virLogSetOutputs(virLogGetDefaultOutput());
-
-return 0;
-}
-
-
-
 /* Display version information. */
 static void
 virLockDaemonVersion(const char *argv0)
@@ -1186,7 +1134,9 @@ int main(int argc, char **argv) {
 }
 VIR_FREE(remote_config_file);
 
-if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) {
+if (virDaemonSetupLogging((virDaemonLogConfigPtr)(&(config->log_level)),
+  "virtlockd", privileged,
+  verbose, godaemon)< 0) {
 VIR_ERROR(_("Can't initialize logging"));
 exit(EXIT_FAILURE);
 }
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 772bbb805b..9f962300ed 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -45,6 +45,7 @@
 #include "virstring.h"
 #include "virgettext.h"
 #include "virenum.h"
+#include "virdaemon.h"
 
 #include "log_daemon_dispatch.h"
 #include "log_protocol.h"
@@ -419,51 +420,6 @@ virLogDaemonErrorHandler(void *opaque G_GNUC_UNUSED,
 }
 
 
-static void
-virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
- bool privileged,
- bool verbose,
- bool godaemon)
-{
-virLogReset();
-
-/*
- * Libvirtd's order of precedence is:
- * cmdline > environment > config
- *
- * Given the precedence, we must process the variables in the opposite
- * order, each one overriding the previous.
- */
-if (config->log_level != 0)
-virLogSetDefaultPriority(config->log_level);
-
-/* In case the config is empty, both filters and outputs will become empty,
- * however we can't start with empty outputs, thus we'll need to define and
- * setup a default one.
- */
-ignore_value

Re: [PATCH v2 00/30] Configurable policy for handling deprecated interfaces

[Markus Armbruster]

Peter Maydell  writes:

> On Tue, 3 Mar 2020 at 16:37, Markus Armbruster  wrote:
>>
>> Based-on: <20200227144531.24309-1-arm...@redhat.com>
>>
>> This series extends QMP introspection to cover deprecation.
>> Additionally, new option -compat lets you configure what to do when
>> deprecated interfaces get used.  This is intended for testing users of
>> the management interfaces.  It is experimental.
>
> How much do you think this is likely to affect the
> generate-rst-from-qapi-docs series? I'd really like
> that to go in for this release, but this looks like
> it's shaping up to be a big conflict :-(

I paused reviewing your series to post this one, because "I'd really
like that to go in for this release" :)

My series touches 21 existing commented definitions in qapi/, six more
in tests/qapi-schema/doc-good.json, and adds new module
qapi/compat.json.  Consolidated diff appended.

Rule of thumb for reducing conflict resolution labor: the bigger manual
change goes first.  Yours is bigger, but I don't remember how manual it
is.

Let's try to get both series reviewed, then figure out together how to
get them merged with the least pain.



$ git-diff -p --stat master posted/qapi-features \*.json
 qapi/block-core.json| 69 +++--
 qapi/block.json |  9 ++--
 qapi/char.json  |  1 +
 qapi/compat.json| 52 +++
 qapi/control.json   | 11 ++--
 qapi/introspect.json| 26 +-
 qapi/machine.json   | 34 ++--
 qapi/migration.json | 36 -
 qapi/misc.json  | 13 ++---
 qapi/qapi-schema.json   |  1 +
 tests/qapi-schema/doc-good.json | 22 +++-
 tests/qapi-schema/features-deprecated-type.json |  3 ++
 tests/qapi-schema/qapi-schema-test.json | 48 +++--
 13 files changed, 239 insertions(+), 86 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 85e27bb61f..bade02760c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -297,7 +297,7 @@
 #
 # @encrypted: true if the backing device is encrypted
 #
-# @encryption_key_missing: Deprecated; always false
+# @encryption_key_missing: always false
 #
 # @detect_zeroes: detect and optimize zero writes (Since 2.1)
 #
@@ -363,13 +363,19 @@
 # @dirty-bitmaps: dirty bitmaps information (only present if node
 # has one or more dirty bitmaps) (Since 4.2)
 #
+# Features:
+# @deprecated: Member @encryption_key_missing is deprecated.  It is
+# always false.
+#
 # Since: 0.14.0
 #
 ##
 { 'struct': 'BlockDeviceInfo',
   'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str',
 '*backing_file': 'str', 'backing_file_depth': 'int',
-'encrypted': 'bool', 'encryption_key_missing': 'bool',
+'encrypted': 'bool',
+'encryption_key_missing': { 'type': 'bool',
+'features': [ 'deprecated' ] },
 'detect_zeroes': 'BlockdevDetectZeroesOptions',
 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
@@ -475,7 +481,7 @@
 #
 # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
 #
-# @status: Deprecated in favor of @recording and @locked. (since 2.4)
+# @status: current status of the dirty bitmap (since 2.4)
 #
 # @recording: true if the bitmap is recording new writes from the guest.
 # Replaces `active` and `disabled` statuses. (since 4.0)
@@ -492,11 +498,17 @@
 #@busy to be false. This bitmap cannot be used. To remove
 #it, use @block-dirty-bitmap-remove. (Since 4.0)
 #
+# Features:
+# @deprecated: Member @status is deprecated.  Use @recording and
+# @locked instead.
+#
 # Since: 1.3
 ##
 { 'struct': 'BlockDirtyInfo',
   'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-   'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus',
+   'recording': 'bool', 'busy': 'bool',
+   'status': { 'type': 'DirtyBitmapStatus',
+   'features': [ 'deprecated' ] },
'persistent': 'bool', '*inconsistent': 'bool' } }
 
 ##
@@ -659,7 +671,6 @@
 #
 # @dirty-bitmaps: dirty bitmaps information (only present if the
 # driver has one or more dirty bitmaps) (Since 2.0)
-# Deprecated in 4.2; see BlockDeviceInfo instead.
 #
 # @io-status: @BlockDeviceIoStatus. Only present if the device
 # supports it and the VM is configured to stop on errors
@@ -669,13 +680,18 @@
 # @inserted: @BlockDeviceInfo describing the device if media is
 #present
 #
+# Features:
+# @deprecated: Member @dirty-bitmaps is deprecated.  Use @inserted
+#   

RE: [PATCH V4 0/5] Introduce Advanced Watch Dog module

[Zhang, Chen]

> >>> Subject: Re: [PATCH V4 0/5] Introduce Advanced Watch Dog module
> >>>
> >>>
> >>> On 2020/1/19 下午5:10, Zhang, Chen wrote:
>  Hi~
> 
>  Anyone have comments about this module?
> >>> Hi Chen:
> >>>
> >>> I will take a look at this series.
> >> Sorry for slow reply due to CNY and extend leave.
> >> OK, waiting your comments~ Thanks~
> >>
> >>> Two general questions:
> >>>
> >>> - if it can detect more than network stall, it should not belong to
> >>> /net
> >> This module use network connection status to detect all the issue(Host to
> Guest/Host to Host/Host to Admin...).
> >> The target is more than network but all use network way. So it is looks a
> tricky problem.
> >
> > Ok.
> >
> >
> >>> - need to convince libvirt guys for this proposal, since usually
> >>> it's the duty of upper layer instead of qemu itself
> >>>
> >> Yes, It looks a upper layer responsibility, but In the cover latter I have
> explained the reason why we need this in Qemu.
> >>try to make this module as simple as possible. This module give upper
> layer software a new way to connect/monitoring Qemu.
> >> And due to all the COLO code implement in Qemu side, Many customer
> >> want to use this FT solution without other dependencies, it is very easy to
> integrated to real product.
> >>
> >> Thanks
> >> Zhang Chen
> >
> > I would like to hear from libvirt about such design.
> 
> 
> Hi Jason,
> 
> OK. I add the libvirt mailing list in this thread.
> 
> The full mail discussion and patches:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg02611.html
> 
> 
> By the way, I noticed Eric is libvirt maintianer.
> 
> Hi Eric and Paolo, Can you give some comments about this series?
> 
> 

No news for a while...
We already have some users(Cloud Service Provider) try to use is module in 
their product.
But they also need to follow the Qemu upstream code.

Thanks
Zhang Chen


> Thanks
> 
> Zhang Chen
> 
> 
> >
> > Thanks
> >