Re: [libvirt] [PATCH V7] libxl: add migration support

On Thu, May 08, 2014 at 03:56:51PM -0600, Jim Fehlig wrote: > This patch adds initial migration support to the libxl driver, > using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration > functions. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > > V6 here > https://www.redhat.com/archives/libvir-list/2014-May/msg00017.html > > In V7: > There were no comments on V6, but during testing I noticed that > 'virsh migrate --suspend ...' was not being honored. Fixed that > in this version so that the domain is paused on the destination > once migration completes. > > po/POTFILES.in | 1 + > src/Makefile.am | 3 +- > src/libxl/libxl_conf.h | 6 + > src/libxl/libxl_domain.h | 1 + > src/libxl/libxl_driver.c | 235 ++++++++++++++++++ > src/libxl/libxl_migration.c | 581 ++++++++++++++++++++++++++++++++++++++++++++ > src/libxl/libxl_migration.h | 79 ++++++ > 7 files changed, 905 insertions(+), 1 deletion(-) > > @@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged, > LIBXL_VNC_PORT_MAX))) > goto error; > > + /* Allocate bitmap for migration port reservation */ > + if (!(libxl_driver->migrationPorts = > + virPortAllocatorNew(_("migration"), > + LIBXL_MIGRATION_PORT_MIN, > + LIBXL_MIGRATION_PORT_MAX))) > + goto error; > + > No need todo it in this patch, but experiance with QEMU is that people will want this range to be configurable in /etc/libvirt/libxl.conf. So I'd suggest adding this later

> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c > new file mode 100644 > index 0000000..e3699c5 > --- /dev/null > +++ b/src/libxl/libxl_migration.c > @@ -0,0 +1,581 @@ > +/* > + * libxl_migration.c: methods for handling migration with libxenlight > + * > + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. > + * > + * 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, see > + * <http://www.gnu.org/licenses/>;. > + * > + * Authors: > + * Jim Fehlig <jfehlig(a)suse.com&gt; > + * Chunyan Liu <cyliu(a)suse.com&gt; > + */ > + > +#include <config.h> > + > +#include "internal.h" > +#include "virlog.h" > +#include "virerror.h" > +#include "virconf.h" > +#include "datatypes.h" > +#include "virfile.h" > +#include "viralloc.h" > +#include "viruuid.h" > +#include "vircommand.h" > +#include "virstring.h" > +#include "virobject.h" > +#include "rpc/virnetsocket.h" > +#include "libxl_domain.h" > +#include "libxl_driver.h" > +#include "libxl_conf.h" > +#include "libxl_migration.h" > + > +#define VIR_FROM_THIS VIR_FROM_LIBXL > + > +VIR_LOG_INIT("libxl.libxl_migration"); > + > +typedef struct _libxlMigrationDstArgs { > + virObject parent; > + > + virConnectPtr conn; > + virDomainObjPtr vm; > + unsigned int flags; > + > + /* for freeing listen sockets */ > + virNetSocketPtr *socks; > + size_t nsocks; > +} libxlMigrationDstArgs; > + > +static virClassPtr libxlMigrationDstArgsClass; > + > +static void > +libxlMigrationDstArgsDispose(void *obj) > +{ > + libxlMigrationDstArgs *args = obj; > + > + VIR_FREE(args->socks); > +} > + > +static int > +libxlMigrationDstArgsOnceInit(void) > +{ > + if (!(libxlMigrationDstArgsClass = virClassNew(virClassForObject(), > + "libxlMigrationDstArgs", > + sizeof(libxlMigrationDstArgs), > + libxlMigrationDstArgsDispose))) > + return -1; > + > + return 0; > +} > + > +VIR_ONCE_GLOBAL_INIT(libxlMigrationDstArgs) > + > +static void > +libxlDoMigrateReceive(virNetSocketPtr sock, > + int events ATTRIBUTE_UNUSED, > + void *opaque) > +{ > + libxlMigrationDstArgs *args = opaque; > + virConnectPtr conn = args->conn; > + virDomainObjPtr vm = args->vm; > + virNetSocketPtr *socks = args->socks; > + size_t nsocks = args->nsocks; > + bool paused = args->flags & VIR_MIGRATE_PAUSED; > + libxlDriverPrivatePtr driver = conn->privateData; > + virNetSocketPtr client_sock; > + int recvfd = -1; > + size_t i; > + int ret; > + > + virNetSocketAccept(sock, &client_sock); > + if (client_sock == NULL) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("Fail to accept migration connection")); > + goto cleanup; > + } > + VIR_DEBUG("Accepted migration connection\n"); > + recvfd = virNetSocketDupFD(client_sock, true); > + virObjectUnref(client_sock); > + > + virObjectLock(vm); > + ret = libxlDomainStart(driver, vm, paused, recvfd); > + virObjectUnlock(vm); > + > + if (ret < 0 && !vm->persistent) > + virDomainObjListRemove(driver->domains, vm); > + > + cleanup: > + /* Remove all listen socks from event handler, and close them. */ > + for (i = 0; i < nsocks; i++) { > + virNetSocketUpdateIOCallback(socks[i], 0); > + virNetSocketRemoveIOCallback(socks[i]); > + virNetSocketClose(socks[i]); > + virObjectUnref(socks[i]); > I think you should set socks[i] = NULL and after the loop also set args->nsocks = 0, just to ensure nothing else can access these now free'd socks.

> +char * > +libxlDomainMigrationBegin(virConnectPtr conn, > + virDomainObjPtr vm, > + const char *xmlin) > +{ > + libxlDriverPrivatePtr driver = conn->privateData; > + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); > + virDomainDefPtr tmpdef = NULL; > + virDomainDefPtr def; > + char *xml = NULL; > + > + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) > + goto cleanup; > + > + if (xmlin) { > + if (!(tmpdef = virDomainDefParseString(xmlin, cfg->caps, > + driver->xmlopt, > + 1 << VIR_DOMAIN_VIRT_XEN, > + VIR_DOMAIN_XML_INACTIVE))) > + goto endjob; > I think you want to call an equivalent of qemuDomainDefCheckABIStability here. THis ensures the user supplied XML only contains changes that are not going to impact guest machine ABI.