[PATCH 1/7] test_driver: Replace virAtomicIntAdd() with virAtomicIntInc()

[Michal Privoznik]

Instead of calling virAtomicIntAdd(, 1); we can call
virAtomicIntInc() directly.

Signed-off-by: Michal Privoznik 
---
 src/test/test_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 47c1fc588f..6f617592f3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -695,7 +695,7 @@ testDomainStartState(testDriverPtr privconn,
 int ret = -1;
 
 virDomainObjSetState(dom, VIR_DOMAIN_RUNNING, reason);
-dom->def->id = virAtomicIntAdd(>nextDomID, 1);
+dom->def->id = virAtomicIntInc(>nextDomID);
 
 if (virDomainObjSetDefTransient(privconn->xmlopt,
 dom, NULL) < 0) {
-- 
2.24.1

[PATCH 6/7] src: Drop virAtomicIntDecAndTest() with g_atomic_int_dec_and_test()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/libxl/libxl_domain.c  |  2 +-
 src/lxc/lxc_process.c |  2 +-
 src/nwfilter/nwfilter_dhcpsnoop.c |  8 
 src/qemu/qemu_process.c   |  2 +-
 src/util/viratomic.h  | 11 ---
 src/util/virobject.c  |  2 +-
 6 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 8885af451f..d53363dc02 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -878,7 +878,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
 
 priv->ignoreDeathEvent = false;
 
-if (virAtomicIntDecAndTest(>nactive) && driver->inhibitCallback)
+if (!!g_atomic_int_dec_and_test(>nactive) && 
driver->inhibitCallback)
 driver->inhibitCallback(false, driver->inhibitOpaque);
 
 if ((vm->def->ngraphics == 1) &&
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 2bb2216dc0..d8ddea6d24 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -208,7 +208,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
 vm->pid = -1;
 vm->def->id = -1;
 
-if (virAtomicIntDecAndTest(>nactive) && driver->inhibitCallback)
+if (!!g_atomic_int_dec_and_test(>nactive) && 
driver->inhibitCallback)
 driver->inhibitCallback(false, driver->inhibitOpaque);
 
 virLXCDomainReAttachHostDevices(driver, vm->def);
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index f73913ec9e..c4341ee3e2 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -692,7 +692,7 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req)
 
 virNWFilterSnoopLock();
 
-if (virAtomicIntDecAndTest(>refctr)) {
+if (!!g_atomic_int_dec_and_test(>refctr)) {
 /*
  * delete the request:
  * - if we don't find req on the global list anymore
@@ -868,7 +868,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
  skip_instantiate:
 VIR_FREE(ipl);
 
-ignore_value(virAtomicIntDecAndTest());
+ignore_value(!!g_atomic_int_dec_and_test());
 
  lease_not_found:
 VIR_FREE(ipstr);
@@ -1142,7 +1142,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, 
void *opaque)
_("Instantiation of rules failed on "
  "interface '%s'"), req->binding->portdevname);
 }
-ignore_value(virAtomicIntDecAndTest(job->qCtr));
+ignore_value(!!g_atomic_int_dec_and_test(job->qCtr));
 VIR_FREE(job);
 }
 
@@ -1543,7 +1543,7 @@ virNWFilterDHCPSnoopThread(void *req0)
 pcap_close(pcapConf[i].handle);
 }
 
-ignore_value(virAtomicIntDecAndTest());
+ignore_value(!!g_atomic_int_dec_and_test());
 
 return;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 20c4e3bb5d..ed14ce5ea2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7313,7 +7313,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
 qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
 
-if (virAtomicIntDecAndTest(>nactive) && driver->inhibitCallback)
+if (!!g_atomic_int_dec_and_test(>nactive) && 
driver->inhibitCallback)
 driver->inhibitCallback(false, driver->inhibitOpaque);
 
 /* Wake up anything waiting on domain condition */
diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 152cfcd903..1ddc7019f9 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -26,17 +26,6 @@
 
 #include "internal.h"
 
-/**
- * virAtomicIntDecAndTest:
- * Decrements the value of atomic by 1.
- *
- * Think of this operation as an atomic version of
- * { *atomic -= 1; return *atomic == 0; }
- *
- * This call acts as a full compiler and hardware memory barrier.
- */
-#define virAtomicIntDecAndTest(i) (!!g_atomic_int_dec_and_test(i))
-
 /**
  * virAtomicIntCompareExchange:
  * Compares atomic to oldval and, if equal, sets it to newval. If
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 9185d3e92e..7749d89243 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -344,7 +344,7 @@ virObjectUnref(void *anyobj)
 if (VIR_OBJECT_NOTVALID(obj))
 return false;
 
-bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
+bool lastRef = !!g_atomic_int_dec_and_test(>u.s.refs);
 PROBE(OBJECT_UNREF, "obj=%p", obj);
 if (lastRef) {
 PROBE(OBJECT_DISPOSE, "obj=%p", obj);
-- 
2.24.1

[PATCH 5/7] src: Replace virAtomicIntInc() with g_atomic_int_add()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/libxl/libxl_domain.c  |  2 +-
 src/libxl/libxl_driver.c  |  2 +-
 src/lxc/lxc_process.c |  4 ++--
 src/nwfilter/nwfilter_dhcpsnoop.c | 10 +-
 src/qemu/qemu_process.c   |  4 ++--
 src/test/test_driver.c|  2 +-
 src/util/viratomic.h  | 11 ---
 src/util/virobject.c  |  4 ++--
 src/util/virprocess.c |  2 +-
 9 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index d63eca0bd6..8885af451f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1471,7 +1471,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
 if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
 goto destroy_dom;
 
-if (virAtomicIntInc(>nactive) == 0 && driver->inhibitCallback)
+if (g_atomic_int_add(>nactive, 1) == 0 && driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
 
 /* finally we can call the 'started' hook script if any */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index da9a640db5..41cbb67e3a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -446,7 +446,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
  VIR_DOMAIN_RUNNING_UNKNOWN);
 
-if (virAtomicIntInc(>nactive) == 0 && driver->inhibitCallback)
+if (g_atomic_int_add(>nactive, 1) == 0 && driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
 
 /* Enable domain death events */
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 2e860e2fae..2bb2216dc0 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1468,7 +1468,7 @@ int virLXCProcessStart(virConnectPtr conn,
 if (virCommandHandshakeNotify(cmd) < 0)
 goto cleanup;
 
-if (virAtomicIntInc(>nactive) == 0 && driver->inhibitCallback)
+if (g_atomic_int_add(>nactive, 1) == 0 && driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
 
 if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
@@ -1670,7 +1670,7 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
  VIR_DOMAIN_RUNNING_UNKNOWN);
 
-if (virAtomicIntInc(>nactive) == 0 && driver->inhibitCallback)
+if (g_atomic_int_add(>nactive, 1) == 0 && 
driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
 
 if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index b82779609d..f73913ec9e 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -541,7 +541,7 @@ virNWFilterSnoopReqLeaseTimerRun(virNWFilterSnoopReqPtr req)
 static void
 virNWFilterSnoopReqGet(virNWFilterSnoopReqPtr req)
 {
-virAtomicIntInc(>refctr);
+g_atomic_int_add(>refctr, 1);
 }
 
 /*
@@ -756,7 +756,7 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterSnoopReqPtr req,
 /* put the lease on the req's list */
 virNWFilterSnoopIPLeaseTimerAdd(pl);
 
-virAtomicIntInc();
+g_atomic_int_add(, 1);
 
  exit:
 if (update_leasefile)
@@ -1172,7 +1172,7 @@ virNWFilterSnoopDHCPDecodeJobSubmit(virThreadPoolPtr pool,
 ret = virThreadPoolSendJob(pool, 0, job);
 
 if (ret == 0)
-virAtomicIntInc(qCtr);
+g_atomic_int_add(qCtr, 1);
 else
 VIR_FREE(job);
 
@@ -1649,7 +1649,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr 
techdriver,
 
 threadPuts = true;
 
-virAtomicIntInc();
+g_atomic_int_add(, 1);
 
 req->threadkey = virNWFilterSnoopActivate(req);
 if (!req->threadkey) {
@@ -1777,7 +1777,7 @@ virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr 
ipl)
 goto err_exit;
 
 /* keep dead leases at < ~95% of file size */
-if (virAtomicIntInc() >=
+if (g_atomic_int_add(, 1) >=
 g_atomic_int_get() * 20)
 virNWFilterSnoopLeaseFileLoad();   /* load & refresh lease file */
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 57a60c568a..20c4e3bb5d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5571,7 +5571,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
 qemuDomainSetFakeReboot(driver, vm, false);
 virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, 
VIR_DOMAIN_PAUSED_STARTING_UP);
 
-if (virAtomicIntInc(>nactive) == 0 && driver->inhibitCallback)
+if (g_atomic_int_add(>nactive, 1) == 0 && 
driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
 
 /* Run an early hook to set-up missing devices */
@@ -8146,7 +8146,7 @@ qemuProcessReconnect(void *opaque)
 goto error;
 }
 
-if (virAtomicIntInc(>nactive) == 0 && 

[PATCH 2/7] tests: Drop viratomictest

[Michal Privoznik]

In future commits our virAtomic* APIs will be replaced with their
GLib variants. Instead of trying to update the test after each
commit and eventually removing the test anyway, remove it upfront
and save the hassle.

Signed-off-by: Michal Privoznik 
---
 tests/Makefile.am |   5 --
 tests/viratomictest.c | 175 --
 2 files changed, 180 deletions(-)
 delete mode 100644 tests/viratomictest.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 19705c6b76..ef49b122cd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -179,7 +179,6 @@ test_programs = virshtest sockettest \
virhostcputest virbuftest \
commandtest seclabeltest \
virhashtest virconftest \
-   viratomictest \
utiltest shunloadtest \
virtimetest viruritest virkeyfiletest \
viralloctest \
@@ -1456,10 +1455,6 @@ virhashtest_SOURCES = \
virhashtest.c virhashdata.h testutils.h testutils.c
 virhashtest_LDADD = $(LDADDS)
 
-viratomictest_SOURCES = \
-   viratomictest.c testutils.h testutils.c
-viratomictest_LDADD = $(LDADDS)
-
 virbitmaptest_SOURCES = \
virbitmaptest.c testutils.h testutils.c
 virbitmaptest_LDADD = $(LDADDS)
diff --git a/tests/viratomictest.c b/tests/viratomictest.c
deleted file mode 100644
index e30df66cd7..00
--- a/tests/viratomictest.c
+++ /dev/null
@@ -1,175 +0,0 @@
-/*
- * Copyright (C) 2011-2013 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library.  If not, see
- * .
- *
- */
-
-#include 
-
-#include 
-#include 
-
-#include "testutils.h"
-
-#include "viratomic.h"
-#include "virrandom.h"
-#include "virthread.h"
-
-static int
-testTypes(const void *data G_GNUC_UNUSED)
-{
-unsigned int u, u2;
-int s, s2;
-bool res;
-
-#define testAssertEq(a, b) \
-if (!(a == b)) \
-return -1;
-virAtomicIntSet(, 5);
-u2 = virAtomicIntGet();
-testAssertEq(u2, 5);
-
-res = virAtomicIntCompareExchange(, 6, 7);
-if (res)
-return -1;
-testAssertEq(u, 5);
-
-testAssertEq(virAtomicIntAdd(, 1), 5);
-testAssertEq(u, 6);
-
-testAssertEq(virAtomicIntInc(), 6);
-testAssertEq(u, 7);
-
-res = virAtomicIntDecAndTest();
-if (res)
-return -1;
-testAssertEq(u, 6);
-
-u2 = virAtomicIntAnd(, 5);
-testAssertEq(u2, 6);
-testAssertEq(u, 4);
-
-u2 = virAtomicIntOr(, 8);
-testAssertEq(u2, 4);
-testAssertEq(u, 12);
-
-u2 = virAtomicIntXor(, 4);
-testAssertEq(u2, 12);
-testAssertEq(u, 8);
-
-virAtomicIntSet(, 5);
-s2 = virAtomicIntGet();
-testAssertEq(s2, 5);
-
-res = virAtomicIntCompareExchange(, 6, 7);
-if (res)
-return -1;
-testAssertEq(s, 5);
-
-virAtomicIntAdd(, 1);
-testAssertEq(s, 6);
-
-virAtomicIntInc();
-testAssertEq(s, 7);
-
-res = virAtomicIntDecAndTest();
-if (res)
-return -1;
-testAssertEq(s, 6);
-
-s2 = virAtomicIntAnd(, 5);
-testAssertEq(s2, 6);
-testAssertEq(s, 4);
-
-s2 = virAtomicIntOr(, 8);
-testAssertEq(s2, 4);
-testAssertEq(s, 12);
-
-s2 = virAtomicIntXor(, 4);
-testAssertEq(s2, 12);
-testAssertEq(s, 8);
-
-return 0;
-}
-
-#define THREADS 10
-#define ROUNDS 1
-
-volatile int bucket[THREADS];
-volatile int atomic;
-
-static void
-thread_func(void *data)
-{
-int idx = (intptr_t)data;
-size_t i;
-int d;
-
-for (i = 0; i < ROUNDS; i++) {
-d = virRandomBits(7);
-bucket[idx] += d;
-virAtomicIntAdd(, d);
-#ifdef WIN32
-SleepEx(0, 0);
-#else
-sched_yield();
-#endif
-}
-}
-
-static int
-testThreads(const void *data G_GNUC_UNUSED)
-{
-int sum;
-size_t i;
-virThread threads[THREADS];
-
-atomic = 0;
-for (i = 0; i < THREADS; i++)
-bucket[i] = 0;
-
-for (i = 0; i < THREADS; i++) {
-if (virThreadCreate(&(threads[i]), true, thread_func, 
(void*)(intptr_t)i) < 0)
-return -1;
-}
-
-for (i = 0; i < THREADS; i++)
-virThreadJoin([i]);
-
-sum = 0;
-for (i = 0; i < THREADS; i++)
-sum += bucket[i];
-
-if (sum != atomic)
-return -1;
-
-return 0;
-}
-
-static int
-mymain(void)
-{
-int ret = 0;
-
-if (virTestRun("types", testTypes, NULL) < 0)
-ret = -1;
-if (virTestRun("threads", testThreads, NULL) < 0)
-ret = 

[PATCH 4/7] src: Replace virAtomicIntSet() with g_atomic_int_set()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/nwfilter/nwfilter_dhcpsnoop.c |  2 +-
 src/test/test_driver.c|  4 ++--
 src/util/viratomic.h  |  9 -
 src/util/virobject.c  |  2 +-
 src/util/virsystemd.c | 10 +-
 5 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index 074cffecb2..b82779609d 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1884,7 +1884,7 @@ virNWFilterSnoopLeaseFileRefresh(void)
  TMPLEASEFILE, LEASEFILE);
 unlink(TMPLEASEFILE);
 }
-virAtomicIntSet(, 0);
+g_atomic_int_set(, 0);
 
  skip_rename:
 virNWFilterSnoopLeaseFileOpen();
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6f617592f3..4e159157d7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -106,7 +106,7 @@ struct _testDriver {
 size_t numAuths;
 testAuthPtr auths;
 
-/* virAtomic access only */
+/* g_atomic access only */
 volatile int nextDomID;
 
 /* immutable pointer, immutable object after being initialized with
@@ -448,7 +448,7 @@ testDriverNew(void)
 !(ret->pools = virStoragePoolObjListNew()))
 goto error;
 
-virAtomicIntSet(>nextDomID, 1);
+g_atomic_int_set(>nextDomID, 1);
 
 return ret;
 
diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 6c1764f7bf..2811447a29 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -26,15 +26,6 @@
 
 #include "internal.h"
 
-/**
- * virAtomicIntSet:
- * Sets the value of atomic to newval.
- *
- * This call acts as a full compiler and hardware memory barrier
- * (after the set)
- */
-#define virAtomicIntSet(i, newv) g_atomic_int_set(i, newv)
-
 /**
  * virAtomicIntInc:
  * Increments the value of atomic by 1.
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 5af14234f2..8cece6e735 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -246,7 +246,7 @@ virObjectNew(virClassPtr klass)
 
 obj->u.s.magic = klass->magic;
 obj->klass = klass;
-virAtomicIntSet(>u.s.refs, 1);
+g_atomic_int_set(>u.s.refs, 1);
 
 PROBE(OBJECT_NEW, "obj=%p classname=%s", obj, obj->klass->name);
 
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 5b66094de5..6b47f4fa36 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -160,13 +160,13 @@ virSystemdHasMachined(void)
 
 if ((ret = virDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) {
 if (ret == -2)
-virAtomicIntSet(, -2);
+g_atomic_int_set(, -2);
 return ret;
 }
 
 if ((ret = virDBusIsServiceRegistered("org.freedesktop.systemd1")) == -1)
 return ret;
-virAtomicIntSet(, ret);
+g_atomic_int_set(, ret);
 return ret;
 }
 
@@ -183,14 +183,14 @@ virSystemdHasLogind(void)
 ret = virDBusIsServiceEnabled("org.freedesktop.login1");
 if (ret < 0) {
 if (ret == -2)
-virAtomicIntSet(, -2);
+g_atomic_int_set(, -2);
 return ret;
 }
 
 if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) == -1)
 return ret;
 
-virAtomicIntSet(, ret);
+g_atomic_int_set(, ret);
 return ret;
 }
 
@@ -386,7 +386,7 @@ int virSystemdCreateMachine(const char *name,
 VIR_INFO("CreateMachineWithNetwork isn't supported, switching "
  "to legacy CreateMachine method for 
systemd-machined");
 virResetError();
-virAtomicIntSet(, 0);
+g_atomic_int_set(, 0);
 /* Could re-structure without Using goto, but this
  * avoids another atomic read which would trigger
  * another memory barrier */
-- 
2.24.1

[PATCH 7/7] Drop virAtomic module

[Michal Privoznik]

Now, that every use of virAtomic was replaced with its g_atomic
equivalent, let's remove the module.

Signed-off-by: Michal Privoznik 
---
 configure.ac  |  1 -
 m4/virt-atomic.m4 | 77 --
 src/Makefile.am   |  6 ---
 src/libvirt_atomic.syms   | 11 
 src/libxl/libxl_domain.c  |  1 -
 src/libxl/libxl_driver.c  |  1 -
 src/lxc/lxc_process.c |  1 -
 src/nwfilter/nwfilter_dhcpsnoop.c |  1 -
 src/qemu/qemu_conf.c  |  1 -
 src/qemu/qemu_domain.c|  1 -
 src/qemu/qemu_process.c   |  1 -
 src/test/test_driver.c|  1 -
 src/util/Makefile.inc.am  |  2 -
 src/util/viratomic.c  | 35 
 src/util/viratomic.h  | 90 ---
 src/util/virobject.c  |  1 -
 src/util/virprocess.c |  1 -
 src/util/virsystemd.c |  1 -
 18 files changed, 233 deletions(-)
 delete mode 100644 m4/virt-atomic.m4
 delete mode 100644 src/libvirt_atomic.syms
 delete mode 100644 src/util/viratomic.c
 delete mode 100644 src/util/viratomic.h

diff --git a/configure.ac b/configure.ac
index b1f75fa751..5bd9bc841a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -302,7 +302,6 @@ LIBVIRT_ARG_YAJL
 
 LIBVIRT_CHECK_ACL
 LIBVIRT_CHECK_APPARMOR
-LIBVIRT_CHECK_ATOMIC
 LIBVIRT_CHECK_ATTR
 LIBVIRT_CHECK_AUDIT
 LIBVIRT_CHECK_BASH_COMPLETION
diff --git a/m4/virt-atomic.m4 b/m4/virt-atomic.m4
deleted file mode 100644
index d96f7a2bac..00
--- a/m4/virt-atomic.m4
+++ /dev/null
@@ -1,77 +0,0 @@
-dnl The atomic implementation check
-dnl
-dnl Copyright (C) 2016 Red Hat, Inc.
-dnl
-dnl This library is free software; you can redistribute it and/or
-dnl modify it under the terms of the GNU Lesser General Public
-dnl License as published by the Free Software Foundation; either
-dnl version 2.1 of the License, or (at your option) any later version.
-dnl
-dnl This library is distributed in the hope that it will be useful,
-dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
-dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-dnl Lesser General Public License for more details.
-dnl
-dnl You should have received a copy of the GNU Lesser General Public
-dnl License along with this library.  If not, see
-dnl .
-dnl
-
-AC_DEFUN([LIBVIRT_CHECK_ATOMIC], [
-  AC_REQUIRE([LIBVIRT_CHECK_PTHREAD])
-
-  dnl We need to decide at configure time if libvirt will use real atomic
-  dnl operations ("lock free") or emulated ones with a mutex.
-  dnl
-  dnl Note that the atomic ops are only available with GCC on x86 when
-  dnl using -march=i486 or higher.  If we detect that the atomic ops are
-  dnl not available but would be available given the right flags, we want
-  dnl to abort and advise the user to fix their CFLAGS.  It's better to do
-  dnl that then to silently fall back on emulated atomic ops just because
-  dnl the user had the wrong build environment.
-
-  atomic_ops=
-
-  AC_MSG_CHECKING([for atomic ops implementation])
-
-  AC_TRY_COMPILE([], [__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4;],[
-atomic_ops=gcc
-  ],[])
-
-  if test "$atomic_ops" = "" ; then
-SAVE_CFLAGS="${CFLAGS}"
-CFLAGS="-march=i486"
-AC_TRY_COMPILE([],
-   [__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4;],
-   [AC_MSG_ERROR([Libvirt must be built with -march=i486 or 
later.])],
-   [])
-CFLAGS="${SAVE_CFLAGS}"
-
-case "$host" in
-  *-*-mingw* | *-*-msvc* )
-atomic_ops=win32
-;;
-  *)
-if test "$ac_cv_header_pthread_h" = "yes" ; then
-  atomic_ops=pthread
-else
-  AC_MSG_ERROR([Libvirt must be built with GCC or have pthread.h on 
non-Win32 platforms])
-fi
-;;
-esac
-  fi
-
-  case "$atomic_ops" in
-gcc)
-  AC_DEFINE([VIR_ATOMIC_OPS_GCC],[1],[Use GCC atomic ops])
-  ;;
-win32)
-  AC_DEFINE([VIR_ATOMIC_OPS_WIN32],[1],[Use Win32 atomic ops])
-  ;;
-pthread)
-  AC_DEFINE([VIR_ATOMIC_OPS_PTHREAD],[1],[Use pthread atomic ops 
emulation])
-  ;;
-  esac
-  AM_CONDITIONAL([WITH_ATOMIC_OPS_PTHREAD],[test "$atomic_ops" = "pthread"])
-  AC_MSG_RESULT([$atomic_ops])
-])
diff --git a/src/Makefile.am b/src/Makefile.am
index 58355c5337..7bb6127ca4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -390,12 +390,6 @@ else ! WITH_SSH2
 SYM_FILES += $(srcdir)/libvirt_libssh2.syms
 endif ! WITH_SSH2
 
-if WITH_ATOMIC_OPS_PTHREAD
-USED_SYM_FILES += $(srcdir)/libvirt_atomic.syms
-else ! WITH_ATOMIC_OPS_PTHREAD
-SYM_FILES += $(srcdir)/libvirt_atomic.syms
-endif ! WITH_ATOMIC_OPS_PTHREAD
-
 if WITH_LIBSSH
 USED_SYM_FILES += $(srcdir)/libvirt_libssh.syms
 else ! WITH_LIBSSH
diff --git a/src/libvirt_atomic.syms b/src/libvirt_atomic.syms
deleted file mode 100644
index e2c23637d1..00
--- a/src/libvirt_atomic.syms
+++ /dev/null
@@ -1,11 +0,0 @@
-#
-# 

[PATCH 3/7] src: Replace virAtomicIntGet() with g_atomic_int_get()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/nwfilter/nwfilter_dhcpsnoop.c | 12 ++--
 src/util/viratomic.h  |  9 -
 src/util/virsystemd.c |  6 +++---
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index a1c0c0189e..074cffecb2 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -601,7 +601,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req)
 if (!req)
 return;
 
-if (virAtomicIntGet(>refctr) != 0)
+if (g_atomic_int_get(>refctr) != 0)
 return;
 
 /* free all leases */
@@ -1477,7 +1477,7 @@ virNWFilterDHCPSnoopThread(void *req0)
 unsigned int diff;
 
 /* submit packet to worker thread */
-if (virAtomicIntGet([i].qCtr) >
+if (g_atomic_int_get([i].qCtr) >
 pcapConf[i].maxQSize) {
 if (last_displayed_queue - time(0) > 10) {
 last_displayed_queue = time(0);
@@ -1778,7 +1778,7 @@ virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr 
ipl)
 
 /* keep dead leases at < ~95% of file size */
 if (virAtomicIntInc() >=
-virAtomicIntGet() * 20)
+g_atomic_int_get() * 20)
 virNWFilterSnoopLeaseFileLoad();   /* load & refresh lease file */
 
  err_exit:
@@ -1809,7 +1809,7 @@ virNWFilterSnoopPruneIter(const void *payload,
 /*
  * have the entry removed if it has no leases and no one holds a ref
  */
-del_req = ((req->start == NULL) && (virAtomicIntGet(>refctr) == 0));
+del_req = ((req->start == NULL) && (g_atomic_int_get(>refctr) == 0));
 
 virNWFilterSnoopReqUnlock(req);
 
@@ -1973,9 +1973,9 @@ virNWFilterSnoopLeaseFileLoad(void)
 static void
 virNWFilterSnoopJoinThreads(void)
 {
-while (virAtomicIntGet() != 0) {
+while (g_atomic_int_get() != 0) {
 VIR_WARN("Waiting for snooping threads to terminate: %u",
- virAtomicIntGet());
+ g_atomic_int_get());
 g_usleep(1000 * 1000);
 }
 }
diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 12b116a6cd..6c1764f7bf 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -26,15 +26,6 @@
 
 #include "internal.h"
 
-/**
- * virAtomicIntGet:
- * Gets the current value of atomic.
- *
- * This call acts as a full compiler and hardware memory barrier
- * (before the get)
- */
-#define virAtomicIntGet(v) g_atomic_int_get(v)
-
 /**
  * virAtomicIntSet:
  * Sets the value of atomic to newval.
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index a9ff782fb8..5b66094de5 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -154,7 +154,7 @@ virSystemdHasMachined(void)
 int ret;
 int val;
 
-val = virAtomicIntGet();
+val = g_atomic_int_get();
 if (val != -1)
 return val;
 
@@ -176,7 +176,7 @@ virSystemdHasLogind(void)
 int ret;
 int val;
 
-val = virAtomicIntGet();
+val = g_atomic_int_get();
 if (val != -1)
 return val;
 
@@ -352,7 +352,7 @@ int virSystemdCreateMachine(const char *name,
  */
 
 VIR_DEBUG("Attempting to create machine via systemd");
-if (virAtomicIntGet()) {
+if (g_atomic_int_get()) {
 virError error;
 memset(, 0, sizeof(error));
 
-- 
2.24.1

[PATCH 0/7] Drop virAtomic module

[Michal Privoznik]

Inspired by:

https://www.redhat.com/archives/libvir-list/2020-January/msg01446.html

Instead of using:

  #define virAtomicIntXXX g_atomic_int_XXX

let's use the GLib directly and drop needless defines.

Michal Prívozník (7):
  test_driver: Replace virAtomicIntAdd() with virAtomicIntInc()
  tests: Drop viratomictest
  src: Replace virAtomicIntGet() with g_atomic_int_get()
  src: Replace virAtomicIntSet() with g_atomic_int_set()
  src: Replace virAtomicIntInc() with g_atomic_int_add()
  src: Drop virAtomicIntDecAndTest() with g_atomic_int_dec_and_test()
  Drop virAtomic module

 configure.ac  |   1 -
 m4/virt-atomic.m4 |  77 -
 src/Makefile.am   |   6 -
 src/libvirt_atomic.syms   |  11 --
 src/libxl/libxl_domain.c  |   5 +-
 src/libxl/libxl_driver.c  |   3 +-
 src/lxc/lxc_process.c |   7 +-
 src/nwfilter/nwfilter_dhcpsnoop.c |  33 +++---
 src/qemu/qemu_conf.c  |   1 -
 src/qemu/qemu_domain.c|   1 -
 src/qemu/qemu_process.c   |   7 +-
 src/test/test_driver.c|   7 +-
 src/util/Makefile.inc.am  |   2 -
 src/util/viratomic.c  |  35 --
 src/util/viratomic.h  | 130 --
 src/util/virobject.c  |   9 +-
 src/util/virprocess.c |   3 +-
 src/util/virsystemd.c |  17 ++-
 tests/Makefile.am |   5 -
 tests/viratomictest.c | 175 --
 20 files changed, 41 insertions(+), 494 deletions(-)
 delete mode 100644 m4/virt-atomic.m4
 delete mode 100644 src/libvirt_atomic.syms
 delete mode 100644 src/util/viratomic.c
 delete mode 100644 src/util/viratomic.h
 delete mode 100644 tests/viratomictest.c

-- 
2.24.1

Re: [PATCH] lib: Prohibit parallel connections with tunneled migration

[Jim Fehlig]

Any comments on this patch?

Regards,
Jim

On 1/21/20 8:00 PM, Jim Fehlig wrote:

As discussed on the developer list, parallel migration connections
are not compatible with tunneled migration

https://www.redhat.com/archives/libvir-list/2020-January/msg00463.html

Prohibit the concurrent use of parallel and tunneled migration options.

Signed-off-by: Jim Fehlig 
---

I added the check to all migration entry points except virDomainMigrate3,
where the p2p and tunneled options are already prohibitied.

  src/libvirt-domain.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4074397b30..b910ba6b4d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3546,6 +3546,10 @@ virDomainMigrate(virDomainPtr domain,
   VIR_MIGRATE_NON_SHARED_INC,
   error);
  
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,

+ VIR_MIGRATE_PARALLEL,
+ error);
+
  if (flags & VIR_MIGRATE_OFFLINE) {
  if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
@@ -3701,6 +3705,10 @@ virDomainMigrate2(virDomainPtr domain,
   VIR_MIGRATE_NON_SHARED_INC,
   error);
  
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,

+ VIR_MIGRATE_PARALLEL,
+ error);
+
  if (flags & VIR_MIGRATE_OFFLINE) {
  if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
@@ -4087,6 +4095,10 @@ virDomainMigrateToURI(virDomainPtr domain,
  virCheckReadOnlyGoto(domain->conn->flags, error);
  virCheckNonNullArgGoto(duri, error);
  
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,

+ VIR_MIGRATE_PARALLEL,
+ error);
+
  if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
  goto error;
  
@@ -4159,6 +4171,10 @@ virDomainMigrateToURI2(virDomainPtr domain,

  virCheckDomainReturn(domain, -1);
  virCheckReadOnlyGoto(domain->conn->flags, error);
  
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,

+ VIR_MIGRATE_PARALLEL,
+ error);
+
  if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
  goto error;
  
@@ -4232,6 +4248,10 @@ virDomainMigrateToURI3(virDomainPtr domain,

  virCheckDomainReturn(domain, -1);
  virCheckReadOnlyGoto(domain->conn->flags, error);
  
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED,

+ VIR_MIGRATE_PARALLEL,
+ error);
+
  if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
  goto error;
  

[PATCH 3/5] qemu: Extend QEMU with tpm-spapr support

[Stefan Berger]

Extend QEMU with tpm-spapr support.

Signed-off-by: Stefan Berger 
---
 src/qemu/qemu_command.c| 15 ++-
 src/qemu/qemu_domain.c |  2 ++
 src/qemu/qemu_domain_address.c | 10 ++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c8195cfbb9..3df72232ca 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9064,15 +9064,19 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
 
 
 static char *
-qemuBuildTPMDevStr(const virDomainDef *def)
+qemuBuildTPMDevStr(const virDomainDef *def,
+   virQEMUCapsPtr qemuCaps)
 {
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-const virDomainTPMDef *tpm = def->tpm;
+virDomainTPMDef *tpm = def->tpm;
 const char *model = virDomainTPMModelTypeToString(tpm->model);
 
 virBufferAsprintf(, "%s,tpmdev=tpm-%s,id=%s",
   model, tpm->info.alias, tpm->info.alias);
 
+if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
+return NULL;
+
 return virBufferContentAndReset();
 }
 
@@ -9162,7 +9166,8 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
 
 static int
 qemuBuildTPMCommandLine(virCommandPtr cmd,
-const virDomainDef *def)
+const virDomainDef *def,
+virQEMUCapsPtr qemuCaps)
 {
 char *optstr;
 g_autofree char *chardev = NULL;
@@ -9202,7 +9207,7 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
 VIR_FREE(fdset);
 }
 
-if (!(optstr = qemuBuildTPMDevStr(def)))
+if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps)))
 return -1;
 
 virCommandAddArgList(cmd, "-device", optstr, NULL);
@@ -9918,7 +9923,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 chardevStdioLogd) < 0)
 return NULL;
 
-if (qemuBuildTPMCommandLine(cmd, def) < 0)
+if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0)
 return NULL;
 
 if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ace611909d..099a782834 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7795,6 +7795,8 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
 flag = QEMU_CAPS_DEVICE_TPM_CRB;
 break;
 case VIR_DOMAIN_TPM_MODEL_SPAPR:
+flag = QEMU_CAPS_DEVICE_TPM_SPAPR;
+break;
 case VIR_DOMAIN_TPM_MODEL_LAST:
 default:
 virReportEnumRangeError(virDomainTPMModel, tpm->model);
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 409d850a06..ef17e84d7c 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -35,6 +35,7 @@ VIR_LOG_INIT("qemu.qemu_domain_address");
 #define VIO_ADDR_SCSI 0x2000ul
 #define VIO_ADDR_SERIAL 0x3000ul
 #define VIO_ADDR_NVRAM 0x3000ul
+#define VIO_ADDR_TPM 0x4000ul
 
 
 /**
@@ -267,6 +268,15 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
 return -1;
 }
 
+if (def->tpm) {
+if (ARCH_IS_PPC64(def->os.arch) &&
+STRPREFIX(def->os.machine, "pseries"))
+def->tpm->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
+if (qemuDomainAssignSpaprVIOAddress(def, >tpm->info,
+VIO_ADDR_TPM) < 0)
+return -1;
+}
+
 /* No other devices are currently supported on spapr-vio */
 
 return 0;
-- 
2.17.1

[PATCH 0/5] Add support for SPAPR vTPM for pSeries VM

[Stefan Berger]

QEMU 5.0 will have SPAPR vTPM support. This series of patches
adds support for the XML and command line creation of the 
SPAPR vTPM for pSeries VMs along with test cases.

Regards,
   Stefan

Stefan Berger (5):
  conf: Add support for tpm-spapr to domain XML
  qemu: Extend QEMU capabilities with 'tpm-spapr'
  qemu: Extend QEMU with tpm-spapr support
  tests: Extend ppc64 capabilities data with TPM related XML and
responses
  tests: Add test for domain XML with tpm-spapr TPM device model

 docs/formatdomain.html.in | 4 +-
 docs/schemas/domaincommon.rng | 4 +
 src/conf/domain_conf.c|24 +-
 src/conf/domain_conf.h| 1 +
 src/qemu/qemu_capabilities.c  | 6 +
 src/qemu/qemu_capabilities.h  | 1 +
 src/qemu/qemu_command.c   |15 +-
 src/qemu/qemu_domain.c| 8 +-
 src/qemu/qemu_domain_address.c|10 +
 tests/domaincapsdata/qemu_5.0.0.ppc64.xml |   115 +
 .../caps_5.0.0.ppc64.replies  | 24695 
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1128 +
 .../tpm-emulator-spapr.ppc64-latest.args  |45 +
 tests/qemuxml2argvdata/tpm-emulator-spapr.xml |60 +
 tests/qemuxml2argvtest.c  | 4 +
 15 files changed, 26105 insertions(+), 15 deletions(-)
 create mode 100644 tests/domaincapsdata/qemu_5.0.0.ppc64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-spapr.ppc64-latest.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-spapr.xml

-- 
2.17.1

[PATCH 2/5] qemu: Extend QEMU capabilities with 'tpm-spapr'

[Stefan Berger]

Extend the QEMU capabilties with tpm-spapr support.

