Re: [libvirt] [PATCH] add migration APIs to libxl driver
---
src/libxl/libxl_conf.h |4 +
src/libxl/libxl_driver.c | 641
++
src/libxl/libxl_driver.h |5 +
3 files changed, 650 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 8ba0ee4..2041cc2 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -41,6 +41,9 @@
# define LIBXL_VNC_PORT_MIN 5900
# define LIBXL_VNC_PORT_MAX 65535
+# define LIBXL_MIGRATION_PORT_MIN 49152
+# define LIBXL_MIGRATION_PORT_MAX 49216
+
there is a overlap between vnc and migration port. althrought, it will try
next port in virPortAllocatorAcquire after bind fail.
# define LIBXL_CONFIG_DIR SYSCONFDIR /libvirt/libxl
# define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR /autostart
# define LIBXL_STATE_DIR LOCALSTATEDIR /run/libvirt/libxl
@@ -109,6 +112,7 @@ struct _libxlDriverPrivate {
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr reservedVNCPorts;
+virPortAllocatorPtr reservedMigPorts;
/* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e2a6d44..93b7153 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -32,6 +32,12 @@
#include libxl_utils.h
#include fcntl.h
#include regex.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 virlog.h
@@ -52,6 +58,7 @@
#include virsysinfo.h
#include viraccessapicheck.h
#include viratomic.h
+#include rpc/virnetsocket.h
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -69,6 +76,20 @@
static libxlDriverPrivatePtr libxl_driver = NULL;
+typedef struct _libxlMigrateReceiveArgs {
+virConnectPtr conn;
+virDomainObjPtr vm;
+
+/* for freeing listen sockets */
+virNetSocketPtr *socks;
+size_t nsocks;
+} libxlMigrateReceiveArgs;
+
+static const char libxlMigrateReceiverReady[]=
+libvirt libxl migration receiver ready, send binary domain data;
+static const char libxlMigrateReceiverFinish[]=
+domain received, ready to unpause;
+
/* Function declarations */
static int
libxlDomainManagedSaveLoad(virDomainObjPtr vm,
@@ -836,6 +857,12 @@ libxlStateInitialize(bool privileged,
LIBXL_VNC_PORT_MAX)))
goto error;
+/* Allocate bitmap for migration port reservation */
+if (!(libxl_driver-reservedMigPorts =
+ virPortAllocatorNew(LIBXL_MIGRATION_PORT_MIN,
+ LIBXL_MIGRATION_PORT_MAX)))
+goto error;
+
if (!(libxl_driver-domains = virDomainObjListNew()))
goto error;
@@ -4175,11 +4202,620 @@ libxlConnectSupportsFeature(virConnectPtr conn,
int feature)
switch (feature) {
case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
return 1;
+case VIR_DRV_FEATURE_MIGRATION_V3:
+return 1;
default:
return 0;
}
}
+static int
+libxlCheckMessageBanner(int fd, const char *banner, int banner_sz)
+{
+char buf[banner_sz];
+int ret = 0;
+
+do {
+ret = saferead(fd, buf, banner_sz);
+} while (ret == -1 errno == EAGAIN);
+
+if (ret != banner_sz || memcmp(buf, banner, banner_sz)) {
+return -1;
+}
+
+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;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainObjPtr vm;
+virDomainDefPtr def = NULL;
+char *xml = NULL;
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
+
+vm = virDomainObjListFindByUUID(driver-domains, domain-uuid);
+if (!vm) {
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(domain-uuid, uuidstr);
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(no domain with matching uuid '%s'), uuidstr);
+goto cleanup;
+}
libxlDomObjFromDomain is introduced in commit 0d87fd0aa by Jim.
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto cleanup;
+}
+
+if (virDomainMigrateBegin3EnsureACL(domain-conn, vm-def) 0)
+goto cleanup;
+
+if (xmlin) {
+if (!(def =
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
2012/3/19 Daniel P. Berrange berra...@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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
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-
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
2012/3/15 Jim Fehlig jfeh...@suse.com mailto:jfeh...@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Jim Fehlig wrote:
+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;
+char *servname = NULL;
+struct addrinfo hints, *res;
+int sockfd = -1;
+int ret = -1;
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
+
+libxlDriverLock(driver);
+
+if (doParseURI(uri, hostname, port))
+goto cleanup;
+
+VIR_DEBUG(hostname = %s, port = %d, hostname, port);
+
+if (virAsprintf(servname, %d, port ? port : DEFAULT_MIGRATION_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);
Hmm, the entire driver is locked during the migration. I just noticed
libxlDomainSave{Flags} also holds the driver lock during save :-(.
libxlDomainCoreDump drops the lock during the dump and IMO migration
should follow the same pattern.
After some more testing, following the pattern in libxlDomainCoreDump is
not a good idea as it leads to a deadlock.
# virsh dump dom /var/lib/libvirt/libxl/save/test.dump
d1. lock driver
d2. get virDomainObjPtr for domain, acquiring dom obj lock
d3. unlock driver
d4. core dump domain
d5. lock driver
d6. do post dump stuff
d7. unlock driver
d8. unlock dom obj lock
While d4 is in progress, list domains
# virsh list
l1. get num domains
l2. lock driver
l3. call virDomainObjListNumOfDomains, which iterates through domains,
obtaining dom obj lock in process
l3 will block when trying to obtain dom obj lock for the domain being
dumped, and note that it is holding the driver lock. When d4 is
finished, it will attempt to acquire the driver lock - deadlock. A
quick fix would be to lock the driver for duration of the dump, similar
to save. But we really need to prevent locking the driver during these
long running operations.
Question for other libvirt devs:
Many of the libxl driver functions use this pattern
- lock driver
- vm = virDomainFindByUUID // acquires dom obj lock
- unlock driver
- do stuff
- virDomainObjUnlock
In some cases, do stuff requires obtaining the driver lock (e.g.
removing a domain from driver's domain list), in which case there is
potential for deadlock. Any suggestions for preventing the deadlock
*and* avoiding locking the driver during long running operations?
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
On 03/16/2012 11:53 AM, Jim Fehlig wrote: Question for other libvirt devs: Many of the libxl driver functions use this pattern - lock driver - vm = virDomainFindByUUID // acquires dom obj lock - unlock driver - do stuff - virDomainObjUnlock In some cases, do stuff requires obtaining the driver lock (e.g. removing a domain from driver's domain list), in which case there is potential for deadlock. Any suggestions for preventing the deadlock *and* avoiding locking the driver during long running operations? See src/qemu/THREADS.txt. That gives the details on how to drop the driver lock and even the domain object lock, by maintaining a reference count and job condition on each domain to ensure that others can still use the driver, just not for the domain whose job is still occupied. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
2012/3/15 Jim Fehlig jfeh...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
Hi, Jim,
I made some changes to the patch according to your comments:
a. support concurrent migrations, add virBitmapPtr for probing migration ports
b. update doParseURI:
use virAsprintf instead of strdup and snprintf,
support migration URI syntax hostname[:port], remove xlmigr scheme
c. drop lock of driver while doing doMigrateSend
d. fix extra whitespace and parameter alignment and other places mentioned
Not changed:
a. ensure the name provided in xmlin is the same as def-name. Since we
support
domain name change, so I think the name could be different. Keep not
checked.
b. leaks the original name. It seems the original name won't be used again, so
didn't save original name. Keep it as before.
c. about migrate_receive_args. It's only used in dst side. If send logic and
receive logic still matches, extending the structure won't cause problem.
But if send logic and receive logic cannot match, there will problem. Still
think about how to handle it.
Any further comments will be very appreciated. Thanks for your time!
FYI, your mailer mangled this patch and I had to apply it manually.
Probably best to send with 'git send-email' and include a patch version
(e.g. V2) in the subject. See the archives for examples.
Signed-off-by: Chunyan Liu cy...@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 =
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
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 cy...@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,
+
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chun Yan Liu cy...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote: Chun Yan Liu cy...@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
Chunyan Liu wrote:
Add migration APIs for libxl driver.
Implemented in migration version 3. Based on xen 4.1.
I didn't get a chance to test this yet, but have some initial review
comments.
Signed-off-by: Chunyan Liu cy...@suse.com
---
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 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
@@ -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.
+
+typedef struct migrate_receive_args {
+virConnectPtr conn;
+virDomainObjPtr vm;
+int sockfd;
+} migrate_receive_args;
If there is a future need to extend this structure, will it cause
incompatibility issues between a source with the extensions and a
destination without? Or vise versa?
/* Function declarations */
static int
@@ -1095,6 +1108,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 +3866,600 @@ 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;
+char port[16] = 0;
+
+if (strstr (uri, //)) { /* Full URI. */
+virURIPtr uriptr = virURIParse(uri);
+if (!uriptr) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid URI));
+return -1;
+}
+if (uriptr-scheme STRCASENEQ(uriptr-scheme, xlmigr)) {
+libxlError(VIR_ERR_INVALID_ARG,
+_(Only xlmigr://
+ migrations are supported by Xen));
+xmlFreeURI(uriptr);
+return -1;
+}
This is just tcp migration. Any reason for requiring the xlmigr scheme?
+if (!uriptr-server) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(A hostname must be
+ specified in the URI));
+xmlFreeURI(uriptr);
+return -1;
+}
+hostname = strdup(uriptr-server);
I think it would be better to rework this, and other uses of strdup()
and snprintf() in this function, to use virAsprintf().
+if (!hostname) {
+virReportOOMError();
+xmlFreeURI(uriptr);
+return -1;
+}
+if (uriptr-port)
+snprintf(port, sizeof(port), %d, uriptr-port);
+xmlFreeURI(uriptr);
+}
+else if ((p = strrchr(uri, ':')) != NULL) { /* hostname:port */
+int port_nr, n;
+
+if (virStrToLong_i(p+1, NULL, 10, port_nr) 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid port number));
+return -1;
+}
+snprintf(port, sizeof(port), %d, port_nr);
+
+/* Get the hostname. */
+n = p - uri; /* n = Length of hostname in bytes. */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+hostname[n] = '\0';
+}
+else {/* hostname (or IP address) */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+}
+*p_hostname = hostname;
+*p_port = atoi(port);
+return 0;
+}
+
+static bool libxlMigrationFromIsAllowed(libxlDriverPrivatePtr driver,
virDomainObjPtr vm)
+{
+/* to be extended */
+VIR_DEBUG(driver=%p, vm=%p, driver, vm);
+return true;
+}
+
+static bool libxlMigrationToIsAllowed(libxlDriverPrivatePtr driver,
virDomainDefPtr def)
+{
+/* to be extended */
+VIR_DEBUG(driver=%p, def=%p, driver, def);
+return true;
+}
I think these functions (and their call sites) can be added in a future
patch that provides an implementation.
+
+static char *
Re: [libvirt] [PATCH] Add migration APIs for libxl driver
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.
+
+typedef struct migrate_receive_args {
+virConnectPtr conn;
+virDomainObjPtr vm;
+int sockfd;
+} migrate_receive_args;
If there is a future need to extend this structure, will it cause
incompatibility issues between a source with the extensions and a
destination without? Or vise versa?
It will if send logic and receive logic doesn't match. Maybe need to add some
extra check, but seems no better way to completely avoid that?
/* Function declarations */
static int
@@ -1095,6 +1108,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 +3866,600 @@ 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;
+char port[16] = 0;
+
+if (strstr (uri, //)) { /* Full URI. */
+virURIPtr uriptr = virURIParse(uri);
+if (!uriptr) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid URI));
+return -1;
+}
+if (uriptr-scheme STRCASENEQ(uriptr-scheme, xlmigr)) {
+libxlError(VIR_ERR_INVALID_ARG,
+_(Only xlmigr://
+ migrations are supported by Xen));
+xmlFreeURI(uriptr);
+return -1;
+}
This is just tcp migration. Any reason for requiring the xlmigr scheme?
It's not a necessary. Just refer to xen_driver syntax, migration uri could be
[xlmigr://]IP[:Port]. We can also define libxl own syntax, migration uri like
IP[:Port].
+if (!uriptr-server) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(A hostname must be
+ specified in the URI));
+xmlFreeURI(uriptr);
+return -1;
+}
+hostname = strdup(uriptr-server);
I think it would be better to rework this, and other uses of strdup()
and snprintf() in this function, to use virAsprintf().
+if (!hostname) {
+virReportOOMError();
+xmlFreeURI(uriptr);
+return -1;
+}
+if (uriptr-port)
+snprintf(port, sizeof(port), %d, uriptr-port);
+xmlFreeURI(uriptr);
+}
+else if ((p = strrchr(uri, ':')) != NULL) { /* hostname:port */
+int port_nr, n;
+
+if (virStrToLong_i(p+1, NULL, 10, port_nr) 0) {
+libxlError(VIR_ERR_INVALID_ARG,
+ _(Invalid port number));
+return -1;
+}
+snprintf(port, sizeof(port), %d, port_nr);
+
+/* Get the hostname. */
+n = p - uri; /* n = Length of hostname in bytes. */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+hostname[n] = '\0';
+}
+else {/* hostname (or IP address) */
+hostname = strdup(uri);
+if (!hostname) {
+virReportOOMError();
+return -1;
+}
+}
+*p_hostname = hostname;
+*p_port = atoi(port);
+return 0;
+}
+
+static bool libxlMigrationFromIsAllowed(libxlDriverPrivatePtr driver,
virDomainObjPtr vm)
+{
+
