[libvirt] [PATCH 0/n] qemu block-commit support
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
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
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.
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
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.
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- /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):
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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) 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
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
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/06/2012 01:26 AM, Eric Blake wrote:
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.)
-- /kashyap
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake -
Kashyap Chamarthy -
Michal Privoznik