Signed-off-by: Stefan Berger 
---
 src/qemu/qemu_capabilities.c | 6 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8b7fde2bcc..02ca023f27 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -559,6 +559,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "i8042",
   "rng-builtin",
   "virtio-net.failover",
+  "tpm-spapr",
 );
 
 
@@ -1275,6 +1276,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "max-arm-cpu", QEMU_CAPS_ARM_MAX_CPU },
 { "i8042", QEMU_CAPS_DEVICE_I8042 },
 { "rng-builtin", QEMU_CAPS_OBJECT_RNG_BUILTIN },
+{ "tpm-spapr", QEMU_CAPS_DEVICE_TPM_SPAPR },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
@@ -3028,6 +3030,10 @@ const struct tpmTypeToCaps virQEMUCapsTPMModelsToCaps[] 
= {
 .type = VIR_DOMAIN_TPM_MODEL_CRB,
 .caps = QEMU_CAPS_DEVICE_TPM_CRB,
 },
+{
+.type = VIR_DOMAIN_TPM_MODEL_SPAPR,
+.caps = QEMU_CAPS_DEVICE_TPM_SPAPR,
+},
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d86f54a481..b97c11ee1d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -540,6 +540,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_I8042, /* PS/2 controller */
 QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */
 QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */
+QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.17.1

[PATCH 1/5] conf: Add support for tpm-spapr to domain XML

[Stefan Berger]

From: Stefan Berger 

This patch adds support for the tpm-spapr device model for ppc64. The XML for
this type of TPM looks as follows:

 
   
 

Extend the documentation.

Signed-off-by: Stefan Berger 
---
 docs/formatdomain.html.in |  4 +++-
 docs/schemas/domaincommon.rng |  4 
 src/conf/domain_conf.c| 24 +---
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_domain.c|  6 --
 5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 60a103d7c6..5a9835fbfe 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -8508,7 +8508,9 @@ qemu-kvm -net nic,model=? /dev/null
 
   The model attribute specifies what device
   model QEMU provides to the guest. If no model name is provided,
-  tpm-tis will automatically be chosen.
+  tpm-tis will automatically be chosen for non-ppc64
+  architectures. For ppc64/pseries guests, tpm-spapr
+  is the default.
   Since 4.4.0, another available choice
   is the tpm-crb, which should only be used when the
   backend device is a TPM 2.0.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ea237a05e5..9577d26c2a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4383,6 +4383,7 @@
   
 tpm-tis
 tpm-crb
+tpm-spapr
   
 
   
@@ -4390,6 +4391,9 @@
   
 
   
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b60db7ecd..a55be400fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1127,6 +1127,7 @@ VIR_ENUM_IMPL(virDomainTPMModel,
   VIR_DOMAIN_TPM_MODEL_LAST,
   "tpm-tis",
   "tpm-crb",
+  "tpm-spapr",
 );
 
 VIR_ENUM_IMPL(virDomainTPMBackend,
@@ -13242,7 +13243,8 @@ static virDomainTPMDefPtr
 virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 xmlNodePtr node,
 xmlXPathContextPtr ctxt,
-unsigned int flags)
+unsigned int flags,
+virArch arch)
 {
 virDomainTPMDefPtr def;
 VIR_XPATH_NODE_AUTORESTORE(ctxt);
@@ -13258,11 +13260,17 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 return NULL;
 
 model = virXMLPropString(node, "model");
-if (model != NULL &&
-(def->model = virDomainTPMModelTypeFromString(model)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+if (model != NULL) {
+if ((def->model = virDomainTPMModelTypeFromString(model)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown TPM frontend model '%s'"), model);
-goto error;
+goto error;
+}
+} else {
+if (ARCH_IS_PPC64(arch))
+def->model = VIR_DOMAIN_TPM_MODEL_SPAPR;
+else
+def->model = VIR_DOMAIN_TPM_MODEL_TIS;
 }
 
 ctxt->node = node;
@@ -16639,7 +16647,8 @@ virDomainDeviceDefParse(const char *xmlStr,
 return NULL;
 break;
 case VIR_DOMAIN_DEVICE_TPM:
-if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, 
flags)))
+if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, 
flags,
+  def->os.arch)))
 return NULL;
 break;
 case VIR_DOMAIN_DEVICE_PANIC:
@@ -21464,7 +21473,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 
 if (n > 0) {
-if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, 
flags)))
+if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags,
+ def->os.arch)))
 goto error;
 }
 VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e144f3aad3..19732fcfc9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1254,6 +1254,7 @@ struct _virDomainHubDef {
 typedef enum {
 VIR_DOMAIN_TPM_MODEL_TIS,
 VIR_DOMAIN_TPM_MODEL_CRB,
+VIR_DOMAIN_TPM_MODEL_SPAPR,
 
 VIR_DOMAIN_TPM_MODEL_LAST
 } virDomainTPMModel;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3045b4bcd..ace611909d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7756,9 +7756,10 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm,
 
 switch (tpm->version) {
 case VIR_DOMAIN_TPM_VERSION_1_2:
-/* only TIS available for emulator */
+/* only TIS available for emulator (non-ppc64 case) */
 if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
-tpm->model != VIR_DOMAIN_TPM_MODEL_TIS) {
+tpm->model != VIR_DOMAIN_TPM_MODEL_TIS &&
+!ARCH_IS_PPC64(def->os.arch)) {
 

[PATCH 5/5] tests: Add test for domain XML with tpm-spapr TPM device model

[Stefan Berger]

This patch adds a test case for domain XML with the tpm-spapr TPM device
model.

Signed-off-by: Stefan Berger 
---
 .../tpm-emulator-spapr.ppc64-latest.args  | 45 ++
 tests/qemuxml2argvdata/tpm-emulator-spapr.xml | 60 +++
 tests/qemuxml2argvtest.c  |  4 ++
 3 files changed, 109 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-spapr.ppc64-latest.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-spapr.xml

diff --git a/tests/qemuxml2argvdata/tpm-emulator-spapr.ppc64-latest.args 
b/tests/qemuxml2argvdata/tpm-emulator-spapr.ppc64-latest.args
new file mode 100644
index 00..f4c8a062c1
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator-spapr.ppc64-latest.args
@@ -0,0 +1,45 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-TPM-VM \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-TPM-VM/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-TPM-VM/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-TPM-VM/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest=TPM-VM,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-TPM-VM/master-key.aes \
+-machine pseries-5.0,accel=tcg,usb=off,dump-guest-core=off \
+-cpu POWER9 \
+-m 2048 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot menu=on,strict=on \
+-device pci-ohci,id=usb,bus=pci.0,addr=0x1 \
+-device spapr-vscsi,id=scsi0,reg=0x2000 \
+-device spapr-vscsi,id=scsi1,reg=0x3000 \
+-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi1-0-0-0 \
+-device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,\
+device_id=drive-scsi1-0-0-0,drive=drive-scsi1-0-0-0,id=scsi1-0-0-0,bootindex=1 
\
+-chardev pty,id=charserial0 \
+-device spapr-vty,chardev=charserial0,id=serial0,reg=0x3000 \
+-chardev pty,id=charserial1 \
+-device spapr-vty,chardev=charserial1,id=serial1,reg=0x30001000 \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-spapr,tpmdev=tpm-tpm0,id=tpm0,reg=0x5000 \
+-global spapr-nvram.reg=0x4000 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/tpm-emulator-spapr.xml 
b/tests/qemuxml2argvdata/tpm-emulator-spapr.xml
new file mode 100644
index 00..f9cda19727
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator-spapr.xml
@@ -0,0 +1,60 @@
+
+  TPM-VM
+  11d7cd22-da89-3094-6212-079a48a309a1
+  2097152
+  512288
+  1
+  
+hvm
+
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-ppc64
+
+  
+
+
+
+
+  
+
+
+  
+
+
+
+
+  
+  
+  
+  
+
+
+
+
+  
+
+
+  
+
+
+
+  
+
+
+
+
+  
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a36183bf34..5691317bad 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -738,6 +738,9 @@ mymain(void)
 # define DO_TEST_CAPS_VER(name, ver) \
 DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
 
+# define DO_TEST_CAPS_LATEST_PPC64(name) \
+DO_TEST_CAPS_ARCH_LATEST(name, "ppc64")
+
 # define DO_TEST_CAPS_ARCH_LATEST_FAILURE(name, arch) \
 DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, \
   ARG_FLAGS, FLAG_EXPECT_FAILURE)
@@ -2193,6 +2196,7 @@ mymain(void)
 DO_TEST_CAPS_LATEST("tpm-emulator");
 DO_TEST_CAPS_LATEST("tpm-emulator-tpm2");
 DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc");
+DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr");
 
 DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
 DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE);
-- 
2.17.1

Re: [PATCH v2 0/2] finish qemu-nbd --partition deprecation

[Eric Blake]

ping

On 1/23/20 10:46 AM, Eric Blake wrote:

Based-on: <20200116141511.16849-1-peter.mayd...@linaro.org>
(0/3 convert qemu-nbd, qemu-block-drivers to rST)

In v2:
- rebased on top of rST doc changes
- patch 1 added

Eric Blake (2):
   docs: Fix typo in qemu-nbd -P replacement
   qemu-nbd: Removed deprecated --partition option

  docs/interop/qemu-nbd.rst |  15 ++---
  qemu-deprecated.texi  |  49 ++
  qemu-nbd.c| 133 +-
  3 files changed, 24 insertions(+), 173 deletions(-)



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[libvirt PATCH 2/5] qemu: Add the QEMU_CAPS_CPU_KVM_NO_ADJVTIME capability

[Andrea Bolognani]

We will use this capability to detect whether the QEMU binary
supports the kvm-no-adjvtime CPU feature.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c  | 2 ++
 src/qemu/qemu_capabilities.h  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8b7fde2bcc..4aa77c787d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -559,6 +559,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "i8042",
   "rng-builtin",
   "virtio-net.failover",
+  "cpu.kvm-no-adjvtime",
 );
 
 
@@ -1561,6 +1562,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsMemoryBackendMemfd[]
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = {
 { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES },
+{ "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME },
 };
 
 static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d86f54a481..db2334d4bf 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -540,6 +540,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_I8042, /* PS/2 controller */
 QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */
 QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */
+QEMU_CAPS_CPU_KVM_NO_ADJVTIME, /* -cpu ...,kvm-no-adjvtime */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
index 538b40dd5b..c05cea2eb7 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
@@ -179,6 +179,7 @@
   
   
   
+  
   4002050
   0
   61700241
-- 
2.24.1

[libvirt PATCH 5/5] tests: Add tests for the kvm-no-adjvtime CPU feature

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 .../aarch64-features-misc.aarch64-latest.args | 32 +++
 .../aarch64-features-misc.xml | 17 ++
 tests/qemuxml2argvtest.c  |  2 ++
 3 files changed, 51 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/aarch64-features-misc.aarch64-latest.args
 create mode 100644 tests/qemuxml2argvdata/aarch64-features-misc.xml

diff --git a/tests/qemuxml2argvdata/aarch64-features-misc.aarch64-latest.args 
b/tests/qemuxml2argvdata/aarch64-features-misc.aarch64-latest.args
new file mode 100644
index 00..f1d1017d59
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-features-misc.aarch64-latest.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-guest/master-key.aes \
+-machine virt,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 \
+-cpu host,kvm-no-adjvtime=on \
+-m 1024 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/aarch64-features-misc.xml 
b/tests/qemuxml2argvdata/aarch64-features-misc.xml
new file mode 100644
index 00..49e197ad87
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-features-misc.xml
@@ -0,0 +1,17 @@
+
+  guest
+  1ccfd97d-5eb4-478a-bbe6-88d254c16db7
+  1048576
+  1
+  
+hvm
+  
+  
+
+  
+  
+/usr/bin/qemu-system-aarch64
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a36183bf34..81b82bcd1f 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2725,6 +2725,8 @@ mymain(void)
 /* SVE aarch64 CPU features work on modern QEMU */
 DO_TEST_CAPS_ARCH_LATEST("aarch64-features-sve", "aarch64");
 
+DO_TEST_CAPS_ARCH_LATEST("aarch64-features-misc", "aarch64");
+
 qemuTestSetHostArch(, VIR_ARCH_NONE);
 
 DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);
-- 
2.24.1

[libvirt PATCH 3/5] cpu: Add the kvm-no-adjvtime CPU feature

[Andrea Bolognani]

The feature is currently available on ARM only, and so it only
needs to be known to the corresponding CPU driver.

Signed-off-by: Andrea Bolognani 
---
 src/cpu_map/arm_features.xml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/cpu_map/arm_features.xml b/src/cpu_map/arm_features.xml
index 8a53384463..13d00f07a8 100644
--- a/src/cpu_map/arm_features.xml
+++ b/src/cpu_map/arm_features.xml
@@ -19,4 +19,7 @@
   
   
 
+  
+  
+
 
-- 
2.24.1

[libvirt PATCH 4/5] qemu: Validate the kvm-no-adjvtime CPU feature

[Andrea Bolognani]

Error out if the QEMU binary doesn't support it.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_domain.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3045b4bcd..8b0c91d6c1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4625,7 +4625,8 @@ qemuDomainDefSetDefaultCPU(virDomainDefPtr def,
 
 
 static int
-qemuDomainDefCPUPostParse(virDomainDefPtr def)
+qemuDomainDefCPUPostParse(virDomainDefPtr def,
+  virQEMUCapsPtr qemuCaps)
 {
 virCPUFeatureDefPtr sveFeature = NULL;
 bool sveVectorLengthsProvided = false;
@@ -4696,6 +4697,15 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def)
 } else if (STRPREFIX(feature->name, "sve")) {
 sveVectorLengthsProvided = true;
 }
