* [Qemu-devel] [PATCH for-1.6 1/4] rdma: use resp.len after validation in qemu_rdma_registration_stop
2013-08-07 16:05 [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling mrhines
@ 2013-08-07 16:05 ` mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 2/4] rdma: validate RDMAControlHeader::len mrhines
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2013-08-07 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, aliguori, quintela, owasserm, mrhines, pbonzini
From: Isaku Yamahata <yamahata@private.email.ne.jp>
resp.len is given from remote host. So should be validated before use.
Otherwise memcpy can access beyond the buffer.
Cc: Michael R. Hines <mrhines@us.ibm.com>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
migration-rdma.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/migration-rdma.c b/migration-rdma.c
index 3a380d4..6721266 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -3045,10 +3045,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
return ret;
}
- qemu_rdma_move_header(rdma, reg_result_idx, &resp);
- memcpy(rdma->block,
- rdma->wr_data[reg_result_idx].control_curr, resp.len);
-
nb_remote_blocks = resp.len / sizeof(RDMARemoteBlock);
/*
@@ -3070,6 +3066,9 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
return -EINVAL;
}
+ qemu_rdma_move_header(rdma, reg_result_idx, &resp);
+ memcpy(rdma->block,
+ rdma->wr_data[reg_result_idx].control_curr, resp.len);
for (i = 0; i < nb_remote_blocks; i++) {
network_to_remote_block(&rdma->block[i]);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH for-1.6 2/4] rdma: validate RDMAControlHeader::len
2013-08-07 16:05 [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 1/4] rdma: use resp.len after validation in qemu_rdma_registration_stop mrhines
@ 2013-08-07 16:05 ` mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 3/4] rdma: check if RDMAControlHeader::len match transferred byte mrhines
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2013-08-07 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, aliguori, quintela, owasserm, mrhines, pbonzini
From: Isaku Yamahata <yamahata@private.email.ne.jp>
RMDAControlHeader::len is provided from remote, so validate it.
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
migration-rdma.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/migration-rdma.c b/migration-rdma.c
index 6721266..ebe1f55 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -1424,6 +1424,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf,
* The copy makes the RDMAControlHeader simpler to manipulate
* for the time being.
*/
+ assert(head->len <= RDMA_CONTROL_MAX_BUFFER - sizeof(*head));
memcpy(wr->control, head, sizeof(RDMAControlHeader));
control_to_network((void *) wr->control);
@@ -1504,6 +1505,10 @@ static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
control_desc[head->type], head->type, head->len);
return -EIO;
}
+ if (head->len > RDMA_CONTROL_MAX_BUFFER - sizeof(*head)) {
+ fprintf(stderr, "too long length: %d\n", head->len);
+ return -EINVAL;
+ }
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH for-1.6 3/4] rdma: check if RDMAControlHeader::len match transferred byte
2013-08-07 16:05 [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 1/4] rdma: use resp.len after validation in qemu_rdma_registration_stop mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 2/4] rdma: validate RDMAControlHeader::len mrhines
@ 2013-08-07 16:05 ` mrhines
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling mrhines
2013-08-14 16:27 ` [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, " Anthony Liguori
4 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2013-08-07 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, aliguori, quintela, owasserm, mrhines, pbonzini
From: Isaku Yamahata <yamahata@private.email.ne.jp>
RDMAControlHeader::len is provided from remote, so check if the value
match the actual transferred byte_len.
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp>
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
migration-rdma.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/migration-rdma.c b/migration-rdma.c
index ebe1f55..30e08cd 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -1214,7 +1214,8 @@ static void qemu_rdma_signal_unregister(RDMAContext *rdma, uint64_t index,
* (of any kind) has completed.
* Return the work request ID that completed.
*/
-static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out)
+static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out,
+ uint32_t *byte_len)
{
int ret;
struct ibv_wc wc;
@@ -1285,6 +1286,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out)
}
*wr_id_out = wc.wr_id;
+ if (byte_len) {
+ *byte_len = wc.byte_len;
+ }
return 0;
}
@@ -1302,7 +1306,8 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out)
* completions only need to be recorded, but do not actually
* need further processing.
*/
-static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested)
+static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested,
+ uint32_t *byte_len)
{
int num_cq_events = 0, ret = 0;
struct ibv_cq *cq;
@@ -1314,7 +1319,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested)
}
/* poll cq first */
while (wr_id != wrid_requested) {
- ret = qemu_rdma_poll(rdma, &wr_id_in);
+ ret = qemu_rdma_poll(rdma, &wr_id_in, byte_len);
if (ret < 0) {
return ret;
}
@@ -1356,7 +1361,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested)
}
while (wr_id != wrid_requested) {
- ret = qemu_rdma_poll(rdma, &wr_id_in);
+ ret = qemu_rdma_poll(rdma, &wr_id_in, byte_len);
if (ret < 0) {
goto err_block_for_wrid;
}
@@ -1442,7 +1447,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf,
return ret;
}
- ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL);
+ ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL);
if (ret < 0) {
fprintf(stderr, "rdma migration: send polling control error!\n");
}
@@ -1483,7 +1488,9 @@ static int qemu_rdma_post_recv_control(RDMAContext *rdma, int idx)
static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
RDMAControlHeader *head, int expecting, int idx)
{
- int ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RECV_CONTROL + idx);
+ uint32_t byte_len;
+ int ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RECV_CONTROL + idx,
+ &byte_len);
if (ret < 0) {
fprintf(stderr, "rdma migration: recv polling control error!\n");
@@ -1509,6 +1516,11 @@ static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
fprintf(stderr, "too long length: %d\n", head->len);
return -EINVAL;
}
+ if (sizeof(*head) + head->len != byte_len) {
+ fprintf(stderr, "Malformed length: %d byte_len %d\n",
+ head->len, byte_len);
+ return -EINVAL;
+ }
return 0;
}
@@ -1738,7 +1750,7 @@ retry:
count++, current_index, chunk,
sge.addr, length, rdma->nb_sent, block->nb_chunks);
- ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
if (ret < 0) {
fprintf(stderr, "Failed to Wait for previous write to complete "
@@ -1882,7 +1894,7 @@ retry:
if (ret == ENOMEM) {
DDPRINTF("send queue is full. wait a little....\n");
- ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
if (ret < 0) {
fprintf(stderr, "rdma migration: failed to make "
"room in full send queue! %d\n", ret);
@@ -2471,7 +2483,7 @@ static int qemu_rdma_drain_cq(QEMUFile *f, RDMAContext *rdma)
}
while (rdma->nb_sent) {
- ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE);
+ ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
if (ret < 0) {
fprintf(stderr, "rdma migration: complete polling error!\n");
return -EIO;
@@ -2607,7 +2619,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
*/
while (1) {
uint64_t wr_id, wr_id_in;
- int ret = qemu_rdma_poll(rdma, &wr_id_in);
+ int ret = qemu_rdma_poll(rdma, &wr_id_in, NULL);
if (ret < 0) {
fprintf(stderr, "rdma migration: polling error! %d\n", ret);
goto err;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling
2013-08-07 16:05 [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling mrhines
` (2 preceding siblings ...)
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 3/4] rdma: check if RDMAControlHeader::len match transferred byte mrhines
@ 2013-08-07 16:05 ` mrhines
2013-08-08 6:12 ` Orit Wasserman
2013-08-14 16:27 ` [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, " Anthony Liguori
4 siblings, 1 reply; 8+ messages in thread
From: mrhines @ 2013-08-07 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, aliguori, quintela, owasserm, mrhines, pbonzini
From: "Michael R. Hines" <mrhines@us.ibm.com>
getaddrinfo() already knows what it's doing,
wqand can potentially return multiple addresses.
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
migration-rdma.c | 56 ++++++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 27 deletions(-)
diff --git a/migration-rdma.c b/migration-rdma.c
index 30e08cd..d71cca5 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -392,7 +392,6 @@ typedef struct RDMAContext {
uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
GHashTable *blockmap;
- bool ipv6;
} RDMAContext;
/*
@@ -745,7 +744,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
char port_str[16];
struct rdma_cm_event *cm_event;
char ip[40] = "unknown";
- int af = rdma->ipv6 ? PF_INET6 : PF_INET;
+ struct addrinfo *e;
if (rdma->host == NULL || !strcmp(rdma->host, "")) {
ERROR(errp, "RDMA hostname has not been set");
@@ -775,18 +774,23 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
goto err_resolve_get_addr;
}
- inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
- ip, sizeof ip);
- DPRINTF("%s => %s\n", rdma->host, ip);
+ for (e = res; e != NULL; e = e->ai_next) {
+ inet_ntop(e->ai_family,
+ &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
+ DPRINTF("Trying %s => %s\n", rdma->host, ip);
- /* resolve the first address */
- ret = rdma_resolve_addr(rdma->cm_id, NULL, res->ai_addr,
- RDMA_RESOLVE_TIMEOUT_MS);
- if (ret) {
- ERROR(errp, "could not resolve address %s", rdma->host);
- goto err_resolve_get_addr;
+ /* resolve the first address */
+ ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_addr,
+ RDMA_RESOLVE_TIMEOUT_MS);
+ if (!ret) {
+ goto route;
+ }
}
+ ERROR(errp, "could not resolve address %s", rdma->host);
+ goto err_resolve_get_addr;
+
+route:
qemu_rdma_dump_gid("source_resolve_addr", rdma->cm_id);
ret = rdma_get_cm_event(rdma->channel, &cm_event);
@@ -2260,8 +2264,6 @@ err_rdma_source_connect:
static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
{
int ret = -EINVAL, idx;
- int af = rdma->ipv6 ? PF_INET6 : PF_INET;
- struct sockaddr_in sin;
struct rdma_cm_id *listen_id;
char ip[40] = "unknown";
struct addrinfo *res;
@@ -2292,35 +2294,36 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
goto err_dest_init_create_listen_id;
}
- memset(&sin, 0, sizeof(sin));
- sin.sin_family = af;
- sin.sin_port = htons(rdma->port);
snprintf(port_str, 16, "%d", rdma->port);
port_str[15] = '\0';
if (rdma->host && strcmp("", rdma->host)) {
+ struct addrinfo *e;
+
ret = getaddrinfo(rdma->host, port_str, NULL, &res);
if (ret < 0) {
ERROR(errp, "could not getaddrinfo address %s", rdma->host);
goto err_dest_init_bind_addr;
}
+ for (e = res; e != NULL; e = e->ai_next) {
+ inet_ntop(e->ai_family,
+ &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
+ DPRINTF("Trying %s => %s\n", rdma->host, ip);
+ ret = rdma_bind_addr(listen_id, e->ai_addr);
+ if (!ret) {
+ goto listen;
+ }
+ }
- inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
- ip, sizeof ip);
+ ERROR(errp, "Error: could not rdma_bind_addr!");
+ goto err_dest_init_bind_addr;
} else {
ERROR(errp, "migration host and port not specified!");
ret = -EINVAL;
goto err_dest_init_bind_addr;
}
-
- DPRINTF("%s => %s\n", rdma->host, ip);
-
- ret = rdma_bind_addr(listen_id, res->ai_addr);
- if (ret) {
- ERROR(errp, "Error: could not rdma_bind_addr!");
- goto err_dest_init_bind_addr;
- }
+listen:
rdma->listen_id = listen_id;
qemu_rdma_dump_gid("dest_init", listen_id);
@@ -2351,7 +2354,6 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
if (addr != NULL) {
rdma->port = atoi(addr->port);
rdma->host = g_strdup(addr->host);
- rdma->ipv6 = addr->ipv6;
} else {
ERROR(errp, "bad RDMA migration address '%s'", host_port);
g_free(rdma);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling mrhines
@ 2013-08-08 6:12 ` Orit Wasserman
2013-08-08 14:44 ` Michael R. Hines
0 siblings, 1 reply; 8+ messages in thread
From: Orit Wasserman @ 2013-08-08 6:12 UTC (permalink / raw)
To: mrhines; +Cc: yamahata, aliguori, quintela, qemu-devel, mrhines, pbonzini
On 08/07/2013 07:05 PM, mrhines@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
>
> getaddrinfo() already knows what it's doing,
> wqand can potentially return multiple addresses.
>
> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
> ---
> migration-rdma.c | 56 ++++++++++++++++++++++++++++--------------------------
> 1 file changed, 29 insertions(+), 27 deletions(-)
>
> diff --git a/migration-rdma.c b/migration-rdma.c
> index 30e08cd..d71cca5 100644
> --- a/migration-rdma.c
> +++ b/migration-rdma.c
> @@ -392,7 +392,6 @@ typedef struct RDMAContext {
> uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
>
> GHashTable *blockmap;
> - bool ipv6;
> } RDMAContext;
>
> /*
> @@ -745,7 +744,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
> char port_str[16];
> struct rdma_cm_event *cm_event;
> char ip[40] = "unknown";
> - int af = rdma->ipv6 ? PF_INET6 : PF_INET;
> + struct addrinfo *e;
>
> if (rdma->host == NULL || !strcmp(rdma->host, "")) {
> ERROR(errp, "RDMA hostname has not been set");
> @@ -775,18 +774,23 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
> goto err_resolve_get_addr;
> }
>
> - inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
> - ip, sizeof ip);
> - DPRINTF("%s => %s\n", rdma->host, ip);
> + for (e = res; e != NULL; e = e->ai_next) {
> + inet_ntop(e->ai_family,
> + &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
> + DPRINTF("Trying %s => %s\n", rdma->host, ip);
>
> - /* resolve the first address */
> - ret = rdma_resolve_addr(rdma->cm_id, NULL, res->ai_addr,
> - RDMA_RESOLVE_TIMEOUT_MS);
> - if (ret) {
> - ERROR(errp, "could not resolve address %s", rdma->host);
> - goto err_resolve_get_addr;
> + /* resolve the first address */
> + ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_addr,
> + RDMA_RESOLVE_TIMEOUT_MS);
> + if (!ret) {
> + goto route;
> + }
> }
>
> + ERROR(errp, "could not resolve address %s", rdma->host);
> + goto err_resolve_get_addr;
> +
> +route:
> qemu_rdma_dump_gid("source_resolve_addr", rdma->cm_id);
>
> ret = rdma_get_cm_event(rdma->channel, &cm_event);
> @@ -2260,8 +2264,6 @@ err_rdma_source_connect:
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> int ret = -EINVAL, idx;
> - int af = rdma->ipv6 ? PF_INET6 : PF_INET;
> - struct sockaddr_in sin;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> struct addrinfo *res;
> @@ -2292,35 +2294,36 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> goto err_dest_init_create_listen_id;
> }
>
> - memset(&sin, 0, sizeof(sin));
> - sin.sin_family = af;
> - sin.sin_port = htons(rdma->port);
> snprintf(port_str, 16, "%d", rdma->port);
> port_str[15] = '\0';
>
> if (rdma->host && strcmp("", rdma->host)) {
> + struct addrinfo *e;
> +
> ret = getaddrinfo(rdma->host, port_str, NULL, &res);
> if (ret < 0) {
> ERROR(errp, "could not getaddrinfo address %s", rdma->host);
> goto err_dest_init_bind_addr;
> }
>
> + for (e = res; e != NULL; e = e->ai_next) {
> + inet_ntop(e->ai_family,
> + &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
> + DPRINTF("Trying %s => %s\n", rdma->host, ip);
> + ret = rdma_bind_addr(listen_id, e->ai_addr);
> + if (!ret) {
> + goto listen;
> + }
> + }
>
> - inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
> - ip, sizeof ip);
> + ERROR(errp, "Error: could not rdma_bind_addr!");
> + goto err_dest_init_bind_addr;
> } else {
> ERROR(errp, "migration host and port not specified!");
> ret = -EINVAL;
> goto err_dest_init_bind_addr;
> }
> -
> - DPRINTF("%s => %s\n", rdma->host, ip);
> -
> - ret = rdma_bind_addr(listen_id, res->ai_addr);
> - if (ret) {
> - ERROR(errp, "Error: could not rdma_bind_addr!");
> - goto err_dest_init_bind_addr;
> - }
> +listen:
>
> rdma->listen_id = listen_id;
> qemu_rdma_dump_gid("dest_init", listen_id);
> @@ -2351,7 +2354,6 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
> if (addr != NULL) {
> rdma->port = atoi(addr->port);
> rdma->host = g_strdup(addr->host);
> - rdma->ipv6 = addr->ipv6;
> } else {
> ERROR(errp, "bad RDMA migration address '%s'", host_port);
> g_free(rdma);
>
Looks good,
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling
2013-08-08 6:12 ` Orit Wasserman
@ 2013-08-08 14:44 ` Michael R. Hines
0 siblings, 0 replies; 8+ messages in thread
From: Michael R. Hines @ 2013-08-08 14:44 UTC (permalink / raw)
To: qemu-devel
On 08/08/2013 02:12 AM, Orit Wasserman wrote:
> On 08/07/2013 07:05 PM, mrhines@linux.vnet.ibm.com wrote:
>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>
>> getaddrinfo() already knows what it's doing,
>> wqand can potentially return multiple addresses.
>>
>> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
>> ---
>> migration-rdma.c | 56 ++++++++++++++++++++++++++++--------------------------
>> 1 file changed, 29 insertions(+), 27 deletions(-)
>>
>> diff --git a/migration-rdma.c b/migration-rdma.c
>> index 30e08cd..d71cca5 100644
>> --- a/migration-rdma.c
>> +++ b/migration-rdma.c
>> @@ -392,7 +392,6 @@ typedef struct RDMAContext {
>> uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
>>
>> GHashTable *blockmap;
>> - bool ipv6;
>> } RDMAContext;
>>
>> /*
>> @@ -745,7 +744,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
>> char port_str[16];
>> struct rdma_cm_event *cm_event;
>> char ip[40] = "unknown";
>> - int af = rdma->ipv6 ? PF_INET6 : PF_INET;
>> + struct addrinfo *e;
>>
>> if (rdma->host == NULL || !strcmp(rdma->host, "")) {
>> ERROR(errp, "RDMA hostname has not been set");
>> @@ -775,18 +774,23 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
>> goto err_resolve_get_addr;
>> }
>>
>> - inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
>> - ip, sizeof ip);
>> - DPRINTF("%s => %s\n", rdma->host, ip);
>> + for (e = res; e != NULL; e = e->ai_next) {
>> + inet_ntop(e->ai_family,
>> + &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
>> + DPRINTF("Trying %s => %s\n", rdma->host, ip);
>>
>> - /* resolve the first address */
>> - ret = rdma_resolve_addr(rdma->cm_id, NULL, res->ai_addr,
>> - RDMA_RESOLVE_TIMEOUT_MS);
>> - if (ret) {
>> - ERROR(errp, "could not resolve address %s", rdma->host);
>> - goto err_resolve_get_addr;
>> + /* resolve the first address */
>> + ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_addr,
>> + RDMA_RESOLVE_TIMEOUT_MS);
>> + if (!ret) {
>> + goto route;
>> + }
>> }
>>
>> + ERROR(errp, "could not resolve address %s", rdma->host);
>> + goto err_resolve_get_addr;
>> +
>> +route:
>> qemu_rdma_dump_gid("source_resolve_addr", rdma->cm_id);
>>
>> ret = rdma_get_cm_event(rdma->channel, &cm_event);
>> @@ -2260,8 +2264,6 @@ err_rdma_source_connect:
>> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
>> {
>> int ret = -EINVAL, idx;
>> - int af = rdma->ipv6 ? PF_INET6 : PF_INET;
>> - struct sockaddr_in sin;
>> struct rdma_cm_id *listen_id;
>> char ip[40] = "unknown";
>> struct addrinfo *res;
>> @@ -2292,35 +2294,36 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
>> goto err_dest_init_create_listen_id;
>> }
>>
>> - memset(&sin, 0, sizeof(sin));
>> - sin.sin_family = af;
>> - sin.sin_port = htons(rdma->port);
>> snprintf(port_str, 16, "%d", rdma->port);
>> port_str[15] = '\0';
>>
>> if (rdma->host && strcmp("", rdma->host)) {
>> + struct addrinfo *e;
>> +
>> ret = getaddrinfo(rdma->host, port_str, NULL, &res);
>> if (ret < 0) {
>> ERROR(errp, "could not getaddrinfo address %s", rdma->host);
>> goto err_dest_init_bind_addr;
>> }
>>
>> + for (e = res; e != NULL; e = e->ai_next) {
>> + inet_ntop(e->ai_family,
>> + &((struct sockaddr_in *) e->ai_addr)->sin_addr, ip, sizeof ip);
>> + DPRINTF("Trying %s => %s\n", rdma->host, ip);
>> + ret = rdma_bind_addr(listen_id, e->ai_addr);
>> + if (!ret) {
>> + goto listen;
>> + }
>> + }
>>
>> - inet_ntop(af, &((struct sockaddr_in *) res->ai_addr)->sin_addr,
>> - ip, sizeof ip);
>> + ERROR(errp, "Error: could not rdma_bind_addr!");
>> + goto err_dest_init_bind_addr;
>> } else {
>> ERROR(errp, "migration host and port not specified!");
>> ret = -EINVAL;
>> goto err_dest_init_bind_addr;
>> }
>> -
>> - DPRINTF("%s => %s\n", rdma->host, ip);
>> -
>> - ret = rdma_bind_addr(listen_id, res->ai_addr);
>> - if (ret) {
>> - ERROR(errp, "Error: could not rdma_bind_addr!");
>> - goto err_dest_init_bind_addr;
>> - }
>> +listen:
>>
>> rdma->listen_id = listen_id;
>> qemu_rdma_dump_gid("dest_init", listen_id);
>> @@ -2351,7 +2354,6 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
>> if (addr != NULL) {
>> rdma->port = atoi(addr->port);
>> rdma->host = g_strdup(addr->host);
>> - rdma->ipv6 = addr->ipv6;
>> } else {
>> ERROR(errp, "bad RDMA migration address '%s'", host_port);
>> g_free(rdma);
>>
> Looks good,
> Reviewed-by: Orit Wasserman <owasserm@redhat.com>
>
>
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling
2013-08-07 16:05 [Qemu-devel] [PATCH for-1.6 0/4] rdma: additional cleanups, proper getaddrinfo() handling mrhines
` (3 preceding siblings ...)
2013-08-07 16:05 ` [Qemu-devel] [PATCH for-1.6 4/4] rdma: proper getaddrinfo() handling mrhines
@ 2013-08-14 16:27 ` Anthony Liguori
4 siblings, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2013-08-14 16:27 UTC (permalink / raw)
To: mrhines, qemu-devel
Cc: yamahata, aliguori, quintela, owasserm, mrhines, pbonzini
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 8+ messages in thread