9:44 a.m.
I'm slowly making progress towards having block-commit doing something
useful. First up is online support, using qemu 1.3 block-commit (hmm,
that commit is in Kevin's pull request, but Anthony hasn't yet taken
it into qemu.git proper, and the interface might still have more tweaks
before the actual qemu 1.3 release in December, so this may be a case
of me posting patches but not applying them to libvirt for another
month or two). I'm still trying to test this code, as well as to
write followup patches to allow things to work with SELinux enforcing,
but wanted to at least get this thread started.
Eric Blake (2):
blockjob: manage qemu block-commit monitor command
blockjob: wire up online qemu block-commit
src/qemu/qemu_capabilities.c | 3 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 30 +++++++++++++++++
src/qemu/qemu_monitor.h | 7 ++++
src/qemu/qemu_monitor_json.c | 34 +++++++++++++++++++
src/qemu/qemu_monitor_json.h | 7 ++++
7 files changed, 161 insertions(+)
--
1.7.11.4
9:44 a.m.
New subject: [libvirt] [PATCH 1/2] blockjob: manage qemu block-commit monitor command
qemu 1.3 will be adding a 'block-commit' monitor command, per
https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html
It matches nicely to the libvirt API virDomainBlockCommit.
* src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_COMMIT): New bit.
* src/qemu/qemu_capabilities.c (qemuCapsProbeQMPCommands): Set it.
* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): New prototype.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Implement it.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise.
(qemuMonitorJSONHandleBlockJobImpl)
(emuMonitorJSONGetBlockJobInfoOne): Handle new event type.
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_monitor.c | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 7 +++++++
src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 7 +++++++
6 files changed, 82 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a5eb995..65f1307 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -187,6 +187,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"reboot-timeout", /* 110 */
"dump-guest-core",
"seamless-migration",
+ "block-commit",
);
struct _qemuCaps {
@@ -1880,6 +1881,8 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
qemuCapsSet(caps, QEMU_CAPS_SPICE);
else if (STREQ(name, "query-kvm"))
qemuCapsSet(caps, QEMU_CAPS_KVM);
+ else if (STREQ(name, "block-commit"))
+ qemuCapsSet(caps, QEMU_CAPS_BLOCK_COMMIT);
VIR_FREE(name);
}
VIR_FREE(commands);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 5d343c1..6939c45 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -150,6 +150,7 @@ enum qemuCapsFlags {
QEMU_CAPS_REBOOT_TIMEOUT = 110, /* -boot reboot-timeout */
QEMU_CAPS_DUMP_GUEST_CORE = 111, /* dump-guest-core-parameter */
QEMU_CAPS_SEAMLESS_MIGRATION = 112, /* seamless-migration for SPICE */
+ QEMU_CAPS_BLOCK_COMMIT = 113, /* block-commit */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 85b0bc2..68c6d3f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2786,6 +2786,36 @@ qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr
actions)
return ret;
}
+/* Start a block-commit block job. bandwidth is in MB/sec. */
+int
+qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
+ const char *top, const char *base,
+ unsigned long bandwidth)
+{
+ int ret = -1;
+ unsigned long long speed;
+
+ VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld",
+ mon, device, NULLSTR(top), NULLSTR(base), bandwidth);
+
+ /* Convert bandwidth MiB to bytes */
+ speed = bandwidth;
+ if (speed > ULLONG_MAX / 1024 / 1024) {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("bandwidth must be less than %llu"),
+ ULLONG_MAX / 1024 / 1024);
+ return -1;
+ }
+ speed <<= 20;
+
+ if (mon->json)
+ ret = qemuMonitorJSONBlockCommit(mon, device, top, base, speed);
+ else
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("block-commit requires JSON monitor"));
+ return ret;
+}
+
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
const char *cmd,
char **reply,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 54b3a99..9bdc802 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -521,6 +521,13 @@ int qemuMonitorDiskSnapshot(qemuMonitorPtr mon,
int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuMonitorBlockCommit(qemuMonitorPtr mon,
+ const char *device,
+ const char *top,
+ const char *base,
+ unsigned long bandwidth)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
const char *cmd,
char **reply,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bd52ce4..501b338 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -803,6 +803,8 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
if (STREQ(type_str, "stream"))
type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
+ else if (STREQ(type_str, "commit"))
+ type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
switch ((virConnectDomainEventBlockJobStatus) event) {
case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
@@ -3269,6 +3271,36 @@ cleanup:
return ret;
}
+/* speed is in bytes/sec */
+int
+qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
+ const char *top, const char *base,
+ unsigned long speed)
+{
+ int ret = -1;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+
+ cmd = qemuMonitorJSONMakeCommand("block-commit",
+ "s:device", device,
+ "U:speed", speed,
+ "s:top", top,
+ base ? "s:base" : NULL, base,
+ NULL);
+ if (!cmd)
+ return -1;
+
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ goto cleanup;
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+
int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
const char *cmd_str,
char **reply_str,
@@ -3385,6 +3417,8 @@ static int qemuMonitorJSONGetBlockJobInfoOne(virJSONValuePtr entry,
}
if (STREQ(type, "stream"))
info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
+ else if (STREQ(type, "commit"))
+ info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
else
info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 63b84c6..71bc6aa 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -235,6 +235,13 @@ int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon,
bool reuse);
int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions);
+int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
+ const char *device,
+ const char *top,
+ const char *base,
+ unsigned long bandwidth)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
const char *cmd_str,
char **reply_str,
--
1.7.11.4
9 a.m.
New subject: [libvirt] [PATCH 1/2] blockjob: manage qemu block-commit monitor command
On 04.10.2012 16:44, Eric Blake wrote:
qemu 1.3 will be adding a 'block-commit' monitor command,
per
https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg05199.html
It matches nicely to the libvirt API virDomainBlockCommit.
* src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_COMMIT): New bit.
* src/qemu/qemu_capabilities.c (qemuCapsProbeQMPCommands): Set it.
* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): New prototype.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Implement it.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise.
(qemuMonitorJSONHandleBlockJobImpl)
(emuMonitorJSONGetBlockJobInfoOne): Handle new event type.
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_monitor.c | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 7 +++++++
src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 7 +++++++
6 files changed, 82 insertions(+)
Looks good.
9:44 a.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
This is the bare minimum to kick off a block commit. In particular,
flags support is missing (shallow requires us to crawl the backing
chain to determine the file name to pass to the qemu monitor command;
delete requires us to track what needs to be deleted at the time
the completion event fires), and top must be non-NULL (matching the
fact that the current qemu code does not support committing the
active layer, although it is still planned to add that before 1.3).
Since the active layer won't change, we have it easy and do not have
to alter the domain XML. We are also relying on qemu to do some
error detection on our behalf (such as validating 'top' and 'base'
as being members of the backing chain). Additionally, this will
fail if SELinux is enforcing, because we fail to grant qemu proper
read/write access to the files it will be modifying.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): New function.
(qemuDriver): Register it.
---
src/qemu/qemu_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fddb446..137df7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12529,6 +12529,84 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned
long bandwidth,
return qemuDomainBlockRebase(dom, path, NULL, bandwidth, flags);
}
+
+static int
+qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
+ const char *top, unsigned long bandwidth,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = dom->conn->privateData;
+ qemuDomainObjPrivatePtr priv;
+ virDomainObjPtr vm = NULL;
+ char *device = NULL;
+ int ret = -1;
+ virDomainEventPtr event = NULL;
+ int idx;
+ virDomainDiskDefPtr disk;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+ priv = vm->privateData;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto endjob;
+ }
+ if (!qemuCapsGet(priv->caps, QEMU_CAPS_BLOCK_COMMIT) || !top) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("online commit of active image not supported with "
+ "this QEMU binary"));
+ goto endjob;
+ }
+
+ device = qemuDiskPathToAlias(vm, path, &idx);
+ if (!device)
+ goto endjob;
+ disk = vm->def->disks[idx];
+
+ if (!disk->src) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk %s has no source file to be committed"),
+ disk->dst);
+ goto endjob;
+ }
+
+ /* XXX For now, we are relying on qemu to check that 'top' and
+ * 'base' resolve to members of the backing chain in correct
+ * order; but if we ever get more paranoid and track the backing
+ * chain ourself, we should be pre-validating the data rather than
+ * relying on qemu. For that matter, we need to be changing the
+ * SELinux label on both 'base' and the parent of 'top', so that
+ * qemu can open(O_RDWR) those files for the duration of the
+ * commit. */
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorBlockCommit(priv->mon, device, top, base, bandwidth);
+ qemuDomainObjExitMonitor(driver, vm);
+
+endjob:
+ if (qemuDomainObjEndJob(driver, vm) == 0) {
+ vm = NULL;
+ goto cleanup;
+ }
+
+cleanup:
+ VIR_FREE(device);
+ if (vm)
+ virDomainObjUnlock(vm);
+ if (event) {
+ qemuDriverLock(driver);
+ qemuDomainEventQueue(driver, event);
+ qemuDriverUnlock(driver);
+ }
+ return ret;
+}
+
static int
qemuDomainOpenGraphics(virDomainPtr dom,
unsigned int idx,
@@ -13871,6 +13949,7 @@ static virDriver qemuDriver = {
.domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
.domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
.domainBlockRebase = qemuDomainBlockRebase, /* 0.9.10 */
+ .domainBlockCommit = qemuDomainBlockCommit, /* 0.10.3 */
.isAlive = qemuIsAlive, /* 0.9.8 */
.nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */
.domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */
--
1.7.11.4
9 a.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 04.10.2012 16:44, Eric Blake wrote:
This is the bare minimum to kick off a block commit. In particular,
flags support is missing (shallow requires us to crawl the backing
chain to determine the file name to pass to the qemu monitor command;
delete requires us to track what needs to be deleted at the time
the completion event fires), and top must be non-NULL (matching the
fact that the current qemu code does not support committing the
active layer, although it is still planned to add that before 1.3).
Since the active layer won't change, we have it easy and do not have
to alter the domain XML. We are also relying on qemu to do some
error detection on our behalf (such as validating 'top' and 'base'
as being members of the backing chain). Additionally, this will
fail if SELinux is enforcing, because we fail to grant qemu proper
read/write access to the files it will be modifying.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): New function.
(qemuDriver): Register it.
---
src/qemu/qemu_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fddb446..137df7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12529,6 +12529,84 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned
long bandwidth,
return qemuDomainBlockRebase(dom, path, NULL, bandwidth, flags);
}
+
+static int
+qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
+ const char *top, unsigned long bandwidth,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = dom->conn->privateData;
+ qemuDomainObjPrivatePtr priv;
+ virDomainObjPtr vm = NULL;
+ char *device = NULL;
+ int ret = -1;
+ virDomainEventPtr event = NULL;
Not used yet. But since this is a stub only it doesn't really matter.
+ int idx;
+ virDomainDiskDefPtr disk;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+ priv = vm->privateData;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto endjob;
+ }
+ if (!qemuCapsGet(priv->caps, QEMU_CAPS_BLOCK_COMMIT) || !top) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("online commit of active image not supported with "
+ "this QEMU binary"));
+ goto endjob;
+ }
+
+ device = qemuDiskPathToAlias(vm, path, &idx);
+ if (!device)
+ goto endjob;
+ disk = vm->def->disks[idx];
+
+ if (!disk->src) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk %s has no source file to be committed"),
+ disk->dst);
+ goto endjob;
+ }
+
+ /* XXX For now, we are relying on qemu to check that 'top' and
+ * 'base' resolve to members of the backing chain in correct
+ * order; but if we ever get more paranoid and track the backing
+ * chain ourself, we should be pre-validating the data rather than
+ * relying on qemu. For that matter, we need to be changing the
+ * SELinux label on both 'base' and the parent of 'top', so that
+ * qemu can open(O_RDWR) those files for the duration of the
+ * commit. */
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorBlockCommit(priv->mon, device, top, base, bandwidth);
+ qemuDomainObjExitMonitor(driver, vm);
+
+endjob:
+ if (qemuDomainObjEndJob(driver, vm) == 0) {
+ vm = NULL;
+ goto cleanup;
+ }
+
+cleanup:
+ VIR_FREE(device);
+ if (vm)
+ virDomainObjUnlock(vm);
+ if (event) {
+ qemuDriverLock(driver);
+ qemuDomainEventQueue(driver, event);
+ qemuDriverUnlock(driver);
+ }
+ return ret;
+}
+
static int
qemuDomainOpenGraphics(virDomainPtr dom,
unsigned int idx,
@@ -13871,6 +13949,7 @@ static virDriver qemuDriver = {
.domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
.domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
.domainBlockRebase = qemuDomainBlockRebase, /* 0.9.10 */
+ .domainBlockCommit = qemuDomainBlockCommit, /* 0.10.3 */
.isAlive = qemuIsAlive, /* 0.9.8 */
.nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */
.domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */
Looks good.
10:03 a.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/05/2012 08:00 AM, Michal Privoznik wrote:
On 04.10.2012 16:44, Eric Blake wrote:
> This is the bare minimum to kick off a block commit. In particular,
> flags support is missing (shallow requires us to crawl the backing
> chain to determine the file name to pass to the qemu monitor command;
> delete requires us to track what needs to be deleted at the time
> the completion event fires), and top must be non-NULL (matching the
> fact that the current qemu code does not support committing the
> active layer, although it is still planned to add that before 1.3).
> Since the active layer won't change, we have it easy and do not have
> to alter the domain XML. We are also relying on qemu to do some
> error detection on our behalf (such as validating 'top' and 'base'
> as being members of the backing chain). Additionally, this will
> fail if SELinux is enforcing, because we fail to grant qemu proper
> read/write access to the files it will be modifying.
>
> +
> +static int
> +qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
> + const char *top, unsigned long bandwidth,
> + unsigned int flags)
> +{
> + struct qemud_driver *driver = dom->conn->privateData;
> + qemuDomainObjPrivatePtr priv;
> + virDomainObjPtr vm = NULL;
> + char *device = NULL;
> + int ret = -1;
> + virDomainEventPtr event = NULL;
Not used yet. But since this is a stub only it doesn't really matter.
I'll scrub it out before pushing (it's leftover from my other
unsubmitted branch for doing offline block commit, where we have to
synthesize the event because qemu is not running).
Looks good.
And Anthony just merged Jeff's patches into qemu.git today (now commit
ed61fc10), so I'll push this as soon as I complete another round of
testing, as well as work towards proper SELinux behavior.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:53 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/05/2012 08:33 PM, Eric Blake wrote:
On 10/05/2012 08:00 AM, Michal Privoznik wrote:
> On 04.10.2012 16:44, Eric Blake wrote:
>> This is the bare minimum to kick off a block commit. In particular,
>> flags support is missing (shallow requires us to crawl the backing
>> chain to determine the file name to pass to the qemu monitor command;
>> delete requires us to track what needs to be deleted at the time
>> the completion event fires), and top must be non-NULL (matching the
>> fact that the current qemu code does not support committing the
>> active layer, although it is still planned to add that before 1.3).
>> Since the active layer won't change, we have it easy and do not have
>> to alter the domain XML. We are also relying on qemu to do some
>> error detection on our behalf (such as validating 'top' and
'base'
>> as being members of the backing chain). Additionally, this will
>> fail if SELinux is enforcing, because we fail to grant qemu proper
>> read/write access to the files it will be modifying.
>>
>> +
>> +static int
>> +qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
>> + const char *top, unsigned long bandwidth,
>> + unsigned int flags)
>> +{
>> + struct qemud_driver *driver = dom->conn->privateData;
>> + qemuDomainObjPrivatePtr priv;
>> + virDomainObjPtr vm = NULL;
>> + char *device = NULL;
>> + int ret = -1;
>> + virDomainEventPtr event = NULL;
>
> Not used yet. But since this is a stub only it doesn't really matter.
I'll scrub it out before pushing (it's leftover from my other
unsubmitted branch for doing offline block commit, where we have to
synthesize the event because qemu is not running).
>
> Looks good.
And Anthony just merged Jeff's patches into qemu.git today (now commit
ed61fc10), so I'll push this as soon as I complete another round of
testing, as well as work towards proper SELinux behavior.
Eric, Let me know how I can help you test here.
I'm trying to test the below scenario(based on Jcody's email to qemu-devel) for
Block Commit :
Original state: [base] <-- [snap-1] <-- [snap-2] <-- [snap-3] <--
[active-layer]
Try to turn the into:
Case-1/ [base] <-- [active-layer] # where snap1, snap2, snap3 are committed into
the 'base' image (and thus, rightfully invalidating snap1 & snap2)
[or]
Case-2/ [base] <--- [snap-1] <--- [active-layer] # where data from snap2 &
snap3 are
committed into 'snap1' (and thus, rightfully invalidating snap2 & snap3)
[or]
Case-3/ [base] <--- [snap-3] <--- [active-layer], # where data from snap1 &
snap2 are
committed into the 'base' image (and thus, invalidating snap1 & snap2)
(All of the above, while the guest is live and running)
Please let me know if the above understanding is correct.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
/kashyap
2:04 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/05/2012 12:53 PM, Kashyap Chamarthy wrote:
>
> And Anthony just merged Jeff's patches into qemu.git today (now commit
> ed61fc10), so I'll push this as soon as I complete another round of
> testing, as well as work towards proper SELinux behavior.
Eric, Let me know how I can help you test here.
With qemu.git and with my patches applied (they are not yet in
libvirt.git, so you'll have to apply them yourself to test):
I'm trying to test the below scenario(based on Jcody's email to qemu-devel) for
Block Commit :
Original state: [base] <-- [snap-1] <-- [snap-2] <-- [snap-3] <--
[active-layer]
Assuming that you have a domain with just one disk, labeled 'vda'
according to 'virsh domblklist', then...
Try to turn the into:
Case-1/ [base] <-- [active-layer] # where snap1, snap2, snap3 are committed into
the 'base' image (and thus, rightfully invalidating snap1 & snap2)
virsh blockcommit $dom vda --wait --base /path/to/base --top /path/to/snap-3
Note that your chain has to use absolute backing file names in the
current state of qemu.git (Jeff is working on a patch to work with
chains that used relative backing file names).
Also, note that the patches I have posted so far do not interact with
existing libvirt snapshots, so while you can verify that the disk images
have been properly modified (for example, after the commit completes,
stop the domain, then use qemu-img info active-layer to see that
active-layer's backing image is now base instead of snap-3), the fact
that you did a commit may make it harder to use any snapshots you
created via 'virsh snapshot-create --disk-only' to create the long chain
in the first place.
[or]
Case-2/ [base] <--- [snap-1] <--- [active-layer] # where data from snap2 &
snap3 are
committed into 'snap1' (and thus, rightfully invalidating snap2 & snap3)
Similar to case-1, except that now you use --base snap-1.
[or]
Case-3/ [base] <--- [snap-3] <--- [active-layer], # where data from snap1 &
snap2 are
committed into the 'base' image (and thus, invalidating snap1 & snap2)
Similar to case-1, except that now you use --top snap-2.
(All of the above, while the guest is live and running)
Yep - requires a running guest at the moment (I'm still trying to get
patches to support offline guest commits, using qemu-img, but it's
taking a while to iron out the design work necessary to track a child
qemu-img across libvirtd restarts).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:22 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/06/2012 12:34 AM, Eric Blake wrote:
--
/kashyap
On 10/05/2012 12:53 PM, Kashyap Chamarthy wrote:
>>
>> And Anthony just merged Jeff's patches into qemu.git today (now commit
>> ed61fc10), so I'll push this as soon as I complete another round of
>> testing, as well as work towards proper SELinux behavior.
>
> Eric, Let me know how I can help you test here.
With qemu.git and with my patches applied (they are not yet in
libvirt.git, so you'll have to apply them yourself to test):
Yes, I can do that. I'll be testing this over the weekend.
I have the latest qemu.git(with the above commit from Anthony ed61fc10), compiled for
arch x86_64.
So, just to clarify, so I don't mess up, on libvirt's latest git, I'm
applying only these
two patches as below:
[PATCH 1/2] blockjob: manage qemu block-commit monitor command
[PATCH 2/2] blockjob: wire up online qemu block-commit
# git am -3 /path-to-patch1
# git am -3 /path-to-patch2
# ./autogen.sh
# make
# make check
Then run the below (while using the qemu binary from it's git #
./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -smp 2 -m 1024
/home/kashyap/vmimages/f17vm2.qcow2)
And use the 'virsh' to do the testing from the fresh compile:
# ./run ./tools/virsh ...
>
> I'm trying to test the below scenario(based on Jcody's email to qemu-devel)
for Block Commit :
>
> Original state: [base] <-- [snap-1] <-- [snap-2] <-- [snap-3] <--
[active-layer]
Assuming that you have a domain with just one disk, labeled 'vda'
according to 'virsh domblklist', then...
>
> Try to turn the into:
>
> Case-1/ [base] <-- [active-layer] # where snap1, snap2, snap3 are committed
into
> the 'base' image (and thus, rightfully invalidating snap1 & snap2)
virsh blockcommit $dom vda --wait --base /path/to/base --top /path/to/snap-3
Note that your chain has to use absolute backing file names in the
current state of qemu.git (Jeff is working on a patch to work with
chains that used relative backing file names).
Also, note that the patches I have posted so far do not interact with
existing libvirt snapshots, so while you can verify that the disk images
have been properly modified (for example, after the commit completes,
stop the domain, then use qemu-img info active-layer to see that
active-layer's backing image is now base instead of snap-3), the fact
that you did a commit may make it harder to use any snapshots you
created via 'virsh snapshot-create --disk-only' to create the long chain
in the first place.
>
> [or]
>
> Case-2/ [base] <--- [snap-1] <--- [active-layer] # where data from snap2
& snap3 are
> committed into 'snap1' (and thus, rightfully invalidating snap2 & snap3)
Similar to case-1, except that now you use --base snap-1.
>
> [or]
>
> Case-3/ [base] <--- [snap-3] <--- [active-layer], # where data from snap1
& snap2 are
> committed into the 'base' image (and thus, invalidating snap1 & snap2)
Similar to case-1, except that now you use --top snap-2.
>
>
> (All of the above, while the guest is live and running)
Yep - requires a running guest at the moment (I'm still trying to get
patches to support offline guest commits, using qemu-img, but it's
taking a while to iron out the design work necessary to track a child
qemu-img across libvirtd restarts).
Thanks, that's clear.
2:25 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/06/2012 12:52 AM, Kashyap Chamarthy wrote:
On 10/06/2012 12:34 AM, Eric Blake wrote:
> On 10/05/2012 12:53 PM, Kashyap Chamarthy wrote:
>
>>>
>>> And Anthony just merged Jeff's patches into qemu.git today (now commit
>>> ed61fc10), so I'll push this as soon as I complete another round of
>>> testing, as well as work towards proper SELinux behavior.
>>
>> Eric, Let me know how I can help you test here.
>
> With qemu.git and with my patches applied (they are not yet in
> libvirt.git, so you'll have to apply them yourself to test):
Yes, I can do that. I'll be testing this over the weekend.
I have the latest qemu.git(with the above commit from Anthony ed61fc10), compiled for
arch x86_64.
So, just to clarify, so I don't mess up, on libvirt's latest git, I'm
applying only these
two patches as below:
[PATCH 1/2] blockjob: manage qemu block-commit monitor command
[PATCH 2/2] blockjob: wire up online qemu block-commit
# git am -3 /path-to-patch1
# git am -3 /path-to-patch2
# ./autogen.sh
# make
# make check
Then run the below (while using the qemu binary from it's git #
./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -smp 2 -m 1024
/home/kashyap/vmimages/f17vm2.qcow2)
(of-course with relevant changes to the 'emulator' attribute to the domain's
libvirt xml file)
And use the 'virsh' to do the testing from the fresh compile:
# ./run ./tools/virsh ...
>
>>
>> I'm trying to test the below scenario(based on Jcody's email to
qemu-devel) for Block Commit :
>>
>> Original state: [base] <-- [snap-1] <-- [snap-2] <-- [snap-3] <--
[active-layer]
>
> Assuming that you have a domain with just one disk, labeled 'vda'
> according to 'virsh domblklist', then...
>
>>
>> Try to turn the into:
>>
>> Case-1/ [base] <-- [active-layer] # where snap1, snap2, snap3 are
committed into
>> the 'base' image (and thus, rightfully invalidating snap1 & snap2)
>
> virsh blockcommit $dom vda --wait --base /path/to/base --top /path/to/snap-3
>
> Note that your chain has to use absolute backing file names in the
> current state of qemu.git (Jeff is working on a patch to work with
> chains that used relative backing file names).
>
> Also, note that the patches I have posted so far do not interact with
> existing libvirt snapshots, so while you can verify that the disk images
> have been properly modified (for example, after the commit completes,
> stop the domain, then use qemu-img info active-layer to see that
> active-layer's backing image is now base instead of snap-3), the fact
> that you did a commit may make it harder to use any snapshots you
> created via 'virsh snapshot-create --disk-only' to create the long chain
> in the first place.
>
>>
>> [or]
>>
>> Case-2/ [base] <--- [snap-1] <--- [active-layer] # where data from snap2
& snap3 are
>> committed into 'snap1' (and thus, rightfully invalidating snap2 &
snap3)
>
> Similar to case-1, except that now you use --base snap-1.
>
>>
>> [or]
>>
>> Case-3/ [base] <--- [snap-3] <--- [active-layer], # where data from snap1
& snap2 are
>> committed into the 'base' image (and thus, invalidating snap1 &
snap2)
>
> Similar to case-1, except that now you use --top snap-2.
>
>>
>>
>> (All of the above, while the guest is live and running)
>
> Yep - requires a running guest at the moment (I'm still trying to get
> patches to support offline guest commits, using qemu-img, but it's
> taking a while to iron out the design work necessary to track a child
> qemu-img across libvirtd restarts).
Thanks, that's clear.
>
--
/kashyap
2:56 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/05/2012 01:25 PM, Kashyap Chamarthy wrote:
>
> So, just to clarify, so I don't mess up, on libvirt's latest git, I'm
applying only these
> two patches as below:
>
> [PATCH 1/2] blockjob: manage qemu block-commit monitor command
> [PATCH 2/2] blockjob: wire up online qemu block-commit
>
> # git am -3 /path-to-patch1
> # git am -3 /path-to-patch2
> # ./autogen.sh
Here, I tend to use './autogen.sh --system CFLAGS=-g' if I intend to
debug anything under gdb; the --system makes it more likely that the
just-built libvirtd can be run without having to install it first. But
the ./run script also helps there.
> # make
> # make check
>
> Then run the below (while using the qemu binary from it's git #
> ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -smp 2 -m 1024
> /home/kashyap/vmimages/f17vm2.qcow2)
(of-course with relevant changes to the 'emulator' attribute to the domain's
libvirt xml file)
Yep, and/or install the just-built qemu into the system location.
>
> And use the 'virsh' to do the testing from the fresh compile:
>
> # ./run ./tools/virsh ...
>
Yep, that's about what I'm doing in my tests.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:26 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/06/2012 01:26 AM, Eric Blake wrote:
--
/kashyap
On 10/05/2012 01:25 PM, Kashyap Chamarthy wrote:
>>
>> So, just to clarify, so I don't mess up, on libvirt's latest git,
I'm applying only these
>> two patches as below:
>>
>> [PATCH 1/2] blockjob: manage qemu block-commit monitor command
>> [PATCH 2/2] blockjob: wire up online qemu block-commit
>>
>> # git am -3 /path-to-patch1
>> # git am -3 /path-to-patch2
>> # ./autogen.sh
Here, I tend to use './autogen.sh --system CFLAGS=-g' if I intend to
debug anything under gdb; the --system makes it more likely that the
just-built libvirtd can be run without having to install it first. But
the ./run script also helps there.
>> # make
>> # make check
>>
>> Then run the below (while using the qemu binary from it's git #
>> ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -smp 2 -m 1024
>> /home/kashyap/vmimages/f17vm2.qcow2)
>
> (of-course with relevant changes to the 'emulator' attribute to the
domain's libvirt xml file)
Yep, and/or install the just-built qemu into the system location.
>
>>
>> And use the 'virsh' to do the testing from the fresh compile:
>>
>> # ./run ./tools/virsh ...
>>
Yep, that's about what I'm doing in my tests.
Cool, patches applied just fine, 'make check' says all 68 tests passed. and I
could just
see the help output for 'blockcommit' once I compiled the git. Will continue with
real
testing tomorrow. And report results/findings in a separate thread.
(I'm not quite familiar w/ libvirt-tck framework (also, not a perl expert). I'll
see if I
can write some shell/python scripts, if it's of any value.)
2:12 p.m.
New subject: [libvirt] [PATCH 2/2] blockjob: wire up online qemu block-commit
On 10/05/2012 12:53 PM, Kashyap Chamarthy wrote:
Eric, Let me know how I can help you test here.
Other ideas of things to test - pass invalid arguments, and make sure
you get sane error messages (for example, a file that doesn't exist, or
omit --base, omit --top, specify --base and --top out of order)
Test regular files vs. raw block devices in the backing chain.
Test commits into the base file when it is not of type qcow2.
Testing with relative vs. absolute naming will eventually be helpful,
although right now we know of some relative name handling bugs that
qemu.git is still trying to iron out.
Obviously, testing with SELinux enforcing will be helpful, once I get
further patches submitted to attempt that, but for now, you will
probably hit failures unless you have selinux in permissive mode.
Try testing the case of a backing file smaller than the top file being
committed:
base(1M) <- snap(2M) <- active(3M)
and with data located at various offsets (here, it may be easier to test
via hot-plugging a dummy device to an existing guest, and then using dd
within the guest to write to that hot-plug device); committing snap into
base should make base expand to be the size of snap but not the size of
active.
Whatever else you can dream up - the more tests we come up with, the
better this code will be :) That is, knowing HOW people want to use it
will make it easier to ensure that those use cases work.
If you can write libvirt-tck tests that cover your tests, that would be
even nicer, as then we would have a framework for easily repeating the
tests and ensure we don't regress.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4432
days inactive
4433
days old
12 comments
3 participants
participants (3)
-
Eric Blake -
Kashyap Chamarthy -
Michal Privoznik