[libvirt] [PATCH V2 13/13] libxl: add migration support
Jim Fehlig
jfehlig at suse.com
Wed Mar 19 22:52:17 UTC 2014
Michal Privoznik wrote:
> On 13.03.2014 23:11, 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 at suse.com>
>> ---
>>
>> V2:
>> - Now that the libxl driver supports hostdev passthrough, ensure
>> domain has no hostdevs before migration
>> - Fail early in libxlDomainMigrateBegin3Params if domain is inactive
>> - Change name of local variable in libxlDoMigrateSend from 'live'
>> to 'xl_flags'.
>>
>> 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 | 225 +++++++++++++++++
>> src/libxl/libxl_migration.c | 598
>> ++++++++++++++++++++++++++++++++++++++++++++
>> src/libxl/libxl_migration.h | 78 ++++++
>> 7 files changed, 911 insertions(+), 1 deletion(-)
>>
>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 3306dc1..6fc5266 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>
>> @@ -4313,6 +4323,216 @@ cleanup:
>> return ret;
>> }
>>
>> +static char *
>> +libxlDomainMigrateBegin3Params(virDomainPtr domain,
>> + virTypedParameterPtr params,
>> + int nparams,
>> + char **cookieout ATTRIBUTE_UNUSED,
>> + int *cookieoutlen ATTRIBUTE_UNUSED,
>> + unsigned int flags)
>> +{
>> + const char *xmlin = NULL;
>> + const char *dname = NULL;
>> + virDomainObjPtr vm = NULL;
>> +
>> + virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
>> + if (virTypedParamsValidate(params, nparams,
>> LIBXL_MIGRATION_PARAMETERS) < 0)
>> + return NULL;
>> +
>> + if (virTypedParamsGetString(params, nparams,
>> + VIR_MIGRATE_PARAM_DEST_XML,
>> + &xmlin) < 0 ||
>> + virTypedParamsGetString(params, nparams,
>> + VIR_MIGRATE_PARAM_DEST_NAME,
>> + &dname) < 0)
>
> I'd expect @dname to be used somewhere...
Seems I followed the qemu driver too closely, where AFAICT dname isn't
used in the begin phase either. Is there even anything to do with dname
in the begin phase on the migration source?
>
>> + return NULL;
>> +
>> + if (!(vm = libxlDomObjFromDomain(domain)))
>> + return NULL;
>> +
>> + if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def)
>> < 0) {
>> + virObjectUnlock(vm);
>> + return NULL;
>> + }
>> +
>> + if (!virDomainObjIsActive(vm)) {
>> + virReportError(VIR_ERR_OPERATION_INVALID,
>> + "%s", _("domain is not running"));
>> + virObjectUnlock(vm);
>> + return NULL;
>> + }
>> +
>> + return libxlDomainMigrationBegin(domain->conn, vm, xmlin);
>> +}
>> +
>
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> new file mode 100644
>> index 0000000..01023db
>> --- /dev/null
>> +++ b/src/libxl/libxl_migration.c
>> @@ -0,0 +1,598 @@
>> +/*
>> + * 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 at suse.com>
>> + * Chunyan Liu <cyliu at suse.com>
>> + */
>> +
>> +#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 "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
>> +
>> +typedef struct _libxlMigrateReceiveArgs {
>> + virConnectPtr conn;
>> + virDomainObjPtr vm;
>> +
>> + /* for freeing listen sockets */
>> + virNetSocketPtr *socks;
>> + size_t nsocks;
>> +} libxlMigrateReceiveArgs;
>> +
>> +static const char libxlMigrateReceiverReady[] =
>> + "libvirt libxl migration receiver ready, send binary domain data";
>> +static const char libxlMigrateReceiverFinish[] =
>> + "domain received, ready to unpause";
>
> So you're using these to sync source and destination on migration.
> Kudos for using text protocol not a binary blob. I haven't followed v1
> closely, so what was resolution on this? I mean, my concern is if this
> is extensible.
>
>> +
>> +
>> +static int
>> +libxlCheckMessageBanner(int fd, const char *banner, int banner_sz)
>> +{
>> + char buf[banner_sz];
>> + int ret = 0;
>> +
>> + do {
>> + ret = saferead(fd, buf, banner_sz);
>> + } while (ret == -1 && errno == EAGAIN);
>
> The @fd is in blocking state, so we should never get EAGAIN here. But
> it doesn't hurt to check.
>
>> +
>> + if (ret != banner_sz || memcmp(buf, banner, banner_sz)) {
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void
>> +libxlDoMigrateReceive(virNetSocketPtr sock,
>> + int events ATTRIBUTE_UNUSED,
>> + void *opaque)
>> +{
>> + libxlMigrateReceiveArgs *data = opaque;
>> + virConnectPtr conn = data->conn;
>> + virDomainObjPtr vm = data->vm;
>> + virNetSocketPtr *socks = data->socks;
>> + size_t nsocks = data->nsocks;
>> + libxlDriverPrivatePtr driver = conn->privateData;
>> + virNetSocketPtr client_sock;
>> + int recv_fd;
>> + int len;
>> + 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\n");
>> + recv_fd = virNetSocketDupFD(client_sock, true);
>> + virObjectUnref(client_sock);
>> +
>> + len = sizeof(libxlMigrateReceiverReady);
>> + if (safewrite(recv_fd, libxlMigrateReceiverReady, len) != len) {
>> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> + _("Failed to write libxlMigrateReceiverReady"));
>> + goto cleanup;
>> + }
>> +
>> + virObjectLock(vm);
>> + ret = libxlDomainStart(driver, vm, false, recv_fd);
>> + virObjectUnlock(vm);
>> +
>> + if (ret < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Failed to restore domain with libxenlight"));
>> + if (!vm->persistent) {
>> + virDomainObjListRemove(driver->domains, vm);
>> + vm = NULL;
>> + }
>> + goto cleanup;
>> + }
>> +
>> + len = sizeof(libxlMigrateReceiverFinish);
>> + if (safewrite(recv_fd, libxlMigrateReceiverFinish, len) != len) {
>> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> + _("Failed to write
>> libxlMigrateReceiverFinish"));
>> + }
>> +
>> + /* Remove all listen socks from event handler, and close them. */
>> + if (nsocks) {
>
> Useless 'if'
>
>> + for (i = 0; i < nsocks; i++) {
>> + virNetSocketUpdateIOCallback(socks[i], 0);
>> + virNetSocketRemoveIOCallback(socks[i]);
>> + virNetSocketClose(socks[i]);
>> + virObjectUnref(socks[i]);
>> + }
>> + VIR_FREE(socks);
>> + }
>> +
>> +cleanup:
>> + VIR_FORCE_CLOSE(recv_fd);
>> + VIR_FREE(opaque);
>> + return;
>> +}
>> +
>
>> +int
>> +libxlDomainMigrationPrepare(virConnectPtr dconn,
>> + virDomainDefPtr def,
>> + const char *uri_in,
>> + char **uri_out)
>> +{
>> + libxlDriverPrivatePtr driver = dconn->privateData;
>> + virDomainObjPtr vm = NULL;
>> + char *hostname = NULL;
>> + unsigned short port;
>> + char portstr[100];
>> + virURIPtr uri = NULL;
>> + virNetSocketPtr *socks = NULL;
>> + size_t nsocks = 0;
>> + int nsocks_listen = 0;
>> + libxlMigrateReceiveArgs *args;
>> + size_t i;
>> + int ret = -1;
>> +
>> + if (!(vm = virDomainObjListAdd(driver->domains, def,
>> + driver->xmlopt,
>> + VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
>> + VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
>> + NULL)))
>> + goto cleanup;
>> +
>> + /* Create socket connection to receive migration data */
>> + if (!uri_in) {
>> + if ((hostname = virGetHostname()) == NULL)
>> + goto cleanup;
>> +
>> + if (STRPREFIX(hostname, "localhost")) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("hostname on destination resolved to
>> localhost,"
>> + " but migration requires an FQDN"));
>> + goto cleanup;
>> + }
>> +
>> + if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
>> + goto cleanup;
>> +
>> + if (virAsprintf(uri_out, "tcp://%s:%d", hostname, port) < 0)
>> + goto cleanup;
>> + } else {
>> + if (!(STRPREFIX(uri_in, "tcp://"))) {
>> + /* not full URI, add prefix tcp:// */
>> + char *tmp;
>> + if (virAsprintf(&tmp, "tcp://%s", uri_in) < 0)
>> + goto cleanup;
>> + uri = virURIParse(tmp);
>> + VIR_FREE(tmp);
>> + } else {
>> + uri = virURIParse(uri_in);
>> + }
>> +
>> + if (uri == NULL) {
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("unable to parse URI: %s"),
>> + uri_in);
>> + goto cleanup;
>> + }
>> +
>> + if (uri->server == NULL) {
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("missing host in migration URI: %s"),
>> + uri_in);
>> + goto cleanup;
>> + } else {
>> + hostname = uri->server;
>> + }
>> +
>> + if (uri->port == 0) {
>> + if (virPortAllocatorAcquire(driver->migrationPorts,
>> &port) < 0)
>> + goto cleanup;
>> +
>> + } else {
>> + port = uri->port;
>> + }
>> +
>> + if (virAsprintf(uri_out, "tcp://%s:%d", hostname, port) < 0)
>> + goto cleanup;
>> + }
>> +
>> + snprintf(portstr, sizeof(portstr), "%d", port);
>> +
>> + if (virNetSocketNewListenTCP(hostname, portstr, &socks, &nsocks)
>> < 0) {
>> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> + _("Fail to create socket for incoming
>> migration"));
>> + goto cleanup;
>> + }
>> +
>> + if (VIR_ALLOC(args) < 0)
>> + goto cleanup;
>> +
>> + args->conn = dconn;
>> + args->vm = vm;
>> + args->socks = socks;
>> + args->nsocks = nsocks;
>> +
>> + for (i = 0; i < nsocks; i++) {
>> + if (virNetSocketSetBlocking(socks[i], true) < 0)
>> + continue;
>> + if (virNetSocketListen(socks[i], 1) < 0)
>> + continue;
>> +
>> + if (virNetSocketAddIOCallback(socks[i],
>> + 0,
>> + libxlDoMigrateReceive,
>> + args,
>> + NULL) < 0) {
>> + continue;
>> + }
>> +
>> + virNetSocketUpdateIOCallback(socks[i],
>> VIR_EVENT_HANDLE_READABLE);
>> + nsocks_listen++;
>> + }
>> +
>> + if (!nsocks_listen)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + goto done;
>> +
>> +cleanup:
>> + if (nsocks) {
>
> There's no need for this 'if'.
>
>> + for (i = 0; i < nsocks; i++) {
>> + virNetSocketClose(socks[i]);
>> + virObjectUnref(socks[i]);
>> + }
>> + VIR_FREE(socks);
>> + }
>> +
>> +done:
>> + virURIFree(uri);
>> + if (vm)
>> + virObjectUnlock(vm);
>> + return ret;
>> +}
>> +
>> +int
>> +libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>> + virDomainObjPtr vm,
>> + const char *dom_xml ATTRIBUTE_UNUSED,
>> + const char *dconnuri ATTRIBUTE_UNUSED,
>> + const char *uri_str,
>> + const char *dname ATTRIBUTE_UNUSED,
>> + unsigned int flags)
>> +{
>> + char *hostname = NULL;
>> + unsigned short port = 0;
>> + char portstr[100];
>> + virURIPtr uri = NULL;
>> + virNetSocketPtr sock;
>> + int sockfd = -1;
>> + int saved_errno = EINVAL;
>> + int ret = -1;
>> +
>
> Missing virCheckFlags(VIR_MIGRATE_LIVE, -1);
Flags are checked in the calling function, libxlDomainMigratePerform3Params.
I've fixed up the other issues you mentioned, but will wait to send a V3
until clarifying the use of dname in the begin phase.
Regards,
Jim
More information about the libvir-list
mailing list