Message ID | 1392690846-30926-1-git-send-email-mrhines@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) > From: "Michael R. Hines" <mrhines@us.ibm.com> > > 1. Fix small memory leak in parsing inet address from command line in data_init() > 2. Fix ibv_post_send() return value check and pass error code back up correctly. > 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > > Reported-by: frank.yangjie@gmail.com > Reported-by: dgilbert@redhat.com > Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > migration-rdma.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/migration-rdma.c b/migration-rdma.c > index f94f3b4..29351a6 100644 > --- a/migration-rdma.c > +++ b/migration-rdma.c > @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > } > > > - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > - return -1; > - } > + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > > - if (ret < 0) { > + if (ret > 0) { > fprintf(stderr, "Failed to use post IB SEND for control!\n"); > - return ret; > + return -ret; > } > > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > } > } > > - if (rdma->qp) { > - rdma_destroy_qp(rdma->cm_id); > - rdma->qp = NULL; > - } > if (rdma->cq) { > ibv_destroy_cq(rdma->cq); > rdma->cq = NULL; > @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > rdma->listen_id = NULL; > } > if (rdma->cm_id) { > + if (rdma->qp) { > + rdma_destroy_qp(rdma->cm_id); > + rdma->qp = NULL; > + } > rdma_destroy_id(rdma->cm_id); > rdma->cm_id = NULL; > } > @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > } else { > ERROR(errp, "bad RDMA migration address '%s'", host_port); > g_free(rdma); > - return NULL; > + rdma = NULL; > } > + > + qapi_free_InetSocketAddress(addr); > } > > return rdma; > -- > 1.8.1.2
Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) > From: "Michael R. Hines" <mrhines@us.ibm.com> > > 1. Fix small memory leak in parsing inet address from command line in data_init() > 2. Fix ibv_post_send() return value check and pass error code back up correctly. > 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > > Reported-by: frank.yangjie@gmail.com > Reported-by: dgilbert@redhat.com > Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> Ping: last call for 1.7.1 (freeze today) > --- > migration-rdma.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/migration-rdma.c b/migration-rdma.c > index f94f3b4..29351a6 100644 > --- a/migration-rdma.c > +++ b/migration-rdma.c > @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > } > > > - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > - return -1; > - } > + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > > - if (ret < 0) { > + if (ret > 0) { > fprintf(stderr, "Failed to use post IB SEND for control!\n"); > - return ret; > + return -ret; > } > > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > } > } > > - if (rdma->qp) { > - rdma_destroy_qp(rdma->cm_id); > - rdma->qp = NULL; > - } > if (rdma->cq) { > ibv_destroy_cq(rdma->cq); > rdma->cq = NULL; > @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > rdma->listen_id = NULL; > } > if (rdma->cm_id) { > + if (rdma->qp) { > + rdma_destroy_qp(rdma->cm_id); > + rdma->qp = NULL; > + } > rdma_destroy_id(rdma->cm_id); > rdma->cm_id = NULL; > } > @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > } else { > ERROR(errp, "bad RDMA migration address '%s'", host_port); > g_free(rdma); > - return NULL; > + rdma = NULL; > } > + > + qapi_free_InetSocketAddress(addr); > } > > return rdma; > -- > 1.8.1.2
I think that's correct. It doesn't meet cold filter criteria. Pelle -----Original Message----- From: qemu-stable-bounces+pelle=netapp.com@nongnu.org [mailto:qemu-stable-bounces+pelle=netapp.com@nongnu.org] On Behalf Of Michael Roth Sent: Thursday, February 27, 2014 10:49 AM To: mrhines@linux.vnet.ibm.com; qemu-devel@nongnu.org Cc: Michael R. Hines; hinesmr@cn.ibm.com; qemu-stable@nongnu.org; quintela@redhat.com Subject: Re: [Qemu-stable] [PATCH] rdma: bug fixes Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) > From: "Michael R. Hines" <mrhines@us.ibm.com> > > 1. Fix small memory leak in parsing inet address from command line in data_init() > 2. Fix ibv_post_send() return value check and pass error code back up correctly. > 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > > Reported-by: frank.yangjie@gmail.com > Reported-by: dgilbert@redhat.com > Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> Ping: last call for 1.7.1 (freeze today) > --- > migration-rdma.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/migration-rdma.c b/migration-rdma.c > index f94f3b4..29351a6 100644 > --- a/migration-rdma.c > +++ b/migration-rdma.c > @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > } > > > - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > - return -1; > - } > + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > > - if (ret < 0) { > + if (ret > 0) { > fprintf(stderr, "Failed to use post IB SEND for control!\n"); > - return ret; > + return -ret; > } > > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > } > } > > - if (rdma->qp) { > - rdma_destroy_qp(rdma->cm_id); > - rdma->qp = NULL; > - } > if (rdma->cq) { > ibv_destroy_cq(rdma->cq); > rdma->cq = NULL; > @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > rdma->listen_id = NULL; > } > if (rdma->cm_id) { > + if (rdma->qp) { > + rdma_destroy_qp(rdma->cm_id); > + rdma->qp = NULL; > + } > rdma_destroy_id(rdma->cm_id); > rdma->cm_id = NULL; > } > @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > } else { > ERROR(errp, "bad RDMA migration address '%s'", host_port); > g_free(rdma); > - return NULL; > + rdma = NULL; > } > + > + qapi_free_InetSocketAddress(addr); > } > > return rdma; > -- > 1.8.1.2
Quoting Wahlstrom, Pelle (2014-02-27 09:57:33) > I think that's correct. It doesn't meet cold filter criteria. Not sure I understand, what criteria are you referring to? > > Pelle > > -----Original Message----- > From: qemu-stable-bounces+pelle=netapp.com@nongnu.org [mailto:qemu-stable-bounces+pelle=netapp.com@nongnu.org] On Behalf Of Michael Roth > Sent: Thursday, February 27, 2014 10:49 AM > To: mrhines@linux.vnet.ibm.com; qemu-devel@nongnu.org > Cc: Michael R. Hines; hinesmr@cn.ibm.com; qemu-stable@nongnu.org; quintela@redhat.com > Subject: Re: [Qemu-stable] [PATCH] rdma: bug fixes > > Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) > > From: "Michael R. Hines" <mrhines@us.ibm.com> > > > > 1. Fix small memory leak in parsing inet address from command line in data_init() > > 2. Fix ibv_post_send() return value check and pass error code back up correctly. > > 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > > > > Reported-by: frank.yangjie@gmail.com > > Reported-by: dgilbert@redhat.com > > Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> > > Ping: last call for 1.7.1 (freeze today) > > > --- > > migration-rdma.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/migration-rdma.c b/migration-rdma.c > > index f94f3b4..29351a6 100644 > > --- a/migration-rdma.c > > +++ b/migration-rdma.c > > @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > > } > > > > > > - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > > - return -1; > > - } > > + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > > > > - if (ret < 0) { > > + if (ret > 0) { > > fprintf(stderr, "Failed to use post IB SEND for control!\n"); > > - return ret; > > + return -ret; > > } > > > > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > > @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > > } > > } > > > > - if (rdma->qp) { > > - rdma_destroy_qp(rdma->cm_id); > > - rdma->qp = NULL; > > - } > > if (rdma->cq) { > > ibv_destroy_cq(rdma->cq); > > rdma->cq = NULL; > > @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > > rdma->listen_id = NULL; > > } > > if (rdma->cm_id) { > > + if (rdma->qp) { > > + rdma_destroy_qp(rdma->cm_id); > > + rdma->qp = NULL; > > + } > > rdma_destroy_id(rdma->cm_id); > > rdma->cm_id = NULL; > > } > > @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > > } else { > > ERROR(errp, "bad RDMA migration address '%s'", host_port); > > g_free(rdma); > > - return NULL; > > + rdma = NULL; > > } > > + > > + qapi_free_InetSocketAddress(addr); > > } > > > > return rdma; > > -- > > 1.8.1.2
On 02/27/2014 11:49 PM, Michael Roth wrote: > Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) >> From: "Michael R. Hines" <mrhines@us.ibm.com> >> >> 1. Fix small memory leak in parsing inet address from command line in data_init() >> 2. Fix ibv_post_send() return value check and pass error code back up correctly. >> 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. >> >> Reported-by: frank.yangjie@gmail.com >> Reported-by: dgilbert@redhat.com >> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> > Ping: last call for 1.7.1 (freeze today) Is this ping for me or the maintainer? Did I miss anything? - Michael >> --- >> migration-rdma.c | 20 ++++++++++---------- >> 1 file changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/migration-rdma.c b/migration-rdma.c >> index f94f3b4..29351a6 100644 >> --- a/migration-rdma.c >> +++ b/migration-rdma.c >> @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, >> } >> >> >> - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { >> - return -1; >> - } >> + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); >> >> - if (ret < 0) { >> + if (ret > 0) { >> fprintf(stderr, "Failed to use post IB SEND for control!\n"); >> - return ret; >> + return -ret; >> } >> >> ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); >> @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) >> } >> } >> >> - if (rdma->qp) { >> - rdma_destroy_qp(rdma->cm_id); >> - rdma->qp = NULL; >> - } >> if (rdma->cq) { >> ibv_destroy_cq(rdma->cq); >> rdma->cq = NULL; >> @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) >> rdma->listen_id = NULL; >> } >> if (rdma->cm_id) { >> + if (rdma->qp) { >> + rdma_destroy_qp(rdma->cm_id); >> + rdma->qp = NULL; >> + } >> rdma_destroy_id(rdma->cm_id); >> rdma->cm_id = NULL; >> } >> @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) >> } else { >> ERROR(errp, "bad RDMA migration address '%s'", host_port); >> g_free(rdma); >> - return NULL; >> + rdma = NULL; >> } >> + >> + qapi_free_InetSocketAddress(addr); >> } >> >> return rdma; >> -- >> 1.8.1.2
Quoting Michael R. Hines (2014-03-26 22:14:27) > On 02/27/2014 11:49 PM, Michael Roth wrote: > > Quoting mrhines@linux.vnet.ibm.com (2014-02-17 20:34:06) > >> From: "Michael R. Hines" <mrhines@us.ibm.com> > >> > >> 1. Fix small memory leak in parsing inet address from command line in data_init() > >> 2. Fix ibv_post_send() return value check and pass error code back up correctly. > >> 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > >> > >> Reported-by: frank.yangjie@gmail.com > >> Reported-by: dgilbert@redhat.com > >> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> > > Ping: last call for 1.7.1 (freeze today) > > Is this ping for me or the maintainer? Did I miss anything? Just a ping for the maintainers, to try to get it committed upstream prior to stable freeze. Ship has sailed for 1.7.1, but hopefully we can get it in for 1.7.2: ping :) > > - Michael > > >> --- > >> migration-rdma.c | 20 ++++++++++---------- > >> 1 file changed, 10 insertions(+), 10 deletions(-) > >> > >> diff --git a/migration-rdma.c b/migration-rdma.c > >> index f94f3b4..29351a6 100644 > >> --- a/migration-rdma.c > >> +++ b/migration-rdma.c > >> @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > >> } > >> > >> > >> - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > >> - return -1; > >> - } > >> + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > >> > >> - if (ret < 0) { > >> + if (ret > 0) { > >> fprintf(stderr, "Failed to use post IB SEND for control!\n"); > >> - return ret; > >> + return -ret; > >> } > >> > >> ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > >> @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > >> } > >> } > >> > >> - if (rdma->qp) { > >> - rdma_destroy_qp(rdma->cm_id); > >> - rdma->qp = NULL; > >> - } > >> if (rdma->cq) { > >> ibv_destroy_cq(rdma->cq); > >> rdma->cq = NULL; > >> @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > >> rdma->listen_id = NULL; > >> } > >> if (rdma->cm_id) { > >> + if (rdma->qp) { > >> + rdma_destroy_qp(rdma->cm_id); > >> + rdma->qp = NULL; > >> + } > >> rdma_destroy_id(rdma->cm_id); > >> rdma->cm_id = NULL; > >> } > >> @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > >> } else { > >> ERROR(errp, "bad RDMA migration address '%s'", host_port); > >> g_free(rdma); > >> - return NULL; > >> + rdma = NULL; > >> } > >> + > >> + qapi_free_InetSocketAddress(addr); > >> } > >> > >> return rdma; > >> -- > >> 1.8.1.2
On 02/18/2014 10:34 AM, mrhines@linux.vnet.ibm.com wrote: > From: "Michael R. Hines" <mrhines@us.ibm.com> > > 1. Fix small memory leak in parsing inet address from command line in data_init() > 2. Fix ibv_post_send() return value check and pass error code back up correctly. > 3. Fix rdma_destroy_qp() segfault after failure to connect to destination. > > Reported-by: frank.yangjie@gmail.com > Reported-by: dgilbert@redhat.com > Signed-off-by: Michael R. Hines <mrhines@us.ibm.com> > --- > migration-rdma.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/migration-rdma.c b/migration-rdma.c > index f94f3b4..29351a6 100644 > --- a/migration-rdma.c > +++ b/migration-rdma.c > @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, > } > > > - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { > - return -1; > - } > + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); > > - if (ret < 0) { > + if (ret > 0) { > fprintf(stderr, "Failed to use post IB SEND for control!\n"); > - return ret; > + return -ret; > } > > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); > @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > } > } > > - if (rdma->qp) { > - rdma_destroy_qp(rdma->cm_id); > - rdma->qp = NULL; > - } > if (rdma->cq) { > ibv_destroy_cq(rdma->cq); > rdma->cq = NULL; > @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) > rdma->listen_id = NULL; > } > if (rdma->cm_id) { > + if (rdma->qp) { > + rdma_destroy_qp(rdma->cm_id); > + rdma->qp = NULL; > + } > rdma_destroy_id(rdma->cm_id); > rdma->cm_id = NULL; > } > @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) > } else { > ERROR(errp, "bad RDMA migration address '%s'", host_port); > g_free(rdma); > - return NULL; > + rdma = NULL; > } > + > + qapi_free_InetSocketAddress(addr); > } > > return rdma; Juan? Ping again? - Michael
"Michael R. Hines" <mrhines@linux.vnet.ibm.com> wrote: > On 02/18/2014 10:34 AM, mrhines@linux.vnet.ibm.com wrote: >> From: "Michael R. Hines" <mrhines@us.ibm.com> Applied, would go in next pull request. >> @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) >> } else { >> ERROR(errp, "bad RDMA migration address '%s'", host_port); >> g_free(rdma); >> - return NULL; >> + rdma = NULL; >> } >> + >> + qapi_free_InetSocketAddress(addr); >> } >> >> return rdma; > > Juan? Ping again? I obviously missed this one, sorry :-( Later, Juan.)
diff --git a/migration-rdma.c b/migration-rdma.c index f94f3b4..29351a6 100644 --- a/migration-rdma.c +++ b/migration-rdma.c @@ -1589,13 +1589,11 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf, } - if (ibv_post_send(rdma->qp, &send_wr, &bad_wr)) { - return -1; - } + ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr); - if (ret < 0) { + if (ret > 0) { fprintf(stderr, "Failed to use post IB SEND for control!\n"); - return ret; + return -ret; } ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_SEND_CONTROL, NULL); @@ -2237,10 +2235,6 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) } } - if (rdma->qp) { - rdma_destroy_qp(rdma->cm_id); - rdma->qp = NULL; - } if (rdma->cq) { ibv_destroy_cq(rdma->cq); rdma->cq = NULL; @@ -2258,6 +2252,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) rdma->listen_id = NULL; } if (rdma->cm_id) { + if (rdma->qp) { + rdma_destroy_qp(rdma->cm_id); + rdma->qp = NULL; + } rdma_destroy_id(rdma->cm_id); rdma->cm_id = NULL; } @@ -2512,8 +2510,10 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp) } else { ERROR(errp, "bad RDMA migration address '%s'", host_port); g_free(rdma); - return NULL; + rdma = NULL; } + + qapi_free_InetSocketAddress(addr); } return rdma;