[libvirt] [PATCH 0/4] incremental backup: QMP interactions
I'm trying to break my incremental backup work into smaller pieces so I can make progress and actually get some code committed, while I continue to hammer away at preparing a v9 series that addresses even more review comments and things learned during testing. These few patches represent the bulk of 14-15/21 from v8, except that the operation to update Checkpoint size must come later in the series when checkpoint_conf.h is ready. Another reason to commit this now, even without the incremental backup clients of the new capabilities, is that capability additions tend to be a conflict magnet, so I'd rather stabilize this and reduce the amount of rebase work I have been doing. Changes from v8: - improve testing of nbd-server-start - split up QMP changes into more manageable pieces - address formatting review comments Eric Blake (4): backup: Prepare for Unix sockets in QMP nbd-server-start backup: Add two new qemu capabilities backup: Add new qemu monitor bitmap backup: Add new parameters to qemu monitor nbd-server-add src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_monitor.h | 29 +++- src/qemu/qemu_monitor_json.h | 24 ++- src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_migration.c | 9 +- src/qemu/qemu_monitor.c | 74 ++++++++- src/qemu/qemu_monitor_json.c | 149 +++++++++++++++++- .../caps_4.0.0.aarch64.xml | 2 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 + .../caps_4.0.0.riscv32.xml | 2 + .../caps_4.0.0.riscv64.xml | 2 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 + .../caps_4.0.0.x86_64.xml | 2 + tests/qemumonitorjsontest.c | 60 ++++++- 14 files changed, 341 insertions(+), 26 deletions(-) -- 2.20.1
Migration always uses a TCP socket for NBD servers, because we don't support same-host migration. But upcoming pull-mode incremental backup needs to also support a Unix socket, for retrieving the backup from the same host. Support this by plumbing virStorageNetHostDef through the monitor calls, since that is a nice reusable struct that can track both TCP and Unix sockets. Update qemumonitorjsontest to not only test the response to the command, but to actually verify that the command itself uses the two correct QMP forms. I'm actually a bit surprised that we are not utilizing qemuMonitorTestAddItemVerbatim more frequently. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 6 ++-- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_migration.c | 7 ++++- src/qemu/qemu_monitor.c | 13 +++++--- src/qemu/qemu_monitor_json.c | 23 ++++++++++---- tests/qemumonitorjsontest.c | 58 ++++++++++++++++++++++++++++++++++-- 6 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index dee594fa66..fa84ff821e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1094,9 +1094,9 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon, char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); int qemuMonitorNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, - const char *tls_alias); + const virStorageNetHostDef *server, + const char *tls_alias) + ATTRIBUTE_NONNULL(2); int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index acef1a0a79..e41bdc8c4f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -459,8 +459,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias); int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 32b3040473..267a729c6f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -380,6 +380,10 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, unsigned short port = 0; char *diskAlias = NULL; size_t i; + virStorageNetHostDef server = { + .name = (char *)listenAddr, /* cast away const */ + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + }; if (nbdPort < 0 || nbdPort > USHRT_MAX) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -415,7 +419,8 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto exit_monitor; - if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, tls_alias) < 0) + server.port = port; + if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) goto exit_monitor; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6b731cd91a..187513a986 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3925,15 +3925,20 @@ qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, int qemuMonitorNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias) { - VIR_DEBUG("host=%s port=%u tls_alias=%s", host, port, NULLSTR(tls_alias)); + /* Peek inside the struct for nicer logging */ + if (server->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) + VIR_DEBUG("server={tcp host=%s port=%u} tls_alias=%s", + NULLSTR(server->name), server->port, NULLSTR(tls_alias)); + else + VIR_DEBUG("server={unix socket=%s} tls_alias=%s", + NULLSTR(server->socket), NULLSTR(tls_alias)); QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONNBDServerStart(mon, host, port, tls_alias); + return qemuMonitorJSONNBDServerStart(mon, server, tls_alias); } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 53a7de8b77..93113d4e8a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6684,8 +6684,7 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path) int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias) { int ret = -1; @@ -6694,10 +6693,22 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, virJSONValuePtr addr = NULL; char *port_str = NULL; - if (virAsprintf(&port_str, "%u", port) < 0) - return ret; - - if (!(addr = qemuMonitorJSONBuildInetSocketAddress(host, port_str))) + switch ((virStorageNetHostTransport)server->transport) { + case VIR_STORAGE_NET_HOST_TRANS_TCP: + if (virAsprintf(&port_str, "%u", server->port) < 0) + return ret; + addr = qemuMonitorJSONBuildInetSocketAddress(server->name, port_str); + break; + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + addr = qemuMonitorJSONBuildUnixSocketAddress(server->socket); + break; + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid server address")); + goto cleanup; + } + if (!addr) goto cleanup; if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start", diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0894e748ae..9d707fcc7c 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1348,7 +1348,6 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "back GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") GEN_TEST_FUNC(qemuMonitorJSONScreendump, "devicename", 1, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) -GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345, "test-alias") GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1") GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) @@ -1356,6 +1355,61 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode") +static int +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque; + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); + int ret = -1; + virStorageNetHostDef server_tcp = { + .name = (char *)"localhost", + .port = 12345, + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + }; + virStorageNetHostDef server_unix = { + .socket = (char *)"/tmp/sock", + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX, + }; + + if (!test) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"inet\",\"data\":{" + " \"host\":\"localhost\"," + " \"port\":\"12345\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-1\"}", + NULL, "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"unix\",\"data\":{" + " \"path\":\"/tmp/sock\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-2\"}", + NULL, "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test), + &server_tcp, "test-alias") < 0) + goto cleanup; + + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test), + &server_unix, "test-alias") < 0) + goto cleanup; + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} + static bool testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a, struct qemuMonitorQueryCpusEntry *b) @@ -3014,7 +3068,6 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONDrivePivot); DO_TEST_GEN(qemuMonitorJSONScreendump); DO_TEST_GEN(qemuMonitorJSONOpenGraphics); - DO_TEST_GEN(qemuMonitorJSONNBDServerStart); DO_TEST_GEN(qemuMonitorJSONNBDServerAdd); DO_TEST_GEN(qemuMonitorJSONDetachCharDev); DO_TEST_GEN(qemuMonitorJSONBlockdevTrayOpen); @@ -3036,6 +3089,7 @@ mymain(void) DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability); DO_TEST(qemuMonitorJSONSendKeyHoldtime); DO_TEST(qemuMonitorSupportsActiveCommit); + DO_TEST(qemuMonitorJSONNBDServerStart); DO_TEST_CPU_DATA("host"); DO_TEST_CPU_DATA("full"); -- 2.20.1
On Wed, Jun 05, 2019 at 22:01:07 -0500, Eric Blake wrote:
Migration always uses a TCP socket for NBD servers, because we don't support same-host migration. But upcoming pull-mode incremental backup needs to also support a Unix socket, for retrieving the backup from the same host. Support this by plumbing virStorageNetHostDef through the monitor calls, since that is a nice reusable struct that can track both TCP and Unix sockets.
Update qemumonitorjsontest to not only test the response to the command, but to actually verify that the command itself uses the two correct QMP forms. I'm actually a bit surprised that we are not utilizing qemuMonitorTestAddItemVerbatim more frequently.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 6 ++-- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_migration.c | 7 ++++- src/qemu/qemu_monitor.c | 13 +++++--- src/qemu/qemu_monitor_json.c | 23 ++++++++++---- tests/qemumonitorjsontest.c | 58 ++++++++++++++++++++++++++++++++++-- 6 files changed, 92 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index dee594fa66..fa84ff821e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1094,9 +1094,9 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon, char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
int qemuMonitorNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, - const char *tls_alias); + const virStorageNetHostDef *server, + const char *tls_alias) + ATTRIBUTE_NONNULL(2); int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index acef1a0a79..e41bdc8c4f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -459,8 +459,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon);
int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias); int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 32b3040473..267a729c6f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -380,6 +380,10 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, unsigned short port = 0; char *diskAlias = NULL; size_t i; + virStorageNetHostDef server = { + .name = (char *)listenAddr, /* cast away const */ + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + };
if (nbdPort < 0 || nbdPort > USHRT_MAX) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -415,7 +419,8 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto exit_monitor;
- if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, tls_alias) < 0) + server.port = port; + if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) goto exit_monitor; }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6b731cd91a..187513a986 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3925,15 +3925,20 @@ qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
int qemuMonitorNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias) { - VIR_DEBUG("host=%s port=%u tls_alias=%s", host, port, NULLSTR(tls_alias)); + /* Peek inside the struct for nicer logging */ + if (server->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) + VIR_DEBUG("server={tcp host=%s port=%u} tls_alias=%s", + NULLSTR(server->name), server->port, NULLSTR(tls_alias)); + else + VIR_DEBUG("server={unix socket=%s} tls_alias=%s", + NULLSTR(server->socket), NULLSTR(tls_alias));
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONNBDServerStart(mon, host, port, tls_alias); + return qemuMonitorJSONNBDServerStart(mon, server, tls_alias); }
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 53a7de8b77..93113d4e8a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6684,8 +6684,7 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path)
int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, - const char *host, - unsigned int port, + const virStorageNetHostDef *server, const char *tls_alias) { int ret = -1; @@ -6694,10 +6693,22 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, virJSONValuePtr addr = NULL; char *port_str = NULL;
- if (virAsprintf(&port_str, "%u", port) < 0) - return ret; - - if (!(addr = qemuMonitorJSONBuildInetSocketAddress(host, port_str))) + switch ((virStorageNetHostTransport)server->transport) { + case VIR_STORAGE_NET_HOST_TRANS_TCP: + if (virAsprintf(&port_str, "%u", server->port) < 0) + return ret; + addr = qemuMonitorJSONBuildInetSocketAddress(server->name, port_str); + break; + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + addr = qemuMonitorJSONBuildUnixSocketAddress(server->socket); + break; + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid server address")); + goto cleanup; + } + if (!addr) goto cleanup;
if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start", diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0894e748ae..9d707fcc7c 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1348,7 +1348,6 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "back GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") GEN_TEST_FUNC(qemuMonitorJSONScreendump, "devicename", 1, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) -GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345, "test-alias") GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1") GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) @@ -1356,6 +1355,61 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode")
+static int +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque; + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
I'd suggest you use qemuMonitorTestNewSchema here so that we preserve the schema checks.
+ int ret = -1; + virStorageNetHostDef server_tcp = { + .name = (char *)"localhost", + .port = 12345, + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + }; + virStorageNetHostDef server_unix = { + .socket = (char *)"/tmp/sock", + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX, + }; + + if (!test) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"inet\",\"data\":{" + " \"host\":\"localhost\"," + " \"port\":\"12345\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-1\"}", + NULL, "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"unix\",\"data\":{" + " \"path\":\"/tmp/sock\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-2\"}", + NULL, "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test), + &server_tcp, "test-alias") < 0) + goto cleanup; + + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test), + &server_unix, "test-alias") < 0) + goto cleanup; + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} + static bool testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a, struct qemuMonitorQueryCpusEntry *b) @@ -3014,7 +3068,6 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONDrivePivot); DO_TEST_GEN(qemuMonitorJSONScreendump); DO_TEST_GEN(qemuMonitorJSONOpenGraphics); - DO_TEST_GEN(qemuMonitorJSONNBDServerStart);
Which are deleted here.
DO_TEST_GEN(qemuMonitorJSONNBDServerAdd); DO_TEST_GEN(qemuMonitorJSONDetachCharDev); DO_TEST_GEN(qemuMonitorJSONBlockdevTrayOpen); @@ -3036,6 +3089,7 @@ mymain(void)
ACK with schema checks preserved.
On 6/6/19 7:53 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:07 -0500, Eric Blake wrote:
Migration always uses a TCP socket for NBD servers, because we don't support same-host migration. But upcoming pull-mode incremental backup needs to also support a Unix socket, for retrieving the backup from the same host. Support this by plumbing virStorageNetHostDef through the monitor calls, since that is a nice reusable struct that can track both TCP and Unix sockets.
Update qemumonitorjsontest to not only test the response to the command, but to actually verify that the command itself uses the two correct QMP forms. I'm actually a bit surprised that we are not utilizing qemuMonitorTestAddItemVerbatim more frequently.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
+static int +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque; + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
I'd suggest you use qemuMonitorTestNewSchema here so that we preserve the schema checks.
Oh, I _completely_ missed what TestNewSchema was providing.
+ int ret = -1; + virStorageNetHostDef server_tcp = { + .name = (char *)"localhost", + .port = 12345, + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + }; + virStorageNetHostDef server_unix = { + .socket = (char *)"/tmp/sock", + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX, + }; + + if (!test) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"inet\",\"data\":{" + " \"host\":\"localhost\"," + " \"port\":\"12345\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-1\"}", + NULL, "{\"return\":{}}") < 0)
Testing for a verbatim response proves that we generated at least one form of acceptable JSON, but if TestNewSchema is able to validate that the entire command complies with the introspected schema advertised by qemu, then that also serves to validate things, and with a lot less fragility. Yes, I'll fix that, now that I _finally_ understand what you were asking for (you made a similar comment in your v8 review, and I misunderstood it).
@@ -3036,6 +3089,7 @@ mymain(void)
ACK with schema checks preserved.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Jun 06, 2019 at 08:40:14 -0500, Eric Blake wrote:
On 6/6/19 7:53 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:07 -0500, Eric Blake wrote:
Migration always uses a TCP socket for NBD servers, because we don't support same-host migration. But upcoming pull-mode incremental backup needs to also support a Unix socket, for retrieving the backup from the same host. Support this by plumbing virStorageNetHostDef through the monitor calls, since that is a nice reusable struct that can track both TCP and Unix sockets.
Update qemumonitorjsontest to not only test the response to the command, but to actually verify that the command itself uses the two correct QMP forms. I'm actually a bit surprised that we are not utilizing qemuMonitorTestAddItemVerbatim more frequently.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
+static int +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque; + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
I'd suggest you use qemuMonitorTestNewSchema here so that we preserve the schema checks.
Oh, I _completely_ missed what TestNewSchema was providing.
It has one catch though that I've figured out now. qemuMonitorTestAddItemVerbatim actually does not support schema checking yet. I've implemented only a limited subset of the internals to support it. Probably the best course of actions for this test will be to swithch it to qemuMonitorTestAddItem which does schema checking. In this case I feel it's more useful to do the check against the schema rather than to see that the resposne is the same. Alternatively I can see whether it's reasonably feasible to do schema checking also in qemuMonitorTestAddItemVerbatim.
+ int ret = -1; + virStorageNetHostDef server_tcp = { + .name = (char *)"localhost", + .port = 12345, + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + }; + virStorageNetHostDef server_unix = { + .socket = (char *)"/tmp/sock", + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX, + }; + + if (!test) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"nbd-server-start\"," + " \"arguments\":{\"addr\":{" + " \"type\":\"inet\",\"data\":{" + " \"host\":\"localhost\"," + " \"port\":\"12345\"}}," + " \"tls-creds\":\"test-alias\"}," + " \"id\":\"libvirt-1\"}", + NULL, "{\"return\":{}}") < 0)
Testing for a verbatim response proves that we generated at least one form of acceptable JSON, but if TestNewSchema is able to validate that the entire command complies with the introspected schema advertised by
Well, it still validates only the one given syntax at this point, because there's no sane way to compare everything what libvirt is able to generate.
qemu, then that also serves to validate things, and with a lot less fragility. Yes, I'll fix that, now that I _finally_ understand what you were asking for (you made a similar comment in your v8 review, and I misunderstood it).
The huge advantage of this is that I don't actually have to check for typos and other things manually against qemu's schema :)
@@ -3036,6 +3089,7 @@ mymain(void)
ACK with schema checks preserved.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 07, 2019 at 10:13:04 +0200, Peter Krempa wrote:
On Thu, Jun 06, 2019 at 08:40:14 -0500, Eric Blake wrote:
On 6/6/19 7:53 AM, Peter Krempa wrote:
[...]
In this case I feel it's more useful to do the check against the schema rather than to see that the resposne is the same.
Alternatively I can see whether it's reasonably feasible to do schema checking also in qemuMonitorTestAddItemVerbatim.
https://www.redhat.com/archives/libvir-list/2019-June/msg00210.html So we can keep using qemuMonitorTestAddItemVerbatim here.
On 6/7/19 7:06 AM, Peter Krempa wrote:
On Fri, Jun 07, 2019 at 10:13:04 +0200, Peter Krempa wrote:
On Thu, Jun 06, 2019 at 08:40:14 -0500, Eric Blake wrote:
On 6/6/19 7:53 AM, Peter Krempa wrote:
[...]
In this case I feel it's more useful to do the check against the schema rather than to see that the resposne is the same.
Alternatively I can see whether it's reasonably feasible to do schema checking also in qemuMonitorTestAddItemVerbatim.
https://www.redhat.com/archives/libvir-list/2019-June/msg00210.html
So we can keep using qemuMonitorTestAddItemVerbatim here.
AddItemVerbatim is a pain to maintain; I'd rather stick with AddItem + schema checks. But in doing that, I found that a lot of existing code in the test did not do schema tests; hence I'm planning on pushing these four patches (amended per your review) only after a prerequisite fix of the existing tests: https://www.redhat.com/archives/libvir-list/2019-June/msg00283.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Mon, Jun 10, 2019 at 12:11:34 -0500, Eric Blake wrote:
On 6/7/19 7:06 AM, Peter Krempa wrote:
On Fri, Jun 07, 2019 at 10:13:04 +0200, Peter Krempa wrote:
On Thu, Jun 06, 2019 at 08:40:14 -0500, Eric Blake wrote:
On 6/6/19 7:53 AM, Peter Krempa wrote:
[...]
In this case I feel it's more useful to do the check against the schema rather than to see that the resposne is the same.
Alternatively I can see whether it's reasonably feasible to do schema checking also in qemuMonitorTestAddItemVerbatim.
https://www.redhat.com/archives/libvir-list/2019-June/msg00210.html
So we can keep using qemuMonitorTestAddItemVerbatim here.
AddItemVerbatim is a pain to maintain; I'd rather stick with AddItem + schema checks. But in doing that, I found that a lot of existing code in the test did not do schema tests; hence I'm planning on pushing these four patches (amended per your review) only after a prerequisite fix of
I agree. If there isn't a particular reason to check the data on the monitor as well, using the AddItem is sufficient when we do a schema check. The AddItemVerbatim function makes sense when we couple it with functional testing of other code as well where we need to validate that libvirt's commands are correct as well e.g. as we do for the cpu hotplug tests.
Add two capabilities for testing features required for the upcoming virDomainBackupBegin: use block-dirty-bitmap-merge as the generic witness of bitmap support needed for checkpoints (since all of the bitmap management functionalities were finalized in the same qemu 4.0 release), and the bitmap parameter to nbd-server-add for pull-mode backup support. Even though both capabilities are likely to be present or absent together (that is, it is unlikely to encounter a qemu that backports only one of the two), it still makes sense to keep two capabilities as the two uses are orthogonal (full backups don't require checkpoints, push mode backups don't require NBD bitmap support, and checkpoints can be used for more than just incremental backups). Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_capabilities.c | 6 ++++++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 ++ 8 files changed, 22 insertions(+) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0f282ad239..3032d4edcd 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -509,6 +509,10 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_QUERY_CURRENT_MACHINE, /* query-current-machine command */ QEMU_CAPS_MACHINE_VIRT_IOMMU, /* -machine virt,iommu */ + /* 330 */ + QEMU_CAPS_BITMAP_MERGE, /* block-dirty-bitmap-merge */ + QEMU_CAPS_NBD_BITMAP, /* nbd-server-add supports bitmap */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c8d724aed5..d0fdd1da9f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -526,6 +526,10 @@ VIR_ENUM_IMPL(virQEMUCaps, "overcommit", "query-current-machine", "machine.virt.iommu", + + /* 330 */ + "bitmap-merge", + "nbd-bitmap", ); @@ -970,6 +974,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES }, { "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL }, { "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE }, + { "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { @@ -1264,6 +1269,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "block-commit/arg-type/*top", QEMU_CAPS_ACTIVE_COMMIT }, { "query-iothreads/ret-type/poll-max-ns", QEMU_CAPS_IOTHREAD_POLLING }, { "query-display-options/ret-type/+egl-headless/rendernode", QEMU_CAPS_EGL_HEADLESS_RENDERNODE }, + { "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml index e071bc0c8d..250b7edd52 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml @@ -161,6 +161,8 @@ <flag name='overcommit'/> <flag name='query-current-machine'/> <flag name='machine.virt.iommu'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml index 9c4e6583ec..24b55002a6 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml @@ -166,6 +166,8 @@ <flag name='virtio-pci-non-transitional'/> <flag name='overcommit'/> <flag name='query-current-machine'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index e987cdd415..230e1e7c99 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -164,6 +164,8 @@ <flag name='virtio-pci-non-transitional'/> <flag name='overcommit'/> <flag name='query-current-machine'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index e1a07dd945..4b2f4cf628 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -164,6 +164,8 @@ <flag name='virtio-pci-non-transitional'/> <flag name='overcommit'/> <flag name='query-current-machine'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 8cb537eaf4..a1ac2587a0 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -129,6 +129,8 @@ <flag name='virtio-pci-non-transitional'/> <flag name='overcommit'/> <flag name='query-current-machine'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100758</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index ca7174e3b9..68845cca74 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -202,6 +202,8 @@ <flag name='virtio-pci-non-transitional'/> <flag name='overcommit'/> <flag name='query-current-machine'/> + <flag name='bitmap-merge'/> + <flag name='nbd-bitmap'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100758</microcodeVersion> -- 2.20.1
On Wed, Jun 05, 2019 at 22:01:08 -0500, Eric Blake wrote:
Add two capabilities for testing features required for the upcoming virDomainBackupBegin: use block-dirty-bitmap-merge as the generic witness of bitmap support needed for checkpoints (since all of the bitmap management functionalities were finalized in the same qemu 4.0 release), and the bitmap parameter to nbd-server-add for pull-mode backup support. Even though both capabilities are likely to be present or absent together (that is, it is unlikely to encounter a qemu that backports only one of the two), it still makes sense to keep two capabilities as the two uses are orthogonal (full backups don't require checkpoints, push mode backups don't require NBD bitmap support, and checkpoints can be used for more than just incremental backups).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_capabilities.c | 6 ++++++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 ++ 8 files changed, 22 insertions(+)
So the code looks good to me, but we need to clarify one thing before ACK. Is there anything that would change libvirt's behaviour incompatibly if these are specified? In some cases these also imply a libvirt behaviour change (e.g. different command line) and thus not pushing those in the same release as the implementation might cause problems.
On 6/6/19 7:51 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:08 -0500, Eric Blake wrote:
Add two capabilities for testing features required for the upcoming virDomainBackupBegin: use block-dirty-bitmap-merge as the generic witness of bitmap support needed for checkpoints (since all of the bitmap management functionalities were finalized in the same qemu 4.0 release), and the bitmap parameter to nbd-server-add for pull-mode backup support. Even though both capabilities are likely to be present or absent together (that is, it is unlikely to encounter a qemu that backports only one of the two), it still makes sense to keep two capabilities as the two uses are orthogonal (full backups don't require checkpoints, push mode backups don't require NBD bitmap support, and checkpoints can be used for more than just incremental backups).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_capabilities.c | 6 ++++++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 ++ 8 files changed, 22 insertions(+)
So the code looks good to me, but we need to clarify one thing before ACK.
Is there anything that would change libvirt's behaviour incompatibly if these are specified? In some cases these also imply a libvirt behaviour change (e.g. different command line) and thus not pushing those in the same release as the implementation might cause problems.
So far, the only things that depend on the capabilities are the new code additions for checkpoints and pull-mode backups; existing libvirt.git should have no change in behavior whether or not the capabilities are detected. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Jun 06, 2019 at 08:35:15 -0500, Eric Blake wrote:
On 6/6/19 7:51 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:08 -0500, Eric Blake wrote:
Add two capabilities for testing features required for the upcoming virDomainBackupBegin: use block-dirty-bitmap-merge as the generic witness of bitmap support needed for checkpoints (since all of the bitmap management functionalities were finalized in the same qemu 4.0 release), and the bitmap parameter to nbd-server-add for pull-mode backup support. Even though both capabilities are likely to be present or absent together (that is, it is unlikely to encounter a qemu that backports only one of the two), it still makes sense to keep two capabilities as the two uses are orthogonal (full backups don't require checkpoints, push mode backups don't require NBD bitmap support, and checkpoints can be used for more than just incremental backups).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_capabilities.c | 6 ++++++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 ++ 8 files changed, 22 insertions(+)
So the code looks good to me, but we need to clarify one thing before ACK.
Is there anything that would change libvirt's behaviour incompatibly if these are specified? In some cases these also imply a libvirt behaviour change (e.g. different command line) and thus not pushing those in the same release as the implementation might cause problems.
So far, the only things that depend on the capabilities are the new code additions for checkpoints and pull-mode backups; existing libvirt.git should have no change in behavior whether or not the capabilities are detected.
ACK in that case. If we don't modify the behaviour incompatibly after the rest of the code is pushed this is safe to do.
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability). Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index fa84ff821e..30474c325d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -646,6 +646,25 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon, unsigned long long newmem); int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, bool online); +int qemuMonitorAddBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap, + bool persistent) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int qemuMonitorEnableBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int qemuMonitorMergeBitmaps(qemuMonitorPtr mon, + const char *node, + const char *dst, + virJSONValuePtr *src) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int qemuMonitorDeleteBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + /* XXX should we pass the virDomainDiskDefPtr instead * and hide dev_name details inside monitor. Reconsider diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e41bdc8c4f..8f92e6de35 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -580,5 +580,22 @@ int qemuMonitorJSONGetCurrentMachineInfo(qemuMonitorPtr mon, qemuMonitorCurrentMachineInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONAddBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap, + bool persistent); + +int qemuMonitorJSONEnableBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap); + +int qemuMonitorJSONMergeBitmaps(qemuMonitorPtr mon, + const char *node, + const char *dst, + virJSONValuePtr *src); + +int qemuMonitorJSONDeleteBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap); #endif /* LIBVIRT_QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 187513a986..a371f7d425 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4488,3 +4488,54 @@ qemuMonitorGetCurrentMachineInfo(qemuMonitorPtr mon, return qemuMonitorJSONGetCurrentMachineInfo(mon, info); } + + +int +qemuMonitorAddBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap, + bool persistent) +{ + VIR_DEBUG("node=%s bitmap=%s persistent=%d", node, bitmap, persistent); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONAddBitmap(mon, node, bitmap, persistent); +} + +int +qemuMonitorEnableBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) +{ + VIR_DEBUG("node=%s bitmap=%s", node, bitmap); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONEnableBitmap(mon, node, bitmap); +} + +int +qemuMonitorMergeBitmaps(qemuMonitorPtr mon, + const char *node, + const char *dst, + virJSONValuePtr *src) +{ + VIR_DEBUG("node=%s dst=%s", node, dst); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONMergeBitmaps(mon, node, dst, src); +} + +int +qemuMonitorDeleteBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) +{ + VIR_DEBUG("node=%s bitmap=%s", node, bitmap); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONDeleteBitmap(mon, node, bitmap); +} diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 93113d4e8a..41eef0c38c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8510,3 +8510,122 @@ qemuMonitorJSONGetCurrentMachineInfo(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int +qemuMonitorJSONAddBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap, + bool persistent) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-add", + "s:node", node, + "s:name", bitmap, + "b:persistent", persistent, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +int +qemuMonitorJSONEnableBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-enable", + "s:node", node, + "s:name", bitmap, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +int +qemuMonitorJSONMergeBitmaps(qemuMonitorPtr mon, + const char *node, + const char *dst, + virJSONValuePtr *src) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-merge", + "s:node", node, + "s:target", dst, + "a:bitmaps", src, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(*src); + *src = NULL; + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +int +qemuMonitorJSONDeleteBitmap(qemuMonitorPtr mon, + const char *node, + const char *bitmap) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-remove", + "s:node", node, + "s:name", bitmap, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} -- 2.20.1
On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+)
I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's simple to add and gives you checking of the arguments against the QAPI schema for free.
On 6/6/19 7:48 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+)
I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's simple to add and gives you checking of the arguments against the QAPI schema for free.
Will do. Do you need to see the amended patch with that added, or should I go ahead and push once I have it working? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Jun 06, 2019 at 08:41:26 -0500, Eric Blake wrote:
On 6/6/19 7:48 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+)
I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's simple to add and gives you checking of the arguments against the QAPI schema for free.
Will do. Do you need to see the amended patch with that added, or should I go ahead and push once I have it working?
ACK if you add those with all the fields excercised.
On 6/7/19 2:08 AM, Peter Krempa wrote:
On Thu, Jun 06, 2019 at 08:41:26 -0500, Eric Blake wrote:
On 6/6/19 7:48 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+)
I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's simple to add and gives you checking of the arguments against the QAPI schema for free.
Will do. Do you need to see the amended patch with that added, or should I go ahead and push once I have it working?
ACK if you add those with all the fields excercised.
Here's what I'm squashing in. The MergeBitmaps test depends on my cleanup to use TestNewSchema everywhere. Also, I noticed that if we wanted to use VIR_AUTOPTR(qemuMonitorTest) it might make a lot of the file easier to read, but that should be a separate patch (including fixing: qemumonitorjsontest.c:1431:5: error: cleanup argument not a function VIR_AUTOPTR(qemuMonitorTest) test = NULL; ^~~~~~~~~~~ ) diff --git i/tests/qemumonitorjsontest.c w/tests/qemumonitorjsontest.c index 2e7e976e9b..c07d8bf548 100644 --- i/tests/qemumonitorjsontest.c +++ w/tests/qemumonitorjsontest.c @@ -1375,6 +1375,9 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode") +GEN_TEST_FUNC(qemuMonitorJSONAddBitmap, "node", "bitmap", true) +GEN_TEST_FUNC(qemuMonitorJSONEnableBitmap, "node", "bitmap") +GEN_TEST_FUNC(qemuMonitorJSONDeleteBitmap, "node", "bitmap") static int testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) @@ -1419,6 +1422,44 @@ testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) return ret; } +static int +testQemuMonitorJSONqemuMonitorJSONMergeBitmaps(const void *opaque) +{ + const testGenericData *data = opaque; + virDomainXMLOptionPtr xmlopt = data->xmlopt; + qemuMonitorTestPtr test = qemuMonitorTestNewSchema(xmlopt, data->schema); + int ret = -1; + VIR_AUTOPTR(virJSONValue) arr = NULL; + + + if (!test) + return -1; + + if (!(arr = virJSONValueNewArray())) + goto cleanup; + + if (virJSONValueArrayAppendString(arr, "b1") < 0 || + virJSONValueArrayAppendString(arr, "b2") < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "block-dirty-bitmap-merge", + "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorJSONMergeBitmaps(qemuMonitorTestGetMonitor(test), + "node", "dst", &arr) < 0) + goto cleanup; + + if (arr) + goto cleanup; + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} + static bool testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a, struct qemuMonitorQueryCpusEntry *b) @@ -3109,6 +3150,9 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONBlockdevTrayClose); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumRemove); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert); + DO_TEST_GEN(qemuMonitorJSONAddBitmap); + DO_TEST_GEN(qemuMonitorJSONEnableBitmap); + DO_TEST_GEN(qemuMonitorJSONDeleteBitmap); DO_TEST(qemuMonitorJSONGetBalloonInfo); DO_TEST(qemuMonitorJSONGetBlockInfo); DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo); @@ -3125,6 +3169,7 @@ mymain(void) DO_TEST(qemuMonitorJSONSendKeyHoldtime); DO_TEST(qemuMonitorSupportsActiveCommit); DO_TEST(qemuMonitorJSONNBDServerStart); + DO_TEST(qemuMonitorJSONMergeBitmaps); DO_TEST_CPU_DATA("host"); DO_TEST_CPU_DATA("full");
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Mon, Jun 10, 2019 at 21:49:51 -0500, Eric Blake wrote:
On 6/7/19 2:08 AM, Peter Krempa wrote:
On Thu, Jun 06, 2019 at 08:41:26 -0500, Eric Blake wrote:
On 6/6/19 7:48 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of various qcow2 bitmap manipulations as the basis to virDomainCheckpoints and incremental backups. Add four functions to expose block-dirty-bitmap-{add,enable,disable,merge} (this is the recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 19 ++++++ src/qemu/qemu_monitor_json.h | 17 +++++ src/qemu/qemu_monitor.c | 51 +++++++++++++++ src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+)
I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's simple to add and gives you checking of the arguments against the QAPI schema for free.
Will do. Do you need to see the amended patch with that added, or should I go ahead and push once I have it working?
ACK if you add those with all the fields excercised.
Here's what I'm squashing in. The MergeBitmaps test depends on my cleanup to use TestNewSchema everywhere. Also, I noticed that if we wanted to use VIR_AUTOPTR(qemuMonitorTest) it might make a lot of the file easier to read, but that should be a separate patch (including fixing: qemumonitorjsontest.c:1431:5: error: cleanup argument not a function VIR_AUTOPTR(qemuMonitorTest) test = NULL;
As I've pointed out in the RFC patch you've sent, adding the automatic freeing function is a good idea, but refactoring old code does seem to be a waste of time. You certainly can use the new format in the code you are adding though.
^~~~~~~~~~~ )
diff --git i/tests/qemumonitorjsontest.c w/tests/qemumonitorjsontest.c index 2e7e976e9b..c07d8bf548 100644 --- i/tests/qemumonitorjsontest.c +++ w/tests/qemumonitorjsontest.c @@ -1375,6 +1375,9 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode") +GEN_TEST_FUNC(qemuMonitorJSONAddBitmap, "node", "bitmap", true) +GEN_TEST_FUNC(qemuMonitorJSONEnableBitmap, "node", "bitmap") +GEN_TEST_FUNC(qemuMonitorJSONDeleteBitmap, "node", "bitmap")
static int testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) @@ -1419,6 +1422,44 @@ testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) return ret; }
+static int +testQemuMonitorJSONqemuMonitorJSONMergeBitmaps(const void *opaque) +{ + const testGenericData *data = opaque; + virDomainXMLOptionPtr xmlopt = data->xmlopt; + qemuMonitorTestPtr test = qemuMonitorTestNewSchema(xmlopt, data->schema); + int ret = -1; + VIR_AUTOPTR(virJSONValue) arr = NULL; + + + if (!test) + return -1; + + if (!(arr = virJSONValueNewArray())) + goto cleanup; + + if (virJSONValueArrayAppendString(arr, "b1") < 0 || + virJSONValueArrayAppendString(arr, "b2") < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "block-dirty-bitmap-merge", + "{\"return\":{}}") < 0) + goto cleanup; + + if (qemuMonitorJSONMergeBitmaps(qemuMonitorTestGetMonitor(test), + "node", "dst", &arr) < 0) + goto cleanup; + + if (arr) + goto cleanup; + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} + static bool testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a, struct qemuMonitorQueryCpusEntry *b) @@ -3109,6 +3150,9 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONBlockdevTrayClose); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumRemove); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert); + DO_TEST_GEN(qemuMonitorJSONAddBitmap); + DO_TEST_GEN(qemuMonitorJSONEnableBitmap); + DO_TEST_GEN(qemuMonitorJSONDeleteBitmap); DO_TEST(qemuMonitorJSONGetBalloonInfo); DO_TEST(qemuMonitorJSONGetBlockInfo); DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo); @@ -3125,6 +3169,7 @@ mymain(void) DO_TEST(qemuMonitorJSONSendKeyHoldtime); DO_TEST(qemuMonitorSupportsActiveCommit); DO_TEST(qemuMonitorJSONNBDServerStart); + DO_TEST(qemuMonitorJSONMergeBitmaps);
DO_TEST_CPU_DATA("host"); DO_TEST_CPU_DATA("full");
ACK to the squash-in regardless of if you decide to use AUTOPTR for the test monitor in testQemuMonitorJSONqemuMonitorJSONMergeBitmaps.
The upcoming virDomainBackup() API needs to take advantage of the ability to expose a bitmap as part of nbd-server-add for a pull-mode backup (this is the recently-added QEMU_CAPS_NBD_BITMAP capability). Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 4 +++- src/qemu/qemu_monitor_json.h | 4 +++- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 10 +++++++--- src/qemu/qemu_monitor_json.c | 7 ++++++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 30474c325d..482d51c41d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1118,7 +1118,9 @@ int qemuMonitorNBDServerStart(qemuMonitorPtr mon, ATTRIBUTE_NONNULL(2); int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, - bool writable); + const char *export, + bool writable, + const char *bitmap); int qemuMonitorNBDServerStop(qemuMonitorPtr); int qemuMonitorGetTPMModels(qemuMonitorPtr mon, char ***tpmmodels); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 8f92e6de35..85d8f00fc0 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -463,7 +463,9 @@ int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *tls_alias); int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, - bool writable); + const char *export, + bool writable, + const char *bitmap); int qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon); int qemuMonitorJSONGetTPMModels(qemuMonitorPtr mon, char ***tpmmodels) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 267a729c6f..a62cb668fe 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -424,7 +424,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, goto exit_monitor; } - if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) + if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, NULL, true, NULL) < 0) goto exit_monitor; if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a371f7d425..39df201eca 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3945,13 +3945,17 @@ qemuMonitorNBDServerStart(qemuMonitorPtr mon, int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, - bool writable) + const char *export, + bool writable, + const char *bitmap) { - VIR_DEBUG("deviceID=%s", deviceID); + VIR_DEBUG("deviceID=%s, export=%s, bitmap=%s", deviceID, NULLSTR(export), + NULLSTR(bitmap)); QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONNBDServerAdd(mon, deviceID, writable); + return qemuMonitorJSONNBDServerAdd(mon, deviceID, export, writable, + bitmap); } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 41eef0c38c..6b66f6ef55 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6736,15 +6736,20 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, - bool writable) + const char *export, + bool writable, + const char *bitmap) { int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; + /* Note: bitmap must be NULL if QEMU_CAPS_NBD_BITMAP is lacking */ if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-add", "s:device", deviceID, + "S:name", export, "b:writable", writable, + "S:bitmap", bitmap, NULL))) return ret; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 9d707fcc7c..2a9e6cc75f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1348,7 +1348,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "back GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") GEN_TEST_FUNC(qemuMonitorJSONScreendump, "devicename", 1, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) -GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) +GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", NULL, true, NULL) GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1") GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") -- 2.20.1
On Wed, Jun 05, 2019 at 22:01:10 -0500, Eric Blake wrote:
The upcoming virDomainBackup() API needs to take advantage of the ability to expose a bitmap as part of nbd-server-add for a pull-mode backup (this is the recently-added QEMU_CAPS_NBD_BITMAP capability).
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor.h | 4 +++- src/qemu/qemu_monitor_json.h | 4 +++- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 10 +++++++--- src/qemu/qemu_monitor_json.c | 7 ++++++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 21 insertions(+), 8 deletions(-)
[...]
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 9d707fcc7c..2a9e6cc75f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1348,7 +1348,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "back GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") GEN_TEST_FUNC(qemuMonitorJSONScreendump, "devicename", 1, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) -GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) +GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", NULL, true, NULL)
Please use non-NULL attributes here to ensure schema checking.
GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1") GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev")
ACK with the non-null test added and make check re-run.
participants (2)
-
Eric Blake -
Peter Krempa