+
+if (STREQ(feature->name, "kvm-no-adjvtime") &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME))
+{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("The '%s' feature is not supported by this "
+ "QEMU binary"), feature->name);
+return -1;
+}
 }
 
 if (sveVectorLengthsProvided) {
@@ -4858,7 +4868,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 if (qemuDomainDefVcpusPostParse(def) < 0)
 return -1;
 
-if (qemuDomainDefCPUPostParse(def) < 0)
+if (qemuDomainDefCPUPostParse(def, qemuCaps) < 0)
 return -1;
 
 if (qemuDomainDefTsegPostParse(def, qemuCaps) < 0)
-- 
2.24.1

[libvirt PATCH 1/5] tests: Add capabilities for QEMU 5.0.0 on aarch64

[Andrea Bolognani]

This is the first QEMU version that supports the kvm-no-adjvtime
CPU feature.

Signed-off-by: Andrea Bolognani 
---
This patch has been trimmed.

 .../qemu_5.0.0-virt.aarch64.xml   |   156 +
 tests/domaincapsdata/qemu_5.0.0.aarch64.xml   |   150 +
 .../caps_5.0.0.aarch64.replies| 22717 
 .../caps_5.0.0.aarch64.xml|   456 +
 4 files changed, 23479 insertions(+)
 create mode 100644 tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml
 create mode 100644 tests/domaincapsdata/qemu_5.0.0.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml

diff --git a/tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml 
b/tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml
new file mode 100644
index 00..f92ae951ca
--- /dev/null
+++ b/tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml
@@ -0,0 +1,156 @@
+
+  /usr/bin/qemu-system-aarch64
+  kvm
+  virt-5.0
+  aarch64
[...]
diff --git a/tests/domaincapsdata/qemu_5.0.0.aarch64.xml 
b/tests/domaincapsdata/qemu_5.0.0.aarch64.xml
new file mode 100644
index 00..232aacec2f
--- /dev/null
+++ b/tests/domaincapsdata/qemu_5.0.0.aarch64.xml
@@ -0,0 +1,150 @@
+
+  /usr/bin/qemu-system-aarch64
+  kvm
+  integratorcp
+  aarch64
[...]
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.replies 
b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.replies
new file mode 100644
index 00..a3136a0966
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.replies
@@ -0,0 +1,22717 @@
+{
+  "execute": "qmp_capabilities",
+  "id": "libvirt-1"
+}
+
+{
+  "return": {
+  },
+  "id": "libvirt-1"
+}
+
+{
+  "execute": "query-version",
+  "id": "libvirt-2"
+}
+
+{
+  "return": {
+"qemu": {
+  "micro": 50,
+  "minor": 2,
+  "major": 4
+},
+"package": "v4.2.0-1157-gadcd6e93b9"
+  },
+  "id": "libvirt-2"
+}
[...]
+{
+  "execute": "query-cpu-model-expansion",
+  "arguments": {
+"type": "full",
+"model": {
+  "name": "host"
+}
+  },
+  "id": "libvirt-42"
+}
+
+{
+  "return": {
+"model": {
+  "name": "host",
+  "props": {
+"sve768": false,
+"sve128": false,
+"sve1024": false,
+"sve1280": false,
+"sve896": false,
+"sve256": false,
+"sve1536": false,
+"sve1792": false,
+"sve384": false,
+"sve": false,
+"sve2048": false,
+"kvm-no-adjvtime": false,
+"sve512": false,
+"aarch64": true,
+"pmu": true,
+"sve1920": false,
+"sve1152": false,
+"sve640": false,
+"sve1408": false,
+"sve1664": false
+  }
+}
+  },
+  "id": "libvirt-42"
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
new file mode 100644
index 00..538b40dd5b
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
@@ -0,0 +1,456 @@
+
+  /usr/bin/qemu-system-aarch64
[...]
+  4002050
+  0
+  61700241
+  v4.2.0-1157-gadcd6e93b9
+  aarch64
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
[...]
-- 
2.24.1

[libvirt PATCH 0/5] qemu: Add support for the kvm-no-adjvtime CPU feature

[Andrea Bolognani]

This ARM-specific CPU feature has been recently added to QEMU with

  https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg04124.html

Patch 1/5 has been trimmed quite liberally: to obtain the unabridged
version of this series, use

  $ git fetch https://gitlab.com/abologna/libvirt kvm-no-adjvtime

Andrea Bolognani (5):
  tests: Add capabilities for QEMU 5.0.0 on aarch64
  qemu: Add the QEMU_CAPS_CPU_KVM_NO_ADJVTIME capability
  cpu: Add the kvm-no-adjvtime CPU feature
  qemu: Validate the kvm-no-adjvtime CPU feature
  tests: Add tests for the kvm-no-adjvtime CPU feature

 src/cpu_map/arm_features.xml  | 3 +
 src/qemu/qemu_capabilities.c  | 2 +
 src/qemu/qemu_capabilities.h  | 1 +
 src/qemu/qemu_domain.c|14 +-
 .../qemu_5.0.0-virt.aarch64.xml   |   156 +
 tests/domaincapsdata/qemu_5.0.0.aarch64.xml   |   150 +
 .../caps_5.0.0.aarch64.replies| 22717 
 .../caps_5.0.0.aarch64.xml|   457 +
 .../aarch64-features-misc.aarch64-latest.args |32 +
 .../aarch64-features-misc.xml |17 +
 tests/qemuxml2argvtest.c  | 2 +
 11 files changed, 23549 insertions(+), 2 deletions(-)
 create mode 100644 tests/domaincapsdata/qemu_5.0.0-virt.aarch64.xml
 create mode 100644 tests/domaincapsdata/qemu_5.0.0.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
 create mode 100644 
tests/qemuxml2argvdata/aarch64-features-misc.aarch64-latest.args
 create mode 100644 tests/qemuxml2argvdata/aarch64-features-misc.xml

-- 
2.24.1

Re: [libvirt PATCH v2 56/56] gnulib: delete all gnulib integration

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:37PM +, Daniel P. Berrangé wrote:
> This deletes all trace of gnulib from libvirt. We still
> have the keycodemapdb submodule to deal with. The simple
> solution taken was to update it when running autogen.sh.
> 
> Previously gnulib could auto-trigger refresh when running
> 'make' too. We could figure out a solution for this, but
> with the pending meson rewrite it isn't worth worrying
> about, given how infrequently keycodemapdb changes.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  .color_coded.in |2 -
>  .gitignore  |9 +-
>  .gitmodules |3 -
>  .gnulib |1 -
>  .ycm_extra_conf.py.in   |2 -
>  Makefile.am |2 +-
>  README-hacking  |9 +-
>  autogen.sh  |  219 +--
>  bootstrap   | 1073 ---
>  bootstrap.conf  |  100 ---
>  build-aux/syntax-check.mk   |  129 +---
>  ci/build.sh |4 +-
>  config-post.h   |5 +-
>  configure.ac|   11 +-
>  docs/compiling.html.in  |   25 -
>  docs/hacking.html.in|5 +-
>  gnulib/lib/Makefile.am  |   30 -
>  libvirt.spec.in |2 -
>  m4/virt-compile-warnings.m4 |   18 +-
>  src/Makefile.am |7 +-
>  src/admin/Makefile.inc.am   |1 -
>  src/bhyve/Makefile.inc.am   |1 -
>  src/interface/Makefile.inc.am   |1 -
>  src/libxl/Makefile.inc.am   |1 -
>  src/locking/Makefile.inc.am |3 -
>  src/logging/Makefile.inc.am |1 -
>  src/lxc/Makefile.inc.am |2 -
>  src/network/Makefile.inc.am |3 +-
>  src/node_device/Makefile.inc.am |2 -
>  src/nwfilter/Makefile.inc.am|1 -
>  src/qemu/Makefile.inc.am|1 -
>  src/remote/Makefile.inc.am  |1 -
>  src/rpc/virnetsocket.c  |6 -
>  src/secret/Makefile.inc.am  |1 -
>  src/security/Makefile.inc.am|1 -
>  src/storage/Makefile.inc.am |   16 -
>  src/util/viralloc.h |3 +-
>  src/util/virbitmap.c|4 +-
>  src/util/virfile.c  |7 +-
>  src/util/virsocket.h|   15 -
>  src/vbox/Makefile.inc.am|1 -
>  src/vz/Makefile.inc.am  |1 -
>  tests/Makefile.am   |   21 +-
>  tests/virstringtest.c   |3 +-
>  tools/Makefile.am   |9 +-
>  45 files changed, 62 insertions(+), 1700 deletions(-)
>  delete mode 16 .gnulib
>  delete mode 100755 bootstrap
>  delete mode 100644 bootstrap.conf
>  delete mode 100644 gnulib/lib/Makefile.am

You've missed some gnulib related things in build-aux/syntax-check.mk,
with that fixed:

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 55/56] src: remove virFilePrintf in favour of g_fprintf

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:36PM +, Daniel P. Berrangé wrote:
> The virFilePrintf function was a wrapper for fprintf() to provide
> Windows portability, since gnulib's fprintf() replacement was
> license restricted. This is no longer needed now we have the
> g_fprintf function available.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/libvirt_private.syms |  1 -
>  src/util/virfile.c   | 31 -
>  src/util/virfile.h   |  3 ---
>  tests/commandhelper.c|  3 +--
>  tests/fdstreamtest.c | 42 
>  tests/qemusecuritytest.c |  2 +-
>  tests/shunloadtest.c |  2 +-
>  tests/ssh.c  |  3 +--
>  tests/testutils.h|  6 --
>  tests/testutilsqemu.c|  4 ++--
>  tests/virstringtest.c| 34 
>  11 files changed, 44 insertions(+), 87 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 54/56] src: ensure use of g_printf / g_fprintf functions

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:35PM +, Daniel P. Berrangé wrote:
> The printf/fprintf function impls provided on Windows do
> not follow the normal syntax for format specifiers as
> the UNIX hosts. Currently we use GNULIB to provide a
> portability fix for this. GLib has also imported the
> GNULIB impl for this same reason, and thus we can rely
> on the g_printf / g_fprintf functions.
> 
> This identified a couple of places not explicitly
> linking to glib.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/internal.h  | 14 ++
>  src/locking/Makefile.inc.am |  5 -
>  2 files changed, 18 insertions(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 53/56] src: optionally include xlocale.h header

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:34PM +, Daniel P. Berrangé wrote:
> On macOS some definitions are in xlocale.h, instead of in
> locale.h. GNULIB hides this difference by making the latter
> include the former.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure.ac  | 1 +
>  src/util/virgettext.c | 3 +++
>  src/util/virstring.c  | 3 +++
>  tests/vshtabletest.c  | 3 +++
>  4 files changed, 10 insertions(+)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 52/56] src: add define of ENOMSG for MinGW

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:33PM +, Daniel P. Berrangé wrote:
> MinGW lacks ENOMSG until version 6.0.0 (Fedora 31).
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/internal.h | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [PATCH] build: Drop execute permissions on virsh bash completion command

[Ján Tomko]

On Fri, Jan 31, 2020 at 09:01:41AM -0700, Jim Fehlig wrote:

On 1/31/20 8:39 AM, Michal Privoznik wrote:

On 1/31/20 3:38 PM, Jim Fehlig wrote:

rpmlint complains about "script-without-shebang" due to the execute
permissions on /usr/share/bash-completion/completions/vsh. Use
INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
execute permissions.

Signed-off-by: Jim Fehlig 
---

BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
on my installation, only vsh has execute permissions set.

  tools/Makefile.am | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 426a6ba661..d9d1a2f43f 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -357,7 +357,7 @@ libvirt-guests.service: 
libvirt-guests.service.in $(top_builddir)/config.status

  if WITH_BASH_COMPLETION
  install-bash-completion:
  $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
-    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+    $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
  "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
  ( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
  rm -f virsh virt-admin && \



Frankly, I don't know why I wanted it to be executable. Perhaps I 
thought that if it wasn't bash would not load it, but apparently, 
that's not true. BTW, I have another executable file there, but the 
majority isn't.


I should have been as precise :-). Turns out there are a lot of links 
in that directory.



# find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/perf


/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/colormgr


# find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
576


615



$ find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/fwupdate
$ find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
629

If you need more data points.

Jano


Reviewed-by: Michal Privoznik 


Thanks!

Regards,
Jim




signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 51/56] src: assume sys/sysmacros.h always exists on Linux

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:32PM +, Daniel P. Berrangé wrote:
> All our supported Linux distros now have this header.
> It has never existed on FreeBSD / macOS / Mingw.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/conf/domain_audit.c  | 4 +---
>  src/lxc/lxc_controller.c | 4 +---
>  src/lxc/lxc_driver.c | 4 +---
>  src/qemu/qemu_domain.c   | 4 +---
>  src/util/vircgroup.c | 8 +---
>  src/util/virdevmapper.c  | 4 +---
>  src/util/virutil.c   | 4 +---
>  tests/vircgroupmock.c| 8 +---
>  8 files changed, 8 insertions(+), 32 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 50/56] m4: add check for pthread library

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:31PM +, Daniel P. Berrangé wrote:
> When we get rid of GNULIB, we need to check for -lpthread
> support.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  m4/virt-pthread.m4 | 26 ++
>  src/Makefile.am|  2 --
>  tests/Makefile.am  |  2 +-
>  3 files changed, 15 insertions(+), 15 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [PATCH] build: Drop execute permissions on virsh bash completion command

[Jim Fehlig]

On 1/31/20 8:39 AM, Michal Privoznik wrote:

On 1/31/20 3:38 PM, Jim Fehlig wrote:

rpmlint complains about "script-without-shebang" due to the execute
permissions on /usr/share/bash-completion/completions/vsh. Use
INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
execute permissions.

Signed-off-by: Jim Fehlig 
---

BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
on my installation, only vsh has execute permissions set.

  tools/Makefile.am | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 426a6ba661..d9d1a2f43f 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in 
$(top_builddir)/config.status

  if WITH_BASH_COMPLETION
  install-bash-completion:
  $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
-    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+    $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
  "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
  ( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
  rm -f virsh virt-admin && \



Frankly, I don't know why I wanted it to be executable. Perhaps I thought that 
if it wasn't bash would not load it, but apparently, that's not true. BTW, I 
have another executable file there, but the majority isn't.


I should have been as precise :-). Turns out there are a lot of links in that 
directory.



# find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/perf


/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/colormgr


# find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
576


615


Reviewed-by: Michal Privoznik 


Thanks!

Regards,
Jim

Re: [libvirt PATCH v2 48/56] src: convert code to use virPipe APIs

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:29PM +, Daniel P. Berrangé wrote:
> This addreses portability to Windows and standardizes
> error reporting. This fixes a number of places which
> failed to set O_CLOEXEC or failed to report errors.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/libxl/libxl_migration.c |  6 ++
>  src/locking/lock_daemon.c   |  2 +-
>  src/logging/log_daemon.c|  2 +-
>  src/logging/log_handler.c   |  6 ++
>  src/lxc/lxc_process.c   |  5 +
>  src/qemu/qemu_migration.c   | 25 +
>  src/qemu/qemu_tpm.c |  5 +
>  src/remote/remote_daemon.c  |  2 +-
>  src/rpc/virnetdaemon.c  |  5 +
>  src/rpc/virnetsocket.c  |  5 +
>  src/util/vircommand.c   | 18 +-
>  src/util/virfdstream.c  |  5 +
>  src/util/virfile.c  |  5 +
>  src/util/virpolkit.c|  2 +-
>  src/util/virprocess.c   |  5 +
>  tests/commandtest.c |  4 ++--
>  tests/eventtest.c   |  2 +-
>  tests/testutils.c   |  2 +-
>  tools/virsh-domain.c|  8 
>  tools/vsh.c |  8 +++-
>  20 files changed, 40 insertions(+), 82 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 49/56] tools: conditionalize use of O_SYNC flag

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:30PM +, Daniel P. Berrangé wrote:
> The O_SYNC flag is not defined on Windows platforms.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tools/vsh.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

[libvirt PATCH] qemu: drop unused variable

[Ján Tomko]

The g_auto conversion made clang realize the variable is unused:
../../src/qemu/qemu_domain.c:10349:36: error: unused variable
'cfg' [-Werror,-Wunused-variable]
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);

Signed-off-by: Ján Tomko 
Fixes: 20fa2bc6e52e01feaf39d12d38bcf8eaec4c9a46
---
Pushed as a mundanely trivial and depressingly boring build fix.

 src/qemu/qemu_domain.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c5ae3a5210..d3045b4bcd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10346,8 +10346,6 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr 
driver,
  virDomainDiskDefPtr disk,
  qemuDomainLogContextPtr logCtxt)
 {
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-
 if (disk->rawio == VIR_TRISTATE_BOOL_YES)
 qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
logCtxt);
-- 
2.19.2

Re: [libvirt PATCH v2 47/56] src: introduce a wrapper for the pipe2() system call

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:28PM +, Daniel P. Berrangé wrote:
> This hides the differences between Windows and UNIX,
> and adds standard error reporting.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure.ac |  1 +
>  src/libvirt_private.syms |  3 ++
>  src/util/virutil.c   | 64 
>  src/util/virutil.h   | 34 +
>  4 files changed, 102 insertions(+)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH] qemu: Fix domain ID allocation

[Michal Privoznik]

On 1/31/20 4:45 PM, Ján Tomko wrote:

On Fri, Jan 31, 2020 at 04:33:50PM +0100, Michal Privoznik wrote:

On 1/31/20 3:43 PM, Ján Tomko wrote:

The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.

Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:

$ virsh list
 Id   Name   State
--
 1    f28live    running
 1    f28live1   running

Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.

This also restores the usual numbering from 1 instead of 0.

Signed-off-by: Ján Tomko 
Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
---
 src/qemu/qemu_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e5051027fc..0b119cbe78 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1858,7 +1858,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 int qemuDriverAllocateID(virQEMUDriverPtr driver)
 {
-    return virAtomicIntInc(>lastvmid);
+    return g_atomic_int_add(>lastvmid, 1) + 1;
 }



Does it makes sense to replace all virAtomic with g_atomic or do we 


That does make sense. It is encouraged in viratomic.h, as I said in the
commit message. The reason I did not cleanup all of them is the bugfix
nature of this patch.


Aha! So we are already doing that. From our src/util/viratomic.h:

#define virAtomicIntInc(i) g_atomic_int_add(i, 1)

So all we need to do is just drop viratomic.

Michal

Re: [libvirt PATCH] qemu: Fix domain ID allocation

[Ján Tomko]

On Fri, Jan 31, 2020 at 04:33:50PM +0100, Michal Privoznik wrote:

On 1/31/20 3:43 PM, Ján Tomko wrote:

The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.

Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:

$ virsh list
 Id   Name   State
--
 1f28liverunning
 1f28live1   running

Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.

This also restores the usual numbering from 1 instead of 0.

Signed-off-by: Ján Tomko 
Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
---
 src/qemu/qemu_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e5051027fc..0b119cbe78 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1858,7 +1858,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 int qemuDriverAllocateID(virQEMUDriverPtr driver)
 {
-return virAtomicIntInc(>lastvmid);
+return g_atomic_int_add(>lastvmid, 1) + 1;
 }



Does it makes sense to replace all virAtomic with g_atomic or do we 


That does make sense. It is encouraged in viratomic.h, as I said in the
commit message. The reason I did not cleanup all of them is the bugfix
nature of this patch.

still pretend that we care about client library and we don't crash on 
OOM in it?


I sincerely hope that none of the g_atomic functions allocate memory.

Jano



Michal


signature.asc
Description: PGP signature

Re: [PATCH] build: Drop execute permissions on virsh bash completion command

[Michal Privoznik]

On 1/31/20 3:38 PM, Jim Fehlig wrote:

rpmlint complains about "script-without-shebang" due to the execute
permissions on /usr/share/bash-completion/completions/vsh. Use
INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
execute permissions.

Signed-off-by: Jim Fehlig 
---

BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
on my installation, only vsh has execute permissions set.

  tools/Makefile.am | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 426a6ba661..d9d1a2f43f 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in 
$(top_builddir)/config.status
  if WITH_BASH_COMPLETION
  install-bash-completion:
$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
-   $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+   $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
rm -f virsh virt-admin && \



Frankly, I don't know why I wanted it to be executable. Perhaps I 
thought that if it wasn't bash would not load it, but apparently, that's 
not true. BTW, I have another executable file there, but the majority isn't.


# find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/perf

# find /usr/share/bash-completion/completions/ -type f \! -executable | 
wc -l

576


Reviewed-by: Michal Privoznik 

Michal

Re: [libvirt PATCH] qemu: Fix domain ID allocation

[Michal Privoznik]

On 1/31/20 3:43 PM, Ján Tomko wrote:

The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.

Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:

$ virsh list
  Id   Name   State
--
  1f28liverunning
  1f28live1   running

Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.

This also restores the usual numbering from 1 instead of 0.

Signed-off-by: Ján Tomko 
Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
---
  src/qemu/qemu_conf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e5051027fc..0b119cbe78 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1858,7 +1858,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
  
  int qemuDriverAllocateID(virQEMUDriverPtr driver)

  {
-return virAtomicIntInc(>lastvmid);
+return g_atomic_int_add(>lastvmid, 1) + 1;
  }
  
  



Does it makes sense to replace all virAtomic with g_atomic or do we 
still pretend that we care about client library and we don't crash on 
OOM in it?


Michal

Re: [libvirt PATCH v2 45/56] configure: request 64-bit APIs on 32-bit platforms

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:26PM +, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure.ac | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 46/56] examples: remove obsolete workaround for mingw

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:27PM +, Daniel P. Berrangé wrote:
> The build still succeeds with the workaround removed, so
> whatever was causing the problem no longer exists.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  examples/c/domain/domtop.c  | 15 ---
>  examples/c/domain/suspend.c | 14 --
>  2 files changed, 29 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 44/56] src: remove sys/wait.h from many files

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:25PM +, Daniel P. Berrangé wrote:
> Most code now uses the virProcess / virCommand APIs, so
> the need for sys/wait.h is quite limited. Removing this
> include removes the dependency on GNULIB providing a
> dummy sys/wait.h for Windows.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/libvirt.c | 1 -
>  src/lxc/lxc_container.c   | 1 -
>  src/network/bridge_driver.c   | 1 -
>  src/openvz/openvz_conf.c  | 1 -
>  src/openvz/openvz_driver.c| 1 -
>  src/qemu/qemu_capabilities.c  | 1 -
>  src/qemu/qemu_conf.c  | 1 -
>  src/qemu/qemu_driver.c| 1 -
>  src/remote/remote_daemon.c| 1 -
>  src/rpc/virnetsocket.c| 1 -
>  src/storage/storage_backend_iscsi.c   | 1 -
>  src/storage/storage_backend_logical.c | 1 -
>  src/storage/storage_util.c| 1 -
>  src/util/virfdstream.c| 1 -
>  src/util/virfile.c| 1 -
>  src/util/virhook.c| 1 -
>  src/util/viriptables.c| 1 -
>  src/util/virnetdevveth.c  | 2 --
>  src/util/virsysinfo.c | 1 -
>  src/vz/vz_driver.c| 1 -
>  tests/commandtest.c   | 4 +++-
>  tests/testutils.c | 1 -
>  22 files changed, 3 insertions(+), 23 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 43/56] util: conditionalize FD stream to exclude WIN32

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:24PM +, Daniel P. Berrangé wrote:
> Almost none of the virFDStream code will actually work
> on WIN32 builds, nor is it used except for in the
> virtualbox driver for screenshots. It is simpler to
> wrap it all in a '#ifndef WIN32'.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/util/virfdstream.c | 122 +++--
>  1 file changed, 93 insertions(+), 29 deletions(-)

