Hi All, Here''s a first cut of libxenlight driver for libvirt. The driver is stateful and provides functionality for managed (persistent) domains. The driver only maintains state for and manages domains under its control, ignoring domains created by other libxenlight clients, e.g. xl. As you can see, a fair number of the driver functions are implemented, at least what I think constitutes a minimal set for upstream inclusion. Are there any required functions missing? I''m still looking into a problem with failure to restart a domain, persistent or otherwise. E.g. virsh start|create domU; virsh shutdown domU; virsh start|create domU. The second start/create fails in libxenlight, but I''ve not yet tracked down the cause. Also, I''ve not yet handled restart of libvirtd if domains are running - not sure if this is a requirement for initial inclusion. TIA for your review and comment. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, 14 Feb 2011, Jim Fehlig wrote:> Hi All, > > Here''s a first cut of libxenlight driver for libvirt. The driver is > stateful and provides functionality for managed (persistent) domains. > The driver only maintains state for and manages domains under its > control, ignoring domains created by other libxenlight clients, e.g. xl. > > As you can see, a fair number of the driver functions are implemented, > at least what I think constitutes a minimal set for upstream inclusion. > Are there any required functions missing? > > I''m still looking into a problem with failure to restart a domain, > persistent or otherwise. E.g. virsh start|create domU; virsh shutdown > domU; virsh start|create domU. The second start/create fails in > libxenlight, but I''ve not yet tracked down the cause. Also, I''ve not > yet handled restart of libvirtd if domains are running - not sure if > this is a requirement for initial inclusion. > > TIA for your review and comment. > > Regards, > Jim > > > Add a new xen driver based on libxenlight [1], which is the primary > toolstack starting with Xen 4.1.0. The driver is stateful, runs > privileged only, and is accessed with libxl:/// URI. >---8<---> +static int > +libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) > +{ > + virDomainDiskDefPtr *l_disks = def->disks; > + int ndisks = def->ndisks; > + libxl_device_disk *x_disks; > + int i; > + > + if (VIR_ALLOC_N(x_disks, ndisks) < 0) { > + virReportOOMError(); > + return -1; > + } > + > + for (i = 0; i < ndisks; i++) { > + if (l_disks[i]->src && > + (x_disks[i].physpath = strdup(l_disks[i]->src)) == NULL) { > + virReportOOMError(); > + goto error; > + } > + > + if (l_disks[i]->dst && > + (x_disks[i].virtpath = strdup(l_disks[i]->dst)) == NULL) { > + virReportOOMError(); > + goto error; > + } > + > + if (l_disks[i]->driverName) { > + if (STREQ(l_disks[i]->driverName, "tap") || > + STREQ(l_disks[i]->driverName, "tap2")) { > + if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "qcow2")) > + x_disks[i].phystype = PHYSTYPE_QCOW2; > + else if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "aio")) > + x_disks[i].phystype = PHYSTYPE_AIO; > + else if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "vhd")) > + x_disks[i].phystype = PHYSTYPE_VHD; > + } else if (STREQ(l_disks[i]->driverName, "file")) { > + x_disks[i].phystype = PHYSTYPE_FILE; > + } else if (STREQ(l_disks[i]->driverName, "phy")) { > + x_disks[i].phystype = PHYSTYPE_PHY; > + } > + } else { > + /* Default to file?? */ > + x_disks[i].phystype = PHYSTYPE_FILE; > + }Just an heads up on disk parsing: we are about to change the disk API, before the xen 4.1 release. That''s because we realized that the current API entangles the disk backend and the disk format together; so we are going to separate them, using two separate fields: backend and format. An change in the xl disk config parser will also follow, that should help you make the corresponding changes to this function. Everything else on the libxenlight side looks good! _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Here''s the latest version of a libxenlight driver for libvirt. I''ve added a per-domain libxl_ctx in addition to the driver wide context. The former is stored in virDomainObject privateData and used for operations on the domain. The latter is stored in driver private data and is used for non-domain related libxl calls, e.g. getVersion, getNodeInfo, etc. This approach was suggested by Ian Jackson and Stefano Stabellini and appears to be working much better than a single, driver wide libxl_ctx. I no longer have the restart issues described in the first patch posting [1]. Your review and comments are much appreciated! Thanks, Jim [1] https://www.redhat.com/archives/libvir-list/2011-February/msg00409.html _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 18 Feb 2011, Jim Fehlig wrote:> +static int > +libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) > +{ > + virDomainDiskDefPtr *l_disks = def->disks; > + int ndisks = def->ndisks; > + libxl_device_disk *x_disks; > + int i; > + > + if (VIR_ALLOC_N(x_disks, ndisks) < 0) { > + virReportOOMError(); > + return -1; > + } > + > + for (i = 0; i < ndisks; i++) { > + if (l_disks[i]->src && > + (x_disks[i].physpath = strdup(l_disks[i]->src)) == NULL) { > + virReportOOMError(); > + goto error; > + } > + > + if (l_disks[i]->dst && > + (x_disks[i].virtpath = strdup(l_disks[i]->dst)) == NULL) { > + virReportOOMError(); > + goto error; > + } > + > + if (l_disks[i]->driverName) { > + if (STREQ(l_disks[i]->driverName, "tap") || > + STREQ(l_disks[i]->driverName, "tap2")) { > + if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "qcow2")) > + x_disks[i].phystype = PHYSTYPE_QCOW2; > + else if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "aio")) > + x_disks[i].phystype = PHYSTYPE_AIO; > + else if (l_disks[i]->driverType && > + STREQ(l_disks[i]->driverType, "vhd")) > + x_disks[i].phystype = PHYSTYPE_VHD; > + } else if (STREQ(l_disks[i]->driverName, "file")) { > + x_disks[i].phystype = PHYSTYPE_FILE; > + } else if (STREQ(l_disks[i]->driverName, "phy")) { > + x_disks[i].phystype = PHYSTYPE_PHY; > + } > + } else { > + /* Default to file?? */ > + x_disks[i].phystype = PHYSTYPE_FILE; > + }few days ago the patch that removes phystype and introduces backend and format has been applied, so this code needs an update ...> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > new file mode 100644 > index 0000000..d581e7b > --- /dev/null > +++ b/src/libxl/libxl_driver.c > @@ -0,0 +1,1280 @@ > +/*---------------------------------------------------------------------------*/ > +/* 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 > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + */ > +/*---------------------------------------------------------------------------*/ > + > +#include <config.h> > + > +#include <sys/utsname.h> > +#include <libxl.h> > + > +#include "internal.h" > +#include "logging.h" > +#include "virterror_internal.h" > +#include "datatypes.h" > +#include "files.h" > +#include "memory.h" > +#include "event.h" > +#include "uuid.h" > +#include "libxl_driver.h" > +#include "libxl_conf.h" > + > + > +#define VIR_FROM_THIS VIR_FROM_LIBXL > + > +#define LIBXL_DOM_REQ_POWEROFF 0 > +#define LIBXL_DOM_REQ_REBOOT 1 > +#define LIBXL_DOM_REQ_SUSPEND 2 > +#define LIBXL_DOM_REQ_CRASH 3 > +#define LIBXL_DOM_REQ_HALT 4 > + > +static libxlDriverPrivatePtr libxl_driver = NULL; > + > + > +/* Function declarations */ > +static int > +libxlVmStart(virDomainObjPtr vm, bool start_paused); > + > + > +/* Function definitions */ > +static void > +libxlDriverLock(libxlDriverPrivatePtr driver) > +{ > + virMutexLock(&driver->lock); > +} > + > +static void > +libxlDriverUnlock(libxlDriverPrivatePtr driver) > +{ > + virMutexUnlock(&driver->lock); > +} > + > +static void *libxlDomainObjPrivateAlloc(void) > +{ > + libxlDomainObjPrivatePtr priv; > + > + if (VIR_ALLOC(priv) < 0) > + return NULL; > + > + libxl_ctx_init(&priv->ctx, LIBXL_VERSION, libxl_driver->logger); > + priv->waiterFD = -1; > + priv->eventHdl = -1; > + > + return priv; > +} > + > +static void libxlDomainObjPrivateFree(void *data) > +{ > + libxlDomainObjPrivatePtr priv = data; > + > + if (priv->dWaiter) { > + libxl_free_waiter(priv->dWaiter); > + VIR_FREE(priv->dWaiter); > + } > + libxl_ctx_free(&priv->ctx); > + VIR_FREE(priv); > +} > + > +static void libxlEventHandler(int watch, > + int fd, > + int events, > + void *data) > +{ > + libxlDriverPrivatePtr driver = libxl_driver; > + virDomainObjPtr vm = data; > + libxlDomainObjPrivatePtr priv; > + libxl_event event; > + libxl_dominfo info; > + > + libxlDriverLock(driver); > + virDomainObjLock(vm); > + libxlDriverUnlock(driver); > + > + priv = vm->privateData; > + > + memset(&event, 0, sizeof(event)); > + memset(&info, 0, sizeof(info)); > + > + if (priv->waiterFD != fd || priv->eventHdl != watch) { > + virEventRemoveHandle(watch); > + goto cleanup; > + } > + > + if (!(events & VIR_EVENT_HANDLE_READABLE)) { > + goto cleanup; > + } > + > + if (libxl_get_event(&priv->ctx, &event)) { > + goto cleanup; > + } > + > + if (event.type == LIBXL_EVENT_DOMAIN_DEATH) { > + /* libxl_event_get_domain_death_info returns 1 if death > + * event was for this domid */ > + if (libxl_event_get_domain_death_info(&priv->ctx, > + vm->def->id, > + &event, > + &info) != 1) { > + goto cleanup; > + } > + > + virEventRemoveHandle(watch); > + if (info.shutdown_reason == SHUTDOWN_poweroff) { > + libxl_domain_destroy(&priv->ctx, vm->def->id, 0); > + if (vm->persistent) { > + vm->def->id = -1; > + vm->state = VIR_DOMAIN_SHUTOFF; > + } else { > + libxl_free_waiter(priv->dWaiter); > + VIR_FREE(priv->dWaiter); > + virDomainRemoveInactive(&driver->domains, vm); > + vm = NULL; > + } > + } else if (info.shutdown_reason == SHUTDOWN_reboot) { > + libxl_domain_destroy(&priv->ctx, vm->def->id, 0); > + vm->def->id = -1; > + vm->state = VIR_DOMAIN_SHUTOFF; > + sleep(1); > + libxlVmStart(vm, 0);we need to handle at least SHUTDOWN_suspend and SHUTDOWN_crash too> + } else { > + VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason); > + } > + }we should call libxl_free_event before returning, otherwise we leak two strings> + > +cleanup: > + if (vm) > + virDomainObjUnlock(vm); > +} > + > +static int > +libxlCreateDomEvents(virDomainObjPtr vm) > +{ > + libxlDomainObjPrivatePtr priv = vm->privateData; > + int fd; > + > + /* Do we already have a waiter for this domain? */ > + if (priv->dWaiter == NULL) { > + if (VIR_ALLOC(priv->dWaiter) < 0) { > + virReportOOMError(); > + return -1; > + } > + > + if (libxl_wait_for_domain_death(&priv->ctx, vm->def->id, priv->dWaiter)) { > + goto error; > + } > + } > + > + libxl_get_wait_fd(&priv->ctx, &fd); > + if (fd < 0) { > + goto error; > + } > + > + priv->waiterFD = fd; > + if ((priv->eventHdl = virEventAddHandle( > + fd, > + VIR_EVENT_HANDLE_READABLE | VIR_EVENT_HANDLE_ERROR, > + libxlEventHandler, > + vm, NULL)) < 0) { > + goto error; > + } > + > + return 0; > + > +error: > + libxl_free_waiter(priv->dWaiter); > + VIR_FREE(priv->dWaiter); > + return -1; > +} > + > +static int > +libxlVmStart(virDomainObjPtr vm, bool start_paused) > +{ > + libxl_domain_config d_config; > + virDomainDefPtr def = vm->def; > + int ret; > + uint32_t domid = 0; > + char *dom_xml = NULL; > + pid_t child_console_pid = -1; > + libxlDomainObjPrivatePtr priv = vm->privateData; > + > + memset(&d_config, 0, sizeof(d_config)); > + > + if (libxlBuildDomainConfig(def, &d_config) < 0 ) > + return -1; > + > + //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 (ret) { > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("libxenlight failed to create new domain ''%s''"), > + d_config.c_info.name); > + goto error; > + } > + > + def->id = domid; > + if ((dom_xml = virDomainDefFormat(def, 0)) == NULL) > + goto error; > + > + if(libxl_userdata_store(&priv->ctx, domid, "libvirt-xml", > + (uint8_t *)dom_xml, strlen(dom_xml) + 1)) { > + libxlError(VIR_ERR_INTERNAL_ERROR, > + _("libxenlight failed to store userdata")); > + goto error; > + } > + > + if (libxlCreateDomEvents(vm) < 0) > + goto error; > + > + if (!start_paused) { > + libxl_domain_unpause(&priv->ctx, domid); > + vm->state = VIR_DOMAIN_RUNNING; > + } else { > + vm->state = VIR_DOMAIN_PAUSED; > + } > + > + libxl_domain_config_destroy(&d_config); > + VIR_FREE(dom_xml); > + return 0; > + > +error: > + if (domid > 0) { > + libxl_domain_destroy(&priv->ctx, domid, 0); > + def->id = -1; > + } > + libxl_domain_config_destroy(&d_config); > + VIR_FREE(dom_xml); > + return -1; > +} > + > +static int > +libxlShutdown(void) > +{ > + if (!libxl_driver) > + return -1; > + > + libxlDriverLock(libxl_driver); > + virCapabilitiesFree(libxl_driver->caps); > + libxl_ctx_free(&libxl_driver->ctx); > + xtl_logger_destroy(libxl_driver->logger); > + if (libxl_driver->logger_file) > + VIR_FORCE_FCLOSE(libxl_driver->logger_file); > + > + virDomainObjListDeinit(&libxl_driver->domains); > + > + VIR_FREE(libxl_driver->configDir); > + VIR_FREE(libxl_driver->autostartDir); > + VIR_FREE(libxl_driver->logDir); > + VIR_FREE(libxl_driver->stateDir); > + VIR_FREE(libxl_driver->libDir); > + VIR_FREE(libxl_driver->saveDir); > + > + libxlDriverUnlock(libxl_driver); > + virMutexDestroy(&libxl_driver->lock); > + VIR_FREE(libxl_driver); > + > + return 0; > +} > + > +static int > +libxlStartup(int privileged) { > + const libxl_version_info *ver_info; > + char *log_file = NULL; > + > + /* Check that the user is root, silently disable if not */ > + if (!privileged) { > + VIR_INFO0("Not running privileged, disabling libxenlight driver"); > + return 0; > + } > + > + if (VIR_ALLOC(libxl_driver) < 0) > + return -1; > + > + if (virMutexInit(&libxl_driver->lock) < 0) { > + VIR_ERROR0(_("cannot initialize mutex")); > + VIR_FREE(libxl_driver); > + return -1; > + } > + libxlDriverLock(libxl_driver); > + > + if (virDomainObjListInit(&libxl_driver->domains) < 0) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->configDir, > + "%s", LIBXL_CONFIG_DIR) == -1) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->autostartDir, > + "%s", LIBXL_AUTOSTART_DIR) == -1) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->logDir, > + "%s", LIBXL_LOG_DIR) == -1) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->stateDir, > + "%s", LIBXL_STATE_DIR) == -1) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->libDir, > + "%s", LIBXL_LIB_DIR) == -1) > + goto out_of_memory; > + > + if (virAsprintf(&libxl_driver->saveDir, > + "%s", LIBXL_SAVE_DIR) == -1) > + goto out_of_memory; > + > + if (virFileMakePath(libxl_driver->logDir) != 0) { > + char ebuf[1024]; > + VIR_ERROR(_("Failed to create log dir ''%s'': %s"), > + libxl_driver->logDir, virStrerror(errno, ebuf, sizeof ebuf)); > + goto error; > + } > + if (virFileMakePath(libxl_driver->stateDir) != 0) { > + char ebuf[1024]; > + VIR_ERROR(_("Failed to create state dir ''%s'': %s"), > + libxl_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf)); > + goto error; > + } > + if (virFileMakePath(libxl_driver->libDir) != 0) { > + char ebuf[1024]; > + VIR_ERROR(_("Failed to create lib dir ''%s'': %s"), > + libxl_driver->libDir, virStrerror(errno, ebuf, sizeof ebuf)); > + goto error; > + } > + if (virFileMakePath(libxl_driver->saveDir) != 0) { > + char ebuf[1024]; > + VIR_ERROR(_("Failed to create save dir ''%s'': %s"), > + libxl_driver->saveDir, virStrerror(errno, ebuf, sizeof ebuf)); > + goto error; > + } > + > + if (virAsprintf(&log_file, "%s/libxl.log", libxl_driver->logDir) < 0) { > + goto out_of_memory; > + } > + > + if ((libxl_driver->logger_file = fopen(log_file, "a")) == NULL) { > + virReportSystemError(errno, > + _("failed to create logfile %s"), > + log_file); > + goto error; > + } > + VIR_FREE(log_file); > + > + libxl_driver->logger > + (xentoollog_logger*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0); > + if (!libxl_driver->logger) { > + VIR_ERROR0(_("cannot create logger for libxenlight")); > + goto error; > + } > + > + if (libxl_ctx_init(&libxl_driver->ctx, > + LIBXL_VERSION, > + libxl_driver->logger)) { > + VIR_ERROR0(_("cannot initialize libxenlight context")); > + goto error; > + } > + > + if ((ver_info = libxl_get_version_info(&libxl_driver->ctx)) == NULL) { > + VIR_ERROR0(_("cannot version information from libxenlight")); > + goto error; > + } > + libxl_driver->version = (ver_info->xen_version_major * 1000000) + > + (ver_info->xen_version_minor * 1000); > + > + if ((libxl_driver->caps > + libxlMakeCapabilities(&libxl_driver->ctx)) == NULL) { > + VIR_ERROR0(_("cannot create capabilities for libxenlight")); > + goto error; > + } > + > + libxl_driver->caps->privateDataAllocFunc = libxlDomainObjPrivateAlloc; > + libxl_driver->caps->privateDataFreeFunc = libxlDomainObjPrivateFree;If I understand correctly privateDataAllocFunc is called at each domain creation _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel P. Berrange
2011-Feb-21 17:15 UTC
[Xen-devel] Re: [libvirt] [PATCH] libxenlight driver
On Thu, Feb 17, 2011 at 06:28:36PM -0700, Jim Fehlig wrote:> Here''s the latest version of a libxenlight driver for libvirt. > > I''ve added a per-domain libxl_ctx in addition to the driver wide > context. The former is stored in virDomainObject privateData and used > for operations on the domain. The latter is stored in driver private > data and is used for non-domain related libxl calls, e.g. getVersion, > getNodeInfo, etc. This approach was suggested by Ian Jackson and > Stefano Stabellini and appears to be working much better than a single, > driver wide libxl_ctx. I no longer have the restart issues described in > the first patch posting [1]. > > Your review and comments are much appreciated!I''ve not had a chance todo a proper review yet, but after skimming the patch I think your impl / architecture looks pretty good. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini wrote:> On Fri, 18 Feb 2011, Jim Fehlig wrote: > >> +static int >> +libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) >> +{ >> + virDomainDiskDefPtr *l_disks = def->disks; >> + int ndisks = def->ndisks; >> + libxl_device_disk *x_disks; >> + int i; >> + >> + if (VIR_ALLOC_N(x_disks, ndisks) < 0) { >> + virReportOOMError(); >> + return -1; >> + } >> + >> + for (i = 0; i < ndisks; i++) { >> + if (l_disks[i]->src && >> + (x_disks[i].physpath = strdup(l_disks[i]->src)) == NULL) { >> + virReportOOMError(); >> + goto error; >> + } >> + >> + if (l_disks[i]->dst && >> + (x_disks[i].virtpath = strdup(l_disks[i]->dst)) == NULL) { >> + virReportOOMError(); >> + goto error; >> + } >> + >> + if (l_disks[i]->driverName) { >> + if (STREQ(l_disks[i]->driverName, "tap") || >> + STREQ(l_disks[i]->driverName, "tap2")) { >> + if (l_disks[i]->driverType && >> + STREQ(l_disks[i]->driverType, "qcow2")) >> + x_disks[i].phystype = PHYSTYPE_QCOW2; >> + else if (l_disks[i]->driverType && >> + STREQ(l_disks[i]->driverType, "aio")) >> + x_disks[i].phystype = PHYSTYPE_AIO; >> + else if (l_disks[i]->driverType && >> + STREQ(l_disks[i]->driverType, "vhd")) >> + x_disks[i].phystype = PHYSTYPE_VHD; >> + } else if (STREQ(l_disks[i]->driverName, "file")) { >> + x_disks[i].phystype = PHYSTYPE_FILE; >> + } else if (STREQ(l_disks[i]->driverName, "phy")) { >> + x_disks[i].phystype = PHYSTYPE_PHY; >> + } >> + } else { >> + /* Default to file?? */ >> + x_disks[i].phystype = PHYSTYPE_FILE; >> + } >> > > few days ago the patch that removes phystype and introduces backend and > format has been applied, so this code needs an update >Thanks for the review and comments. I''ve posted a new version based on xen-unstable c/s 22940.>> +static void libxlEventHandler(int watch, >> + int fd, >> + int events, >> + void *data) >> +{ >> + libxlDriverPrivatePtr driver = libxl_driver; >> + virDomainObjPtr vm = data; >> + libxlDomainObjPrivatePtr priv; >> + libxl_event event; >> + libxl_dominfo info; >> + >> + libxlDriverLock(driver); >> + virDomainObjLock(vm); >> + libxlDriverUnlock(driver); >> + >> + priv = vm->privateData; >> + >> + memset(&event, 0, sizeof(event)); >> + memset(&info, 0, sizeof(info)); >> + >> + if (priv->waiterFD != fd || priv->eventHdl != watch) { >> + virEventRemoveHandle(watch); >> + goto cleanup; >> + } >> + >> + if (!(events & VIR_EVENT_HANDLE_READABLE)) { >> + goto cleanup; >> + } >> + >> + if (libxl_get_event(&priv->ctx, &event)) { >> + goto cleanup; >> + } >> + >> + if (event.type == LIBXL_EVENT_DOMAIN_DEATH) { >> + /* libxl_event_get_domain_death_info returns 1 if death >> + * event was for this domid */ >> + if (libxl_event_get_domain_death_info(&priv->ctx, >> + vm->def->id, >> + &event, >> + &info) != 1) { >> + goto cleanup; >> + } >> + >> + virEventRemoveHandle(watch); >> + if (info.shutdown_reason == SHUTDOWN_poweroff) { >> + libxl_domain_destroy(&priv->ctx, vm->def->id, 0); >> + if (vm->persistent) { >> + vm->def->id = -1; >> + vm->state = VIR_DOMAIN_SHUTOFF; >> + } else { >> + libxl_free_waiter(priv->dWaiter); >> + VIR_FREE(priv->dWaiter); >> + virDomainRemoveInactive(&driver->domains, vm); >> + vm = NULL; >> + } >> + } else if (info.shutdown_reason == SHUTDOWN_reboot) { >> + libxl_domain_destroy(&priv->ctx, vm->def->id, 0); >> + vm->def->id = -1; >> + vm->state = VIR_DOMAIN_SHUTOFF; >> + sleep(1); >> + libxlVmStart(vm, 0); >> > > we need to handle at least SHUTDOWN_suspend and SHUTDOWN_crash too >The updated patch handles SHUTDOWN_crash similar to SHUTDOWN_poweroff. I''ve ignored SHUTDOWN_suspend since the driver does not yet implement domainSave function. Also notice I''ve ignored any user-specified actions on these events, e.g. coredump and restart. But IMO, these type of improvements can be added incrementally. I''d like to get a basic driver upstream so others can easily contribute.> >> + } else { >> + VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason); >> + } >> + } >> > > we should call libxl_free_event before returning, otherwise we leak two > strings >Fixed in updated patch. [...]>> + >> + libxl_driver->caps->privateDataAllocFunc = libxlDomainObjPrivateAlloc; >> + libxl_driver->caps->privateDataFreeFunc = libxlDomainObjPrivateFree; >> > > If I understand correctly privateDataAllocFunc is called at each domain creation >It is called when a domain is introduced to libvirt, either by defining a persistent domain (e.g. virsh define dom.xml) or creating a transient domain (e.g. virsh create dom.xml). The free func is called when a domain is removed from libvirt, either by undefining a persistent domain (e.g. virsh undefine dom-name) or poweroff of a transient domain. Starting/stopping a persistent domain does not invoke these functions. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel