[libvirt] [patch 0/4] checkpoint functionality

OK. Another run at producing a function that helps the likes of me backup my kvm Windows servers. 'virsh checkpoint domain file [script]' is what the following patch set (against cvs) enables. Modelled on the virDomainSave function it takes an optional script which it will execute (and pass the name of the domain as an argument) while the domain is paused, then resume the domain. By the by stability is coming along nicely since 0.6.0. Thanks! Matt McCowan

This refactors the path check routine used in virDomainSave into util
Matt McCowan
diff -ur libvirt.orig/src/util.h libvirt-0.6.1.1/src/util.h --- libvirt.orig/src/util.h 2009-02-27 01:27:51.000000000 +0900 +++ libvirt-0.6.1.1/src/util.h 2009-03-05 11:44:42.000000000 +0900 @@ -111,6 +111,8 @@ int virFileDeletePid(const char *dir, const char *name); +char *virFileAbsPath(const char *str); + char *virArgvToString(const char *const *argv); int virStrToLong_i(char const *s, --- libvirt.orig/src/util.c 2009-03-03 21:03:44.000000000 +0900 +++ libvirt-0.6.1.1/src/util.c 2009-03-09 14:17:17.000000000 +0900 @@ -136,6 +136,28 @@ return 1; } + +char * +virFileAbsPath(const char *str) +{ + char filepath[4096]; + char *ret; + unsigned int len, t; + + t = strlen(str); + if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) + return NULL; + len = strlen(filepath); + /* that should be covered by getcwd() semantic, but be 100% sure */ + if (len > sizeof(filepath) - (t + 3)) + return NULL; + filepath[len] = '/'; + strcpy(&filepath[len + 1], str); + ret = &filepath[0]; + + return ret; +} + char * virArgvToString(const char *const *argv) {

Patches for the drivers and header Here's hoping jolly Evolution doesn't play silly buggers with wrapping. If so I'll be resending with claws :(
Matt McCowan
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
--- libvirt.orig/src/driver.h 2009-03-03 18:14:28.000000000 +0900 +++ libvirt-0.6.1.1/src/driver.h 2009-03-09 17:28:54.000000000 +0900 @@ -141,6 +141,12 @@ typedef int (*virDrvDomainSave) (virDomainPtr domain, const char *to); + +typedef int + (*virDrvDomainCheckpoint) (virDomainPtr domain, + const char *to, + const char *script); + typedef int (*virDrvDomainRestore) (virConnectPtr conn, const char *from); @@ -372,6 +378,7 @@ virDrvDomainSetMemory domainSetMemory; virDrvDomainGetInfo domainGetInfo; virDrvDomainSave domainSave; + virDrvDomainCheckpoint domainCheckpoint; virDrvDomainRestore domainRestore; virDrvDomainCoreDump domainCoreDump; virDrvDomainSetVcpus domainSetVcpus; diff -ur libvirt.orig/src/lxc_driver.c libvirt-0.6.1.1/src/lxc_driver.c --- libvirt.orig/src/lxc_driver.c 2009-03-09 12:42:43.000000000 +0900 +++ libvirt-0.6.1.1/src/lxc_driver.c 2009-03-05 11:44:42.000000000 +0900 @@ -1435,6 +1423,7 @@ NULL, /* domainSetMemory */ lxcDomainGetInfo, /* domainGetInfo */ NULL, /* domainSave */ + NULL, /* domainCheckpoint */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ NULL, /* domainSetVcpus */ diff -ur libvirt.orig/src/openvz_driver.c libvirt-0.6.1.1/src/openvz_driver.c --- libvirt.orig/src/openvz_driver.c 2009-03-03 18:14:28.000000000 +0900 +++ libvirt-0.6.1.1/src/openvz_driver.c 2009-03-05 11:44:42.000000000 +0900 @@ -1293,6 +1293,7 @@ NULL, /* domainSetMemory */ openvzDomainGetInfo, /* domainGetInfo */ NULL, /* domainSave */ + NULL, /* domainCheckpoint */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ openvzDomainSetVcpus, /* domainSetVcpus */ diff -ur libvirt.orig/src/proxy_internal.c libvirt-0.6.1.1/src/proxy_internal.c --- libvirt.orig/src/proxy_internal.c 2009-01-29 21:10:32.000000000 +0900 +++ libvirt-0.6.1.1/src/proxy_internal.c 2009-03-05 14:39:58.000000000 +0900 @@ -67,6 +67,7 @@ NULL, /* domainSetMemory */ xenProxyDomainGetInfo, /* domainGetInfo */ NULL, /* domainSave */ + NULL, /* domainCheckpoint */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ NULL, /* domainSetVcpus */ diff -ur libvirt.orig/src/test.c libvirt-0.6.1.1/src/test.c --- libvirt.orig/src/test.c 2009-03-03 18:14:28.000000000 +0900 +++ libvirt-0.6.1.1/src/test.c 2009-03-05 11:44:42.000000000 +0900 @@ -3495,6 +3495,7 @@ testSetMemory, /* domainSetMemory */ testGetDomainInfo, /* domainGetInfo */ testDomainSave, /* domainSave */ + NULL, /* domainCheckpoint */ testDomainRestore, /* domainRestore */ testDomainCoreDump, /* domainCoreDump */ testSetVcpus, /* domainSetVcpus */ diff -ur libvirt.orig/src/uml_driver.c libvirt-0.6.1.1/src/uml_driver.c --- libvirt.orig/src/uml_driver.c 2009-03-03 18:14:28.000000000 +0900 +++ libvirt-0.6.1.1/src/uml_driver.c 2009-03-05 11:44:42.000000000 +0900 @@ -1848,6 +1848,7 @@ umlDomainSetMemory, /* domainSetMemory */ umlDomainGetInfo, /* domainGetInfo */ NULL, /* domainSave */ + NULL, /* domainCheckpoint */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ NULL, /* domainSetVcpus */ diff -ur libvirt.orig/src/qemu_driver.c libvirt-0.6.1.1/src/qemu_driver.c --- libvirt.orig/src/qemu_driver.c 2009-03-04 10:24:50.000000000 +0900 +++ libvirt-0.6.1.1/src/qemu_driver.c 2009-03-06 16:30:39.000000000 +0900 @@ -2718,6 +2718,156 @@ return ret; } +static int qemudDomainCheckpoint(virDomainPtr dom, + const char *path, + const char *script) { + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + char *command = NULL; + char *info = NULL; + int fd = -1; + char *safe_path = NULL; + char *xml = NULL; + struct qemud_save_header header; + int ret = -1; + virDomainEventPtr event = NULL; + const char *argv[3]; + + memset(&header, 0, sizeof(header)); + memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); + header.version = QEMUD_SAVE_VERSION; + + qemuDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + + if (!vm) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching id %d"), dom->id); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("domain is not running")); + goto cleanup; + } + + /* Pause */ + if (vm->state == VIR_DOMAIN_RUNNING) { + header.was_running = 1; + if (qemudMonitorCommand(vm, "stop", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("suspend operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_PAUSED; + qemudDebug("Reply %s", info); + VIR_FREE(info); + } + + /* Get XML for the domain */ + xml = virDomainDefFormat(dom->conn, vm->def, VIR_DOMAIN_XML_SECURE); + if (!xml) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("failed to get domain xml")); + goto cleanup; + } + header.xml_len = strlen(xml) + 1; + + /* Write header to file, followed by XML */ + if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to create '%s'"), path); + goto cleanup; + } + + if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("failed to write save header")); + goto cleanup; + } + + if (safewrite(fd, xml, header.xml_len) != header.xml_len) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("failed to write xml")); + goto cleanup; + } + + if (close(fd) < 0) { + virReportSystemError(dom->conn, errno, + _("unable to save file %s"), + path); + goto cleanup; + } + fd = -1; + + /* Migrate to file */ + safe_path = qemudEscapeShellArg(path); + if (!safe_path) { + virReportOOMError(dom->conn); + goto cleanup; + } + if (virAsprintf(&command, "migrate \"exec:" + "dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path) == -1) { + virReportOOMError(dom->conn); + command = NULL; + goto cleanup; + } + + if (qemudMonitorCommand(vm, command, &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("migrate operation failed")); + goto cleanup; + } + + DEBUG ("migrate reply: %s", info); + + /* If the command isn't supported then qemu prints: + * unknown command: migrate" */ + if (strstr(info, "unknown command:")) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, + "%s", + _("'migrate' not supported by this qemu")); + goto cleanup; + } + + /* Call optional script */ + if (script != NULL ) { + argv[0] = qemudEscapeShellArg(script); + argv[1] = virDomainGetName(dom); + argv[2] = NULL; + if (virRun(dom->conn, argv, NULL) < 0) + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("Failed to run '%s'"), script); + } + + if (qemudMonitorCommand(vm, "cont", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("resume operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_RUNNING; + DEBUG ("cont reply: %s", info); + + ret = 0; + +cleanup: + if (fd != -1) + close(fd); + VIR_FREE(xml); + VIR_FREE(safe_path); + VIR_FREE(command); + VIR_FREE(info); + if (ret != 0) + unlink(path); + if (vm) + virDomainObjUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); + qemuDriverUnlock(driver); + return ret; +} static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; @@ -4931,6 +5081,7 @@ qemudDomainSetMemory, /* domainSetMemory */ qemudDomainGetInfo, /* domainGetInfo */ qemudDomainSave, /* domainSave */ + qemudDomainCheckpoint, /* domainCheckpoint */ qemudDomainRestore, /* domainRestore */ NULL, /* domainCoreDump */ qemudDomainSetVcpus, /* domainSetVcpus */

(only 3 patches sorry). Finally the rest. Note, will probably have to 'make rpcgen' and 'make remote.c' in qemud to reparse remote_protocol.x and friends
Matt McCowan
diff -ur libvirt.orig/include/libvirt/libvirt.h.in libvirt-0.6.1.1/include/libvirt/libvirt.h.in --- libvirt.orig/include/libvirt/libvirt.h.in 2009-03-03 18:09:00.000000000 +0900 +++ libvirt-0.6.1.1/include/libvirt/libvirt.h.in 2009-03-05 11:44:42.000000000 +0900 @@ -529,10 +529,13 @@ int virDomainResume (virDomainPtr domain); /* - * Domain save/restore + * Domain save/checkpoint/restore */ int virDomainSave (virDomainPtr domain, const char *to); +int virDomainCheckpoint (virDomainPtr domain, + const char *to, + const char *script); int virDomainRestore (virConnectPtr conn, const char *from); diff -ur libvirt.orig/qemud/remote_protocol.x libvirt-0.6.1.1/qemud/remote_protocol.x --- libvirt.orig/qemud/remote_protocol.x 2009-03-03 18:27:03.000000000 +0900 +++ libvirt-0.6.1.1/qemud/remote_protocol.x 2009-03-05 12:35:34.000000000 +0900 @@ -498,6 +498,12 @@ remote_nonnull_string to; }; +struct remote_domain_checkpoint_args { + remote_nonnull_domain dom; + remote_nonnull_string to; + remote_string script; +}; + struct remote_domain_restore_args { remote_nonnull_string from; }; @@ -1270,7 +1276,8 @@ REMOTE_PROC_NODE_DEVICE_RESET = 120, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL = 121, - REMOTE_PROC_NODE_GET_SECURITY_MODEL = 122 + REMOTE_PROC_NODE_GET_SECURITY_MODEL = 122, + REMOTE_PROC_DOMAIN_CHECKPOINT = 123 }; /* Custom RPC structure. */ diff -ur libvirt.orig/src/libvirt_public.syms libvirt-0.6.1.1/src/libvirt_public.syms --- libvirt.orig/src/libvirt_public.syms 2009-03-03 18:09:00.000000000 +0900 +++ libvirt-0.6.1.1/src/libvirt_public.syms 2009-03-05 11:44:42.000000000 +0900 @@ -254,6 +254,7 @@ virNodeDeviceDettach; virNodeDeviceReAttach; virNodeDeviceReset; + virDomainCheckpoint; virDomainGetSecurityLabel; virNodeGetSecurityModel; } LIBVIRT_0.6.0; --- libvirt.orig/qemud/remote.c 2009-03-03 18:27:03.000000000 +0900 +++ libvirt-0.6.1.1/qemud/remote.c 2009-03-09 14:10:41.000000000 +0900 @@ -1940,6 +1940,33 @@ return 0; } + +static int +remoteDispatchDomainCheckpoint (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_error *rerr, + remote_domain_checkpoint_args *args, + void *ret ATTRIBUTE_UNUSED) +{ + virDomainPtr dom; + + dom = get_nonnull_domain (conn, args->dom); + if (dom == NULL) { + remoteDispatchConnError(rerr, conn); + return -1; + } + + if (virDomainCheckpoint (dom, args->to, + args->script ? *args->script : NULL) == -1) { + virDomainFree(dom); + remoteDispatchConnError(rerr, conn); + return -1; + } + virDomainFree(dom); + return 0; +} + static int remoteDispatchDomainCoreDump (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, --- libvirt.orig/src/remote_internal.c 2009-03-04 10:24:58.000000000 +0900 +++ libvirt-0.6.1.1/src/remote_internal.c 2009-03-09 14:14:22.000000000 +0900 @@ -2089,6 +2089,32 @@ return rv; } + +static int +remoteDomainCheckpoint (virDomainPtr domain, const char *to, const char *script) +{ + int rv = -1; + remote_domain_checkpoint_args args; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.to = (char *) to; + args.script = script == NULL ? NULL : (char **) &script; + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CHECKPOINT, + (xdrproc_t) xdr_remote_domain_checkpoint_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto done; + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + static int remoteDomainRestore (virConnectPtr conn, const char *from) { @@ -6855,6 +6881,7 @@ .domainSetMemory = remoteDomainSetMemory, .domainGetInfo = remoteDomainGetInfo, .domainSave = remoteDomainSave, + .domainCheckpoint = remoteDomainCheckpoint, .domainRestore = remoteDomainRestore, .domainCoreDump = remoteDomainCoreDump, .domainSetVcpus = remoteDomainSetVcpus, --- libvirt.orig/src/virsh.c 2009-03-05 10:17:49.000000000 +0900 +++ libvirt-0.6.1.1/src/virsh.c 2009-03-09 15:43:58.000000000 +0900 @@ -1062,6 +1062,61 @@ return ret; } + +/* + * "checkpoint" command + */ +static const vshCmdInfo info_checkpoint[] = { + {"help", gettext_noop("checkpoint a domain state to a file")}, + {"desc", gettext_noop("Checkpoint a running domain.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_checkpoint[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to save the data")}, + {"script", VSH_OT_DATA, 0, gettext_noop("script run while domain suspended")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdCheckpoint(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + char *name; + char *to; + char *script; + int found; + int ret = TRUE; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(to = vshCommandOptString(cmd, "file", NULL))) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return FALSE; + + script = vshCommandOptString(cmd, "script", NULL); + + if (script != NULL) { + script = vshCommandOptString(cmd, "script", &found); + if (!found) + return FALSE; + } + + if (virDomainCheckpoint(dom, to, script) == 0) { + vshPrint(ctl, _("Domain %s saved to %s running %s\n"), name, to, script); + } else { + vshError(ctl, FALSE, _("Failed to save domain %s to %s running %s"), name, to, script); + ret = FALSE; + } + + virDomainFree(dom); + return ret; +} + /* * "save" command */ @@ -5761,6 +5816,7 @@ {"reboot", cmdReboot, opts_reboot, info_reboot}, {"restore", cmdRestore, opts_restore, info_restore}, {"resume", cmdResume, opts_resume, info_resume}, + {"checkpoint", cmdCheckpoint, opts_checkpoint, info_checkpoint}, {"save", cmdSave, opts_save, info_save}, {"schedinfo", cmdSchedinfo, opts_schedinfo, info_schedinfo}, {"dump", cmdDump, opts_dump, info_dump}, --- libvirt.orig/src/libvirt.c 2009-03-04 10:24:46.000000000 +0900 +++ libvirt-0.6.1.1/src/libvirt.c 2009-03-09 17:24:38.000000000 +0900 @@ -1934,6 +1934,77 @@ return -1; } + +/** + * virDomainCheckpoint: + * @domain: a domain object + * @to: path for the output file + * @script: script + * + * This method will suspend a domain and save its memory contents to + * a file on disk. If specified a script will be executed before resuming the + * domain. + * + * script is optional and so could be NULL + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainCheckpoint(virDomainPtr domain, const char *to, const char *script) +{ + virConnectPtr conn; + + DEBUG("domain=%p, to=%s, script=%s", domain, to, script ? script : ""); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + return (-1); + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + conn = domain->conn; + if (to == NULL) { + virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + /* + * We must absolutize the file path as the save is done out of process + * TODO: check for URI when libxml2 is linked in. + */ + if (to[0] != '/') + to = virFileAbsPath(to); + + if (to == NULL) + return (-1); + + if (script != NULL) { + if (script[0] != '/') + script = virFileAbsPath(script); + if (script == NULL) + return (-1); + } + + if (conn->driver->domainCheckpoint) { + int ret; + ret = conn->driver->domainCheckpoint(domain, to, script); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(domain->conn); + return -1; +} + /** * virDomainSave: * @domain: a domain object

On Mon, Mar 09, 2009 at 11:14:56PM +0900, Matt McCowan wrote:
OK. Another run at producing a function that helps the likes of me backup my kvm Windows servers. 'virsh checkpoint domain file [script]' is what the following patch set (against cvs) enables. Modelled on the virDomainSave function it takes an optional script which it will execute (and pass the name of the domain as an argument) while the domain is paused, then resume the domain.
Okay, I quickly looked at the patch and the new API, so no stylistic review at this point but more a general feedback. I appreciate the completeness of the patch (only bit missing is filling up the virsh man page with the new option) I think this can help administrators in a controlled situation, but I'm hoping a real snapshotting API will be possible at some point where libvirt goes though the list of storage resources used by the domain and properly make a snapshot using a storage API or return an error if that's not possible. This looks more as a script extension to virDomainSave/Restore than really a snapshotting API as I would like libvirt to get at some point. To me it doesn't implement snapshotting, it implement running a script on a frozen domain. There is a lot of issues with that: - the script need to be in place and there is no hint of what it may contain - what the script does is unbounded, there is a potential very serious security issue there - this does not garantee sucess of the operation and in case of failure the script can't even propagate back an error code and description In my opinion this is a bit too ad-hoc to be turned into a full API maintained forever, and I'm worried about remote execution capabilities this opens up. I really hope we will have one day a real checkpointing API in libvirt, but that requires making more progresses on the storage management side. Still thanks for sending the patches, even if I think this is not ripe for inclusion, it reminds us that snapshotting is a big issue and need to be taken care of !
By the by stability is coming along nicely since 0.6.0. Thanks!
Yeah, there were a lot of changes, and bugs are ironed out progressively. Check the version from CVS/git/snapshot to get the latest changes as usual, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Mar 10, 2009 at 05:18:54PM +0100, Daniel Veillard wrote:
On Mon, Mar 09, 2009 at 11:14:56PM +0900, Matt McCowan wrote:
OK. Another run at producing a function that helps the likes of me backup my kvm Windows servers. 'virsh checkpoint domain file [script]' is what the following patch set (against cvs) enables. Modelled on the virDomainSave function it takes an optional script which it will execute (and pass the name of the domain as an argument) while the domain is paused, then resume the domain.
Okay, I quickly looked at the patch and the new API, so no stylistic review at this point but more a general feedback. I appreciate the completeness of the patch (only bit missing is filling up the virsh man page with the new option) I think this can help administrators in a controlled situation, but I'm hoping a real snapshotting API will be possible at some point where libvirt goes though the list of storage resources used by the domain and properly make a snapshot using a storage API or return an error if that's not possible. This looks more as a script extension to virDomainSave/Restore than really a snapshotting API as I would like libvirt to get at some point. To me it doesn't implement snapshotting, it implement running a script on a frozen domain. There is a lot of issues with that: - the script need to be in place and there is no hint of what it may contain - what the script does is unbounded, there is a potential very serious security issue there - this does not garantee sucess of the operation and in case of failure the script can't even propagate back an error code and description
Also this kind of API means that all applications using the API need to know what script to use with what storage in order to take a disk snapshot. This is quite a burden for apps, and means that most of them won't be able to use this snapshot capability in general case.
In my opinion this is a bit too ad-hoc to be turned into a full API maintained forever, and I'm worried about remote execution capabilities this opens up. I really hope we will have one day a real checkpointing API in libvirt, but that requires making more progresses on the storage management side.
In terms of API I think I'd like to see snapshotting available as part of a more generic save/restore API. I tend to think of the current API as providing 'unmanaged save/restore', in so much as libvirt / users of libvirt have no way of knowing whether there is a saved image for the guest available currently. As an example of why this is a problem, consider the often requested ability to save/restore guests across host reboot. When libvirtd starts, it looks at the autostart flag for a guest, and decides may thus automatically boot the guest at that time. libvirtd has no way of knowing whether there was an existing saved image of the guest available to restore, since it doesn't track saved images. Thus I think the first step towards a general snapshot facility would be to provide an API for 'managed save/restore' where we explicitly track saved images. - To save a guest: enum { VIR_DOMAIN_SAVE_LIVE, /* Don't shutdown guest after saving */ } virDomainSaveFlags; virDomainCreateSaveImage(virDomainPtr dom, const char *imagename, unsigned int flags); By default, imagename would be NULL, and flags would be zero. In this case we'd simply be saving to /var/lib/libvirt/saved/qemu/$VMNAME.img Providing equivalence to current virDomainSave(dom, filename), but with libvirt deciding the actual filename used. Now to do snapshots in this scenario, you'd pass a 'imagename' for the snapshot - give each snapshot you take some meaningful name as you desire. And if VIR_DOMAIN_SAVE_LIVE is set then the guest will be allowed to continue running after the snapshot is saved. Obviously this capability would require some way to take a snapshot of the storage, as well as the memory image. We could allow a script to do this, or implement it directly for a handful of storage types we know can do this (eg, LVM, qcow2). - Next, we'd need a way to list saved images virDomainListSaveImages(virDomainPtr dom, const char **imagenames, unsigned int *numimagenames); This would just give back a list of all known imagenames for the VM. - Then a way to restore from an image enum { VIR_DOMAIN_RESTORE_DELETE } virDomainRestoreFlags; virDomainRestoreSaveImage(virDomainPtr dom, const char *imagename, unsigned int flags); This would restore from the named image. If the VIR_DOMAIN_RESTORE_DELETE flag was given, the named image would be deleted after restore (eg to prevent accidental duplicated restore attempts) - Perhaps also need a way to delete saved imaged virDomainDeleteSaveImage(virDomainPtr dom, const char *imagename); If VIR_DOMAIN_SAVE_LIVE is set, the guest won't be shutdown after saving its state. - Perhaps also need a wa yto get more metaata about the save image struct virDomainSaveImageInfo { int savedDate; unsigned long long imageSize; }; virDomainGetSaveImgeInfo(virDomainptr dom, const char *imagename, virDomainSaveImageInfoPtr info); Then again, perhaps the virDomainListSaveImages should just return a list of virDomainSaveImageInfoPtr instances directly, instead of requiring this extra API call - it'd be faster for remote usage in particular because it'd have less round-trips. With this, you could configure libvirtd, so that when starting up, it used virDomainListSaveImages to see if the guest was suspended before the previous host shutdown, and if so, then restore from that saved image automatically. Or make it skip autostart completely, if any save images exist, and allow an admin defined initscript to do auto restore from the save image. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
Matt McCowan
-
Matt McCowan