If we are OK with breaking screenshots on virtualbox.  I guess that if
someone is really using it we will have a bug report and we can solve it.

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH] qemu: Fix domain ID allocation

[Peter Krempa]

On Fri, Jan 31, 2020 at 15:43:14 +0100, Ján Tomko wrote:
> The rewrite to use GLib's atomic ops functions changed the behavior
> of virAtomicIntInc - before it returned the pre-increment value.
> 
> Most of the callers using its value were adjusted, but the one
> in qemuDriverAllocateID was not. If libvirtd would reconnect to
> a running domain during startup, the next started domain would get
> the same ID:
> 
> $ virsh list
>  Id   Name   State
> --
>  1f28liverunning
>  1f28live1   running
> 
> Use the g_atomic_add function directly (as recommended in viratomic.h)
> and add 1 to the result.
> 
> This also restores the usual numbering from 1 instead of 0.
> 
> Signed-off-by: Ján Tomko 
> Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
> ---
>  src/qemu/qemu_conf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Peter Krempa 

[libvirt PATCH] docs: fix a typo

[Ján Tomko]

s/axnd/and/

Signed-off-by: Ján Tomko 
Fixes: f0f34056ab26eaa9f903a51cd1fa155088fd640f
---
Pushed as trivial

 docs/formatdomain.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 20bf6a9b84..60a103d7c6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5877,7 +5877,7 @@
 
 
   Since 6.1.0 (QEMU and KVM only, requires
-QEMU 4.2.0 or newer axnd a guest virtio-net driver supporting
+QEMU 4.2.0 or newer and a guest virtio-net driver supporting
 the "failover" feature, such as the one included in Linux
 kernel 4.18 and newer)
   
-- 
2.19.2

Re: [libvirt PATCH v2 42/56] src: conditionalize / remove use of poll.h

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:23PM +, Daniel P. Berrangé wrote:
> Remove imports of poll.h which are redundant, and
> conditionalize remaining usage that needs to compile
> on Windows platforms.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/util/virpolkit.c  | 2 +-
>  src/util/virutil.c| 1 -
>  tests/commandhelper.c | 2 +-
>  3 files changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 41/56] util: delete the poll() based event loop impl

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:22PM +, Daniel P. Berrangé wrote:
> It is no longer require since switching to the GLib based
> event loop impl.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  po/POTFILES.in   |   1 -
>  src/libvirt_private.syms |  13 -
>  src/util/Makefile.inc.am |   2 -
>  src/util/vireventpoll.c  | 772 ---
>  src/util/vireventpoll.h  | 126 ---
>  5 files changed, 914 deletions(-)
>  delete mode 100644 src/util/vireventpoll.c
>  delete mode 100644 src/util/vireventpoll.h

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 40/56] util: switch to use the GLib event loop impl

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:21PM +, Daniel P. Berrangé wrote:
> This sets the GLib event loop as the impl when calling
> virEventRegisterDefaultImpl(). This remains a private
> impl detail of libvirt, so applications must *NOT*
> assume that a call to virEventRegisterDefaultImpl()
> results in a GLib based event loop.
> 
> They should continue to use the libvirt-glib API
> gvir_event_register() if they explicitly want to guarantee
> a GLib event loop.
> 
> This follows the general principal that the libvirt public
> API should not expose the fact that GLib is being used
> internally.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/util/virevent.c | 21 +++--
>  1 file changed, 3 insertions(+), 18 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 39/56] util: import an event loop impl based on GMainContext

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:20PM +, Daniel P. Berrangé wrote:
> The libvirt-glib project has provided a GMainContext based
> event loop impl for applications. This imports it and sets
> it up for use by libvirt as the primary event loop. This
> remains a private impl detail of libvirt.
> 
> IOW, applications must *NOT* assume that a call to
> "virEventRegisterDefaultImpl" results in a GLib based
> event loop. They should continue to use the libvirt-glib
> API gvir_event_register() if they explicitly want to
> guarantee a GLib event loop.
> 
> This follows the general principal that the libvirt public
> API should not expose the fact that GLib is being used
> internally.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  build-aux/syntax-check.mk |   2 +-
>  po/POTFILES.in|   1 +
>  src/libvirt_private.syms  |   5 +
>  src/util/Makefile.inc.am  |   2 +
>  src/util/vireventglib.c   | 455 ++
>  src/util/vireventglib.h   |  28 +++
>  6 files changed, 492 insertions(+), 1 deletion(-)
>  create mode 100644 src/util/vireventglib.c
>  create mode 100644 src/util/vireventglib.h

[...]

> diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
> new file mode 100644
> index 00..be057c8e3c
> --- /dev/null
> +++ b/src/util/vireventglib.c
> @@ -0,0 +1,455 @@
> +/*
> + * vireventglib.c: GMainContext based event loop
> + *
> + * Copyright (C) 2008 Daniel P. Berrange
> + * Copyright (C) 2010-2019 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * .
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "vireventglib.h"
> +#include "vireventglibwatch.h"
> +#include "virerror.h"
> +#include "virlog.h"
> +
> +#ifdef G_OS_WIN32
> +# include 
> +#endif
> +
> +#define VIR_FROM_THIS VIR_FROM_EVENT
> +
> +VIR_LOG_INIT("util.eventglib");
> +
> +struct virEventGLibHandle
> +{
> +int watch;
> +int fd;
> +int events;
> +int removed;
> +guint source;
> +virEventHandleCallback cb;
> +void *opaque;
> +virFreeCallback ff;
> +};
> +
> +struct virEventGLibTimeout
> +{
> +int timer;
> +int interval;
> +int removed;
> +guint source;
> +virEventTimeoutCallback cb;
> +void *opaque;
> +virFreeCallback ff;
> +};
> +
> +static GMutex *eventlock;
> +
> +static int nextwatch = 1;
> +static GPtrArray *handles;
> +
> +static int nexttimer = 1;
> +static GPtrArray *timeouts;
> +
> +static gboolean
> +virEventGLibHandleDispatch(int fd G_GNUC_UNUSED,
> +   GIOCondition condition,
> +   gpointer opaque)
> +{
> +struct virEventGLibHandle *data = opaque;
> +int events = 0;
> +
> +if (condition & G_IO_IN)
> +events |= VIR_EVENT_HANDLE_READABLE;
> +if (condition & G_IO_OUT)
> +events |= VIR_EVENT_HANDLE_WRITABLE;
> +if (condition & G_IO_HUP)
> +events |= VIR_EVENT_HANDLE_HANGUP;
> +if (condition & G_IO_ERR)
> +events |= VIR_EVENT_HANDLE_ERROR;
> +
> +VIR_DEBUG("Dispatch handler %p %d %d %d %p", data, data->watch, 
> data->fd, events, data->opaque);

Missing names for the formatted data:

"Dispatch handler data=%p watch=%d fd=%d evetns=%d opaque=%p".

The same thing is done for timer and it improves the debug log.

There are some more places that should be fixed [1].

> +
> +(data->cb)(data->watch, data->fd, events, data->opaque);
> +
> +return TRUE;
> +}
> +
> +
> +static int
> +virEventGLibHandleAdd(int fd,
> +  int events,
> +  virEventHandleCallback cb,
> +  void *opaque,
> +  virFreeCallback ff)
> +{
> +struct virEventGLibHandle *data;
> +GIOCondition cond = 0;
> +int ret;
> +
> +g_mutex_lock(eventlock);
> +
> +data = g_new0(struct virEventGLibHandle, 1);
> +
> +if (events & VIR_EVENT_HANDLE_READABLE)
> +cond |= G_IO_IN;
> +if (events & VIR_EVENT_HANDLE_WRITABLE)
> +cond |= G_IO_OUT;

Aren't we missing VIR_EVENT_HANDLE_ERROR and VIR_EVENT_HANDLE_HANGUP?

> +
> +data->watch = nextwatch++;
> +data->fd = fd;
> +data->events = events;
> +data->cb = cb;
> +data->opaque = opaque;
> +data->ff = ff;
> +
> +VIR_DEBUG("Add handle %p %d %d %d %p", data, data->watch, data->fd, 
> 

[PATCH] build: Drop execute permissions on virsh bash completion command

[Jim Fehlig]

rpmlint complains about "script-without-shebang" due to the execute
permissions on /usr/share/bash-completion/completions/vsh. Use
INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
execute permissions.

Signed-off-by: Jim Fehlig 
---

BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
on my installation, only vsh has execute permissions set.

 tools/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 426a6ba661..d9d1a2f43f 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in 
$(top_builddir)/config.status
 if WITH_BASH_COMPLETION
 install-bash-completion:
$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
-   $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+   $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
rm -f virsh virt-admin && \
-- 
2.16.4

[libvirt PATCH] qemu: Fix domain ID allocation

[Ján Tomko]

The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.

Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:

$ virsh list
 Id   Name   State
--
 1f28liverunning
 1f28live1   running

Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.

This also restores the usual numbering from 1 instead of 0.

Signed-off-by: Ján Tomko 
Fixes: 7b9645a7d127a374b8d1c83fdf9789706dbab2c9
---
 src/qemu/qemu_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e5051027fc..0b119cbe78 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1858,7 +1858,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 
 int qemuDriverAllocateID(virQEMUDriverPtr driver)
 {
-return virAtomicIntInc(>lastvmid);
+return g_atomic_int_add(>lastvmid, 1) + 1;
 }
 
 
-- 
2.21.0

[PATCH 05/19] tests: qemublock: Add checkpoint deletion test for deep backing chain

[Peter Krempa]

Add test cases for merging various pairs of bitmaps when snapshots were
created together with checkpoints.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  6 ++
 .../snapshots-current-out.json| 29 +
 .../snapshots-intermediate1-out.json  | 22 +++
 .../snapshots-intermediate2-out.json  | 59 +++
 .../snapshots-intermediate3-out.json  | 59 +++
 .../snapshots-noparent-out.json   | 23 
 6 files changed, 198 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 6194c45ec8..1c04929e81 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1019,6 +1019,12 @@ mymain(void)
 TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c", "basic");
 TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d", "basic");

+TEST_CHECKPOINT_DELETE_MERGE("snapshots-noparent", "a", NULL, "snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate1", "b", "a", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate2", "c", "b", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", 
"snapshots");
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
new file mode 100644
index 00..29fefeea63
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "a",
+  "bitmaps": [
+{
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
new file mode 100644
index 00..4da21a9df7
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
@@ -0,0 +1,59 @@
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "b",
+  "persistent": true,
+  "disabled": true,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  },
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-3-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "c"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
new file mode 

[PATCH 18/19] tests: qemublock: Add tests for qemuBlockBitmapsHandleBlockcopy

[Peter Krempa]

Use some of the existing bitmap data to add tests for
qemuBlockBitmapsHandleBlockcopy.

As the output depends on the ordering in the hash table we must also
install the "virdeterministichash" mock preload library.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  71 +-
 .../bitmapblockcopy/basic-deep-out.json   | 117 +++
 .../bitmapblockcopy/basic-shallow-out.json| 117 +++
 .../bitmapblockcopy/snapshots-deep-out.json   | 133 ++
 .../snapshots-shallow-out.json|  48 +++
 5 files changed, 485 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json
 create mode 100644 
tests/qemublocktestdata/bitmapblockcopy/basic-shallow-out.json
 create mode 100644 
tests/qemublocktestdata/bitmapblockcopy/snapshots-deep-out.json
 create mode 100644 
tests/qemublocktestdata/bitmapblockcopy/snapshots-shallow-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index ada3608e53..06d89b9230 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -803,6 +803,56 @@ testQemuBlockBitmapValidate(const void *opaque)
 }


+static const char *blockcopyPrefix = "qemublocktestdata/bitmapblockcopy/";
+
+struct testQemuBlockBitmapBlockcopyData {
+const char *name;
+bool shallow;
+virStorageSourcePtr chain;
+const char *nodedatafile;
+};
+
+
+static int
+testQemuBlockBitmapBlockcopy(const void *opaque)
+{
+const struct testQemuBlockBitmapBlockcopyData *data = opaque;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_autoptr(virJSONValue) actions = NULL;
+g_autoptr(virJSONValue) nodedatajson = NULL;
+g_autoptr(virHashTable) nodedata = NULL;
+g_autoptr(virStorageSource) fakemirror = virStorageSourceNew();
+
+if (!fakemirror)
+return -1;
+
+fakemirror->nodeformat = g_strdup("mirror-format-node");
+
+expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
+ blockcopyPrefix, 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 (qemuBlockBitmapsHandleBlockcopy(data->chain, fakemirror, nodedata,
+data->shallow, ) < 0)
+return -1;
+
+if (actions &&
+!(actual = virJSONValueToString(actions, true)))
+return -1;
+
+return virTestCompareToFile(actual, expectpath);
+}
+
+
 static int
 mymain(void)
 {
@@ -814,6 +864,7 @@ mymain(void)
 struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata;
 struct testQemuCheckpointDeleteMergeData checkpointdeletedata;
 struct testQemuBlockBitmapValidateData blockbitmapvalidatedata;
+struct testQemuBlockBitmapBlockcopyData blockbitmapblockcopydata;
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 g_autoptr(virStorageSource) bitmapSourceChain = NULL;
@@ -1124,6 +1175,24 @@ mymain(void)
 TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "d", false);
 TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "current", true);

+#define TEST_BITMAP_BLOCKCOPY(testname, shllw, ndf) \
+do { \
+blockbitmapblockcopydata.name = testname; \
+blockbitmapblockcopydata.shallow = shllw; \
+blockbitmapblockcopydata.nodedatafile = ndf; \
+blockbitmapblockcopydata.chain = bitmapSourceChain;\
+if (virTestRun("bitmap block copy " testname, \
+   testQemuBlockBitmapBlockcopy, \
+   ) < 0) \
+ret = -1; \
+} while (0)
+
+TEST_BITMAP_BLOCKCOPY("basic-shallow", true, "basic");
+TEST_BITMAP_BLOCKCOPY("basic-deep", false, "basic");
+
+TEST_BITMAP_BLOCKCOPY("snapshots-shallow", true, "snapshots");
+TEST_BITMAP_BLOCKCOPY("snapshots-deep", false, "snapshots");
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
@@ -1133,4 +1202,4 @@ mymain(void)
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }

-VIR_TEST_MAIN(mymain)
+VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdeterministichash"))
diff --git a/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json 
b/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json
new file mode 100644
index 00..4ed2b97e95
--- /dev/null
+++ b/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json
@@ -0,0 +1,117 @@
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "mirror-format-node",
+  "name": "a",
+  "persistent": true,
+  "disabled": true,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": 

[PATCH 17/19] qemu: block: Introduce function to calculate bitmap handling for block-copy

[Peter Krempa]

Add a function calculating which bitmaps to copy to the mirror during
a block-copy operation.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 138 ++
 src/qemu/qemu_block.h |   7 +++
 2 files changed, 145 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b19290e677..63116ef5f2 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2728,3 +2728,141 @@ qemuBlockBitmapChainIsValid(virStorageSourcePtr src,

 return chain_started;
 }
+
+
+struct qemuBlockBitmapsHandleBlockcopyConcatData {
+virHashTablePtr bitmaps_merge;
+virJSONValuePtr actions;
+const char *mirrornodeformat;
+bool has_bitmaps;
+};
+
+
+static int
+qemuBlockBitmapsHandleBlockcopyConcatActions(void *payload,
+ const void *name,
+ void *opaque)
+{
+struct qemuBlockBitmapsHandleBlockcopyConcatData *data = opaque;
+virJSONValuePtr createactions = payload;
+const char *bitmapname = name;
+g_autoptr(virJSONValue) mergebitmaps = virHashSteal(data->bitmaps_merge, 
bitmapname);
+
+data->has_bitmaps = true;
+
+virJSONValueArrayConcat(data->actions, createactions);
+
+if (qemuMonitorTransactionBitmapMerge(data->actions,
+  data->mirrornodeformat,
+  bitmapname,
+  ) < 0)
+return -1;
+
+return 0;
+}
+
+
+/**
+ * qemuBlockBitmapsHandleBlockcopy:
+ * @src: disk source
+ * @mirror: mirror source
+ * @blockNamedNodeData: hash table containing data about bitmaps
+ * @shallow: whether shallow copy is requested
+ * @actions: filled with arguments for a 'transaction' command
+ *
+ * Calculates which bitmaps to copy and merge during a virDomainBlockCopy job.
+ * This is designed to be called when the job is already synchronised as it
+ * may result in active bitmaps being created.
+ *
+ * Returns 0 on success and -1 on error. If @actions is NULL when 0 is returned
+ * there are no actions to perform for the given job.
+ */
+int
+qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src,
+virStorageSourcePtr mirror,
+virHashTablePtr blockNamedNodeData,
+bool shallow,
+virJSONValuePtr *actions)
+{
+g_autoptr(virHashTable) bitmaps = virHashNew(virJSONValueHashFree);
+g_autoptr(virHashTable) bitmaps_merge = virHashNew(virJSONValueHashFree);
+g_autoptr(virHashTable) bitmaps_skip = virHashNew(NULL);
+g_autoptr(virJSONValue) tmpactions = virJSONValueNewArray();
+qemuBlockNamedNodeDataPtr entry;
+virStorageSourcePtr n;
+size_t i;
+struct qemuBlockBitmapsHandleBlockcopyConcatData data = { .bitmaps_merge = 
bitmaps_merge,
+  .actions = 
tmpactions,
+  
.mirrornodeformat = mirror->nodeformat,
+  .has_bitmaps = 
false, };
+
+for (n = src; n; n = n->backingStore) {
+if (!(entry = virHashLookup(blockNamedNodeData, n->nodeformat)))
+continue;
+
+for (i = 0; i < entry->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
+virJSONValuePtr bitmap_merge;
+
+if (virHashHasEntry(bitmaps_skip, bitmap->name))
+continue;
+
+if (!(bitmap_merge = virHashLookup(bitmaps_merge, bitmap->name))) {
+g_autoptr(virJSONValue) tmp = NULL;
+bool disabled = !bitmap->recording;
+
+/* disable any non top-layer bitmaps */
+if (n != src)
+disabled = true;
+
+if (!bitmap->persistent ||
+!(qemuBlockBitmapChainIsValid(n, bitmap->name,
+  blockNamedNodeData))) {
+ignore_value(virHashAddEntry(bitmaps_skip, bitmap->name, 
NULL));
+continue;
+}
+
+/* prepare the data for adding the bitmap to the mirror */
+tmp = virJSONValueNewArray();
+
+if (qemuMonitorTransactionBitmapAdd(tmp,
+mirror->nodeformat,
+bitmap->name,
+true,
+disabled,
+bitmap->granularity) < 0)
+return -1;
+
+if (virHashAddEntry(bitmaps, bitmap->name, tmp) < 0)
+return -1;
+
+tmp = NULL;
+
+/* prepare array for merging all the bitmaps 

[PATCH 13/19] qemu: blockjob: Store 'jobflags' with block job data

[Peter Krempa]

Add a variable which will store the contents of the 'flags' variable as
passed in by the individual block jobs. Since the flags may influence
behaviour of the jobs it's important to preserve it to the finalization
steps.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.h | 3 +++
 src/qemu/qemu_domain.c   | 7 +++
 tests/qemustatusxml2xmldata/backup-pull-in.xml   | 2 +-
 tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 8 
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 2f29e8209c..9e55382f15 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -129,6 +129,9 @@ struct _qemuBlockJobData {
 virStorageSourcePtr chain; /* Reference to the chain the job operates on. 
*/
 virStorageSourcePtr mirrorChain; /* reference to 'mirror' part of the job 
*/

+unsigned int jobflags; /* per job flags */
+bool jobflagsmissing; /* job flags were not stored */
+
 union {
 qemuBlockJobPullData pull;
 qemuBlockJobCommitData commit;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05a8d3de38..0c7a2641ea 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2582,6 +2582,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void 
*payload,
 virBufferEscapeString(, " newstate='%s'", newstate);
 if (job->brokentype != QEMU_BLOCKJOB_TYPE_NONE)
 virBufferEscapeString(, " brokentype='%s'", 
qemuBlockjobTypeToString(job->brokentype));
+if (!job->jobflagsmissing)
+virBufferAsprintf(, " jobflags='0x%x'", job->jobflags);
 virBufferEscapeString(, "%s", job->errmsg);

 if (job->disk) {
@@ -3294,6 +3296,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr 
vm,
 int newstate = -1;
 bool invalidData = false;
 xmlNodePtr tmp;
+unsigned long jobflags = 0;

 ctxt->node = node;

@@ -,6 +3336,9 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr 
vm,
 STRNEQ(mirror, "yes"))
 invalidData = true;

+if (virXPathULongHex("string(./@jobflags)", ctxt, ) != 0)
+job->jobflagsmissing = true;
+
 if (!disk && !invalidData) {
 if ((tmp = virXPathNode("./chains/disk", ctxt)) &&
 !(job->chain = qemuDomainObjPrivateXMLParseBlockjobChain(tmp, 
ctxt, xmlopt)))
@@ -3352,6 +3358,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr 
vm,

 job->state = state;
 job->newstate = newstate;
+job->jobflags = jobflags;
 job->errmsg = virXPathString("string(./errmsg)", ctxt);
 job->invalidData = invalidData;
 job->disk = disk;
diff --git a/tests/qemustatusxml2xmldata/backup-pull-in.xml 
b/tests/qemustatusxml2xmldata/backup-pull-in.xml
index 3c69c41840..1db978a3ac 100644
--- a/tests/qemustatusxml2xmldata/backup-pull-in.xml
+++ b/tests/qemustatusxml2xmldata/backup-pull-in.xml
@@ -235,7 +235,7 @@
   
   
   
-
+
   
   
   
diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml 
b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
index b5d62fd4ab..ca6d110179 100644
--- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
+++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
@@ -261,19 +261,19 @@
 
   
 
-
+
   
 
-
+
   
   
   
   
 
-
+
   
 
-
+
   
 
   
-- 
2.24.1

[PATCH 10/19] virJSONValueNewArray: Use g_new0 to allocate and remove NULL checks from callers

[Peter Krempa]

Use the glib allocation function that never returns NULL and remove the
now dead-code checks from all callers.

Signed-off-by: Peter Krempa 
---
 src/locking/lock_daemon.c|  4 ++--
 src/logging/log_handler.c|  3 +--
 src/network/leaseshelper.c   |  6 +-
 src/qemu/qemu_agent.c|  6 +-
 src/qemu/qemu_backup.c   |  6 ++
 src/qemu/qemu_block.c|  9 +++--
 src/qemu/qemu_blockjob.c |  3 +--
 src/qemu/qemu_checkpoint.c   |  9 +++--
 src/qemu/qemu_driver.c   |  3 +--
 src/qemu/qemu_firmware.c | 12 
 src/qemu/qemu_migration_params.c |  3 +--
 src/qemu/qemu_monitor_json.c |  3 +--
 src/rpc/virnetserver.c   |  6 ++
 src/rpc/virnetserverservice.c|  3 +--
 src/util/virjson.c   | 13 ++---
 src/util/virlockspace.c  |  6 ++
 src/util/virmacmap.c |  8 
 tests/qemublocktest.c|  3 +--
 tests/qemumonitorjsontest.c  |  5 ++---
 19 files changed, 35 insertions(+), 76 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 7ea228ce37..fd376fef2a 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -949,8 +949,8 @@ virLockDaemonPreExecRestart(const char *state_file,
 goto cleanup;
 }

-if (!(lockspaces = virJSONValueNewArray()))
-goto cleanup;
+lockspaces = virJSONValueNewArray();
+
 if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0) {
 virJSONValueFree(lockspaces);
 goto cleanup;
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index 030c9d66e3..973c52c7cd 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -619,8 +619,7 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler)
 if (!ret)
 return NULL;

-if (!(files = virJSONValueNewArray()))
-goto error;
+files = virJSONValueNewArray();

 if (virJSONValueObjectAppend(ret, "files", files) < 0) {
 virJSONValueFree(files);
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index f1a061066e..dd1d5f70ee 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -200,11 +200,7 @@ main(int argc, char **argv)
 break;
 }

-if (!(leases_array_new = virJSONValueNewArray())) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("failed to create json"));
-goto cleanup;
-}
+leases_array_new = virJSONValueNewArray();

 if (virLeaseReadCustomLeaseFile(leases_array_new, custom_lease_file,
 delete ? ip : NULL, _duid) < 0)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 4374235d34..7d01d21a11 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1201,9 +1201,6 @@ qemuAgentMakeStringsArray(const char **strings, unsigned 
int len)
 size_t i;
 virJSONValuePtr ret = virJSONValueNewArray(), str;

-if (!ret)
-return NULL;
-
 for (i = 0; i < len; i++) {
 str = virJSONValueNewString(strings[i]);
 if (!str)
@@ -1534,8 +1531,7 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr mon,
 *nmodified = 0;

 /* create the key data array */
-if (!(cpus = virJSONValueNewArray()))
-goto cleanup;
+cpus = virJSONValueNewArray();

 for (i = 0; i < ninfo; i++) {
 qemuAgentCPUInfoPtr in = [i];
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 8b1e9a7e19..2cc6ff7a42 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -180,8 +180,7 @@ qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr 
*incremental,
 g_autoptr(virJSONValue) ret = NULL;
 size_t incridx = 0;

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

 if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
  backingChain,
@@ -819,8 +818,7 @@ qemuBackupBegin(virDomainObjPtr vm,
 !(incremental = qemuBackupBeginCollectIncrementalCheckpoints(vm, 
def->incremental)))
 goto endjob;

-if (!(actions = virJSONValueNewArray()))
-goto endjob;
+actions = virJSONValueNewArray();

 /* The 'chk' checkpoint must be rolled back if the transaction command
  * which creates it on disk is not executed or fails */
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 13e240fdac..03f029368e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -524,8 +524,7 @@ 
qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
 virStorageNetHostDefPtr host;
 size_t i;

-if (!(servers = virJSONValueNewArray()))
-return NULL;
+servers = virJSONValueNewArray();

 for (i = 0; i < src->nhosts; i++) {
 host = src->hosts + i;
@@ -590,8 +589,7 @@ 

[PATCH 08/19] qemu: block: Extract calls of qemuBlockGetNamedNodeData into a helper function

[Peter Krempa]

Create a wrapper for qemuBlockGetNamedNodeData named
qemuBlockGetNamedNodeData. The purpose of the wrapper is to integrate
the monitor handling functionality and in the future possible
qemuCaps-based flags.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_backup.c |  5 +
 src/qemu/qemu_block.c  | 20 
 src/qemu/qemu_block.h  |  4 
 src/qemu/qemu_driver.c | 16 
 4 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 23518a5d40..8b1e9a7e19 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -830,10 +830,7 @@ qemuBackupBegin(virDomainObjPtr vm,
 goto endjob;
 }

-if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, 
QEMU_ASYNC_JOB_BACKUP) < 0)
-goto endjob;
-blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
-if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData)
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, 
QEMU_ASYNC_JOB_BACKUP)))
 goto endjob;

 if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental, 
