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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list