This patch adds save/restore functionality to the libxl driver. It is a v2 of this patch: https://www.redhat.com/archives/libvir-list/2011-April/msg00338.html v2: * header is now padded and has a version field * the correct restore function from libxl is used * only create the restore event once in libxlVmStart

However I ran into a reproducible segfault. Assume you saved a vm with: # virsh save domU foo.img

If you restore the save image, destroy the vm and restore it again, a segfault occurs: # virsh restore foo.img # virsh destroy domU # virsh restore foo.img # segfault If you restart libvirt between the restores no segfault occurs: # virsh restore foo.img # virsh destroy domU # restart libvirt # virsh restore foo.img According to a gdb backtrace a memcpy from the function xc_domain_restore is causing the segfault. Then I saved a vm with the xl tool (which also uses the libxenlight interface) and restored it twice. No segfault occured. So I figured something must went wrong in libvirt. However I was unable to identify the responsible part of code. Now I am hoping that a review will possibly solve this issue. Here is the gdb backtrace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x42ba1950 (LWP 23813)] 0x00007f1bc17d906b in memcpy () from /lib/libc.so.6 (gdb) bt #0 0x00007f1bc17d906b in memcpy () from /lib/libc.so.6 #1 0x00007f1bc2b49346 in xc_domain_restore () from /usr/lib/libxenguest.so.4.0 #2 0x00007f1bc31a80f3 in ?? () from /usr/lib/libxenlight.so.1.0 #3 0x00007f1bc31a105e in ?? () from /usr/lib/libxenlight.so.1.0 #4 0x000000000049389a in libxlVmStart (driver=0x7f1bb8003e80, vm=0x7f1bb8053400, start_paused=false, restore_fd=20) at libxl/libxl_driver.c:531 #5 0x00000000004945c5 in libxlDomainRestore (conn=<value optimized out>, from=<value optimized out>) at libxl/libxl_driver.c:1745 #6 0x00007f1bc1f85a4b in virDomainRestore (conn=0x20a4d40, from=0x7f1bb804d260 "/root/tt.img") at libvirt.c:2330 #7 0x000000000042c5b8 in remoteDispatchDomainRestore (server=<value optimized out>, client=<value optimized out>, conn=0xaf0, hdr=<value optimized out>, rerr=0x42ba0e20, args=0xffffffff, ret=0x42ba0f00) at remote.c:2616 #8 0x0000000000431cb7 in remoteDispatchClientRequest (server=0x209fe90, client=0x20a4ad0, msg=0x20b1aa0) at dispatch.c:524 #9 0x000000000041dd53 in qemudWorker (data=0x20a8018) at libvirtd.c:1622 #10 0x00007f1bc1cb9fc7 in start_thread () from /lib/libpthread.so.0 #11 0x00007f1bc182b64d in clone () from /lib/libc.so.6 #12 0x0000000000000000 in ?? () Cheers, Markus --- src/libxl/libxl_conf.h | 14 +++ src/libxl/libxl_driver.c | 228 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 225 insertions(+), 17 deletions(-) diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 8c87786..e7dd3a1 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -1,5 +1,6 @@ /*---------------------------------------------------------------------------*/ /* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. + * Copyright (C) 2011 Univention GmbH. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -17,6 +18,7 @@ * * Authors: * Jim Fehlig <jfehlig(a)novell.com&gt; + * Markus Groß <gross(a)univention.de&gt; */ /*---------------------------------------------------------------------------*/ @@ -85,6 +87,18 @@ struct _libxlDomainObjPrivate { int eventHdl; }; +#define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" +#define LIBXL_SAVE_VERSION 1 + +typedef struct _libxlSavefileHeader libxlSavefileHeader; +typedef libxlSavefileHeader *libxlSavefileHeaderPtr; +struct _libxlSavefileHeader { + char magic[sizeof(LIBXL_SAVE_MAGIC)-1]; + uint32_t version; + uint32_t xmlLen; + /* 24 bytes used, pad up to 64 bytes */ + uint32_t unused[10]; +}; # define libxlError(code, ...) \ virReportErrorHelper(VIR_FROM_LIBXL, code, __FILE__, \ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 247d78e..56a62c9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -29,6 +29,7 @@ #include <sys/utsname.h> #include <math.h> #include <libxl.h> +#include <fcntl.h> #include "internal.h" #include "logging.h" @@ -60,11 +61,10 @@ static libxlDriverPrivatePtr libxl_driver = NULL; - /* Function declarations */ static int -libxlVmStart(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, bool start_paused); +libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, + bool start_paused, int restore_fd); /* Function definitions */ @@ -188,7 +188,7 @@ libxlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, virResetLastError(); if (vm->autostart && !virDomainObjIsActive(vm) && - libxlVmStart(driver, vm, false) < 0) { + libxlVmStart(driver, vm, false, -1) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, @@ -378,7 +378,7 @@ static void libxlEventHandler(int watch, break; case SHUTDOWN_reboot: libxlVmReap(driver, vm, 0); - libxlVmStart(driver, vm, 0); + libxlVmStart(driver, vm, 0, -1); break; default: VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason); @@ -504,8 +504,8 @@ cleanup: * virDomainObjPtr should be locked on invocation */ static int -libxlVmStart(libxlDriverPrivatePtr driver, - virDomainObjPtr vm, bool start_paused) +libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, + bool start_paused, int restore_fd) { libxl_domain_config d_config; virDomainDefPtr def = vm->def; @@ -524,12 +524,23 @@ libxlVmStart(libxlDriverPrivatePtr driver, //TODO: Balloon dom0 ?? //ret = freemem(&d_config->b_info, &d_config->dm_info); - ret = libxl_domain_create_new(&priv->ctx, &d_config, - NULL, &child_console_pid, &domid); + if (restore_fd < 0) + ret = libxl_domain_create_new(&priv->ctx, &d_config, + NULL, &child_console_pid, &domid); + else + ret = libxl_domain_create_restore(&priv->ctx, &d_config, NULL, + &child_console_pid, &domid, + restore_fd); + if (ret) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("libxenlight failed to create new domain '%s'"), - d_config.c_info.name); + if (restore_fd < 0) + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to create new domain '%s'"), + d_config.c_info.name); + else + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to restore domain '%s'"), + d_config.c_info.name); goto error; } @@ -562,7 +573,9 @@ libxlVmStart(libxlDriverPrivatePtr driver, goto error; event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_BOOTED); + restore_fd < 0 ? + VIR_DOMAIN_EVENT_STARTED_BOOTED : + VIR_DOMAIN_EVENT_STARTED_RESTORED); libxlDomainEventQueue(driver, event); libxl_domain_config_destroy(&d_config); @@ -1046,7 +1059,8 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; def = NULL; - if (libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0) < 0) { + if (libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, + -1) < 0) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL; goto cleanup; @@ -1566,6 +1580,186 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) } static int +libxlDomainSave(virDomainPtr dom, const char * to) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + libxlDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL; + libxlSavefileHeader hdr; + libxl_domain_suspend_info s_info; + char *xml; + uint32_t xml_len; + int fd; + int ret = -1; + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + libxlError(VIR_ERR_NO_DOMAIN, + _("No domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (vm->state != VIR_DOMAIN_PAUSED) { + memset(&s_info, 0, sizeof(s_info)); + + if ((fd = virFileOpenAs(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR, + getuid(), getgid(), 0)) < 0) { + virReportSystemError(-fd, + _("Failed to create domain save file '%s'"), + to); + goto cleanup; + } + + if ((xml = virDomainDefFormat(vm->def, 0)) == NULL) + goto cleanup; + xml_len = strlen(xml) + 1; + + memset(&hdr, 0, sizeof(hdr)); + memcpy(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic)); + hdr.version = LIBXL_SAVE_VERSION; + hdr.xmlLen = xml_len; + + if (safewrite(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("Failed to write save file header")); + goto cleanup; + } + + if (safewrite(fd, xml, xml_len) != xml_len) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("Failed to write xml description")); + goto cleanup; + } + + if (libxl_domain_suspend(&priv->ctx, &s_info, dom->id, fd) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to save domain '%d' with libxenlight"), + dom->id); + goto cleanup; + } + + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_SAVED); + + if (libxlVmReap(driver, vm, 1) != 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to destroy domain '%d'"), dom->id); + goto cleanup; + } + + if (!vm->persistent) { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + ret = 0; + } +cleanup: + VIR_FREE(xml); + if (VIR_CLOSE(fd) < 0) + virReportSystemError(errno, "%s", _("cannot close file")); + if (vm) + virDomainObjUnlock(vm); + if (event) + libxlDomainEventQueue(driver, event); + libxlDriverUnlock(driver); + return ret; +} + +static int +libxlDomainRestore(virConnectPtr conn, const char * from) +{ + libxlDriverPrivatePtr driver = conn->privateData; + virDomainDefPtr def = NULL; + virDomainObjPtr vm = NULL; + libxlSavefileHeader hdr; + char *xml = NULL; + int fd; + int ret = -1; + + libxlDriverLock(driver); + + if ((fd = virFileOpenAs(from, O_RDONLY, 0, getuid(), getgid(), 0)) < 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + "%s", _("cannot read domain image")); + goto cleanup; + } + + if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) { + libxlError(VIR_ERR_OPERATION_FAILED, + "%s", _("failed to read libxl header")); + goto cleanup; + } + + if (memcmp(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic))) { + libxlError(VIR_ERR_INVALID_ARG, "%s", _("image magic is incorrect")); + goto cleanup; + } + + if (hdr.version > LIBXL_SAVE_VERSION) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("image version is not supported (%d > %d)"), + hdr.version, LIBXL_SAVE_VERSION); + goto cleanup; + } + + if (hdr.xmlLen <= 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("invalid XML length: %d"), hdr.xmlLen); + goto cleanup; + } + + if (VIR_ALLOC_N(xml, hdr.xmlLen) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) { + libxlError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML")); + goto cleanup; + } + + if (!(def = virDomainDefParseString(driver->caps, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; + + if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) + goto cleanup; + + def = NULL; + + if ((ret = libxlVmStart(driver, vm, false, fd)) < 0 && + !vm->persistent) { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + +cleanup: + VIR_FREE(xml); + virDomainDefFree(def); + if (VIR_CLOSE(fd) < 0) + virReportSystemError(errno, "%s", _("cannot close file")); + if (vm) + virDomainObjUnlock(vm); + libxlDriverUnlock(driver); + return ret; +} + +static int libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { @@ -2036,7 +2230,7 @@ libxlDomainCreateWithFlags(virDomainPtr dom, goto cleanup; } - ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0); + ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1); cleanup: if (vm) @@ -2672,8 +2866,8 @@ static virDriver libxlDriver = { NULL, /* domainSetBlkioParameters */ NULL, /* domainGetBlkioParameters */ libxlDomainGetInfo, /* domainGetInfo */ - NULL, /* domainSave */ - NULL, /* domainRestore */ + libxlDomainSave, /* domainSave */ + libxlDomainRestore, /* domainRestore */ NULL, /* domainCoreDump */ libxlDomainSetVcpus, /* domainSetVcpus */ libxlDomainSetVcpusFlags, /* domainSetVcpusFlags */ -- 1.7.1 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Markus Groß wrote: > Am Montag 23 Mai 2011 04:30:12 schrieb Jim Fehlig: > >> Jim Fehlig wrote: >> >>> Markus Groß wrote: >>> >>> >>>> This patch adds save/restore functionality to the libxl driver. >>>> >>>> It is a v2 of this patch: >>>> https://www.redhat.com/archives/libvir-list/2011-April/msg00338.html >>>> >>>> v2: >>>> * header is now padded and has a version field >>>> * the correct restore function from libxl is used >>>> * only create the restore event once in libxlVmStart >>>> >>>> >>>> >>> Hi Markus, >>> >>> Finally found time to try your patch. Thanks for the patience :-). >>> >>> >>> >>>> However I ran into a reproducible segfault. >>>> Assume you saved a vm with: >>>> # virsh save domU foo.img >>>> >>>> >>>> >>> I think the problem actually lies in the save function. The domain does >>> not appear to be cleaned up properly. From xl's perspective after virsh >>> save domU foo.img >>> >>> xen33: # xl list >>> Name ID Mem VCPUs State >>> Time(s) >>> Domain-0 0 2023 8 >>> r----- 330.0 >>> (null) 11 1 >>> 2 --pssd 27.1 >>> >>> The orphaned domain disappears after libvirtd restart. >>> >> I manged to track down this problem, patch posted to xen-devel >> >> http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01314.html >> >> > > Great! I attached the current version of the save/restore patch. > It is rebased against the current master. > Ok, thanks, I have some comments below. > >>> >>> >>>> If you restore the save image, >>>> destroy the vm and restore it again, a segfault occurs: >>>> # virsh restore foo.img >>>> # virsh destroy domU >>>> # virsh restore foo.img >>>> # segfault >>>> >>>> >> But I still see the segfault, in addition to domain not booting and >> consuming 100% cpu on first restore :-(. I'll look at these issues next. >> I've identified cause of both of these issues. For the domain not _restoring_ (booting doesn't make much sense on a restore) and consuming 100% cpu, I've posted this patch (which still needs some work) https://www.redhat.com/archives/libvir-list/2011-May/msg01450.html The segfault ended up being a bug in libxc. I've posted a fix to xen-devel http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01511.html > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h > index 65110cf..e75e418 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -1,5 +1,6 @@ > > /*---------------------------------------------------------------------------*/ > /* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. > + * Copyright (C) 2011 Univention GmbH. > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -17,6 +18,7 @@ > * > * Authors: > * Jim Fehlig <jfehlig(a)novell.com&gt; > + * Markus Groß <gross(a)univention.de&gt; > */ > > /*---------------------------------------------------------------------------*/ > > @@ -81,6 +83,18 @@ struct _libxlDomainObjPrivate { > int eventHdl; > }; > > +#define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" > +#define LIBXL_SAVE_VERSION 1 > + > +typedef struct _libxlSavefileHeader libxlSavefileHeader; > +typedef libxlSavefileHeader *libxlSavefileHeaderPtr; > +struct _libxlSavefileHeader { > + char magic[sizeof(LIBXL_SAVE_MAGIC)-1]; > + uint32_t version; > + uint32_t xmlLen; > + /* 24 bytes used, pad up to 64 bytes */ > + uint32_t unused[10]; > +}; > > # define libxlError(code, ...) \ > virReportErrorHelper(VIR_FROM_LIBXL, code, __FILE__, \ > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index b2cc0e8..9bc71fd 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -29,6 +29,7 @@ > #include <sys/utsname.h> > #include <math.h> > #include <libxl.h> > +#include <fcntl.h> > > #include "internal.h" > #include "logging.h" > @@ -60,11 +61,10 @@ > > static libxlDriverPrivatePtr libxl_driver = NULL; > > - > /* Function declarations */ > static int > -libxlVmStart(libxlDriverPrivatePtr driver, > - virDomainObjPtr vm, bool start_paused); > +libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, > + bool start_paused, int restore_fd); > > > /* Function definitions */ > @@ -168,7 +168,7 @@ libxlAutostartDomain(void *payload, const void > *name ATTRIBUTE_UNUSED, > virResetLastError(); > > if (vm->autostart && !virDomainObjIsActive(vm) && > - libxlVmStart(driver, vm, false) < 0) { > + libxlVmStart(driver, vm, false, -1) < 0) { > err = virGetLastError(); > VIR_ERROR(_("Failed to autostart VM '%s': %s"), > vm->def->name, > @@ -369,7 +369,7 @@ static void libxlEventHandler(int watch, > break; > case SHUTDOWN_reboot: > libxlVmReap(driver, vm, 0, VIR_DOMAIN_SHUTOFF_SHUTDOWN); > - libxlVmStart(driver, vm, 0); > + libxlVmStart(driver, vm, 0, -1); > break; > You'll need to add a 'case SHUTDOWN_suspend' here and just call libxlVmCleanup(). > default: > VIR_INFO("Unhandled shutdown_reason %d", > info.shutdown_reason); > @@ -535,8 +535,8 @@ libxlFreeMem(libxlDomainObjPrivatePtr priv, > libxl_domain_config *d_config) > * virDomainObjPtr should be locked on invocation > */ > static int > -libxlVmStart(libxlDriverPrivatePtr driver, > - virDomainObjPtr vm, bool start_paused) > +libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, > + bool start_paused, int restore_fd) > { > libxl_domain_config d_config; > virDomainDefPtr def = vm->def; > @@ -559,12 +559,23 @@ libxlVmStart(libxlDriverPrivatePtr driver, > goto error; > } > > - ret = libxl_domain_create_new(&priv->ctx, &d_config, > - NULL, &child_console_pid, &domid); > + if (restore_fd < 0) > + ret = libxl_domain_create_new(&priv->ctx, &d_config, > + NULL, &child_console_pid, &domid); > + else > + ret = libxl_domain_create_restore(&priv->ctx, &d_config, NULL, > + &child_console_pid, &domid, > + restore_fd); > + > if (ret) { > - libxlError(VIR_ERR_INTERNAL_ERROR, > - _("libxenlight failed to create new domain '%s'"), > - d_config.c_info.name); > + if (restore_fd < 0) > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("libxenlight failed to create new domain '%s'"), > + d_config.c_info.name); > + else > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("libxenlight failed to restore domain '%s'"), > + d_config.c_info.name); > goto error; > } > > @@ -597,7 +608,9 @@ libxlVmStart(libxlDriverPrivatePtr driver, > goto error; > > event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, > - VIR_DOMAIN_EVENT_STARTED_BOOTED); > + restore_fd < 0 ? > + VIR_DOMAIN_EVENT_STARTED_BOOTED : > + > VIR_DOMAIN_EVENT_STARTED_RESTORED); > libxlDomainEventQueue(driver, event); > > libxl_domain_config_destroy(&d_config); > @@ -1075,7 +1088,8 @@ libxlDomainCreateXML(virConnectPtr conn, > const char *xml, > goto cleanup; > def = NULL; > > - if (libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) > != 0) < 0) { > + if (libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, > + -1) < 0) { > virDomainRemoveInactive(&driver->domains, vm); > vm = NULL; > goto cleanup; > @@ -1627,6 +1641,184 @@ libxlDomainGetState(virDomainPtr dom, > } > > static int > +libxlDomainSave(virDomainPtr dom, const char *to) > +{ > + libxlDriverPrivatePtr driver = dom->conn->privateData; > + virDomainObjPtr vm; > + libxlDomainObjPrivatePtr priv; > + virDomainEventPtr event = NULL; > + libxlSavefileHeader hdr; > + char *xml; > + uint32_t xml_len; > + int fd; > + int ret = -1; > + > + libxlDriverLock(driver); > + vm = virDomainFindByUUID(&driver->domains, dom->uuid); > + > + if (!vm) { > + char uuidstr[VIR_UUID_STRING_BUFLEN]; > + virUUIDFormat(dom->uuid, uuidstr); > + libxlError(VIR_ERR_NO_DOMAIN, > + _("No domain with matching uuid '%s'"), uuidstr); > + goto cleanup; > + } > + > + if (!virDomainObjIsActive(vm)) { > + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is > not running")); > + goto cleanup; > + } > + > + priv = vm->privateData; > + > + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { > + > + if ((fd = virFileOpenAs(to, O_CREAT|O_TRUNC|O_WRONLY, > S_IRUSR|S_IWUSR, > + getuid(), getgid(), 0)) < 0) { > + virReportSystemError(-fd, > + _("Failed to create domain save > file '%s'"), > + to); > + goto cleanup; > + } > + > + if ((xml = virDomainDefFormat(vm->def, 0)) == NULL) > + goto cleanup; > + xml_len = strlen(xml) + 1; > + > + memset(&hdr, 0, sizeof(hdr)); > + memcpy(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic)); > + hdr.version = LIBXL_SAVE_VERSION; > + hdr.xmlLen = xml_len; > + > + if (safewrite(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + _("Failed to write save file header")); > + goto cleanup; > + } > + > + if (safewrite(fd, xml, xml_len) != xml_len) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + _("Failed to write xml description")); > + goto cleanup; > + } > + > + if (libxl_domain_suspend(&priv->ctx, NULL, dom->id, fd) != 0) { > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("Failed to save domain '%d' with libxenlight"), > + dom->id); > + goto cleanup; > + } > + > + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, > + VIR_DOMAIN_EVENT_STOPPED_SAVED); > + > + if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("Failed to destroy domain '%d'"), dom->id); > + goto cleanup; > + } > And here call libxl_domain_destroy() directly instead of libxlVmReap(), allowing the event handler to cleanup the vm. Can you make these changes and ensure all the save/restore issues are resolved in your environment? Regards, Jim