blockNamedNodeData,
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 22f03da485..13e240fdac 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2670,3 +2670,23 @@ qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr 
blockNamedNodeData,

 return NULL;
 }
+
+
+virHashTablePtr
+qemuBlockGetNamedNodeData(virDomainObjPtr vm,
+  qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return NULL;
+
+blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData)
+return NULL;
+
+return g_steal_pointer();
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 1a38e0eccf..68646cbf2e 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -208,3 +208,7 @@ qemuBlockNamedNodeDataBitmapPtr
 qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData,
   virStorageSourcePtr src,
   const char *bitmap);
+
+virHashTablePtr
+qemuBlockGetNamedNodeData(virDomainObjPtr vm,
+  qemuDomainAsyncJob asyncJob);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8bb845298b..47f0754a1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15642,15 +15642,9 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr 
driver,
 if (!(actions = virJSONValueNewArray()))
 return -1;

-if (blockdev) {
-if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
-return -1;
-
-blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
-
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData)
-return -1;
-}
+if (blockdev &&
+!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))
+return -1;

 /* prepare a list of objects to use in the vm definition so that we don't
  * have to roll back later */
@@ -18354,9 +18348,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
   
priv->qemuCaps)))
 goto endjob;
 } else {
-qemuDomainObjEnterMonitor(driver, vm);
-blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || 
!blockNamedNodeData)
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, 
QEMU_ASYNC_JOB_NONE)))
 goto endjob;

 if (qemuBlockStorageSourceCreateDetectSize(blockNamedNodeData,
-- 
2.24.1

[PATCH 03/19] tests: qemublock: Add synthetic snapshot+checkpoint test data

[Peter Krempa]

Add a faked qemu output which would simulate scenario where libvirt
would take a snapshot and checkpoint simultaneously. This is visible in
libvirt-2-format node where bitmap 'c' appears, but bitmap 'b' which is
active in the previous layer is not present.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |   1 +
 .../snapshots-synthetic-checkpoint.json   | 827 ++
 .../bitmap/snapshots-synthetic-checkpoint.out |  13 +
 3 files changed, 841 insertions(+)
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index e062f1a4cb..861b4e86ae 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -970,6 +970,7 @@ mymain(void)
 TEST_BITMAP_DETECT("basic");
 TEST_BITMAP_DETECT("synthetic");
 TEST_BITMAP_DETECT("snapshots");
+TEST_BITMAP_DETECT("snapshots-synthetic-checkpoint");

 #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \
 do { \
diff --git a/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json 
b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
new file mode 100644
index 00..25cc150d67
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
@@ -0,0 +1,827 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911522",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"full-backing-filename": "/tmp/pull4.qcow2",
+"backing-filename": "/tmp/pull4.qcow2",
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911527",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 217088,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+  

[PATCH 15/19] qemu: block: Add validator for bitmap chains accross backing chains

[Peter Krempa]

Add a validator which checks that a bitmap spanning multiple backing
chain members doesn't look broken. The current rules are that no
intermediate birmaps are missing (unfortunately it's hard to know
whether the topmost or bottommost bitmap is missing) and none of the
components is inconsistent.

We can obviously improve it over time.

The validator is also tested against the existing bitmap data we have
for the backup merging test as well as some of the existing broken
bitmap synthetic test cases.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 41 +
 src/qemu/qemu_block.h |  5 
 tests/qemublocktest.c | 70 +++
 3 files changed, 116 insertions(+)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 03f029368e..b19290e677 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2687,3 +2687,44 @@ qemuBlockGetNamedNodeData(virDomainObjPtr vm,

 return g_steal_pointer();
 }
+
+
+/**
+ * qemuBlockBitmapChainIsValid:
+ *
+ * Validates that the backing chain of @src contains proper consistent bitmap
+ * data for a chain of bitmaps named @bitmapname.
+ *
+ * A valid chain:
+ * 1) bitmaps of same name are in a consecutive subset of images without gap
+ * 2) don't have any inconsistent bitmaps
+ */
+bool
+qemuBlockBitmapChainIsValid(virStorageSourcePtr src,
+const char *bitmapname,
+virHashTablePtr blockNamedNodeData)
+{
+qemuBlockNamedNodeDataBitmapPtr bitmap;
+virStorageSourcePtr n;
+bool chain_started = false;
+bool chain_ended = false;
+
+for (n = src; n; n = n->backingStore) {
+if (!(bitmap = 
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, n, bitmapname))) {
+if (chain_started)
+chain_ended = true;
+
+continue;
+}
+
+if (chain_ended)
+return false;
+
+chain_started = true;
+
+if (bitmap->inconsistent)
+return false;
+}
+
+return chain_started;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 68646cbf2e..cf51b9bf4e 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -212,3 +212,8 @@ qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr 
blockNamedNodeData,
 virHashTablePtr
 qemuBlockGetNamedNodeData(virDomainObjPtr vm,
   qemuDomainAsyncJob asyncJob);
+
+bool
+qemuBlockBitmapChainIsValid(virStorageSourcePtr src,
+const char *bitmapname,
+virHashTablePtr blockNamedNodeData);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index cd61334b4a..133372908a 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -768,6 +768,41 @@ testQemuCheckpointDeleteMerge(const void *opaque)
 }


+struct testQemuBlockBitmapValidateData {
+const char *name;
+const char *bitmapname;
+virStorageSourcePtr chain;
+bool expect;
+};
+
+static int
+testQemuBlockBitmapValidate(const void *opaque)
+{
+const struct testQemuBlockBitmapValidateData *data = opaque;
+g_autoptr(virJSONValue) nodedatajson = NULL;
+g_autoptr(virHashTable) nodedata = NULL;
+bool actual;
+
+if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, data->name,
+ ".json", NULL)))
+return -1;
+
+if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {
+VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
+return -1;
+}
+
+actual = qemuBlockBitmapChainIsValid(data->chain, data->bitmapname, 
nodedata);
+
+if (actual != data->expect) {
+VIR_TEST_VERBOSE("expected rv:'%d' actual rv:'%d'\n", data->expect, 
actual);
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 mymain(void)
 {
@@ -778,6 +813,7 @@ mymain(void)
 struct testQemuImageCreateData imagecreatedata;
 struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata;
 struct testQemuCheckpointDeleteMergeData checkpointdeletedata;
+struct testQemuBlockBitmapValidateData blockbitmapvalidatedata;
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 g_autoptr(virStorageSource) bitmapSourceChain = NULL;
@@ -1045,7 +1081,41 @@ mymain(void)
 
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate3", 
"d", "c", "snapshots-synthetic-checkpoint");
 TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-current", 
"current", "d", "snapshots-synthetic-checkpoint");

