[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add migration APIs for libxl driver



Chunyan Liu wrote:
> Hi, Jim,
> I made some changes to the patch according to your comments:
> a. support concurrent migrations, add virBitmapPtr for probing migration ports
> b. update doParseURI:
>   use virAsprintf instead of strdup and snprintf,
>   support migration URI syntax hostname[:port], remove xlmigr scheme
> c. drop lock of driver while doing doMigrateSend
> d. fix extra whitespace and parameter alignment and other places mentioned
>
> Not changed:
> a. ensure the name provided in xmlin is the same as def->name. Since we support
>   domain name change, so I think the name could be different. Keep not checked.
> b. leaks the original name. It seems the original name won't be used again, so
>   didn't save original name. Keep it as before.
> c. about migrate_receive_args. It's only used in dst side. If send logic and
>   receive logic still matches, extending the structure won't cause problem.
>   But if send logic and receive logic cannot match, there will problem. Still
>   think about how to handle it.
>
> Any further comments will be very appreciated. Thanks for your time!
>   

FYI, your mailer mangled this patch and I had to apply it manually. 
Probably best to send with 'git send-email' and include a patch version
(e.g. V2) in the subject.  See the archives for examples.

> Signed-off-by: Chunyan Liu <cyliu suse com>
> ---
>  src/libxl/libxl_conf.h   |    1 +
>  src/libxl/libxl_driver.c |  634 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/libxl/libxl_driver.h |   20 ++-
>  3 files changed, 653 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 2820afb..dd59817 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -61,6 +61,7 @@ struct _libxlDriverPrivate {
>     libxl_ctx ctx;
>
>     virBitmapPtr reservedVNCPorts;
> +    virBitmapPtr reservedMigPorts; /* reserved migration ports */
>   

Indentation.

>     virDomainObjList domains;
>
>     virDomainEventStatePtr domainEventState;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d5fa64a..5dc29a0 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -30,6 +30,12 @@
>  #include <math.h>
>  #include <libxl.h>
>  #include <fcntl.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <arpa/inet.h>
> +#include <netdb.h>
>
>  #include "internal.h"
>  #include "logging.h"
> @@ -61,6 +67,12 @@
>
>  static libxlDriverPrivatePtr libxl_driver = NULL;
>
> +typedef struct migrate_receive_args {
> +    virConnectPtr conn;
> +    virDomainObjPtr vm;
> +    int sockfd;
> +} migrate_receive_args;
> +
>  /* Function declarations */
>  static int
>  libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> @@ -810,6 +822,7 @@ libxlShutdown(void)
>         VIR_FORCE_FCLOSE(libxl_driver->logger_file);
>
>     virBitmapFree(libxl_driver->reservedVNCPorts);
> +    virBitmapFree(libxl_driver->reservedMigPorts);
>   

Indentation.

>     VIR_FREE(libxl_driver->configDir);
>     VIR_FREE(libxl_driver->autostartDir);
> @@ -865,6 +878,10 @@ libxlStartup(int privileged) {
>          virBitmapAlloc(LIBXL_VNC_PORT_MAX - LIBXL_VNC_PORT_MIN)) == NULL)
>         goto out_of_memory;
>
> +    if ((libxl_driver->reservedMigPorts =
> +         virBitmapAlloc(LIBXL_MIGRATION_MAX_PORT -
> LIBXL_MIGRATION_MIN_PORT)) == NULL)
>   

Mailer wrapped this.

> +        goto out_of_memory;
> +
>     if (virDomainObjListInit(&libxl_driver->domains) < 0)
>         goto out_of_memory;
>
> @@ -1095,6 +1112,17 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED)
>     return 0;
>  }
>
> +static int
> +libxlSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
> +{
> +    switch (feature) {
> +    case VIR_DRV_FEATURE_MIGRATION_V3:
> +        return 1;
> +    default:
> +        return 0;
> +    }
> +}
> +
>  static const char *
>  libxlGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
>  {
> @@ -3842,12 +3870,613 @@ libxlIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED)
>     return 1;
>  }
>
> +static int libxlReadFixedMessage(int fd, const void *msg, int msgsz)
>   

