2012/3/15 Jim Fehlig <jfehlig@suse.com>

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.

There is some difference to handle Migration ports and VNC ports:
VNC port always find a free port from VNC ports range and use it, but migration port could be pointed by user or if not pointed find a free port to use it. There are two places need to set bitmap maybe:
1. The port pointed by user could be a port in default migration ports range, we should set bitmap so that next time finding free port could avoid that port.
2. No port pointed by user, then find a free migration port from default migration ports range, and set bitmap.
Besides, with port pointed, we need to create socket and bind to the port too. libxlFindFreeVNCPort creates socket and binds port and set bitmap in the function, if FindFreeMigPort also does that, then to user pointed port, we need to do same work again.

Seems not cleaner than: FindFreeMigPort only gets the free port, then we can get a port either from FindFreeMigPort or user pointed, then create socket and bind to that port, if that port is in default migration ports range, set bitmap.).

How do you think?

> + 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

I'll check that.

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.

Yes. For HVM, libxl_domain_resume will fail. For PV, libxl_domain_resume should be OK.
### libxl_domain_resume Called domain_resume on non-cooperative hvm domain xxx
The state in xl shows "ss", the state should be updated in libvirt, will check that.
But there is a problem, it seems there is no corresponding virsh command that can handle such a state. It's not a "PAUSE" state as "virsh suspend xxx" which one can issue "virsh resume xxx" to unpause it.

Can you take a look at these issues?

Thanks!
Jim