+#define TEST_BITMAP_VALIDATE(testname, bitmap, rc) \
+do { \
+blockbitmapvalidatedata.name = testname; \
+blockbitmapvalidatedata.chain = bitmapSourceChain; \
+blockbitmapvalidatedata.bitmapname = bitmap; \
+blockbitmapvalidatedata.expect = rc; \
+if (virTestRun("bitmap validate " testname " " bitmap, \
+   

[PATCH 11/19] virhash: Make sure that hash key is always copied

[Peter Krempa]

Fix all implementations of virHashKeyCopy to always return a valid
pointer.

Tweak the return value expectation comment so that it doesn't
necessarily require to allocate memory.

Signed-off-by: Peter Krempa 
---
 src/conf/domain_addr.c | 5 +
 src/util/virhash.c | 4 +---
 src/util/virhash.h | 3 ++-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index f07b3d9725..e0be655772 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -997,10 +997,7 @@ virZPCIAddrKeyEqual(const void *namea,
 static void *
 virZPCIAddrKeyCopy(const void *name)
 {
-unsigned int *copy;
-
-if (VIR_ALLOC(copy) < 0)
-return NULL;
+unsigned int *copy = g_new0(unsigned int, 1);

 *copy = *((unsigned int *)name);
 return (void *)copy;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index d5c5e017a1..c57d9f8292 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -94,9 +94,7 @@ static bool virHashStrEqual(const void *namea, const void 
*nameb)

 static void *virHashStrCopy(const void *name)
 {
-char *ret;
-ret = g_strdup(name);
-return ret;
+return g_strdup(name);
 }


diff --git a/src/util/virhash.h b/src/util/virhash.h
index 08f99d8a3d..143ce52206 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -83,7 +83,8 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void 
*nameb);
  * Create a copy of the hash key, duplicating
  * memory allocation where applicable
  *
- * Returns a newly allocated copy of @name
+ * Returns a copy of @name which will eventually be passed to the
+ * 'virHashKeyFree' callback at the end of it's lifetime.
  */
 typedef void *(*virHashKeyCopy)(const void *name);
 /**
-- 
2.24.1

[PATCH 14/19] qemu: blockjob: Store 'flags' for all the block job types

[Peter Krempa]

The flags may control important aspects of the block job which may
influence also the termination of the job. Store the 'flags' for all
the block job types.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 13 ++---
 src/qemu/qemu_blockjob.h |  9 ++---
 src/qemu/qemu_driver.c   |  7 ---
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 3dc9222a6f..6b59bbeb2c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -252,7 +252,8 @@ qemuBlockJobDiskNew(virDomainObjPtr vm,
 qemuBlockJobDataPtr
 qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
-virStorageSourcePtr base)
+virStorageSourcePtr base,
+unsigned int jobflags)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(qemuBlockJobData) job = NULL;
@@ -269,6 +270,7 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 return NULL;

 job->data.pull.base = base;
+job->jobflags = jobflags;

 if (qemuBlockJobRegister(job, vm, disk, true) < 0)
 return NULL;
@@ -283,7 +285,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
   virStorageSourcePtr topparent,
   virStorageSourcePtr top,
   virStorageSourcePtr base,
-  bool delete_imgs)
+  bool delete_imgs,
+  unsigned int jobflags)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(qemuBlockJobData) job = NULL;
@@ -307,6 +310,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
 job->data.commit.top = top;
 job->data.commit.base = base;
 job->data.commit.deleteCommittedImages = delete_imgs;
+job->jobflags = jobflags;

 if (qemuBlockJobRegister(job, vm, disk, true) < 0)
 return NULL;
@@ -350,7 +354,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 virStorageSourcePtr mirror,
 bool shallow,
-bool reuse)
+bool reuse,
+unsigned int jobflags)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(qemuBlockJobData) job = NULL;
@@ -371,6 +376,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm,
 if (shallow && !reuse)
 job->data.copy.shallownew = true;

+job->jobflags = jobflags;
+
 if (qemuBlockJobRegister(job, vm, disk, true) < 0)
 return NULL;

diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 9e55382f15..72c7fa053e 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -176,7 +176,8 @@ qemuBlockJobDiskNew(virDomainObjPtr vm,
 qemuBlockJobDataPtr
 qemuBlockJobDiskNewPull(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
-virStorageSourcePtr base);
+virStorageSourcePtr base,
+unsigned int jobflags);

 qemuBlockJobDataPtr
 qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
@@ -184,7 +185,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
   virStorageSourcePtr topparent,
   virStorageSourcePtr top,
   virStorageSourcePtr base,
-  bool delete_imgs);
+  bool delete_imgs,
+  unsigned int jobflags);

 qemuBlockJobDataPtr
 qemuBlockJobNewCreate(virDomainObjPtr vm,
@@ -197,7 +199,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 virStorageSourcePtr mirror,
 bool shallow,
-bool reuse);
+bool reuse,
+unsigned int jobflags);

 qemuBlockJobDataPtr
 qemuBlockJobDiskNewBackup(virDomainObjPtr vm,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 021e5a2732..a2481232ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17706,7 +17706,7 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 speed <<= 20;
 }

-if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource)))
+if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, flags)))
 goto endjob;

 if (blockdev) {
@@ -18393,7 +18393,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 goto endjob;
 }

-if (!(job = qemuBlockJobDiskNewCopy(vm, disk, mirror, mirror_shallow, 
mirror_reuse)))
+if (!(job = qemuBlockJobDiskNewCopy(vm, disk, mirror, mirror_shallow, 
mirror_reuse, flags)))
 goto endjob;

 disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
@@ -18814,7 +18814,8 @@ qemuDomainBlockCommit(virDomainPtr dom,

 if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
  

[PATCH 19/19] qemuDomainBlockPivot: Copy bitmaps backing checkpoints for virDomainBlockCopy

[Peter Krempa]

Use qemuBlockBitmapsHandleBlockcopy to calculate bitmaps to copy over
for a block-copy job.

We copy them when pivoting to the new image as at that point we are
certain that we don't dirty any bitmap unnecessarily.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a2481232ad..7df3f53183 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17572,6 +17572,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
+g_autoptr(virJSONValue) actions = NULL;

 switch ((qemuBlockJobType) job->type) {
 case QEMU_BLOCKJOB_TYPE_NONE:
@@ -17592,6 +17593,20 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 return -1;

 case QEMU_BLOCKJOB_TYPE_COPY:
+if (blockdev && !job->jobflagsmissing) {
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW;
+
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, 
QEMU_ASYNC_JOB_NONE)))
+return -1;
+
+if (qemuBlockBitmapsHandleBlockcopy(disk->src, disk->mirror,
+blockNamedNodeData,
+shallow, ) < 0)
+return -1;
+}
+break;
+
 case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
 break;
 }
@@ -17604,10 +17619,17 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 }

 qemuDomainObjEnterMonitor(driver, vm);
-if (blockdev)
-ret = qemuMonitorJobComplete(priv->mon, job->name);
-else
+if (blockdev) {
+int rc = 0;
+
+if (actions)
+rc = qemuMonitorTransaction(priv->mon, );
+
+if (rc == 0)
+ret = qemuMonitorJobComplete(priv->mon, job->name);
+} else {
 ret = qemuMonitorDrivePivot(priv->mon, job->name);
+}
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
 return -1;

-- 
2.24.1

[PATCH 07/19] qemu: checkpoint: Track and relabel images for bitmap merging

[Peter Krempa]

Allow qemu access to modify backing files in case when we want to delete
a checkpoint.

This patch adds tracking of which images need to be relabelled when
calculating the transaction, the code to relabel them and rollback.

To verify that stuff works we also output the list of images to relabel
into the test case output files in qemublocktest.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c| 35 ---
 src/qemu/qemu_checkpoint.h|  3 +-
 tests/qemublocktest.c | 19 --
 .../snapshots-intermediate1-out.json  |  2 ++
 .../snapshots-intermediate2-out.json  |  3 ++
 .../snapshots-intermediate3-out.json  |  2 ++
 .../snapshots-noparent-out.json   |  4 +++
 ...ynthetic-checkpoint-intermediate1-out.json |  2 ++
 ...ynthetic-checkpoint-intermediate2-out.json |  2 ++
 ...ynthetic-checkpoint-intermediate3-out.json |  2 ++
 ...ots-synthetic-checkpoint-noparent-out.json |  4 +++
 11 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 087a740cf8..6ef35ea1a1 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -155,7 +155,8 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
  const char *delbitmap,
  const char *parentbitmap,
  virJSONValuePtr actions,
- const char *diskdst)
+ const char *diskdst,
+ GSList **reopenimages)
 {
 virStorageSourcePtr n = src;

@@ -235,6 +236,9 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
srcbitmap->name) < 0)
 return -1;

+if (n != src)
+*reopenimages = g_slist_prepend(*reopenimages, n);
+
 n = n->backingStore;
 }

@@ -250,9 +254,12 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverPtr driver = priv->driver;
 g_autoptr(virHashTable) blockNamedNodeData = NULL;
-int rc;
+int rc = -1;
 g_autoptr(virJSONValue) actions = NULL;
 size_t i;
+g_autoptr(GSList) reopenimages = NULL;
+g_autoptr(GSList) relabelimages = NULL;
+GSList *next;

 if (!(actions = virJSONValueNewArray()))
 return -1;
@@ -284,16 +291,34 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

 if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData,
  chkdisk->bitmap, parentbitmap,
- actions, domdisk->dst) < 0)
+ actions, domdisk->dst,
+ ) < 0)
 return -1;
 }

+/* label any non-top images for read-write access */
+for (next = reopenimages; next; next = next->next) {
+virStorageSourcePtr src = next->data;
+
+if (qemuDomainStorageSourceAccessAllow(driver, vm, src, false, false) 
< 0)
+goto relabel;
+
+relabelimages = g_slist_prepend(relabelimages, src);
+}
+
 qemuDomainObjEnterMonitor(driver, vm);
 rc = qemuMonitorTransaction(priv->mon, );
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
 return -1;

-return 0;
+ relabel:
+for (next = relabelimages; next; next = next->next) {
+virStorageSourcePtr src = next->data;
+
+ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, true, 
false));
+}
+
+return rc;
 }


diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index 976b1eed0f..cf1e9e46cb 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -78,4 +78,5 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
  const char *delbitmap,
  const char *parentbitmap,
  virJSONValuePtr actions,
- const char *diskdst);
+ const char *diskdst,
+ GSList **reopenimages);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 306f6310ee..50dcb86810 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -721,6 +721,9 @@ testQemuCheckpointDeleteMerge(const void *opaque)
 g_autoptr(virJSONValue) actions = NULL;
 g_autoptr(virJSONValue) nodedatajson = NULL;
 g_autoptr(virHashTable) nodedata = NULL;
+g_autoptr(GSList) reopenimages = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+GSList *tmp;

 expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
  checkpointDeletePrefix, data->name);
@@ -742,14 +745,26 

[PATCH 12/19] virHashAddOrUpdateEntry: Simplify allocation of new entry

[Peter Krempa]

Use g_new0 and skip checking of the return value of keyCopy callback
as both are bound to return a valid pointer.

Signed-off-by: Peter Krempa 
---
 src/util/virhash.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/util/virhash.c b/src/util/virhash.c
index c57d9f8292..36a2d312fc 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -344,7 +344,6 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void 
*name,
 size_t key, len = 0;
 virHashEntryPtr entry;
 virHashEntryPtr last = NULL;
-void *new_name;

 if ((table == NULL) || (name == NULL))
 return -1;
@@ -374,12 +373,8 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void 
*name,
 len++;
 }

-if (VIR_ALLOC(entry) < 0 || !(new_name = table->keyCopy(name))) {
-VIR_FREE(entry);
-return -1;
-}
-
-entry->name = new_name;
+entry = g_new0(virHashEntry, 1);
+entry->name = table->keyCopy(name);
 entry->payload = userdata;

 if (last)
-- 
2.24.1

[PATCH 02/19] tests: qemublock: Add few more test cases for checkpoint deletion

[Peter Krempa]

Add all intermediate steps and deletion of the current checkpoint on a
flat (single-image) disk image.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  4 +++
 .../checkpointdelete/basic-current-out.json   | 29 +++
 .../basic-intermediate1-out.json  | 22 ++
 .../basic-intermediate2-out.json  | 22 ++
 .../basic-intermediate3-out.json  | 22 ++
 5 files changed, 99 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 00056a2b90..e062f1a4cb 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1003,6 +1003,10 @@ mymain(void)
 } while (0)

 TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL);
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a");
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b");
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c");
+TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d");

  cleanup:
 virHashFree(diskxmljsondata.schema);
diff --git a/tests/qemublocktestdata/checkpointdelete/basic-current-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
new file mode 100644
index 00..eccb7ed15f
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "a",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "b"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "b"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
new file mode 100644
index 00..de40e4b5b0
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "c"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json
new file mode 100644
index 00..b5d85f43f0
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "c",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  }
+]
-- 
2.24.1

[PATCH 01/19] tests: qemublock: Add test for checkpoint deletion bitmap merge

[Peter Krempa]

Add test infrastructure and a basic test for bitmap deletion.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c | 59 +++
 .../checkpointdelete/basic-noparent-out.json  |  9 +++
 2 files changed, 68 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3076dc9645..00056a2b90 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -27,6 +27,7 @@
 #include "qemu/qemu_qapi.h"
 #include "qemu/qemu_monitor_json.h"
 #include "qemu/qemu_backup.h"
+#include "qemu/qemu_checkpoint.h"

 #include "qemu/qemu_command.h"

@@ -700,6 +701,50 @@ testQemuBackupIncrementalBitmapCalculate(const void 
*opaque)
 }


+static const char *checkpointDeletePrefix = 
"qemublocktestdata/checkpointdelete/";
+
+struct testQemuCheckpointDeleteMergeData {
+const char *name;
+virStorageSourcePtr chain;
+const char *deletebitmap;
+const char *parentbitmap;
+};
+
+
+static int
+testQemuCheckpointDeleteMerge(const void *opaque)
+{
+const struct testQemuCheckpointDeleteMergeData *data = opaque;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_autoptr(virJSONValue) actions = NULL;
+bool currentcheckpoint;
+
+expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
+ checkpointDeletePrefix, data->name);
+
+if (!(actions = virJSONValueNewArray()))
+return -1;
+
+/* hack to get the 'current' state until the function stops accepting it */
+currentcheckpoint = STREQ("current", data->deletebitmap);
+
+if (qemuCheckpointDiscardDiskBitmaps(data->chain,
+ data->deletebitmap,
+ data->parentbitmap,
+ currentcheckpoint,
+ actions) < 0) {
+VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n");
+return -1;
+}
+
+if (!(actual = virJSONValueToString(actions, true)))
+return -1;
+
+return virTestCompareToFile(actual, expectpath);
+}
+
+
 static int
 mymain(void)
 {
@@ -709,6 +754,7 @@ mymain(void)
 struct testQemuDiskXMLToJSONData diskxmljsondata;
 struct testQemuImageCreateData imagecreatedata;
 struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata;
+struct testQemuCheckpointDeleteMergeData checkpointdeletedata;
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 g_autoptr(virStorageSource) bitmapSourceChain = NULL;
@@ -945,6 +991,19 @@ mymain(void)
 TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, 
"d", "snapshots");
 TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", 
"snapshots");

+#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp) \
+do { \
+checkpointdeletedata.name = testname; \
+checkpointdeletedata.chain = bitmapSourceChain; \
+checkpointdeletedata.deletebitmap = delbmp; \
+checkpointdeletedata.parentbitmap = parbmp; \
+if (virTestRun("checkpoint delete " testname, \
+   testQemuCheckpointDeleteMerge, ) < 
0) \
+ret = -1; \
+} while (0)
+
+TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL);
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git a/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
new file mode 100644
index 00..e87382fdb4
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
@@ -0,0 +1,9 @@
+[
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "a"
+}
+  }
+]
-- 
2.24.1

[PATCH 09/19] util: json: Introduce virJSONValueArrayConcat

[Peter Krempa]

Add a helper that concatenates the second array into the first.

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms |  1 +
 src/util/virjson.c   | 31 +++
 src/util/virjson.h   |  2 ++
 3 files changed, 34 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ebf830791e..b2d2420415 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2284,6 +2284,7 @@ virISCSIScanTargets;
 virJSONStringReformat;
 virJSONValueArrayAppend;
 virJSONValueArrayAppendString;
+virJSONValueArrayConcat;
 virJSONValueArrayForeachSteal;
 virJSONValueArrayGet;
 virJSONValueArraySize;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 988a09e956..50993648eb 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -811,6 +811,37 @@ virJSONValueArrayAppendString(virJSONValuePtr object,
 }


+/**
+ * virJSONValueArrayConcat:
+ * @a: JSON value array (destination)
+ * @c: JSON value array (source)
+ *
+ * Merges the members of @c array into @a. The values are stolen from @c.
+ */
+int
+virJSONValueArrayConcat(virJSONValuePtr a,
+virJSONValuePtr c)
+{
+size_t i;
+
+if (a->type != VIR_JSON_TYPE_ARRAY ||
+c->type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("expecting JSON 
array"));
+return -1;
+}
+
+a->data.array.values = g_renew(virJSONValuePtr, a->data.array.values,
+   a->data.array.nvalues + 
c->data.array.nvalues);
+
+for (i = 0; i < c->data.array.nvalues; i++)
+a->data.array.values[a->data.array.nvalues++] = 
g_steal_pointer(>data.array.values[i]);
+
+c->data.array.nvalues = 0;
+
+return 0;
+}
+
+
 int
 virJSONValueObjectHasKey(virJSONValuePtr object,
  const char *key)
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 7a6b063b17..0894e91b59 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -71,6 +71,8 @@ virJSONValuePtr virJSONValueNewArrayFromBitmap(virBitmapPtr 
bitmap);

 int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, 
virJSONValuePtr value);
 int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value);
+int virJSONValueArrayConcat(virJSONValuePtr a,
+virJSONValuePtr c);

 int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key);
 virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key);
-- 
2.24.1

[PATCH 04/19] qemu: checkpoint: Introduce support for deleting checkpoints accross snapshots

[Peter Krempa]

Allow deleting of checkpoints when snapshots were created along. The
code tracks and modifies the checkpoint list so that backups can still
be taken with such a backing chain. This unfortunately requires to
rename few bitmaps (by copying and deleting them) in some cases.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 112 -
 src/qemu/qemu_checkpoint.h |   5 +-
 tests/qemublocktest.c  |  34 +++
 3 files changed, 111 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index e75cdd0458..087a740cf8 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -24,6 +24,7 @@
 #include "qemu_capabilities.h"
 #include "qemu_monitor.h"
 #include "qemu_domain.h"
+#include "qemu_block.h"

 #include "virerror.h"
 #include "virlog.h"
@@ -150,39 +151,92 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,

 int
 qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ virHashTablePtr blockNamedNodeData,
  const char *delbitmap,
  const char *parentbitmap,
- bool chkcurrent,
- virJSONValuePtr actions)
+ virJSONValuePtr actions,
+ const char *diskdst)
 {
-if (parentbitmap) {
-g_autoptr(virJSONValue) arr = NULL;
+virStorageSourcePtr n = src;

-if (!(arr = virJSONValueNewArray()))
-return -1;
+/* find the backing chain entry with bitmap named '@bitmap' */
+while (n) {
+qemuBlockNamedNodeDataBitmapPtr tmp;

-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
- src->nodeformat,
- delbitmap) < 0)
-return -1;
+if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+ n, delbitmap))) {
+break;
+}
+
+n = n->backingStore;
+}
+
+if (!n) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("bitmap '%s' not found in backing chain of '%s'"),
+   delbitmap, diskdst);
+return -1;
+}