Perhaps this should be renamed to libxlCheckMessageBanner() or similar
since you are not only reading a message, but also checking if it
matches the expected banner.  E.g.

static int libxlCheckMessageBanner(int fd, const char *banner, int
banner_sz)

> +{
> +    char buf[msgsz];
> +
> +    if (saferead(fd, buf, msgsz) != msgsz || memcmp(buf, msg, msgsz)) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int doParseURI(const char *uri, char **p_hostname, int *p_port)
> +{
> +    char *p, *hostname;
> +    int port_nr = 0;
> +
> +    if (uri == NULL)
> +        return -1;
> +
> +    /* URI passed is a string "hostname[:port]" */
> +    if ((p = strrchr(uri, ':')) != NULL) { /* "hostname:port" */
> +        int n;
> +
> +        if (virStrToLong_i(p+1, NULL, 10, &port_nr) < 0) {
> +            libxlError(VIR_ERR_INVALID_ARG,
> +                        _("Invalid port number"));
> +            return -1;
> +        }
> +
> +        /* Get the hostname. */
> +        n = p - uri; /* n = Length of hostname in bytes. */
> +        if (n <= 0) {
> +            libxlError(VIR_ERR_INVALID_ARG,
> +                       _("Hostname must be specified in the URI"));
> +            return -1;
> +        }
> +
> +        if (virAsprintf(&hostname, "%s", uri) < 0) {
> +            virReportOOMError();
> +            return -1;
> +        }
> +
> +        hostname[n] = '\0';
> +    }
> +    else {/* "hostname" (or IP address) */
> +        if (virAsprintf(&hostname, "%s", uri) < 0) {
> +            virReportOOMError();
> +            return -1;
> +        }
> +    }
> +    *p_hostname = hostname;
> +    *p_port = port_nr;
> +    return 0;
> +}
> +
> +static char *
> +libxlDomainMigrateBegin3(virDomainPtr domain,
> +                          const char *xmlin,
> +                          char **cookieout ATTRIBUTE_UNUSED,
> +                          int *cookieoutlen ATTRIBUTE_UNUSED,
> +                          unsigned long flags,
> +                          const char *dname ATTRIBUTE_UNUSED,
> +                          unsigned long resource ATTRIBUTE_UNUSED)
> +{
> +    libxlDriverPrivatePtr driver = domain->conn->privateData;
> +    virDomainObjPtr vm;
> +    virDomainDefPtr def = NULL;
> +    char *xml = NULL;
> +
> +    virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
> +
> +    libxlDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(domain->uuid, uuidstr);
> +        libxlError(VIR_ERR_OPERATION_INVALID,
> +                   _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
>   

You can unlock the driver at this point after successfully acquiring the
domain object lock.

> +
> +    if (!virDomainObjIsActive(vm)) {
> +        libxlError(VIR_ERR_OPERATION_INVALID,
> +                         _("domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    if (xmlin) {
> +        if (!(def = virDomainDefParseString(driver->caps, xmlin,
> +                         1 << VIR_DOMAIN_VIRT_XEN,
> +                         VIR_DOMAIN_XML_INACTIVE)))
> +            goto cleanup;
> +
> +        xml = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
> +    } else {
> +        xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE);
> +    }
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    libxlDriverUnlock(driver);
> +    return xml;
> +}
> +
> +static void doMigrateReceive(void *opaque)
> +{
> +    migrate_receive_args *data = opaque;
> +    virConnectPtr conn = data->conn;
> +    int sockfd = data->sockfd;
> +    virDomainObjPtr vm = data->vm;
> +    libxlDriverPrivatePtr driver = conn->privateData;
> +    int recv_fd;
> +    struct sockaddr_in new_addr;
> +    socklen_t socklen = sizeof(new_addr);
> +    int len;
> +
> +    do {
> +        recv_fd = accept(sockfd, (struct sockaddr *)&new_addr, &socklen);
> +    } while(recv_fd < 0 && errno == EINTR);
> +
> +    if (recv_fd < 0) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Could not accept migration connection"));
> +        goto cleanup;
> +    }
> +    VIR_DEBUG("Accepted migration\n");
> +
> +    len = sizeof(migrate_receiver_banner);
> +    if (safewrite(recv_fd, migrate_receiver_banner, len) != len) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                         _("Failed to write migrate_receiver_banner"));
> +        goto cleanup;
> +    }
> +
> +    if (libxlVmStart(driver, vm, false, recv_fd) < 0) {
> +        libxlError(VIR_ERR_INTERNAL_ERROR,
> +                    _("Failed to restore domain with libxenlight"));
> +        if (!vm->persistent) {
> +            virDomainRemoveInactive(&driver->domains, vm);
> +            vm = NULL;
> +        }
> +        goto cleanup;
> +    }
> +
> +    len = sizeof(migrate_receiver_ready);
> +    if (safewrite(recv_fd, migrate_receiver_ready, len) != len) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                         _("Failed to write migrate_receiver_ready"));
> +    }
> +
> +cleanup:
> +    if (VIR_CLOSE(recv_fd) < 0)
> +        virReportSystemError(errno, "%s", _("cannot close recv_fd"));
> +    if (VIR_CLOSE(sockfd) < 0)
> +        virReportSystemError(errno, "%s", _("cannot close sockfd"));
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    VIR_FREE(opaque);
> +    return;
> +}
> +
> +static int doMigrateSend(virDomainPtr dom, unsigned long flags, int sockfd)
> +{
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    libxlDomainObjPrivatePtr priv;
> +    libxl_domain_suspend_info suspinfo;
> +    virDomainEventPtr event = NULL;
> +    int live = 0;
> +    int ret = -1;
> +
> +    if (flags & VIR_MIGRATE_LIVE)
> +        live = 1;
> +
> +    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(dom->uuid, uuidstr);
> +        libxlError(VIR_ERR_OPERATION_INVALID,
> +                         _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
> +
> +    priv = vm->privateData;
> +
> +    /* read fixed message from dest (ready to receive) */
> +    if (libxlReadFixedMessage(sockfd, migrate_receiver_banner,
> +                              sizeof(migrate_receiver_banner))) {
> +        goto cleanup;
> +    }
> +
> +    /* send suspend data */
> +    memset(&suspinfo, 0, sizeof(suspinfo));
> +    if (live == 1)
> +        suspinfo.flags |= XL_SUSPEND_LIVE;
> +    if (libxl_domain_suspend(&priv->ctx, &suspinfo, vm->def->id,
> sockfd) != 0) {
>   

Wrapped.

> +        libxlError(VIR_ERR_INTERNAL_ERROR,
> +                    _("Failed to save domain '%d' with libxenlight"),
> +                    vm->def->id);
> +        goto cleanup;
> +    }
> +
> +    /* read fixed message from dest (receive completed) */
> +    if (libxlReadFixedMessage(sockfd, migrate_receiver_ready,
> +                              sizeof(migrate_receiver_ready))) {
> +        /* Src side should be resumed, but for ret < 0, virsh won't
> call Src side
>   

Wrapped.

> +         * Confirm3, handle it here.
> +         */
> +        libxl_domain_resume(&priv->ctx, vm->def->id);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    if (event)
> +        libxlDomainEventQueue(driver, event);
> +    return ret;
> +}
> +
> +static int libxlFindFreeMigPort(libxlDriverPrivatePtr driver, int startPort)
> +{
> +    int i;
> +
> +    for (i = startPort ; i < LIBXL_MIGRATION_MAX_PORT; i++) {
> +        bool used = false;
> +        if (virBitmapGetBit(driver->reservedMigPorts,
> +                            i - LIBXL_MIGRATION_MIN_PORT, &used) < 0)
> +            VIR_DEBUG("virBitmapGetBit failed on bit %d", i -
> LIBXL_MIGRATION_MIN_PORT);
>   

Wrapped.

> +
> +        if (!used)
> +            return i;
> +    }
> +
> +    return -1;
> +}
> +
> +static int
> +libxlDomainMigratePrepare3(virConnectPtr dconn,
> +                            const char *cookiein ATTRIBUTE_UNUSED,
> +                            int cookieinlen ATTRIBUTE_UNUSED,
> +                            char **cookieout ATTRIBUTE_UNUSED,
> +                            int *cookieoutlen ATTRIBUTE_UNUSED,
> +                            const char *uri_in,
> +                            char **uri_out,
> +                            unsigned long flags,
> +                            const char *dname,
> +                            unsigned long resource ATTRIBUTE_UNUSED,
> +                            const char *dom_xml)
> +{
> +    libxlDriverPrivatePtr driver = dconn->privateData;
> +    virDomainDefPtr def = NULL;
> +    virDomainObjPtr vm = NULL;
> +    char *hostname = NULL;
> +    int port = 0;
> +    int sockfd = -1;
> +    struct sockaddr_in addr;
> +    virThread migrate_receive_thread;
> +    migrate_receive_args *args;
> +    int ret = -1;
> +
> +    virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
> +
> +    libxlDriverLock(driver);
> +    if (!dom_xml) {
> +        libxlError(VIR_ERR_OPERATION_INVALID,
> +                         _("no domain XML passed"));
> +        goto cleanup;
> +    }
> +
> +    def = virDomainDefParseString(driver->caps, dom_xml,
> +                                 1 << VIR_DOMAIN_VIRT_XEN,
> +                                 VIR_DOMAIN_XML_INACTIVE);
> +
> +    /* Target domain name, maybe renamed. */
> +    if (dname) {
> +        def->name = strdup(dname);
> +        if (def->name == NULL)
> +            goto cleanup;
> +    }
> +
> +    if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
> +        goto cleanup;
> +
> +    if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true)))
> +        goto cleanup;
> +
> +    def = NULL;
> +
> +    /* Create socket connection to receive migration data */
> +    if (!uri_in) {
> +        hostname = virGetHostname(dconn);
> +        if (hostname == NULL)
> +            goto cleanup;
> +
> +        port = libxlFindFreeMigPort(driver, LIBXL_MIGRATION_MIN_PORT);
>   

I think you should reserve the port in libxlFindFreeMigPort(), similar
to libxlNextFreeVncPort().  In fact, you could probably generalize
libxlNextFreeVncPort(), e.g. libxlNextFreePort(virBitmapPtr bitmap, int
start_port, int stop_port) and use it to find available VNC and
migration ports.


> +        if (port < 0) {
> +            libxlError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("Unable to find an unused migration port"));
> +            goto cleanup;
> +        }
> +
> +        if (virAsprintf(uri_out, "%s:%d", hostname, port) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +    } else {
> +        if (doParseURI(uri_in, &hostname, &port))
> +            goto cleanup;
> +
> +        if (port <= 0) {
> +            port = libxlFindFreeMigPort(driver, LIBXL_MIGRATION_MIN_PORT);
> +            if (port < 0) {
> +                libxlError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("Unable to find an unused migration port"));
> +                goto cleanup;
> +            }
> +
> +            if (virAsprintf(uri_out, "%s:%d", hostname, port) < 0) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +        }
> +    }
> +
> +    sockfd = socket(AF_INET, SOCK_STREAM, 0);
> +    if (sockfd == -1) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Failed to create socket for incoming migration"));
> +        goto cleanup;
> +    }
> +
> +    memset(&addr, 0, sizeof(addr));
> +    addr.sin_family = AF_INET;
> +    addr.sin_port = htons(port);
> +    addr.sin_addr.s_addr = htonl(INADDR_ANY);
> +
> +    if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Fail to bind port for incoming migration"));
> +        goto cleanup;
> +    }
> +
> +    if (listen(sockfd, MAXCONN_NUM) < 0){
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Fail to listen to incoming migration"));
> +        goto cleanup;
> +    }
> +
> +    if (VIR_ALLOC(args) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    args->conn = dconn;
> +    args->vm = vm;
> +    args->sockfd = sockfd;
> +    if (virThreadCreate(&migrate_receive_thread,
> +                        true,
> +                        doMigrateReceive, args) < 0 ) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to create migration thread"));
> +        VIR_FREE(args);
> +        goto cleanup;
> +    }
> +
> +    if (LIBXL_MIGRATION_MIN_PORT <= port && port < LIBXL_MIGRATION_MAX_PORT) {
> +        if (virBitmapSetBit(driver->reservedMigPorts,
> +                            port - LIBXL_MIGRATION_MIN_PORT) < 0)
> +            VIR_DEBUG("virBitmapSetBit failed on bit %d",
> +                      port - LIBXL_MIGRATION_MIN_PORT);
> +    }
> +    ret = 0;
> +    goto end;
> +
> +cleanup:
> +    if (VIR_CLOSE(sockfd) < 0)
> +        virReportSystemError(errno, "%s", _("cannot close sockfd"));
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +
> +end:
> +    VIR_FREE(hostname);
> +    libxlDriverUnlock(driver);
> +    return ret;
> +}
> +
> +static int
> +libxlDomainMigratePerform3(virDomainPtr dom,
> +                            const char *xmlin ATTRIBUTE_UNUSED,
> +                            const char *cookiein ATTRIBUTE_UNUSED,
> +                            int cookieinlen ATTRIBUTE_UNUSED,
> +                            char **cookieout ATTRIBUTE_UNUSED,
> +                            int *cookieoutlen ATTRIBUTE_UNUSED,
> +                            const char *dconnuri ATTRIBUTE_UNUSED,
> +                            const char *uri,
> +                            unsigned long flags,
> +                            const char *dname ATTRIBUTE_UNUSED,
> +                            unsigned long resource ATTRIBUTE_UNUSED)
> +{
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
>   

driver is no longer used, so compilation failure with
--enable-compile-warnings=error.

> +    char *hostname = NULL;
> +    int port = 0;
> +    char *servname = NULL;
> +    struct addrinfo hints, *res;
> +    int sockfd = -1;
> +    int ret = -1;
> +
> +    virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
> +
> +    if (doParseURI(uri, &hostname, &port))
> +        goto cleanup;
> +
> +    VIR_DEBUG("hostname = %s, port = %d", hostname, port);
> +
> +    if (port <= 0)
> +        goto cleanup;
> +
> +    if (virAsprintf(&servname, "%d", port) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) < 0 ){
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Failed to create socket"));
> +        goto cleanup;
> +    }
> +
> +    memset(&hints, 0, sizeof(hints));
> +    hints.ai_family = AF_INET;
> +    hints.ai_socktype = SOCK_STREAM;
> +    if (getaddrinfo(hostname, servname, &hints, &res) || res == NULL) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("IP address lookup failed"));
> +        goto cleanup;
> +    }
> +
> +    if (connect(sockfd, res->ai_addr, res->ai_addrlen) < 0) {
> +        libxlError(VIR_ERR_OPERATION_FAILED,
> +                   _("Connect error"));
> +        goto cleanup;
> +    }
> +
> +    ret = doMigrateSend(dom, flags, sockfd);
> +
> +cleanup:
> +    if (VIR_CLOSE(sockfd) < 0)
> +        virReportSystemError(errno, "%s", _("cannot close sockfd"));
> +    VIR_FREE(hostname);
> +    VIR_FREE(servname);
> +    if (res)
> +        freeaddrinfo(res);
> +    return ret;
> +}
> +
> +static virDomainPtr
> +libxlDomainMigrateFinish3(virConnectPtr dconn,
> +                           const char *dname,
> +                           const char *cookiein ATTRIBUTE_UNUSED,
> +                           int cookieinlen ATTRIBUTE_UNUSED,
> +                           char **cookieout ATTRIBUTE_UNUSED,
> +                           int *cookieoutlen ATTRIBUTE_UNUSED,
> +                           const char *dconnuri ATTRIBUTE_UNUSED,
> +                           const char *uri,
> +                           unsigned long flags,
> +                           int cancelled)
> +{
> +    libxlDriverPrivatePtr driver = dconn->privateData;
> +    char *hostname = NULL;
> +    int port = 0;
> +    virDomainObjPtr vm = NULL;
> +    virDomainPtr dom = NULL;
> +    libxlDomainObjPrivatePtr priv;
> +    virDomainEventPtr event = NULL;
> +    int rc;
> +
> +    virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
> +
> +    libxlDriverLock(driver);
> +
> +    if (doParseURI(uri, &hostname, &port))
> +        VIR_DEBUG("Fail to parse port from URI");
> +
> +    if (LIBXL_MIGRATION_MIN_PORT <= port && port < LIBXL_MIGRATION_MAX_PORT) {
> +        if (virBitmapClearBit(driver->reservedMigPorts,
> +                              port - LIBXL_MIGRATION_MIN_PORT) < 0)
> +            VIR_DEBUG("Could not mark port %d as unused", port);
> +    }
> +
> +    vm = virDomainFindByName(&driver->domains, dname);
> +    if (!vm)
> +        goto cleanup;
> +
> +    if (!cancelled) {
> +        if (!(flags & VIR_MIGRATE_PAUSED)) {
> +            priv = vm->privateData;
> +            rc = libxl_domain_unpause(&priv->ctx, vm->def->id);
> +            if (rc) {
> +                libxlError(VIR_ERR_OPERATION_FAILED,
> +                           _("Failed to unpause domain"));
> +                goto error;
> +            }
> +
> +            virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_BOOTED);
>   

Wrapped.

> +            if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> +                goto error;
> +        }
> +
> +        dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
> +        goto cleanup;
> +    }
> +
> +error:
> +    if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED)) {
> +        libxlError(VIR_ERR_INTERNAL_ERROR,
> +                   _("Failed to destroy domain '%d'"), vm->def->id);
> +        goto cleanup;
> +    }
> +    event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> +                                     VIR_DOMAIN_EVENT_STOPPED_SAVED);
> +    if (!vm->persistent) {
> +        virDomainRemoveInactive(&driver->domains, vm);
> +        vm = NULL;
> +    }
> +
> +cleanup:
> +    VIR_FREE(hostname);
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    if (event)
> +        libxlDomainEventQueue(driver, event);
> +    libxlDriverUnlock(driver);
> +    return dom;
> +}
> +
> +static int
> +libxlDomainMigrateConfirm3(virDomainPtr domain,
> +                            const char *cookiein ATTRIBUTE_UNUSED,
> +                            int cookieinlen ATTRIBUTE_UNUSED,
> +                            unsigned long flags,
> +                            int cancelled)
> +{
> +    libxlDriverPrivatePtr driver = domain->conn->privateData;
> +    virDomainObjPtr vm;
> +    libxlDomainObjPrivatePtr priv;
> +    virDomainEventPtr event = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
> +
> +    libxlDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(domain->uuid, uuidstr);
> +        libxlError(VIR_ERR_NO_DOMAIN,
> +                   _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
> +
> +    if (cancelled) {
> +        priv = vm->privateData;
> +        libxlError(VIR_ERR_INTERNAL_ERROR,
> +                   _("migration failed, try to resume on our end"));
> +        if (!libxl_domain_resume(&priv->ctx, vm->def->id))
> +            ret = 0;
> +
> +        goto cleanup;
> +    }
> +
> +    if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED)) {
> +        libxlError(VIR_ERR_INTERNAL_ERROR,
> +                   _("Failed to destroy domain '%d'"), vm->def->id);
> +        goto cleanup;
> +    }
> +
> +    event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> +                                     VIR_DOMAIN_EVENT_STOPPED_SAVED);
> +
> +    if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
> +        virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm);
> +
> +    if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
> +        virDomainRemoveInactive(&driver->domains, vm);
> +        vm = NULL;
> +    }
> +
> +    VIR_DEBUG("Migration successful.\n");
> +    ret = 0;
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    libxlDriverUnlock(driver);
> +    return ret;
> +}
>
>  static virDriver libxlDriver = {
>     .no = VIR_DRV_LIBXL,
>     .name = "xenlight",
>     .open = libxlOpen, /* 0.9.0 */
>     .close = libxlClose, /* 0.9.0 */
> +    .supports_feature = libxlSupportsFeature, /* 0.9.12 */
>     .type = libxlGetType, /* 0.9.0 */
>     .version = libxlGetVersion, /* 0.9.0 */
>     .getHostname = virGetHostname, /* 0.9.0 */
> @@ -3906,6 +4535,11 @@ static virDriver libxlDriver = {
>     .domainGetSchedulerParametersFlags =
> libxlDomainGetSchedulerParametersFlags, /* 0.9.2 */
>     .domainSetSchedulerParameters = libxlDomainSetSchedulerParameters,
> /* 0.9.0 */
>     .domainSetSchedulerParametersFlags =
> libxlDomainSetSchedulerParametersFlags, /* 0.9.2 */
> +    .domainMigrateBegin3 = libxlDomainMigrateBegin3, /* 0.9.12 */
> +    .domainMigratePrepare3 = libxlDomainMigratePrepare3, /* 0.9.12 */
> +    .domainMigratePerform3 = libxlDomainMigratePerform3, /* 0.9.12 */
> +    .domainMigrateFinish3 = libxlDomainMigrateFinish3, /* 0.9.12 */
> +    .domainMigrateConfirm3 = libxlDomainMigrateConfirm3, /* 0.9.12 */
>   

Some wrapping and indentation issues in this hunk.  Also, next version
of libvirt will be 0.9.11.  Hopefully we'll get this patch into shape
before the end of the month.

>     .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>     .domainEventRegister = libxlDomainEventRegister, /* 0.9.0 */
>     .domainEventDeregister = libxlDomainEventDeregister, /* 0.9.0 */
> diff --git a/src/libxl/libxl_driver.h b/src/libxl/libxl_driver.h
> index 4632d33..ad8df1f 100644
> --- a/src/libxl/libxl_driver.h
> +++ b/src/libxl/libxl_driver.h
> @@ -21,9 +21,25 @@
>  /*---------------------------------------------------------------------------*/
>
>  #ifndef LIBXL_DRIVER_H
> -# define LIBXL_DRIVER_H
> +#define LIBXL_DRIVER_H
>
> -# include <config.h>
> +#include <config.h>
>   

Don't change the whitespace in these preprocessor directives. With cpp
installed, it will cause 'make syntax-check' failures.

> +
> +#define LIBXL_MIGRATION_FLAGS                   \
> +    (VIR_MIGRATE_LIVE |                         \
> +     VIR_MIGRATE_UNDEFINE_SOURCE |              \
> +     VIR_MIGRATE_PAUSED)
> +
> +#define MAXCONN_NUM 10
> +#define LIBXL_MIGRATION_MIN_PORT 49512
> +#define LIBXL_MIGRATION_NUM_PORTS 64
> +#define LIBXL_MIGRATION_MAX_PORT                \
> +    (LIBXL_MIGRATION_MIN_PORT + LIBXL_MIGRATION_NUM_PORTS)
>   

You'll need a space between '#' and 'define' in these macros.

> +
> +static const char migrate_receiver_banner[]=
> +    "xl migration receiver ready, send binary domain data";
> +static const char migrate_receiver_ready[]=
> +    "domain received, ready to unpause";
>
>  int libxlRegister(void);
>   

While testing this patch, I noticed some strange problems wrt concurrent
operations in the driver.  E.g. if I start a migration and then query
dominfo on the migrating domain, it kills the migration

xen134: # virsh migrate --live sles11sp1-pv xen+ssh://xen142
error: internal error Failed to save domain '7' with libxenlight

Also, when a migration fails (e.g. destination cannot access the
domain's disk image) the domain appears to be left in a hung state on
the source.  It is as if the domain is paused, but event the xen tools
claim it is running.

Can you take a look at these issues?

Thanks!
Jim


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]