* [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-02-16 7:50 ` Michael Tokarev
0 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-02-16 7:50 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Amit Shah, Michael Tokarev, Juan Quintela
Do not check for rdma->host being empty twice. This removes a large
"if" block, so code indentation is changed. While at it, remove an
ugly goto from the loop, replacing it with a cleaner if logic. And
finally, there's no need to initialize `ret' variable since is always
has a value.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
migration/rdma.c | 51 ++++++++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 6bee30c..76af724 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2366,10 +2366,10 @@ err_rdma_source_connect:
static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
{
- int ret = -EINVAL, idx;
+ int ret, idx;
struct rdma_cm_id *listen_id;
char ip[40] = "unknown";
- struct rdma_addrinfo *res;
+ struct rdma_addrinfo *res, *e;
char port_str[16];
for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
@@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
rdma->wr_data[idx].control_curr = NULL;
}
- if (rdma->host == NULL) {
+ if (!rdma->host || !rdma->host[0]) {
ERROR(errp, "RDMA host is not set!");
rdma->error_state = -EINVAL;
return -1;
@@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
snprintf(port_str, 16, "%d", rdma->port);
port_str[15] = '\0';
- if (rdma->host && strcmp("", rdma->host)) {
- struct rdma_addrinfo *e;
+ ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
+ if (ret < 0) {
+ ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
+ goto err_dest_init_bind_addr;
+ }
- ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
- if (ret < 0) {
- ERROR(errp, "could not rdma_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_dst_addr)->sin_addr, ip, sizeof ip);
+ trace_qemu_rdma_dest_init_trying(rdma->host, ip);
+ ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
+ if (ret) {
+ continue;
}
-
- for (e = res; e != NULL; e = e->ai_next) {
- inet_ntop(e->ai_family,
- &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
- trace_qemu_rdma_dest_init_trying(rdma->host, ip);
- ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
- if (!ret) {
- if (e->ai_family == AF_INET6) {
- ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
- if (ret) {
- continue;
- }
- }
-
- goto listen;
+ if (e->ai_family == AF_INET6) {
+ ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
+ if (ret) {
+ continue;
}
}
+ break;
+ }
+ if (!e) {
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;
}
-listen:
rdma->listen_id = listen_id;
qemu_rdma_dump_gid("dest_init", listen_id);
--
2.1.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-02-16 7:50 ` [Qemu-devel] " Michael Tokarev
@ 2015-02-19 10:25 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-02-19 10:25 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
This looks OK; have you got RDMA hardware to test with, if not I can
give it a go.
Dave
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> migration/rdma.c | 51 ++++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6bee30c..76af724 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2366,10 +2366,10 @@ err_rdma_source_connect:
>
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> - int ret = -EINVAL, idx;
> + int ret, idx;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> - struct rdma_addrinfo *res;
> + struct rdma_addrinfo *res, *e;
> char port_str[16];
>
> for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> @@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> rdma->wr_data[idx].control_curr = NULL;
> }
>
> - if (rdma->host == NULL) {
> + if (!rdma->host || !rdma->host[0]) {
> ERROR(errp, "RDMA host is not set!");
> rdma->error_state = -EINVAL;
> return -1;
> @@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> snprintf(port_str, 16, "%d", rdma->port);
> port_str[15] = '\0';
>
> - if (rdma->host && strcmp("", rdma->host)) {
> - struct rdma_addrinfo *e;
> + ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> + if (ret < 0) {
> + ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> + goto err_dest_init_bind_addr;
> + }
>
> - ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> - ERROR(errp, "could not rdma_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_dst_addr)->sin_addr, ip, sizeof ip);
> + trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> + ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> + if (ret) {
> + continue;
> }
> -
> - for (e = res; e != NULL; e = e->ai_next) {
> - inet_ntop(e->ai_family,
> - &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
> - trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> - ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> - if (!ret) {
> - if (e->ai_family == AF_INET6) {
> - ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> - if (ret) {
> - continue;
> - }
> - }
> -
> - goto listen;
> + if (e->ai_family == AF_INET6) {
> + ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> + if (ret) {
> + continue;
> }
> }
> + break;
> + }
>
> + if (!e) {
> 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;
> }
> -listen:
>
> rdma->listen_id = listen_id;
> qemu_rdma_dump_gid("dest_init", listen_id);
> --
> 2.1.4
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-02-19 10:25 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-02-19 10:25 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
This looks OK; have you got RDMA hardware to test with, if not I can
give it a go.
Dave
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> migration/rdma.c | 51 ++++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6bee30c..76af724 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2366,10 +2366,10 @@ err_rdma_source_connect:
>
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> - int ret = -EINVAL, idx;
> + int ret, idx;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> - struct rdma_addrinfo *res;
> + struct rdma_addrinfo *res, *e;
> char port_str[16];
>
> for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> @@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> rdma->wr_data[idx].control_curr = NULL;
> }
>
> - if (rdma->host == NULL) {
> + if (!rdma->host || !rdma->host[0]) {
> ERROR(errp, "RDMA host is not set!");
> rdma->error_state = -EINVAL;
> return -1;
> @@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> snprintf(port_str, 16, "%d", rdma->port);
> port_str[15] = '\0';
>
> - if (rdma->host && strcmp("", rdma->host)) {
> - struct rdma_addrinfo *e;
> + ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> + if (ret < 0) {
> + ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> + goto err_dest_init_bind_addr;
> + }
>
> - ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> - ERROR(errp, "could not rdma_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_dst_addr)->sin_addr, ip, sizeof ip);
> + trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> + ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> + if (ret) {
> + continue;
> }
> -
> - for (e = res; e != NULL; e = e->ai_next) {
> - inet_ntop(e->ai_family,
> - &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
> - trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> - ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> - if (!ret) {
> - if (e->ai_family == AF_INET6) {
> - ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> - if (ret) {
> - continue;
> - }
> - }
> -
> - goto listen;
> + if (e->ai_family == AF_INET6) {
> + ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> + if (ret) {
> + continue;
> }
> }
> + break;
> + }
>
> + if (!e) {
> 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;
> }
> -listen:
>
> rdma->listen_id = listen_id;
> qemu_rdma_dump_gid("dest_init", listen_id);
> --
> 2.1.4
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-trivial] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-02-19 10:25 ` Dr. David Alan Gilbert
@ 2015-02-28 9:26 ` Michael Tokarev
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-02-28 9:26 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
19.02.2015 13:25, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
>> Do not check for rdma->host being empty twice. This removes a large
>> "if" block, so code indentation is changed. While at it, remove an
>> ugly goto from the loop, replacing it with a cleaner if logic. And
>> finally, there's no need to initialize `ret' variable since is always
>> has a value.
>
> This looks OK; have you got RDMA hardware to test with, if not I can
> give it a go.
No, I don't have any RDMA hw, I only compile-tested this code, and
double-checked the changes does not change behavour.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-02-28 9:26 ` Michael Tokarev
0 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-02-28 9:26 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
19.02.2015 13:25, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
>> Do not check for rdma->host being empty twice. This removes a large
>> "if" block, so code indentation is changed. While at it, remove an
>> ugly goto from the loop, replacing it with a cleaner if logic. And
>> finally, there's no need to initialize `ret' variable since is always
>> has a value.
>
> This looks OK; have you got RDMA hardware to test with, if not I can
> give it a go.
No, I don't have any RDMA hw, I only compile-tested this code, and
double-checked the changes does not change behavour.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-02-16 7:50 ` [Qemu-devel] " Michael Tokarev
@ 2015-03-04 12:16 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-03-04 12:16 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Juan/Amit: There's a space at the end of a line that got in, so
that just needs fixing in the merge; it's after the 'break;'
but the patch is a big improvement on what's there:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
also tested on an RDMA system.
Thanks,
Dave
> ---
> migration/rdma.c | 51 ++++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6bee30c..76af724 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2366,10 +2366,10 @@ err_rdma_source_connect:
>
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> - int ret = -EINVAL, idx;
> + int ret, idx;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> - struct rdma_addrinfo *res;
> + struct rdma_addrinfo *res, *e;
> char port_str[16];
>
> for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> @@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> rdma->wr_data[idx].control_curr = NULL;
> }
>
> - if (rdma->host == NULL) {
> + if (!rdma->host || !rdma->host[0]) {
> ERROR(errp, "RDMA host is not set!");
> rdma->error_state = -EINVAL;
> return -1;
> @@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> snprintf(port_str, 16, "%d", rdma->port);
> port_str[15] = '\0';
>
> - if (rdma->host && strcmp("", rdma->host)) {
> - struct rdma_addrinfo *e;
> + ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> + if (ret < 0) {
> + ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> + goto err_dest_init_bind_addr;
> + }
>
> - ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> - ERROR(errp, "could not rdma_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_dst_addr)->sin_addr, ip, sizeof ip);
> + trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> + ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> + if (ret) {
> + continue;
> }
> -
> - for (e = res; e != NULL; e = e->ai_next) {
> - inet_ntop(e->ai_family,
> - &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
> - trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> - ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> - if (!ret) {
> - if (e->ai_family == AF_INET6) {
> - ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> - if (ret) {
> - continue;
> - }
> - }
> -
> - goto listen;
> + if (e->ai_family == AF_INET6) {
> + ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> + if (ret) {
> + continue;
> }
> }
> + break;
> + }
>
> + if (!e) {
> 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;
> }
> -listen:
>
> rdma->listen_id = listen_id;
> qemu_rdma_dump_gid("dest_init", listen_id);
> --
> 2.1.4
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-03-04 12:16 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-03-04 12:16 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Juan/Amit: There's a space at the end of a line that got in, so
that just needs fixing in the merge; it's after the 'break;'
but the patch is a big improvement on what's there:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
also tested on an RDMA system.
Thanks,
Dave
> ---
> migration/rdma.c | 51 ++++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6bee30c..76af724 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2366,10 +2366,10 @@ err_rdma_source_connect:
>
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> - int ret = -EINVAL, idx;
> + int ret, idx;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> - struct rdma_addrinfo *res;
> + struct rdma_addrinfo *res, *e;
> char port_str[16];
>
> for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> @@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> rdma->wr_data[idx].control_curr = NULL;
> }
>
> - if (rdma->host == NULL) {
> + if (!rdma->host || !rdma->host[0]) {
> ERROR(errp, "RDMA host is not set!");
> rdma->error_state = -EINVAL;
> return -1;
> @@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> snprintf(port_str, 16, "%d", rdma->port);
> port_str[15] = '\0';
>
> - if (rdma->host && strcmp("", rdma->host)) {
> - struct rdma_addrinfo *e;
> + ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> + if (ret < 0) {
> + ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> + goto err_dest_init_bind_addr;
> + }
>
> - ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> - ERROR(errp, "could not rdma_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_dst_addr)->sin_addr, ip, sizeof ip);
> + trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> + ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> + if (ret) {
> + continue;
> }
> -
> - for (e = res; e != NULL; e = e->ai_next) {
> - inet_ntop(e->ai_family,
> - &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
> - trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> - ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> - if (!ret) {
> - if (e->ai_family == AF_INET6) {
> - ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> - if (ret) {
> - continue;
> - }
> - }
> -
> - goto listen;
> + if (e->ai_family == AF_INET6) {
> + ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> + if (ret) {
> + continue;
> }
> }
> + break;
> + }
>
> + if (!e) {
> 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;
> }
> -listen:
>
> rdma->listen_id = listen_id;
> qemu_rdma_dump_gid("dest_init", listen_id);
> --
> 2.1.4
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-03-04 12:16 ` Dr. David Alan Gilbert
@ 2015-03-04 12:22 ` Michael Tokarev
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-03-04 12:22 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
04.03.2015 15:16, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
>> Do not check for rdma->host being empty twice. This removes a large
>> "if" block, so code indentation is changed. While at it, remove an
>> ugly goto from the loop, replacing it with a cleaner if logic. And
>> finally, there's no need to initialize `ret' variable since is always
>> has a value.
>>
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>
> Juan/Amit: There's a space at the end of a line that got in, so
> that just needs fixing in the merge; it's after the 'break;'
I already fixed it in my tree two days ago.
> but the patch is a big improvement on what's there:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> also tested on an RDMA system.
Thank you!
I think it's okay to merge it using the -trivial tree, I'm about to
send a pull request today.
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-03-04 12:22 ` Michael Tokarev
0 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-03-04 12:22 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
04.03.2015 15:16, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
>> Do not check for rdma->host being empty twice. This removes a large
>> "if" block, so code indentation is changed. While at it, remove an
>> ugly goto from the loop, replacing it with a cleaner if logic. And
>> finally, there's no need to initialize `ret' variable since is always
>> has a value.
>>
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>
> Juan/Amit: There's a space at the end of a line that got in, so
> that just needs fixing in the merge; it's after the 'break;'
I already fixed it in my tree two days ago.
> but the patch is a big improvement on what's there:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> also tested on an RDMA system.
Thank you!
I think it's okay to merge it using the -trivial tree, I'm about to
send a pull request today.
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-03-04 12:22 ` Michael Tokarev
@ 2015-03-04 13:32 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-03-04 13:32 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> 04.03.2015 15:16, Dr. David Alan Gilbert wrote:
> > * Michael Tokarev (mjt@tls.msk.ru) wrote:
> >> Do not check for rdma->host being empty twice. This removes a large
> >> "if" block, so code indentation is changed. While at it, remove an
> >> ugly goto from the loop, replacing it with a cleaner if logic. And
> >> finally, there's no need to initialize `ret' variable since is always
> >> has a value.
> >>
> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> >
> > Juan/Amit: There's a space at the end of a line that got in, so
> > that just needs fixing in the merge; it's after the 'break;'
>
> I already fixed it in my tree two days ago.
>
> > but the patch is a big improvement on what's there:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > also tested on an RDMA system.
>
> Thank you!
>
> I think it's okay to merge it using the -trivial tree, I'm about to
> send a pull request today.
Yes, OK, although I'd generally prefer the RDMA stuff to go
through migration unless it's really trivial.
Dave
>
> /mjt
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-03-04 13:32 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2015-03-04 13:32 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
* Michael Tokarev (mjt@tls.msk.ru) wrote:
> 04.03.2015 15:16, Dr. David Alan Gilbert wrote:
> > * Michael Tokarev (mjt@tls.msk.ru) wrote:
> >> Do not check for rdma->host being empty twice. This removes a large
> >> "if" block, so code indentation is changed. While at it, remove an
> >> ugly goto from the loop, replacing it with a cleaner if logic. And
> >> finally, there's no need to initialize `ret' variable since is always
> >> has a value.
> >>
> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> >
> > Juan/Amit: There's a space at the end of a line that got in, so
> > that just needs fixing in the merge; it's after the 'break;'
>
> I already fixed it in my tree two days ago.
>
> > but the patch is a big improvement on what's there:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > also tested on an RDMA system.
>
> Thank you!
>
> I think it's okay to merge it using the -trivial tree, I'm about to
> send a pull request today.
Yes, OK, although I'd generally prefer the RDMA stuff to go
through migration unless it's really trivial.
Dave
>
> /mjt
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-trivial] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-03-04 13:32 ` Dr. David Alan Gilbert
@ 2015-03-04 13:34 ` Michael Tokarev
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-03-04 13:34 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
04.03.2015 16:32, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
[]
>> I think it's okay to merge it using the -trivial tree, I'm about to
>> send a pull request today.
>
> Yes, OK, although I'd generally prefer the RDMA stuff to go
> through migration unless it's really trivial.
I've no prob with either of these. It is just that I'm preparing
a pull request for -trivial tree now... ;) I can drop the patch
from there.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-03-04 13:34 ` Michael Tokarev
0 siblings, 0 replies; 16+ messages in thread
From: Michael Tokarev @ 2015-03-04 13:34 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Amit Shah, qemu-devel, Juan Quintela
04.03.2015 16:32, Dr. David Alan Gilbert wrote:
> * Michael Tokarev (mjt@tls.msk.ru) wrote:
[]
>> I think it's okay to merge it using the -trivial tree, I'm about to
>> send a pull request today.
>
> Yes, OK, although I'd generally prefer the RDMA stuff to go
> through migration unless it's really trivial.
I've no prob with either of these. It is just that I'm preparing
a pull request for -trivial tree now... ;) I can drop the patch
from there.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-trivial] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
2015-02-16 7:50 ` [Qemu-devel] " Michael Tokarev
@ 2015-03-17 13:10 ` Juan Quintela
-1 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2015-03-17 13:10 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel
Michael Tokarev <mjt@tls.msk.ru> wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Applied, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit
@ 2015-03-17 13:10 ` Juan Quintela
0 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2015-03-17 13:10 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, Amit Shah, qemu-devel
Michael Tokarev <mjt@tls.msk.ru> wrote:
> Do not check for rdma->host being empty twice. This removes a large
> "if" block, so code indentation is changed. While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic. And
> finally, there's no need to initialize `ret' variable since is always
> has a value.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Applied, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread