Re: [libvirt] [PATCH] Add migration APIs for libxl driver
"Chun Yan Liu" <cyliu@suse.com> 3/6/2012 2:29 PM >>> I didn't get a chance to test this yet, but have some initial review comments.
Signed-off-by: Chunyan Liu --- src/libxl/libxl_driver.c | 617 ++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_driver.h | 17 ++- 2 files changed, 632 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d5fa64a..4037635 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -30,6 +30,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include
#include "internal.h" #include "logging.h" @@ -60,6 +66,13 @@ #define XEN_SCHED_CREDIT_NPARAM 2
static libxlDriverPrivatePtr libxl_driver = NULL; +static virThread migrate_receive_thread;
This prevents receiving concurrent migrations. Yes. It is a problem. Defined as "static" is to be used in Begin3 function virThreadCreate and in Finish3() function virThreadJoin, but actually the thread will exit when receiving data completed or meets error, so virThreadJoin doesn't have much effect here. If a call of virThreadJoin is not needed, then can specify migrate_receive_thread as a local variable.
About concurrent migrations, there is another problem in migration port. Every time a migration operation is issued, it will create a socket connection between source and host to transfer data. If a port specified, target side will create a socket and bind to that port, otherwise will bind to a DEFAULT_MIGRATION_PORT (current implementation). But if 1st migration occupies the DEFAULT_MIGRATION_PORT, then 2nd migration (without specifying port) will fail to bind to the DEFAULT_MIGRATION_PORT again. So, to ensure concurrent migrations, perhaps we need a group of ports for migration usage, every migration operation probes for a usable port. How do you think? Thanks, Chunyan
Chunyan Liu wrote:
"Chun Yan Liu" <cyliu@suse.com> 3/6/2012 2:29 PM >>>
I didn't get a chance to test this yet, but have some initial review comments.
Signed-off-by: Chunyan Liu --- src/libxl/libxl_driver.c | 617 ++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_driver.h | 17 ++- 2 files changed, 632 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d5fa64a..4037635 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -30,6 +30,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include
#include "internal.h" #include "logging.h" @@ -60,6 +66,13 @@ #define XEN_SCHED_CREDIT_NPARAM 2
static libxlDriverPrivatePtr libxl_driver = NULL; +static virThread migrate_receive_thread;
This prevents receiving concurrent migrations.
Yes. It is a problem. Defined as "static" is to be used in Begin3 function virThreadCreate and in Finish3() function virThreadJoin, but actually the thread will exit when receiving data completed or meets error, so virThreadJoin doesn't have much effect here. If a call of virThreadJoin is not needed, then can specify migrate_receive_thread as a local variable.
About concurrent migrations, there is another problem in migration port. Every time a migration operation is issued, it will create a socket connection between source and host to transfer data. If a port specified, target side will create a socket and bind to that port, otherwise will bind to a DEFAULT_MIGRATION_PORT (current implementation). But if 1st migration occupies the DEFAULT_MIGRATION_PORT, then 2nd migration (without specifying port) will fail to bind to the DEFAULT_MIGRATION_PORT again.
Ah yes, I noticed that but forgot to mention it - sorry.
So, to ensure concurrent migrations, perhaps we need a group of ports for migration usage, every migration operation probes for a usable port. How do you think?
You can use a virBitmap to keep track of used ports. The qemu driver uses a virBitmap to keep track of used vnc ports, e.g. see qemuProcessNextFreePort() in src/qemu/qemu_process.c. Perhaps the same range of ports qemu uses for migration (i.e. when a port is not specified by the user) can be used in the libxl driver, allowing firewalls and the like to be configured similarly between the two. Thanks, Jim
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! 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 */ 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); 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) + 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) +{ + 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; + } + + 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) { + 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 + * 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); + + 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); + 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; + 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); + 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 */ .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> + +#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) + +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); -- 1.7.3.4
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
2012/3/15 Jim Fehlig <jfehlig@suse.com>
Hi, Jim, I made some changes to the patch according to your comments: a. support concurrent migrations, add virBitmapPtr for probing migration
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
Chunyan Liu wrote: ports 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
Chunyan Liu wrote:
2012/3/15 Jim Fehlig <jfehlig@suse.com <mailto:jfehlig@suse.com>>
> + /* 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.
If the user is specifying the port, we should just use it and be done. That is how the VNC port is handled too. If user has specified a vnc port, then we just use it. Otherwise, call libxlNextFreeVncPort to find a free one.
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.
libxlFindFreeVNCPort only binds to the port to see if it is in use. If not in use, it closes the socket, sets the corresponding bit in the bitmap, and returns the port. Caller then knows the port is free and available for use, e.g. binding, listening, connecting, or whatever it pleases to do with the port. IMO, we could have static int libxlNextFreePort(virtBitmapPtr bitmap, int startPort, int numPorts) which is functionally equivalent to libxlNextFreeVncPort(), but examines startPort through startPort+numPorts. Thanks, Jim
2012/3/15 Jim Fehlig <jfehlig@suse.com>
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
Strange. Found that doing whatever operation that needs to open a connect
to the hypervisor (even "virsh version") will cause the same issue to live migration. While trying operation that calls do_open the connect, xc_shadow_control( in xc_domain_save) will fail. Non-live migration is OK.
Thanks! Jim
On Fri, Mar 09, 2012 at 06:55:55PM +0800, Chunyan Liu wrote:
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 +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; +}
Lets not re-invent URI parsing code with wierd special cases for base hostnames. Please just use virURIParse, since there is no compatibility issue here.
+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);
[snip]
+ 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; + }
[snip]
+ 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; + }
I know the QEMU code doesn't follow what I'm about to say, but I would prefer it if all this socket code was re-written to use the internal virNetSocketPtr APIs. In particular this would result in correct usage of getaddrinfo() which is lacking here.
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> + +#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) + +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";
It is better if these were in the .c file, or if they need to be shared across multiple driver files, use the libxl_conf.h Regards, 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 :|
2012/3/19 Daniel P. Berrange <berrange@redhat.com>
On Fri, Mar 09, 2012 at 06:55:55PM +0800, Chunyan Liu wrote:
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 +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; +}
Lets not re-invent URI parsing code with wierd special cases for base hostnames. Please just use virURIParse, since there is no compatibility issue here.
virURIParse can only parse full URI format like xenmigr://destIP:port, cannot handle simple URI like destIP:port correctly. Both xen_driver and qemu_driver have extra code like in doParseURI to handle the simple URI case. For libxl driver, currently use syntax: virsh migrate domU xen+ssh://destIP destIP[:port], so to parse hostname and port, need extra code to handle that but not virURIParse. And since there are several places that need to parse "port", so I extract the code to function doParseURI.
participants (3)
-
Chunyan Liu -
Daniel P. Berrange -
Jim Fehlig