> + > + if (!vm->persistent) { > + virDomainRemoveInactive(&driver->domains, vm); > + vm = NULL; > + } > + ret = 0; > + } > +cleanup: > + VIR_FREE(xml); > + if (VIR_CLOSE(fd) < 0) > + virReportSystemError(errno, "%s", _("cannot close file")); > + if (vm) > + virDomainObjUnlock(vm); > + if (event) > + libxlDomainEventQueue(driver, event); > + libxlDriverUnlock(driver); > + return ret; > +} > + > +static int > +libxlDomainRestore(virConnectPtr conn, const char *from) > +{ > + libxlDriverPrivatePtr driver = conn->privateData; > + virDomainDefPtr def = NULL; > + virDomainObjPtr vm = NULL; > + libxlSavefileHeader hdr; > + char *xml = NULL; > + int fd; > + int ret = -1; > + > + libxlDriverLock(driver); > + > + if ((fd = virFileOpenAs(from, O_RDONLY, 0, getuid(), getgid(), > 0)) < 0) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + "%s", _("cannot read domain image")); > + goto cleanup; > + } > + > + if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + "%s", _("failed to read libxl header")); > + goto cleanup; > + } > + > + if (memcmp(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic))) { > + libxlError(VIR_ERR_INVALID_ARG, "%s", _("image magic is > incorrect")); > + goto cleanup; > + } > + > + if (hdr.version > LIBXL_SAVE_VERSION) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + _("image version is not supported (%d > %d)"), > + hdr.version, LIBXL_SAVE_VERSION); > + goto cleanup; > + } > + > + if (hdr.xmlLen <= 0) { > + libxlError(VIR_ERR_OPERATION_FAILED, > + _("invalid XML length: %d"), hdr.xmlLen); > + goto cleanup; > + } > + > + if (VIR_ALLOC_N(xml, hdr.xmlLen) < 0) { > + virReportOOMError(); > + goto cleanup; > + } > + > + if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) { > + libxlError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to > read XML")); > + goto cleanup; > + } > + > + if (!(def = virDomainDefParseString(driver->caps, xml, > + VIR_DOMAIN_XML_INACTIVE))) > + goto cleanup; > + > + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) > + goto cleanup; > + > + if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, > def, true))) > + goto cleanup; > + > + def = NULL; > + > + if ((ret = libxlVmStart(driver, vm, false, fd)) < 0 && > + !vm->persistent) { > + virDomainRemoveInactive(&driver->domains, vm); > + vm = NULL; > + } > + > +cleanup: > + VIR_FREE(xml); > + virDomainDefFree(def); > + if (VIR_CLOSE(fd) < 0) > + virReportSystemError(errno, "%s", _("cannot close file")); > + if (vm) > + virDomainObjUnlock(vm); > + libxlDriverUnlock(driver); > + return ret; > +} > + > +static int > libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, > unsigned int flags) > { > @@ -2096,7 +2288,7 @@ libxlDomainCreateWithFlags(virDomainPtr dom, > goto cleanup; > } > > - ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0); > + ret = libxlVmStart(driver, vm, (flags & > VIR_DOMAIN_START_PAUSED) != 0, -1); > > cleanup: > if (vm) > @@ -2722,6 +2914,8 @@ static virDriver libxlDriver = { > .domainSetMemoryFlags = libxlDomainSetMemoryFlags, /* 0.9.0 */ > .domainGetInfo = libxlDomainGetInfo, /* 0.9.0 */ > .domainGetState = libxlDomainGetState, /* 0.9.2 */ > + .domainSave = libxlDomainSave, /* 0.9.2 */ > + .domainRestore = libxlDomainRestore, /* 0.9.2 */ > .domainSetVcpus = libxlDomainSetVcpus, /* 0.9.0 */ > .domainSetVcpusFlags = libxlDomainSetVcpusFlags, /* 0.9.0 */ > .domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */ >

Markus Groß wrote: > Am Dienstag 24 Mai 2011 06:06:08 schrieb Jim Fehlig: > >>> + >>> + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, >>> + VIR_DOMAIN_EVENT_STOPPED_SAVED); >>> + >>> + if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { >>> + libxlError(VIR_ERR_INTERNAL_ERROR, >>> + _("Failed to destroy domain '%d'"), dom->id); >>> + goto cleanup; >>> + } >>> >>> >> And here call libxl_domain_destroy() directly instead of libxlVmReap(), >> allowing the event handler to cleanup the vm. >> >> Can you make these changes and ensure all the save/restore issues are >> resolved in your environment? >> > > I made these changes but this leads to another issue. > When using the event handler to cleanup the domain > there seems to be a race condition. > The call to libxl_event_get_domain_death_info returns a value != 1 > That should be fine though. libxl_event_get_domain_death_info() will return 1 if the event is for the specified domid. If it returns a value != 1, we goto cleanup and keep listening. > sometimes and the domain gets not cleaned up. > Did you use the force flag when invoking libxl_event_get_domain_death_info()? I've noticed that no events are generated when using that flag.

> The latest version of the patch I posted in this thread earlier does > not have this issue as it cleans up the domain directly. > How about this adjustment to your latest version? I can't find any issues using this change, and it follows the same pattern used in xl client.

--- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -378,6 +378,9 @@ static void libxlEventHandler(int watch, libxlVmReap(driver, vm, 0, VIR_DOMAIN_SHUTOFF_SHUTDOWN); libxlVmStart(driver, vm, 0, -1); break; + case SHUTDOWN_suspend: + libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED); + break; default: VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason); break; @@ -1719,7 +1722,7 @@ libxlDomainSave(virDomainPtr dom, const char *to) event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); - if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { + if (libxl_domain_destroy(&priv->ctx, dom->id, 0) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain '%d'"), dom->id); goto cleanup;