[libvirt] [PATCH RFC] Add domainSave/Restore to libxl driver
Jim Fehlig
jfehlig at novell.com
Tue May 3 03:18:40 UTC 2011
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 tinkered with
the cleanup code path with no success, but wanted to let you know my
findings before knocking off for the night. I'll continue poking around
tomorrow.
Cheers,
Jim
> 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 at novell.com>
> + * Markus Groß <gross at univention.de>
> */
> /*---------------------------------------------------------------------------*/
>
> @@ -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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list