-if (chkcurrent) {
-if (qemuMonitorTransactionBitmapEnable(actions,
-   src->nodeformat,
-   parentbitmap) < 0)
+while (n) {
+qemuBlockNamedNodeDataBitmapPtr srcbitmap;
+
+if (!(srcbitmap = 
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+n, delbitmap)))
+break;
+
+/* For the actual checkpoint deletion we will merge any bitmap into the
+ * bitmap of the parent checkpoint (@mergebitmap) or for any image
+ * where the parent checkpoint bitmap is not present we must rename
+ * the bitmap of the deleted checkpoint into the bitmap of the parent
+ * checkpoint as qemu can't currently take the allocation map and turn
+ * it into a bitmap and thus we wouldn't be able to do a backup. */
+if (parentbitmap) {
+qemuBlockNamedNodeDataBitmapPtr dstbitmap;
+g_autoptr(virJSONValue) arr = NULL;
+
+dstbitmap = 
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+  n, parentbitmap);
+
+if (dstbitmap) {
+if (srcbitmap->recording && !dstbitmap->recording) {
+if (qemuMonitorTransactionBitmapEnable(actions,
+   n->nodeformat,
+   dstbitmap->name) < 
0)
+return -1;
+}
+
+} else {
+if (qemuMonitorTransactionBitmapAdd(actions,
+n->nodeformat,
+parentbitmap,
+true,
+!srcbitmap->recording,
+srcbitmap->granularity) < 
0)
+return -1;
+}
+
+if (!(arr = virJSONValueNewArray()))
+return -1;
+
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+ n->nodeformat,
+ 
srcbitmap->name) < 0)
+return -1;
+
+if (qemuMonitorTransactionBitmapMerge(actions,
+ 

[PATCH 16/19] tests: qemublocktest: Add another synthetic test case for broken bitmaps

[Peter Krempa]

Add a case where a bitmap spanning multiple images is missing one of the
intermediate components.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |   8 +
 .../bitmap/snapshots-synthetic-broken.json| 819 ++
 .../bitmap/snapshots-synthetic-broken.out |  12 +
 3 files changed, 839 insertions(+)
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.out

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 133372908a..ada3608e53 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1030,6 +1030,7 @@ mymain(void)
 TEST_BITMAP_DETECT("synthetic");
 TEST_BITMAP_DETECT("snapshots");
 TEST_BITMAP_DETECT("snapshots-synthetic-checkpoint");
+TEST_BITMAP_DETECT("snapshots-synthetic-broken");

 #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \
 do { \
@@ -1116,6 +1117,13 @@ mymain(void)
 TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "c", true);
 TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "d", true);
 TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "current", true);
+
+TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "a", false);
+TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "b", true);
+TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "c", true);
+TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "d", false);
+TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "current", true);
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git a/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json 
b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json
new file mode 100644
index 00..bf4963494f
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json
@@ -0,0 +1,819 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911522",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"full-backing-filename": "/tmp/pull4.qcow2",
+"backing-filename": "/tmp/pull4.qcow2",
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911527",
+"cluster-size": 65536,

[PATCH 06/19] tests: qemublock: Add checkpoint deletion tests for some special cases

[Peter Krempa]

Use the synthetic test data to verify that the algorithm correctly picks
bitmaps to merge when the bitmap is changed along with the image itself.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  7 +++
 ...hots-synthetic-checkpoint-current-out.json | 29 +
 ...ynthetic-checkpoint-intermediate1-out.json | 29 +
 ...ynthetic-checkpoint-intermediate2-out.json | 32 ++
 ...ynthetic-checkpoint-intermediate3-out.json | 59 +++
 ...ots-synthetic-checkpoint-noparent-out.json | 23 
 6 files changed, 179 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 1c04929e81..306f6310ee 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1025,6 +1025,13 @@ mymain(void)
 TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", 
"snapshots");
 TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", 
"snapshots");

+TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-noparent", 
"a", NULL, "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate1", 
"b", "a", "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate2", 
"c", "b", "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate3", 
"d", "c", "snapshots-synthetic-checkpoint");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-current", 
"current", "d", "snapshots-synthetic-checkpoint");
+
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
new file mode 100644
index 00..e979691e6f
--- /dev/null
+++ 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "a"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "a",
+  "bitmaps": [
+{
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
new file mode 100644
index 00..e82098918a
--- /dev/null
+++ 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
@@ -0,0 +1,32 @@
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "b",
+  "persistent": true,
+  "disabled": true,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "c"
+  

[PATCH 00/19] qemu: backup: Add support for checkpoint deletion and block copy with checkpoints

[Peter Krempa]

The first 7 patches are technically v2 of

[libvirt] [RFC PATCH 00/16] qemu: checkpoint: Add support for deleting 
checkpoints accross snapshots

https://www.redhat.com/archives/libvir-list/2020-January/msg00430.html

as they were not reviewed, but the handling of block copy would conflict
with them I'm reposting them with two simple bugfixes.

The rest of the series implements handling of bitmaps when doing a block
copy.

Peter Krempa (19):
  tests: qemublock: Add test for checkpoint deletion bitmap merge
  tests: qemublock: Add few more test cases for checkpoint deletion
  tests: qemublock: Add synthetic snapshot+checkpoint test data
  qemu: checkpoint: Introduce support for deleting checkpoints accross
snapshots
  tests: qemublock: Add checkpoint deletion test for deep backing chain
  tests: qemublock: Add checkpoint deletion tests for some special cases
  qemu: checkpoint: Track and relabel images for bitmap merging
  qemu: block: Extract calls of qemuBlockGetNamedNodeData into a helper
function
  util: json: Introduce virJSONValueArrayConcat
  virJSONValueNewArray: Use g_new0 to allocate and remove NULL checks
from callers
  virhash: Make sure that hash key is always copied
  virHashAddOrUpdateEntry: Simplify allocation of new entry
  qemu: blockjob: Store 'jobflags' with block job data
  qemu: blockjob: Store 'flags' for all the block job types
  qemu: block: Add validator for bitmap chains accross backing chains
  tests: qemublocktest: Add another synthetic test case for broken
bitmaps
  qemu: block: Introduce function to calculate bitmap handling for
block-copy
  tests: qemublock: Add tests for qemuBlockBitmapsHandleBlockcopy
  qemuDomainBlockPivot: Copy bitmaps backing checkpoints for
virDomainBlockCopy

 src/conf/domain_addr.c|   5 +-
 src/libvirt_private.syms  |   1 +
 src/locking/lock_daemon.c |   4 +-
 src/logging/log_handler.c |   3 +-
 src/network/leaseshelper.c|   6 +-
 src/qemu/qemu_agent.c |   6 +-
 src/qemu/qemu_backup.c|  11 +-
 src/qemu/qemu_block.c | 208 -
 src/qemu/qemu_block.h |  16 +
 src/qemu/qemu_blockjob.c  |  16 +-
 src/qemu/qemu_blockjob.h  |  12 +-
 src/qemu/qemu_checkpoint.c| 146 +++-
 src/qemu/qemu_checkpoint.h|   6 +-
 src/qemu/qemu_domain.c|   7 +
 src/qemu/qemu_driver.c|  54 +-
 src/qemu/qemu_firmware.c  |  12 +-
 src/qemu/qemu_migration_params.c  |   3 +-
 src/qemu/qemu_monitor_json.c  |   3 +-
 src/rpc/virnetserver.c|   6 +-
 src/rpc/virnetserverservice.c |   3 +-
 src/util/virhash.c|  13 +-
 src/util/virhash.h|   3 +-
 src/util/virjson.c|  44 +-
 src/util/virjson.h|   2 +
 src/util/virlockspace.c   |   6 +-
 src/util/virmacmap.c  |   8 +-
 tests/qemublocktest.c | 250 +-
 .../bitmap/snapshots-synthetic-broken.json| 819 +
 .../bitmap/snapshots-synthetic-broken.out |  12 +
 .../snapshots-synthetic-checkpoint.json   | 827 ++
 .../bitmap/snapshots-synthetic-checkpoint.out |  13 +
 .../bitmapblockcopy/basic-deep-out.json   | 117 +++
 .../bitmapblockcopy/basic-shallow-out.json| 117 +++
 .../bitmapblockcopy/snapshots-deep-out.json   | 133 +++
 .../snapshots-shallow-out.json|  48 +
 .../checkpointdelete/basic-current-out.json   |  29 +
 .../basic-intermediate1-out.json  |  22 +
 .../basic-intermediate2-out.json  |  22 +
 .../basic-intermediate3-out.json  |  22 +
 .../checkpointdelete/basic-noparent-out.json  |   9 +
 .../snapshots-current-out.json|  29 +
 .../snapshots-intermediate1-out.json  |  24 +
 .../snapshots-intermediate2-out.json  |  62 ++
 .../snapshots-intermediate3-out.json  |  61 ++
 .../snapshots-noparent-out.json   |  27 +
 ...hots-synthetic-checkpoint-current-out.json |  29 +
 ...ynthetic-checkpoint-intermediate1-out.json |  31 +
 ...ynthetic-checkpoint-intermediate2-out.json |  34 +
 ...ynthetic-checkpoint-intermediate3-out.json |  61 ++
 ...ots-synthetic-checkpoint-noparent-out.json |  27 +
 tests/qemumonitorjsontest.c   |   5 +-
 .../qemustatusxml2xmldata/backup-pull-in.xml  |   2 +-
 .../blockjob-blockdev-in.xml  |   8 +-
 53 files changed, 3293 insertions(+), 151 deletions(-)
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.out
 create mode 

Re: [libvirt] [PATCH v1 0/8] domain_addr and qemu_domain cleanups

[Michal Privoznik]

On 1/9/20 7:33 PM, Daniel Henrique Barboza wrote:

Hi,

These are a few cleanups in some files that I'll end up
messing with in a future series (which isn't Glibfied ATM).
Figured it's best to clean them up now instead of adding
more non-Glibfied code on top.

Patches 7 and 8 are stuff that I noted some time ago and
figured it might be worth sending as well.


Daniel Henrique Barboza (8):
   conf/domain_addr.c: use g_autofree in strings
   conf/domain_addr.c: remove unneeded 'cleanup' labels
   qemu_domain.c: remove redundant virObjectUnref()
   qemu_domain.c: use g_autofree when possible
   qemu_domain.c: use g_autoptr when possible
   qemu_domain.c: removing unneeded cleanup labels
   qemu_domain.c: turn qemuDomainChrDefDropDefaultPath to void
   qemu_domain_address.c: turn qemuDomainFillDeviceIsolationGroup to void

  src/conf/domain_addr.c | 124 +++-
  src/qemu/qemu_domain.c | 511 +++--
  src/qemu/qemu_domain_address.c |  20 +-
  src/qemu/qemu_domain_address.h |   2 +-
  src/qemu/qemu_hotplug.c|   6 +-
  5 files changed, 227 insertions(+), 436 deletions(-)



Reviewed-by: Michal Privoznik  and pushed. Sorry 
for the delay.


Michal

Re: [hooks PATCH] Don't allow @localhost email addresses in commit message

[Ján Tomko]

On Fri, Jan 31, 2020 at 10:38:56AM +0100, Andrea Bolognani wrote:

On Mon, 2020-01-27 at 16:12 +, Daniel P. Berrangé wrote:

+   allow_localhost_email=$(git config --bool hooks.allowlocalhostemail)


The comment at the beginning of the script documents most hooks.*
configuration options[1], so please document this one as well.


+   git show $rev | grep '@localhost' >/dev/null 2>&1
+   if test $? != 0
+   then
+   echo "*** Update hook: @localhost email address is 
forbidden $rev" >&2
+   exit 1
+   fi


This seems excessively harsh, as it would block commits where
@localhost appears in the diff[2] or is used in the commit message
in a legitimate way[3].

We need to either be way more specific in selecting the parts of
the commit we care about (Author:, Commit:, Signed-off-by: and
Reviewed-by: at the very least) or replace the match with something


Unlike others, Reviewed-by is not generated from git config, so usually
it's prone to different errors than not filling the value :)


like

 grep -E '<.*@localhost.*>'



I think I actually saw people register 'localhost' as a 2nd level domain
somewhere, but we can worry about that after they send a patch.

Jano


which I think should catch most plausible mistakes without running
into false positives.


[1] hooks.allowmissingsob is notably missing, perhaps whoever
   introduced it should go back and document it ;)
[2] 3a3a85c529e92ad767fbe01587186854c175, though of course you
   could argue that such a commit would not have been necessary if
   we had this hook in the first place O:-)
[3] 3f1a7070428df12dae78c5b3963fa02c0b4ef5f1 and a few others
--
Andrea Bolognani / Red Hat / Virtualization



signature.asc
Description: PGP signature

Re: [libvirt PATCH] docs: Update godoc URLs for Go packages

[Ján Tomko]

On Thu, Jan 30, 2020 at 06:15:14PM +0100, Andrea Bolognani wrote:

Since

 commit 201156ec6e65e08cdb04d6fdefe013652fb7fa87
 Author: Daniel P. Berrangé 
 Date:   Mon Dec 16 10:06:50 2019 +

   docs: add pages to support Go module package resolution

it's possible to use

 import "libvirt.org/libvirt-go"
 import "libvirt.org/libvirt-go-xml"

in Go programs, which will result in the Go packages being
fetched directly from libvirt.org instead of GitHub.

The godoc website offers the API documentation for the updated
imports, and that's where we should send users.

Signed-off-by: Andrea Bolognani 
---
docs/downloads.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH] docs: Update download location for Go packages

[Ján Tomko]

On Thu, Jan 30, 2020 at 06:13:55PM +0100, Andrea Bolognani wrote:

We haven't ever made releases of the Go components, which makes
sense because that's just not how that specific ecosystem works.

For people who are looking to use libvirt from Go, the brief
introductory pages located at the same URLs used to import the
packages are a much better entry point than an empty directory
listing, so let's send them there instead.



Yeah, it's a slight improvement.


Signed-off-by: Andrea Bolognani 
---
docs/downloads.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [hooks PATCH] Don't allow @localhost email addresses in commit message

[Andrea Bolognani]

On Mon, 2020-01-27 at 16:12 +, Daniel P. Berrangé wrote:
> + allow_localhost_email=$(git config --bool hooks.allowlocalhostemail)

The comment at the beginning of the script documents most hooks.*
configuration options[1], so please document this one as well.

> + git show $rev | grep '@localhost' >/dev/null 2>&1
> + if test $? != 0
> + then
> + echo "*** Update hook: @localhost email address 
> is forbidden $rev" >&2
> + exit 1
> + fi

This seems excessively harsh, as it would block commits where
@localhost appears in the diff[2] or is used in the commit message
in a legitimate way[3].

We need to either be way more specific in selecting the parts of
the commit we care about (Author:, Commit:, Signed-off-by: and
Reviewed-by: at the very least) or replace the match with something
like

  grep -E '<.*@localhost.*>'

which I think should catch most plausible mistakes without running
into false positives.


[1] hooks.allowmissingsob is notably missing, perhaps whoever
introduced it should go back and document it ;)
[2] 3a3a85c529e92ad767fbe01587186854c175, though of course you
could argue that such a commit would not have been necessary if
we had this hook in the first place O:-)
[3] 3f1a7070428df12dae78c5b3963fa02c0b4ef5f1 and a few others
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v2 38/56] tools: convert to use g_poll instead of poll

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:19PM +, Daniel P. Berrangé wrote:
> g_poll is portable to Windows platforms.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tools/virsh-domain.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 1a48695b4e..04ba44d4f2 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -23,7 +23,6 @@
>  #include "virsh-util.h"
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  
> @@ -4299,15 +4298,15 @@ virshWatchJob(vshControl *ctl,
>  struct sigaction old_sig_action;
>  sigset_t sigmask, oldsigmask;
>  #endif /* !WIN32 */
> -struct pollfd pollfd[2] = {{.fd = pipe_fd, .events = POLLIN, .revents = 
> 0},
> -   {.fd = STDIN_FILENO, .events = POLLIN, 
> .revents = 0}};
> +GPollFD pollfd[2] = {{.fd = pipe_fd, .events = G_IO_IN, .revents = 0},
> + {.fd = STDIN_FILENO, .events = G_IO_IN, .revents = 
> 0}};

This doesn't look right.  Based on the GLib documentation in case of
unix the .fd should be file descriptor but in case of WIN32 it should
be HANDLE.

Looking at the GNULIB implementation it was converting FD into HANDLE
internally in the poll() function so everything was safe but in case
of g_poll() they except the .fd already be converted to HANDLE.

Our implementation of virPipeQuiet calls _poll() for WIN32 but based on
the reference [1] it looks like it returns FD.

I guess that we should create a wrapper around g_poll to covert FD into
HANDLE on WIN32 to be on a safe side.

Pavel

>  unsigned long long start_us, curr_us;
>  virDomainJobInfo jobinfo;
>  int ret = -1;
>  char retchar;
>  bool functionReturn = false;
>  bool jobStarted = false;
> -nfds_t npollfd = 2;
> +int npollfd = 2;
>  
>  #ifndef WIN32
>  sigemptyset();
> @@ -4326,16 +4325,16 @@ virshWatchJob(vshControl *ctl,
>  
>  start_us = g_get_real_time();
>  while (1) {
> -ret = poll((struct pollfd *), npollfd, 500);
> +ret = g_poll(pollfd, npollfd, 500);
>  if (ret > 0) {
> -if (pollfd[1].revents & POLLIN &&
> +if (pollfd[1].revents & G_IO_IN &&
>  saferead(STDIN_FILENO, , sizeof(retchar)) > 0) {
>  if (vshTTYIsInterruptCharacter(ctl, retchar))
>  virDomainAbortJob(dom);
>  continue;
>  }
>  
> -if (pollfd[0].revents & POLLIN &&
> +if (pollfd[0].revents & G_IO_IN &&
>  saferead(pipe_fd, , sizeof(retchar)) > 0 &&
>  retchar == '0') {
>  if (verbose) {
> -- 
> 2.24.1
> 


signature.asc
Description: PGP signature

Re: [libvirt PATCH v2 37/56] tests: refactor event test to not run lock step

[Pavel Hrdina]

On Tue, Jan 28, 2020 at 01:11:18PM +, Daniel P. Berrangé wrote:
> The current event loop test suite has two threads running
> in lockstep. This was just about viable when we have full
> control over the internal details of the event loop impl.
> When we're using the GLib event loop though there are
> things going on that we don't know about, such as use of
> eventfd() file descriptors. This will break the assumptions
> in the test suite, causing non-deterministic failures.
> 
> This change switches the event loop thread to run fully
> asynchronously from the test suite cases. This is slightly
> weaker validation, but the only way we can get a reliable
> test suite.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tests/eventtest.c | 151 +++---
>  1 file changed, 63 insertions(+), 88 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature