6:30 p.m.
Rebased to latest libvirt.git, and still waiting on Jeff's qemu
patches to go into qemu.git before I can push any of these,
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html
but reposting because of a feature Adam had been requesting:
Patch 3/5 is new, and adds a new <activecommit/> marker to the
'virsh capabilities' output if qemu is detected as supporting
active commit. I've resolved some of Peter's review comments
in patches 1 and 2 (patch 1 underwent a bit of a rewrite to
differentiate the qemu capability probe from normal use of the
interface, and 2 dropped mention of RHEL 6.3 except in the
commit message). At this point, I haven't altered 4 or 5,
although I still need to do something about getting a persistent
domain definition updated after a pivot changes the file in
use by a running domain.
I've forced an update to my git repo:
http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster
or git checkout git://repo.or.cz/libvirt/ericb.git gluster
Eric Blake (5):
blockjob: allow omitted arguments to QMP block-commit
blockjob: turn on qemu capability bit for active commit
blockjob: expose active commit capability
blockcommit: track job type in xml
blockcommit: turn on active commit
docs/formatcaps.html.in | 46 +++++++++++++++-
docs/formatdomain.html.in | 20 +++----
docs/schemas/capability.rng | 5 ++
docs/schemas/domaincommon.rng | 13 +++++
src/conf/capabilities.c | 3 +-
src/conf/domain_conf.c | 33 +++++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 13 +++++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 53 ++++++++++++++----
src/qemu/qemu_monitor.c | 12 +++++
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 29 ++++++++--
src/qemu/qemu_monitor_json.h | 3 +-
src/qemu/qemu_process.c | 18 ++++---
tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 ++++++++++++----------
tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 ++++++++++++----------
tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 ++++++++++++----------
tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 ++++++++++++----------
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 ++++++++++++----------
tests/qemumonitorjsontest.c | 47 ++++++++++++++++
.../qemuxml2argv-disk-active-commit.xml | 37 +++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +-
tests/qemuxml2xmltest.c | 1 +
25 files changed, 481 insertions(+), 173 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
--
1.9.3
6:30 p.m.
New subject: [libvirt] [PATCH v4 1/5] blockjob: allow omitted arguments to QMP block-commit
We are about to turn on support for active block commit. Although
qemu 2.0 was the first version to mostly support it, that version
mis-handles 0-length files, and doesn't have anything available for
easy probing. But qemu 2.1 fixed bugs, and made life simpler by
letting the 'top' argument be optional. Unless someone begs for
active commit with qemu 2.0, for now we are just going to enable
it only by probing for qemu 2.1 behavior (anyone backporting active
commit can also backport the optional argument behavior).
Although all our actual uses of block-commit supply arguments for
both base and top, we can omit both arguments and use a bogus
device string to trigger an interesting behavior in qemu. All QMP
commands first do argument validation, failing with GenericError
if a mandatory argument is missing. Once that passes, the code
in the specific command gets to do further checking, and the qemu
developers made sure that if device is the only supplied argument,
then the block-commit code will look up the device first, with a
failure of DeviceNotFound, before attempting any further argument
validation (most other validations fail with GenericError). Thus,
the category of error class can reliably be used to decipher
whether the top argument was optional, which in turn implies a
working active commit. Since we expect our bogus device string to
trigger an error either way, the code is written to return a
distinct return value without spamming the logs.
* src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New
prototype.
* src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit):
Implement it.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Allow NULL for top and base, for probing purposes.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise, implementing the probe.
* tests/qemumonitorjsontest.c (mymain): Enable...
(testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_monitor.c | 12 +++++++++++
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++---
src/qemu/qemu_monitor_json.h | 3 +--
tests/qemumonitorjsontest.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 87 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 80d7b9d..2d584fc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3261,6 +3261,18 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
return ret;
}
+
+/* Probe whether active commits are supported by a given qemu binary. */
+bool
+qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon)
+{
+ if (!mon->json)
+ return false;
+
+ return qemuMonitorJSONBlockCommit(mon, "bogus", NULL, NULL, 0) == -2;
+}
+
+
/* Use the block-job-complete monitor command to pivot a block copy
* job. */
int
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31b3204..63e78d8 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -665,6 +665,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
unsigned long bandwidth)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4);
+bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon);
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
const char *cmd,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bedd959..75b33e8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3454,7 +3454,14 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr
actions)
return ret;
}
-/* speed is in bytes/sec */
+/* speed is in bytes/sec. Returns 0 on success, -1 with error message
+ * emitted on failure.
+ *
+ * Additionally, can be used to probe if active commit is supported:
+ * pass in a bogus device and NULL top and base. The probe return is
+ * -2 if active commit is detected, -3 if absent; with no error
+ * message issued.
+ */
int
qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
@@ -3467,14 +3474,30 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char
*device,
cmd = qemuMonitorJSONMakeCommand("block-commit",
"s:device", device,
"U:speed", speed,
- "s:top", top,
- "s:base", base,
+ "S:top", top,
+ "S:base", base,
NULL);
if (!cmd)
return -1;
if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
goto cleanup;
+ if (!top && !base) {
+ /* Normally we always specify top and base; but omitting them
+ * allows for probing whether qemu is new enough to support
+ * live commit. */
+ if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ VIR_DEBUG("block-commit supports active commit");
+ ret = -2;
+ } else {
+ /* This is a false negative for qemu 2.0; but probably not
+ * worth the additional complexity to worry about it */
+ VIR_DEBUG("block-commit requires 'top' parameter, "
+ "assuming it lacks active commit");
+ ret = -3;
+ }
+ goto cleanup;
+ }
ret = qemuMonitorJSONCheckError(cmd, reply);
cleanup:
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e29158e..89e668c 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -262,8 +262,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
unsigned long long bandwidth)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
- ATTRIBUTE_NONNULL(4);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
const char *cmd_str,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2099dc8..d136576 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1992,6 +1992,52 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void
*data)
}
static int
+testQemuMonitorJSONqemuMonitorSupportsActiveCommit(const void *data)
+{
+ virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+ int ret = -1;
+ const char *error1 =
+ "{"
+ " \"error\": {"
+ " \"class\": \"DeviceNotFound\","
+ " \"desc\": \"Device 'bogus' not
found\""
+ " }"
+ "}";
+ const char *error2 =
+ "{"
+ " \"error\": {"
+ " \"class\": \"GenericError\","
+ " \"desc\": \"Parameter 'top' is
missing\""
+ " }"
+ "}";
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItemParams(test, "block-commit", error1,
+ "device",
"\"bogus\"",
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (!qemuMonitorSupportsActiveCommit(qemuMonitorTestGetMonitor(test)))
+ goto cleanup;
+
+ if (qemuMonitorTestAddItemParams(test, "block-commit", error2,
+ "device",
"\"bogus\"",
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (qemuMonitorSupportsActiveCommit(qemuMonitorTestGetMonitor(test)))
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+static int
testQemuMonitorJSONqemuMonitorJSONGetDumpGuestMemoryCapability(const void *data)
{
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
@@ -2263,6 +2309,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONSendKey);
DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability);
DO_TEST(qemuMonitorJSONSendKeyHoldtime);
+ DO_TEST(qemuMonitorSupportsActiveCommit);
DO_TEST_CPU_DATA("host");
DO_TEST_CPU_DATA("full");
--
1.9.3
2:10 a.m.
New subject: [libvirt] [PATCH v4 1/5] blockjob: allow omitted arguments to QMP block-commit
On 06/24/14 01:30, Eric Blake wrote:
We are about to turn on support for active block commit. Although
qemu 2.0 was the first version to mostly support it, that version
mis-handles 0-length files, and doesn't have anything available for
easy probing. But qemu 2.1 fixed bugs, and made life simpler by
letting the 'top' argument be optional. Unless someone begs for
active commit with qemu 2.0, for now we are just going to enable
it only by probing for qemu 2.1 behavior (anyone backporting active
commit can also backport the optional argument behavior).
Although all our actual uses of block-commit supply arguments for
both base and top, we can omit both arguments and use a bogus
device string to trigger an interesting behavior in qemu. All QMP
commands first do argument validation, failing with GenericError
if a mandatory argument is missing. Once that passes, the code
in the specific command gets to do further checking, and the qemu
developers made sure that if device is the only supplied argument,
then the block-commit code will look up the device first, with a
failure of DeviceNotFound, before attempting any further argument
validation (most other validations fail with GenericError). Thus,
the category of error class can reliably be used to decipher
whether the top argument was optional, which in turn implies a
working active commit. Since we expect our bogus device string to
trigger an error either way, the code is written to return a
distinct return value without spamming the logs.
* src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New
prototype.
* src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit):
Implement it.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Allow NULL for top and base, for probing purposes.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise, implementing the probe.
* tests/qemumonitorjsontest.c (mymain): Enable...
(testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_monitor.c | 12 +++++++++++
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++---
src/qemu/qemu_monitor_json.h | 3 +--
tests/qemumonitorjsontest.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 87 insertions(+), 5 deletions(-)
ACK,
Peter
3:19 p.m.
New subject: [libvirt] [PATCH v4 1/5] blockjob: allow omitted arguments to QMP block-commit
On 06/24/2014 01:10 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
> We are about to turn on support for active block commit. Although
> qemu 2.0 was the first version to mostly support it, that version
> mis-handles 0-length files, and doesn't have anything available for
> easy probing. But qemu 2.1 fixed bugs, and made life simpler by
> letting the 'top' argument be optional. Unless someone begs for
> active commit with qemu 2.0, for now we are just going to enable
> it only by probing for qemu 2.1 behavior (anyone backporting active
> commit can also backport the optional argument behavior).
>
> * src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New
> prototype.
> * src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit):
> Implement it.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
> Allow NULL for top and base, for probing purposes.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
> Likewise, implementing the probe.
> * tests/qemumonitorjsontest.c (mymain): Enable...
> (testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test.
>
ACK,
Now pushed, after amending the commit message to point to the qemu.git
commit this was gated on.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:30 p.m.
New subject: [libvirt] [PATCH v4 2/5] blockjob: turn on qemu capability bit for active commit
Use the probing functionality added in the last patch to turn on
a capability bit when active commit is present, and gate active
commit on that capability.
For my own reference: the difference between BLOCKJOB_SYNC and
BLOCKJOB_ASYNC is whether qemu generated an event at the
conclusion of blockpull; basically, RHEL 6.2 was the only release
of qemu that has the sync semantics and lacks the event. RHEL
6.3 added blockcopy, but also picked up on the upstream style
of qemu generating events. As no one is likely to backport
active commit to RHEL 6.2, it's safe for blockcommit to always
require async blockjob support.
Modifying qemucapabilitiestest is painful; the .replies files would
be so much easier if they had comments correlating which command
generated the given reply. Maybe I'll fix that up later...
* src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New
capability.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Use the new bit
* src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit.
(virQEMUCapsProbeQMPCommands): Set it.
* tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update.
* tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 7 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 10 ++--
tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +++++++++++++-----------
8 files changed, 188 insertions(+), 140 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 245d6b5..d698db9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"usb-kbd", /* 165 */
"host-pci-multidomain",
"msg-timestamp",
+ "active-commit",
);
@@ -2176,6 +2177,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
VIR_FORCE_CLOSE(fd);
}
+ /* Probe for active commit of qemu 2.1 (for now, we are choosing
+ * to ignore the fact that qemu 2.0 can also do active commit) */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_COMMIT) &&
+ qemuMonitorSupportsActiveCommit(mon))
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT);
+
return 0;
}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..a18d298 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain > 0 in host pci address
*/
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
+ QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22a8ca5..b57f77b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15509,7 +15509,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
"%s", _("domain is not running"));
goto endjob;
}
- if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_COMMIT)) {
+ /* Ensure that no one backports commit to RHEL 6.2, where cancel
+ * behaved differently */
+ if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_COMMIT) &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("online commit not supported with this QEMU
binary"));
goto endjob;
@@ -15541,10 +15544,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
* process; qemu 2.1 is further improving active commit. We need
* to start supporting it in libvirt. */
if (topSource == disk->src) {
- /* We assume that no one will backport qemu 2.0 active commit
- * to an earlier qemu without also backporting the block job
- * ready event; but this makes sure of that fact */
- if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("active commit not supported with this QEMU
binary"));
goto endjob;
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.replies
b/tests/qemucapabilitiesdata/caps_1.3.1-1.replies
index 63c18da..04d2141 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.replies
@@ -308,6 +308,14 @@
}
{
+ "id": "libvirt-6",
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'top' is missing"
+ }
+}
+
+{
"return": [
{
"name": "SPICE_MIGRATE_COMPLETED"
@@ -382,7 +390,7 @@
"name": "SHUTDOWN"
}
],
- "id": "libvirt-6"
+ "id": "libvirt-7"
}
{
@@ -871,7 +879,7 @@
"name": "VGA"
}
],
- "id": "libvirt-7"
+ "id": "libvirt-8"
}
{
@@ -973,7 +981,7 @@
"type": "hex32"
}
],
- "id": "libvirt-8"
+ "id": "libvirt-9"
}
{
@@ -1111,11 +1119,11 @@
"type": "on/off"
}
],
- "id": "libvirt-9"
+ "id": "libvirt-10"
}
{
- "id": "libvirt-10",
+ "id": "libvirt-11",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-ccw' not found"
@@ -1123,7 +1131,7 @@
}
{
- "id": "libvirt-11",
+ "id": "libvirt-12",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-ccw' not found"
@@ -1131,7 +1139,7 @@
}
{
- "id": "libvirt-12",
+ "id": "libvirt-13",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-s390' not found"
@@ -1139,7 +1147,7 @@
}
{
- "id": "libvirt-13",
+ "id": "libvirt-14",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-s390' not found"
@@ -1147,7 +1155,7 @@
}
{
- "id": "libvirt-14",
+ "id": "libvirt-15",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'pci-assign' not found"
@@ -1197,7 +1205,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-15"
+ "id": "libvirt-16"
}
{
@@ -1231,7 +1239,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-16"
+ "id": "libvirt-17"
}
{
@@ -1305,7 +1313,7 @@
"type": "drive"
}
],
- "id": "libvirt-17"
+ "id": "libvirt-18"
}
{
@@ -1359,7 +1367,7 @@
"type": "drive"
}
],
- "id": "libvirt-18"
+ "id": "libvirt-19"
}
{
@@ -1401,11 +1409,11 @@
"type": "uint32"
}
],
- "id": "libvirt-19"
+ "id": "libvirt-20"
}
{
- "id": "libvirt-20",
+ "id": "libvirt-21",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-redir' not found"
@@ -1455,7 +1463,7 @@
"type": "uint32"
}
],
- "id": "libvirt-21"
+ "id": "libvirt-22"
}
{
@@ -1481,13 +1489,13 @@
"type": "drive"
}
],
- "id": "libvirt-22"
+ "id": "libvirt-23"
}
{
"return": [
],
- "id": "libvirt-23"
+ "id": "libvirt-24"
}
{
@@ -1497,7 +1505,7 @@
"type": "uint64"
}
],
- "id": "libvirt-24"
+ "id": "libvirt-25"
}
{
@@ -1547,7 +1555,7 @@
"type": "drive"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -1561,7 +1569,7 @@
"type": "hex32"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
@@ -1615,7 +1623,7 @@
"name": "none"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-28"
}
{
@@ -1693,7 +1701,7 @@
"name": "Opteron_G5"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-29"
}
{
@@ -1701,11 +1709,11 @@
"enabled": false,
"present": true
},
- "id": "libvirt-28"
+ "id": "libvirt-30"
}
{
- "id": "libvirt-29",
+ "id": "libvirt-31",
"error": {
"class": "CommandNotFound",
"desc": "The command query-tpm-models has not been found"
@@ -1713,7 +1721,7 @@
}
{
- "id": "libvirt-30",
+ "id": "libvirt-32",
"error": {
"class": "CommandNotFound",
"desc": "The command query-tpm-types has not been found"
@@ -1721,7 +1729,7 @@
}
{
- "id": "libvirt-31",
+ "id": "libvirt-33",
"error": {
"class": "CommandNotFound",
"desc": "The command query-command-line-options has not been
found"
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.replies
b/tests/qemucapabilitiesdata/caps_1.4.2-1.replies
index 4fb4061..aef359c 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.replies
@@ -320,6 +320,14 @@
}
{
+ "id": "libvirt-6",
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'top' is missing"
+ }
+}
+
+{
"return": [
{
"name": "SPICE_MIGRATE_COMPLETED"
@@ -394,7 +402,7 @@
"name": "SHUTDOWN"
}
],
- "id": "libvirt-6"
+ "id": "libvirt-7"
}
{
@@ -910,7 +918,7 @@
"name": "VGA"
}
],
- "id": "libvirt-7"
+ "id": "libvirt-8"
}
{
@@ -1012,7 +1020,7 @@
"type": "hex32"
}
],
- "id": "libvirt-8"
+ "id": "libvirt-9"
}
{
@@ -1158,11 +1166,11 @@
"type": "on/off"
}
],
- "id": "libvirt-9"
+ "id": "libvirt-10"
}
{
- "id": "libvirt-10",
+ "id": "libvirt-11",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-ccw' not found"
@@ -1170,7 +1178,7 @@
}
{
- "id": "libvirt-11",
+ "id": "libvirt-12",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-ccw' not found"
@@ -1178,7 +1186,7 @@
}
{
- "id": "libvirt-12",
+ "id": "libvirt-13",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-s390' not found"
@@ -1186,7 +1194,7 @@
}
{
- "id": "libvirt-13",
+ "id": "libvirt-14",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-s390' not found"
@@ -1194,7 +1202,7 @@
}
{
- "id": "libvirt-14",
+ "id": "libvirt-15",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'pci-assign' not found"
@@ -1244,7 +1252,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-15"
+ "id": "libvirt-16"
}
{
@@ -1278,7 +1286,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-16"
+ "id": "libvirt-17"
}
{
@@ -1352,7 +1360,7 @@
"type": "drive"
}
],
- "id": "libvirt-17"
+ "id": "libvirt-18"
}
{
@@ -1406,7 +1414,7 @@
"type": "drive"
}
],
- "id": "libvirt-18"
+ "id": "libvirt-19"
}
{
@@ -1448,11 +1456,11 @@
"type": "uint32"
}
],
- "id": "libvirt-19"
+ "id": "libvirt-20"
}
{
- "id": "libvirt-20",
+ "id": "libvirt-21",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-redir' not found"
@@ -1502,7 +1510,7 @@
"type": "uint32"
}
],
- "id": "libvirt-21"
+ "id": "libvirt-22"
}
{
@@ -1528,13 +1536,13 @@
"type": "drive"
}
],
- "id": "libvirt-22"
+ "id": "libvirt-23"
}
{
"return": [
],
- "id": "libvirt-23"
+ "id": "libvirt-24"
}
{
@@ -1544,7 +1552,7 @@
"type": "uint64"
}
],
- "id": "libvirt-24"
+ "id": "libvirt-25"
}
{
@@ -1594,7 +1602,7 @@
"type": "drive"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -1608,7 +1616,7 @@
"type": "hex32"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
@@ -1665,7 +1673,7 @@
"name": "none"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-28"
}
{
@@ -1743,7 +1751,7 @@
"name": "qemu64"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-29"
}
{
@@ -1751,11 +1759,11 @@
"enabled": false,
"present": true
},
- "id": "libvirt-28"
+ "id": "libvirt-30"
}
{
- "id": "libvirt-29",
+ "id": "libvirt-31",
"error": {
"class": "CommandNotFound",
"desc": "The command query-tpm-models has not been found"
@@ -1763,7 +1771,7 @@
}
{
- "id": "libvirt-30",
+ "id": "libvirt-32",
"error": {
"class": "CommandNotFound",
"desc": "The command query-tpm-types has not been found"
@@ -1771,7 +1779,7 @@
}
{
- "id": "libvirt-31",
+ "id": "libvirt-33",
"error": {
"class": "CommandNotFound",
"desc": "The command query-command-line-options has not been
found"
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.replies
b/tests/qemucapabilitiesdata/caps_1.5.3-1.replies
index 686fa3e..1f84bb6 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.replies
@@ -335,6 +335,14 @@
}
{
+ "id": "libvirt-6",
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'top' is missing"
+ }
+}
+
+{
"return": [
{
"name": "GUEST_PANICKED"
@@ -415,7 +423,7 @@
"name": "SHUTDOWN"
}
],
- "id": "libvirt-6"
+ "id": "libvirt-7"
}
{
@@ -976,7 +984,7 @@
"name": "VGA"
}
],
- "id": "libvirt-7"
+ "id": "libvirt-8"
}
{
@@ -1078,7 +1086,7 @@
"type": "hex32"
}
],
- "id": "libvirt-8"
+ "id": "libvirt-9"
}
{
@@ -1224,11 +1232,11 @@
"type": "on/off"
}
],
- "id": "libvirt-9"
+ "id": "libvirt-10"
}
{
- "id": "libvirt-10",
+ "id": "libvirt-11",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-ccw' not found"
@@ -1236,7 +1244,7 @@
}
{
- "id": "libvirt-11",
+ "id": "libvirt-12",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-ccw' not found"
@@ -1244,7 +1252,7 @@
}
{
- "id": "libvirt-12",
+ "id": "libvirt-13",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-s390' not found"
@@ -1252,7 +1260,7 @@
}
{
- "id": "libvirt-13",
+ "id": "libvirt-14",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-s390' not found"
@@ -1260,7 +1268,7 @@
}
{
- "id": "libvirt-14",
+ "id": "libvirt-15",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'pci-assign' not found"
@@ -1310,7 +1318,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-15"
+ "id": "libvirt-16"
}
{
@@ -1352,7 +1360,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-16"
+ "id": "libvirt-17"
}
{
@@ -1426,7 +1434,7 @@
"type": "drive"
}
],
- "id": "libvirt-17"
+ "id": "libvirt-18"
}
{
@@ -1480,7 +1488,7 @@
"type": "drive"
}
],
- "id": "libvirt-18"
+ "id": "libvirt-19"
}
{
@@ -1522,11 +1530,11 @@
"type": "uint32"
}
],
- "id": "libvirt-19"
+ "id": "libvirt-20"
}
{
- "id": "libvirt-20",
+ "id": "libvirt-21",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-redir' not found"
@@ -1576,7 +1584,7 @@
"type": "uint32"
}
],
- "id": "libvirt-21"
+ "id": "libvirt-22"
}
{
@@ -1602,13 +1610,13 @@
"type": "drive"
}
],
- "id": "libvirt-22"
+ "id": "libvirt-23"
}
{
"return": [
],
- "id": "libvirt-23"
+ "id": "libvirt-24"
}
{
@@ -1618,7 +1626,7 @@
"type": "uint64"
}
],
- "id": "libvirt-24"
+ "id": "libvirt-25"
}
{
@@ -1668,7 +1676,7 @@
"type": "drive"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -1682,7 +1690,7 @@
"type": "hex32"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
@@ -1755,7 +1763,7 @@
"cpu-max": 1
}
],
- "id": "libvirt-26"
+ "id": "libvirt-28"
}
{
@@ -1833,7 +1841,7 @@
"name": "qemu64"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-29"
}
{
@@ -1841,19 +1849,19 @@
"enabled": false,
"present": true
},
- "id": "libvirt-28"
+ "id": "libvirt-30"
}
{
"return": [
],
- "id": "libvirt-29"
+ "id": "libvirt-31"
}
{
"return": [
],
- "id": "libvirt-30"
+ "id": "libvirt-32"
}
{
@@ -2529,5 +2537,5 @@
"option": "drive"
}
],
- "id": "libvirt-31"
+ "id": "libvirt-33"
}
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.replies
b/tests/qemucapabilitiesdata/caps_1.6.0-1.replies
index 95e0c37..c9dc29b 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.replies
@@ -341,6 +341,14 @@
}
{
+ "id": "libvirt-6",
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'top' is missing"
+ }
+}
+
+{
"return": [
{
"name": "GUEST_PANICKED"
@@ -424,7 +432,7 @@
"name": "SHUTDOWN"
}
],
- "id": "libvirt-6"
+ "id": "libvirt-7"
}
{
@@ -1018,7 +1026,7 @@
"name": "VGA"
}
],
- "id": "libvirt-7"
+ "id": "libvirt-8"
}
{
@@ -1120,7 +1128,7 @@
"type": "hex32"
}
],
- "id": "libvirt-8"
+ "id": "libvirt-9"
}
{
@@ -1274,11 +1282,11 @@
"type": "on/off"
}
],
- "id": "libvirt-9"
+ "id": "libvirt-10"
}
{
- "id": "libvirt-10",
+ "id": "libvirt-11",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-ccw' not found"
@@ -1286,7 +1294,7 @@
}
{
- "id": "libvirt-11",
+ "id": "libvirt-12",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-ccw' not found"
@@ -1294,7 +1302,7 @@
}
{
- "id": "libvirt-12",
+ "id": "libvirt-13",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-s390' not found"
@@ -1302,7 +1310,7 @@
}
{
- "id": "libvirt-13",
+ "id": "libvirt-14",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-s390' not found"
@@ -1310,7 +1318,7 @@
}
{
- "id": "libvirt-14",
+ "id": "libvirt-15",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'pci-assign' not found"
@@ -1360,7 +1368,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-15"
+ "id": "libvirt-16"
}
{
@@ -1402,7 +1410,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-16"
+ "id": "libvirt-17"
}
{
@@ -1476,7 +1484,7 @@
"type": "drive"
}
],
- "id": "libvirt-17"
+ "id": "libvirt-18"
}
{
@@ -1530,7 +1538,7 @@
"type": "drive"
}
],
- "id": "libvirt-18"
+ "id": "libvirt-19"
}
{
@@ -1572,11 +1580,11 @@
"type": "uint32"
}
],
- "id": "libvirt-19"
+ "id": "libvirt-20"
}
{
- "id": "libvirt-20",
+ "id": "libvirt-21",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-redir' not found"
@@ -1630,7 +1638,7 @@
"type": "uint32"
}
],
- "id": "libvirt-21"
+ "id": "libvirt-22"
}
{
@@ -1656,7 +1664,7 @@
"type": "drive"
}
],
- "id": "libvirt-22"
+ "id": "libvirt-23"
}
{
@@ -1666,7 +1674,7 @@
"type": "size"
}
],
- "id": "libvirt-23"
+ "id": "libvirt-24"
}
{
@@ -1680,7 +1688,7 @@
"type": "uint64"
}
],
- "id": "libvirt-24"
+ "id": "libvirt-25"
}
{
@@ -1730,7 +1738,7 @@
"type": "drive"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -1744,7 +1752,7 @@
"type": "hex32"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
@@ -1833,7 +1841,7 @@
"cpu-max": 1
}
],
- "id": "libvirt-26"
+ "id": "libvirt-28"
}
{
@@ -1911,7 +1919,7 @@
"name": "qemu64"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-29"
}
{
@@ -1919,19 +1927,19 @@
"enabled": false,
"present": true
},
- "id": "libvirt-28"
+ "id": "libvirt-30"
}
{
"return": [
],
- "id": "libvirt-29"
+ "id": "libvirt-31"
}
{
"return": [
],
- "id": "libvirt-30"
+ "id": "libvirt-32"
}
{
@@ -2509,5 +2517,5 @@
"option": "drive"
}
],
- "id": "libvirt-31"
+ "id": "libvirt-33"
}
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
index 3ecf185..a60542a 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
@@ -347,6 +347,14 @@
}
{
+ "id": "libvirt-6",
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'top' is missing"
+ }
+}
+
+{
"return": [
{
"name": "BLOCK_IMAGE_CORRUPTED"
@@ -433,7 +441,7 @@
"name": "SHUTDOWN"
}
],
- "id": "libvirt-6"
+ "id": "libvirt-7"
}
{
@@ -1024,7 +1032,7 @@
"name": "VGA"
}
],
- "id": "libvirt-7"
+ "id": "libvirt-8"
}
{
@@ -1126,7 +1134,7 @@
"type": "hex32"
}
],
- "id": "libvirt-8"
+ "id": "libvirt-9"
}
{
@@ -1280,11 +1288,11 @@
"type": "on/off"
}
],
- "id": "libvirt-9"
+ "id": "libvirt-10"
}
{
- "id": "libvirt-10",
+ "id": "libvirt-11",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-ccw' not found"
@@ -1292,7 +1300,7 @@
}
{
- "id": "libvirt-11",
+ "id": "libvirt-12",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-ccw' not found"
@@ -1300,7 +1308,7 @@
}
{
- "id": "libvirt-12",
+ "id": "libvirt-13",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-blk-s390' not found"
@@ -1308,7 +1316,7 @@
}
{
- "id": "libvirt-13",
+ "id": "libvirt-14",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-net-s390' not found"
@@ -1316,7 +1324,7 @@
}
{
- "id": "libvirt-14",
+ "id": "libvirt-15",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'pci-assign' not found"
@@ -1366,7 +1374,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-15"
+ "id": "libvirt-16"
}
{
@@ -1408,7 +1416,7 @@
"type": "pci-host-devaddr"
}
],
- "id": "libvirt-16"
+ "id": "libvirt-17"
}
{
@@ -1482,7 +1490,7 @@
"type": "drive"
}
],
- "id": "libvirt-17"
+ "id": "libvirt-18"
}
{
@@ -1536,7 +1544,7 @@
"type": "drive"
}
],
- "id": "libvirt-18"
+ "id": "libvirt-19"
}
{
@@ -1578,11 +1586,11 @@
"type": "uint32"
}
],
- "id": "libvirt-19"
+ "id": "libvirt-20"
}
{
- "id": "libvirt-20",
+ "id": "libvirt-21",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-redir' not found"
@@ -1590,7 +1598,7 @@
}
{
- "id": "libvirt-21",
+ "id": "libvirt-22",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'usb-host' not found"
@@ -1620,7 +1628,7 @@
"type": "drive"
}
],
- "id": "libvirt-22"
+ "id": "libvirt-23"
}
{
@@ -1630,7 +1638,7 @@
"type": "size"
}
],
- "id": "libvirt-23"
+ "id": "libvirt-24"
}
{
@@ -1644,7 +1652,7 @@
"type": "uint64"
}
],
- "id": "libvirt-24"
+ "id": "libvirt-25"
}
{
@@ -1694,7 +1702,7 @@
"type": "drive"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -1708,7 +1716,7 @@
"type": "hex32"
}
],
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
@@ -1805,7 +1813,7 @@
"cpu-max": 1
}
],
- "id": "libvirt-26"
+ "id": "libvirt-28"
}
{
@@ -1883,7 +1891,7 @@
"name": "qemu64"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-29"
}
{
@@ -1891,19 +1899,19 @@
"enabled": false,
"present": true
},
- "id": "libvirt-28"
+ "id": "libvirt-30"
}
{
"return": [
],
- "id": "libvirt-29"
+ "id": "libvirt-31"
}
{
"return": [
],
- "id": "libvirt-30"
+ "id": "libvirt-32"
}
{
@@ -2486,5 +2494,5 @@
"option": "drive"
}
],
- "id": "libvirt-31"
+ "id": "libvirt-33"
}
--
1.9.3
2:15 a.m.
New subject: [libvirt] [PATCH v4 2/5] blockjob: turn on qemu capability bit for active commit
On 06/24/14 01:30, Eric Blake wrote:
Use the probing functionality added in the last patch to turn on
a capability bit when active commit is present, and gate active
commit on that capability.
For my own reference: the difference between BLOCKJOB_SYNC and
BLOCKJOB_ASYNC is whether qemu generated an event at the
conclusion of blockpull; basically, RHEL 6.2 was the only release
of qemu that has the sync semantics and lacks the event. RHEL
6.3 added blockcopy, but also picked up on the upstream style
of qemu generating events. As no one is likely to backport
active commit to RHEL 6.2, it's safe for blockcommit to always
require async blockjob support.
Modifying qemucapabilitiestest is painful; the .replies files would
be so much easier if they had comments correlating which command
generated the given reply. Maybe I'll fix that up later...
* src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New
capability.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Use the new bit
* src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit.
(virQEMUCapsProbeQMPCommands): Set it.
* tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update.
* tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise.
* tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 7 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 10 ++--
tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +++++++++++++-----------
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +++++++++++++-----------
8 files changed, 188 insertions(+), 140 deletions(-)
ACK,
Peter
3:22 p.m.
New subject: [libvirt] [PATCH v4 2/5] blockjob: turn on qemu capability bit for active commit
On 06/24/2014 01:15 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
> Use the probing functionality added in the last patch to turn on
> a capability bit when active commit is present, and gate active
> commit on that capability.
>
> For my own reference: the difference between BLOCKJOB_SYNC and
> BLOCKJOB_ASYNC is whether qemu generated an event at the
> conclusion of blockpull; basically, RHEL 6.2 was the only release
> of qemu that has the sync semantics and lacks the event. RHEL
> 6.3 added blockcopy, but also picked up on the upstream style
> of qemu generating events. As no one is likely to backport
> active commit to RHEL 6.2, it's safe for blockcommit to always
> require async blockjob support.
>
> Modifying qemucapabilitiestest is painful; the .replies files would
> be so much easier if they had comments correlating which command
> generated the given reply. Maybe I'll fix that up later...
>
ACK,
Pushed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:30 p.m.
New subject: [libvirt] [PATCH v4 3/5] blockjob: expose active commit capability
Add an element to QEMU's capability XML, to show if the underlying
qemu binary supports active commit. This allows the client to know
ahead of time if they can rely on this method, or must fall back
to older techniques such as blockpull. Without this information,
the only way to check for active commit is to attempt one and check
for errors.
This attribute can be a simple binary (active commit is supported
only if <activecommit/> is in the capabilities) rather than a
full-blown toggle definition. (In contrast, the <disksnapshot>
capability had to be a toggle because we forgot to add it at the
same time as turning on the feature of external snapshots; and
therefore, the absence of the attribute is not sufficient to
conclude whether disk snapshots are supported.)
Our documentation for features was rather sparse; this fleshes out
more of the details for other existing capabilities (and cost me
some time trawling git history).
* docs/schemas/capability.rng (features): Add activecommit.
* docs/formatcaps.html.in: Document it, and other features.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitGuestFromBinary):
Set it.
* src/conf/capabilities.c (virCapabilitiesFormatXML): Expose as
simpler binary.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatcaps.html.in | 46 +++++++++++++++++++++++++++++++++++++++++++-
docs/schemas/capability.rng | 5 +++++
src/conf/capabilities.c | 3 ++-
src/qemu/qemu_capabilities.c | 6 ++++++
4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 137af25..6ac0f4a 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -95,7 +95,51 @@
<dt><code>features</code></dt>
<dd>This optional element encases possible features that can be used
- with a guest of described type.</dd>
+ with a guest of described type. Possible subelements are:
+ <dl>
+ <dt>pae</dt><dd>If present, 32-bit guests can use PAE
+ address space extensions, <span class="since">since
+ 0.4.1</span></dd>
+ <dt>nonpae</dt><dd>If present, 32-bit guests can be run
+ without requiring PAE, <span class="since">since
+ 0.4.1</span></dd>
+ <dt>ia64_be</dt><dd>If present, IA64 guests can be run in
+ big-endian mode, <span class="since">since
0.4.1</span></dd>
+ <dt>acpi</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether the
+ hypervisor exposes ACPI to the guest by default, and
+ the <code>toggle</code> attribute describes whether the
+ user can override this
+ default. <span class="since">Since
0.4.1</span></dd>
+ <dt>apic</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether the
+ hypervisor exposes APIC to the guest by default, and
+ the <code>toggle</code> attribute describes whether the
+ user can override this
+ default. <span class="since">Since
0.4.1</span></dd>
+ <dt>cpuselection</dt><dd>If this element is present, the
+ hypervisor supports the <code><cpu></code>
element
+ within a domain definition for fine-grained control over
+ the CPU presented to the
+ guest. <span class="since">Since
0.7.5</span></dd>
+ <dt>deviceboot</dt><dd>If this element is present,
+ the <code><boot order='...'/></code>
element can
+ be used inside devices, rather than the older boot
+ specification by category. <span class="since">Since
+ 0.8.8</span></dd>
+ <dt>disksnapshot</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether
+ external disk snapshots are supported. If absent,
+ external snapshots may still be supported, but it
+ requires attempting the API and checking for an error to
+ find out for sure. <span class="since">Since
+ 1.2.3</span></dd>
+ <dt>activecommit</dt><dd>Active commit (via the
+ virDomainBlockCommit API) is supported only if this
+ element is present. <span class="since">Since
+ 1.2.6</span></dd>
+ </dl>
+ </dd>
</dl>
<h3><a name="elementExamples">Examples</a></h3>
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index f954599..de32ee9 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -399,6 +399,11 @@
<empty/>
</element>
</optional>
+ <optional>
+ <element name='activecommit'>
+ <empty/>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 19359a5..ebf6121 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1005,7 +1005,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
STREQ(caps->guests[i]->features[j]->name,
"nonpae") ||
STREQ(caps->guests[i]->features[j]->name,
"ia64_be") ||
STREQ(caps->guests[i]->features[j]->name,
"cpuselection") ||
- STREQ(caps->guests[i]->features[j]->name,
"deviceboot")) {
+ STREQ(caps->guests[i]->features[j]->name,
"deviceboot") ||
+ STREQ(caps->guests[i]->features[j]->name,
"activecommit")) {
virBufferAsprintf(&buf, "<%s/>\n",
caps->guests[i]->features[j]->name);
} else {
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d698db9..d1d33d5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -822,6 +822,12 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot", hasdisksnapshot,
0))
goto cleanup;
+ if (!virCapabilitiesAddGuestFeature(guest, "activecommit",
+ virQEMUCapsGet(qemubinCaps,
+ QEMU_CAPS_ACTIVE_COMMIT),
+ 0))
+ goto cleanup;
+
if (virCapabilitiesAddGuestDomain(guest,
"qemu",
NULL,
--
1.9.3
3:06 a.m.
New subject: [libvirt] [PATCH v4 3/5] blockjob: expose active commit capability
On 06/24/14 01:30, Eric Blake wrote:
Add an element to QEMU's capability XML, to show if the
underlying
qemu binary supports active commit. This allows the client to know
ahead of time if they can rely on this method, or must fall back
to older techniques such as blockpull. Without this information,
the only way to check for active commit is to attempt one and check
for errors.
This attribute can be a simple binary (active commit is supported
only if <activecommit/> is in the capabilities) rather than a
full-blown toggle definition. (In contrast, the <disksnapshot>
capability had to be a toggle because we forgot to add it at the
same time as turning on the feature of external snapshots; and
therefore, the absence of the attribute is not sufficient to
conclude whether disk snapshots are supported.)
Our documentation for features was rather sparse; this fleshes out
more of the details for other existing capabilities (and cost me
some time trawling git history).
* docs/schemas/capability.rng (features): Add activecommit.
* docs/formatcaps.html.in: Document it, and other features.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitGuestFromBinary):
Set it.
* src/conf/capabilities.c (virCapabilitiesFormatXML): Expose as
simpler binary.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatcaps.html.in | 46 +++++++++++++++++++++++++++++++++++++++++++-
docs/schemas/capability.rng | 5 +++++
src/conf/capabilities.c | 3 ++-
src/qemu/qemu_capabilities.c | 6 ++++++
4 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 137af25..6ac0f4a 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -95,7 +95,51 @@
<dt><code>features</code></dt>
<dd>This optional element encases possible features that can be used
- with a guest of described type.</dd>
+ with a guest of described type. Possible subelements are:
+ <dl>
+ <dt>pae</dt><dd>If present, 32-bit guests can use PAE
+ address space extensions, <span class="since">since
+ 0.4.1</span></dd>
+ <dt>nonpae</dt><dd>If present, 32-bit guests can be run
+ without requiring PAE, <span class="since">since
+ 0.4.1</span></dd>
+ <dt>ia64_be</dt><dd>If present, IA64 guests can be run in
+ big-endian mode, <span class="since">since
0.4.1</span></dd>
+ <dt>acpi</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether the
+ hypervisor exposes ACPI to the guest by default, and
+ the <code>toggle</code> attribute describes whether the
+ user can override this
+ default. <span class="since">Since
0.4.1</span></dd>
+ <dt>apic</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether the
+ hypervisor exposes APIC to the guest by default, and
+ the <code>toggle</code> attribute describes whether the
+ user can override this
+ default. <span class="since">Since
0.4.1</span></dd>
+ <dt>cpuselection</dt><dd>If this element is present, the
+ hypervisor supports the <code><cpu></code>
element
+ within a domain definition for fine-grained control over
+ the CPU presented to the
+ guest. <span class="since">Since
0.7.5</span></dd>
+ <dt>deviceboot</dt><dd>If this element is present,
+ the <code><boot order='...'/></code>
element can
+ be used inside devices, rather than the older boot
+ specification by category. <span class="since">Since
+ 0.8.8</span></dd>
+ <dt>disksnapshot</dt><dd>If this element is present,
+ the <code>default</code> attribute describes whether
+ external disk snapshots are supported. If absent,
+ external snapshots may still be supported, but it
+ requires attempting the API and checking for an error to
+ find out for sure. <span class="since">Since
+ 1.2.3</span></dd>
+ <dt>activecommit</dt><dd>Active commit (via the
+ virDomainBlockCommit API) is supported only if this
+ element is present. <span class="since">Since
+ 1.2.6</span></dd>
+ </dl>
+ </dd>
</dl>
<h3><a name="elementExamples">Examples</a></h3>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 19359a5..ebf6121 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1005,7 +1005,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
STREQ(caps->guests[i]->features[j]->name,
"nonpae") ||
STREQ(caps->guests[i]->features[j]->name,
"ia64_be") ||
STREQ(caps->guests[i]->features[j]->name,
"cpuselection") ||
- STREQ(caps->guests[i]->features[j]->name,
"deviceboot")) {
+ STREQ(caps->guests[i]->features[j]->name,
"deviceboot") ||
+ STREQ(caps->guests[i]->features[j]->name,
"activecommit")) {
virBufferAsprintf(&buf, "<%s/>\n",
caps->guests[i]->features[j]->name);
} else {
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d698db9..d1d33d5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -822,6 +822,12 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
if (!virCapabilitiesAddGuestFeature(guest, "disksnapshot",
hasdisksnapshot, 0))
goto cleanup;
+ if (!virCapabilitiesAddGuestFeature(guest, "activecommit",
+ virQEMUCapsGet(qemubinCaps,
+ QEMU_CAPS_ACTIVE_COMMIT),
+ 0))
Yuck, integers for boolean values. Pre-existing though.
+ goto cleanup;
+
if (virCapabilitiesAddGuestDomain(guest,
"qemu",
NULL,
ACK,
Peter
3:35 p.m.
New subject: [libvirt] [PATCH v4 3/5] blockjob: expose active commit capability
On 06/24/2014 02:06 AM, Peter Krempa wrote:
On 06/24/14 01:30, Eric Blake wrote:
> Add an element to QEMU's capability XML, to show if the underlying
> qemu binary supports active commit. This allows the client to know
> ahead of time if they can rely on this method, or must fall back
> to older techniques such as blockpull. Without this information,
> the only way to check for active commit is to attempt one and check
> for errors.
>
> This attribute can be a simple binary (active commit is supported
> only if <activecommit/> is in the capabilities) rather than a
> full-blown toggle definition. (In contrast, the <disksnapshot>
> capability had to be a toggle because we forgot to add it at the
> same time as turning on the feature of external snapshots; and
> therefore, the absence of the attribute is not sufficient to
> conclude whether disk snapshots are supported.)
>
> Our documentation for features was rather sparse; this fleshes out
> more of the details for other existing capabilities (and cost me
> some time trawling git history).
I'm going to split this patch. The documentation for the pre-existing bits,
> + if (!virCapabilitiesAddGuestFeature(guest,
"activecommit",
> + virQEMUCapsGet(qemubinCaps,
> + QEMU_CAPS_ACTIVE_COMMIT),
> + 0))
Yuck, integers for boolean values. Pre-existing though.
and a cleanup of int-to-bool, is worth having, but now that we have
virConnectGetDomainCapabilities, I'd rather expose this new capability
through that API than by overloading another feature into this code.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:30 p.m.
New subject: [libvirt] [PATCH v4 4/5] blockcommit: track job type in xml
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element. This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>. Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).
* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 20 ++++++------
docs/schemas/domaincommon.rng | 13 ++++++++
src/conf/domain_conf.c | 33 ++++++++++++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 3 ++
src/qemu/qemu_process.c | 18 +++++++----
.../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +--
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +--
tests/qemuxml2xmltest.c | 1 +
10 files changed, 113 insertions(+), 21 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3075e16..16061b8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1878,16 +1878,18 @@
</dd>
<dt><code>mirror</code></dt>
<dd>
- This element is present if the hypervisor has started a block
- copy operation (via the <code>virDomainBlockRebase</code>
- API), where the mirror location in the <code>source</code>
- sub-element will eventually have the same contents as the
- source, and with the file format in the
+ This element is present if the hypervisor has started a
+ long-running block job operation, where the mirror location in
+ the <code>source</code> sub-element will eventually have the
+ same contents as the source, and with the file format in the
sub-element <code>format</code> (which might differ from the
- format of the source). The details of the <code>source</code>
- sub-element are determined by the <code>type</code> attribute
- of the mirror, similar to what is done for the
- overall <code>disk</code> device element. If
+ format of the source). The <code>job</code> attribute
+ mentions which API started the operation ("copy" for
+ the <code>virDomainBlockRebase</code> API, or
"active-commit"
+ for the <code>virDomainBlockCommit</code> API). The details
+ of the <code>source</code> sub-element are determined by
+ the <code>type</code> attribute of the mirror, similar to what
+ is done for the overall <code>disk</code> device element. If
attribute <code>ready</code> is present, then it is known the
disk is ready to pivot; otherwise, the disk is probably still
copying. For now, this element only valid in output; it is
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 33d0308..fe943f9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4186,6 +4186,13 @@
</attribute>
</optional>
<optional>
+ <attribute name='job'>
+ <choice>
+ <value>copy</value>
+ </choice>
+ </attribute>
+ </optional>
+ <optional>
<interleave>
<ref name='diskSourceFile'/>
<optional>
@@ -4195,6 +4202,12 @@
</optional>
</group>
<group> <!-- preferred format -->
+ <attribute name='job'>
+ <choice>
+ <value>copy</value>
+ <value>active-commit</value>
+ </choice>
+ </attribute>
<interleave>
<ref name="diskSource"/>
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02c394f..65a41c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -760,6 +760,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
"unmap",
"ignore")
+/* Internal mapping: subset of block job types that can be present in
+ * <mirror> XML (remaining types are not two-phase). */
+VIR_ENUM_DECL(virDomainBlockJob)
+VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
+ "", "", "copy", "",
"active-commit")
+
#define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE
#define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE
@@ -5377,10 +5383,26 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur->name, BAD_CAST "mirror") &&
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
char *ready;
+ char *blockJob;
if (VIR_ALLOC(def->mirror) < 0)
goto error;
+ blockJob = virXMLPropString(cur, "job");
+ if (blockJob) {
+ def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
+ if (def->mirrorJob <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown mirror job type
'%s'"),
+ blockJob);
+ VIR_FREE(blockJob);
+ goto error;
+ }
+ VIR_FREE(blockJob);
+ } else {
+ def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
+ }
+
mirrorType = virXMLPropString(cur, "type");
if (mirrorType) {
def->mirror->type = virStorageTypeFromString(mirrorType);
@@ -5403,6 +5425,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
_("mirror requires file name"));
goto error;
}
+ if (def->mirrorJob != VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("mirror without type only supported "
+ "by copy job"));
+ goto error;
+ }
mirrorFormat = virXMLPropString(cur, "format");
}
if (mirrorFormat) {
@@ -15198,10 +15226,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
formatStr = virStorageFileFormatTypeToString(def->mirror->format);
virBufferAsprintf(buf, "<mirror type='%s'",
virStorageTypeToString(def->mirror->type));
- if (def->mirror->type == VIR_STORAGE_TYPE_FILE) {
+ if (def->mirror->type == VIR_STORAGE_TYPE_FILE &&
+ def->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
virBufferEscapeString(buf, " file='%s'",
def->mirror->path);
virBufferEscapeString(buf, " format='%s'", formatStr);
}
+ virBufferEscapeString(buf, " job='%s'",
+ virDomainBlockJobTypeToString(def->mirrorJob));
if (def->mirroring)
virBufferAddLit(buf, " ready='yes'");
virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6779a41..c03fe64 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -608,6 +608,7 @@ struct _virDomainDiskDef {
virStorageSourcePtr mirror;
bool mirroring;
+ int mirrorJob; /* virDomainBlockJobType */
struct {
unsigned int cylinders;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b57f77b..79554e9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14993,6 +14993,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
cleanup:
if (resume && virDomainObjIsActive(vm) &&
@@ -15120,6 +15121,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
}
/* With synchronous block cancel, we must synthesize an event, and
@@ -15397,6 +15399,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
need_unlink = false;
disk->mirror = mirror;
mirror = NULL;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
endjob:
if (need_unlink && unlink(dest))
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0b8155b..0c5a3ac 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1023,28 +1023,32 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
if (disk) {
/* Have to generate two variants of the event for old vs. new
* client callbacks */
+ if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+ disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+ type = disk->mirrorJob;
path = virDomainDiskGetSource(disk);
event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
status);
/* XXX If we completed a block pull or commit, then recompute
* the cached backing chain to match. Better would be storing
- * the chain ourselves rather than reprobing, but this
- * requires modifying domain_conf and our XML to fully track
- * the chain across libvirtd restarts. For that matter, if
- * qemu gains support for committing the active layer, we have
- * to update disk->src. */
+ * the chain ourselves rather than reprobing, but we haven't
+ * quite completed that conversion to use our XML tracking. */
if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL ||
- type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT ||
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) &&
status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
qemuDomainDetermineDiskChain(driver, vm, disk, true);
- if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+ if (disk->mirror &&
+ (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
disk->mirroring = true;
} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
}
}
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
new file mode 100644
index 0000000..6ba5724
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
@@ -0,0 +1,37 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source file='/tmp/HostVG/QEMUGuest1-snap'/>
+ <backingStore type='block' index='1'>
+ <format type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <backingStore/>
+ </backingStore>
+ <mirror type='block' job='active-commit'>
+ <format type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ </mirror>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index 72b03c9..abec180 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -17,7 +17,7 @@
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<backingStore/>
- <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy'
ready='yes'>
+ <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy'
job='copy' ready='yes'>
<source file='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
<target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
<backingStore/>
- <mirror type='file' file='/tmp/copy.img'
format='qcow2'>
+ <mirror type='file' file='/tmp/copy.img' format='qcow2'
job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
</mirror>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
index 72b03c9..abec180 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
@@ -17,7 +17,7 @@
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<backingStore/>
- <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy'
ready='yes'>
+ <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy'
job='copy' ready='yes'>
<source file='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
<target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
<backingStore/>
- <mirror type='file' file='/tmp/copy.img'
format='qcow2'>
+ <mirror type='file' file='/tmp/copy.img' format='qcow2'
job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
</mirror>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 200d50f..98b2c15 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -227,6 +227,7 @@ mymain(void)
DO_TEST_DIFFERENT("disk-mirror-old");
DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
+ DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE);
DO_TEST("graphics-listen-network");
DO_TEST("graphics-vnc");
DO_TEST("graphics-vnc-websocket");
--
1.9.3
3:23 a.m.
New subject: [libvirt] [PATCH v4 4/5] blockcommit: track job type in xml
On 06/24/14 01:30, Eric Blake wrote:
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element. This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>. Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).
* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 20 ++++++------
docs/schemas/domaincommon.rng | 13 ++++++++
src/conf/domain_conf.c | 33 ++++++++++++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 3 ++
src/qemu/qemu_process.c | 18 +++++++----
.../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +--
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +--
tests/qemuxml2xmltest.c | 1 +
10 files changed, 113 insertions(+), 21 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
No change, ACK from the previous version stands.
Peter
6:30 p.m.
New subject: [libvirt] [PATCH v4 5/5] blockcommit: turn on active commit
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --verbose --pivot
and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is SOOOO much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!
Still not done: the persistent XML is not updated; which means
stopping and restarting a persistent guest will use the wrong
file and likely fail to boot.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 79554e9..ae81712 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15493,9 +15493,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
unsigned int baseIndex = 0;
const char *top_parent = NULL;
bool clean_access = false;
+ virStorageSourcePtr mirror = NULL;
+
- /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
- virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
+ /* XXX Add support for COMMIT_DELETE */
+ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -15543,9 +15546,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
&top_parent)))
goto endjob;
- /* FIXME: qemu 2.0 supports active commit, but as a two-stage
- * process; qemu 2.1 is further improving active commit. We need
- * to start supporting it in libvirt. */
if (topSource == disk->src) {
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -15559,6 +15559,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
disk->dst);
goto endjob;
}
+ if (disk->mirror) {
+ virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+ _("disk '%s' already in active block job"),
+ disk->dst);
+ goto endjob;
+ }
+ if (VIR_ALLOC(mirror) < 0)
+ goto endjob;
} else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) {
virReportError(VIR_ERR_INVALID_ARG,
_("active commit requested but '%s' is not
active"),
@@ -15605,6 +15613,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE) < 0))
goto endjob;
+ /* For an active commit, clone enough of the base to act as the mirror */
+ if (mirror) {
+ /* XXX Support network commits */
+ if (baseSource->type != VIR_STORAGE_TYPE_FILE &&
+ baseSource->type != VIR_STORAGE_TYPE_BLOCK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("active commit to non-file not supported yet"));
+ goto endjob;
+ }
+ mirror->type = baseSource->type;
+ if (VIR_STRDUP(mirror->path, baseSource->path) < 0)
+ goto endjob;
+ mirror->format = baseSource->format;
+ }
+
/* Start the commit operation. Pass the user's original spelling,
* if any, through to qemu, since qemu may behave differently
* depending on whether the input was specified as relative or
@@ -15617,6 +15640,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
bandwidth);
qemuDomainObjExitMonitor(driver, vm);
+ if (ret == 0 && mirror) {
+ disk->mirror = mirror;
+ mirror = NULL;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
+ }
+
endjob:
if (ret < 0 && clean_access) {
/* Revert access to read-only, if possible. */
@@ -15627,6 +15656,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
top_parent,
VIR_DISK_CHAIN_READ_ONLY);
}
+ virStorageSourceFree(mirror);
if (!qemuDomainObjEndJob(driver, vm))
vm = NULL;
--
1.9.3
3:24 a.m.
New subject: [libvirt] [PATCH v4 5/5] blockcommit: turn on active commit
On 06/24/14 01:30, Eric Blake wrote:
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --verbose --pivot
and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is SOOOO much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!
Still not done: the persistent XML is not updated; which means
stopping and restarting a persistent guest will use the wrong
file and likely fail to boot.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
ACK as previously. This will obviously need a followup to deal with
networked disks too.
Peter
5:12 p.m.
On 06/23/2014 05:30 PM, Eric Blake wrote:
Rebased to latest libvirt.git, and still waiting on Jeff's qemu
patches to go into qemu.git before I can push any of these,
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html
but reposting because of a feature Adam had been requesting:
Patch 3/5 is new, and adds a new <activecommit/> marker to the
'virsh capabilities' output if qemu is detected as supporting
active commit. I've resolved some of Peter's review comments
in patches 1 and 2 (patch 1 underwent a bit of a rewrite to
differentiate the qemu capability probe from normal use of the
interface, and 2 dropped mention of RHEL 6.3 except in the
commit message). At this point, I haven't altered 4 or 5,
although I still need to do something about getting a persistent
domain definition updated after a pivot changes the file in
use by a running domain.
qemu.git (and therefore the upcoming qemu 2.1 release) now has
everything we need:
commit 7676e2c597000eff3a7233b40cca768b358f9bc9
Author: Jeff Cody <jcody(a)redhat.com>
Date: Mon Jun 30 15:14:15 2014 +0200
block: make 'top' argument to block-commit optional
Now that active layer block-commit is supported, the 'top' argument
no longer needs to be mandatory.
Change it to optional, with the default being the active layer in the
device chain.
[kwolf: Rebased and resolved conflict in tests/qemu-iotests/040]
Reviewed-by: Eric Blake <eblake(a)redhat.com>
Reviewed-by: Benoit Canet <benoit(a)irqsave.net>
Signed-off-by: Jeff Cody <jcody(a)redhat.com>
Signed-off-by: Kevin Wolf <kwolf(a)redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha(a)redhat.com>
but I'd rather wait until after the 1.2.6 release to push this series
since I still don't have persistent XML manipulation working, and since
we may want to rework how feature advertisement works.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3794
days inactive
3804
days old
14 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa