| Message ID | 1487147657-166092-4-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/15/2017 04:34 PM, zhanghailiang wrote: > We should release all unhandled packets before finalize colo compare. > Besides, we need to free connection_track_table, or there will be > a memory leak bug. > > Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com> > --- > net/colo-compare.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index a16e2d5..809bad3 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) > return; > } > This function in my patch "colo-compare and filter-rewriter work with colo-frame " Named 'colo_flush_connection', I think use 'flush' instead of 'release' is better, Thanks Zhang Chen > +static void colo_release_packets(void *opaque, void *user_data) > +{ > + CompareState *s = user_data; > + Connection *conn = opaque; > + Packet *pkt = NULL; > + > + while (!g_queue_is_empty(&conn->primary_list)) { > + pkt = g_queue_pop_head(&conn->primary_list); > + compare_chr_send(&s->chr_out, pkt->data, pkt->size); > + packet_destroy(pkt, NULL); > + } > + while (!g_queue_is_empty(&conn->secondary_list)) { > + pkt = g_queue_pop_head(&conn->secondary_list); > + packet_destroy(pkt, NULL); > + } > +} > + > static void colo_compare_class_init(ObjectClass *oc, void *data) > { > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) > g_main_loop_quit(s->compare_loop); > qemu_thread_join(&s->thread); > > + /* Release all unhandled packets after compare thead exited */ > + g_queue_foreach(&s->conn_list, colo_release_packets, s); > > g_queue_free(&s->conn_list); > > + g_hash_table_destroy(s->connection_track_table); > g_free(s->pri_indev); > g_free(s->sec_indev); > g_free(s->outdev);
On 2017年02月15日 16:34, zhanghailiang wrote: > We should release all unhandled packets before finalize colo compare. > Besides, we need to free connection_track_table, or there will be > a memory leak bug. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > net/colo-compare.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index a16e2d5..809bad3 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) > return; > } > > +static void colo_release_packets(void *opaque, void *user_data) > +{ > + CompareState *s = user_data; > + Connection *conn = opaque; > + Packet *pkt = NULL; > + > + while (!g_queue_is_empty(&conn->primary_list)) { > + pkt = g_queue_pop_head(&conn->primary_list); > + compare_chr_send(&s->chr_out, pkt->data, pkt->size); Any reason to send packets here? Thanks > + packet_destroy(pkt, NULL); > + } > + while (!g_queue_is_empty(&conn->secondary_list)) { > + pkt = g_queue_pop_head(&conn->secondary_list); > + packet_destroy(pkt, NULL); > + } > +} > + > static void colo_compare_class_init(ObjectClass *oc, void *data) > { > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) > g_main_loop_quit(s->compare_loop); > qemu_thread_join(&s->thread); > > + /* Release all unhandled packets after compare thead exited */ > + g_queue_foreach(&s->conn_list, colo_release_packets, s); > > g_queue_free(&s->conn_list); > > + g_hash_table_destroy(s->connection_track_table); > g_free(s->pri_indev); > g_free(s->sec_indev); > g_free(s->outdev);
On 2017/2/16 10:34, Jason Wang wrote: > > > On 2017年02月15日 16:34, zhanghailiang wrote: >> We should release all unhandled packets before finalize colo compare. >> Besides, we need to free connection_track_table, or there will be >> a memory leak bug. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> net/colo-compare.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index a16e2d5..809bad3 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) >> return; >> } >> >> +static void colo_release_packets(void *opaque, void *user_data) >> +{ >> + CompareState *s = user_data; >> + Connection *conn = opaque; >> + Packet *pkt = NULL; >> + >> + while (!g_queue_is_empty(&conn->primary_list)) { >> + pkt = g_queue_pop_head(&conn->primary_list); >> + compare_chr_send(&s->chr_out, pkt->data, pkt->size); > > Any reason to send packets here? > Yes, considering the usage case which we shut COLO for the VM to make it as a normal VM without FT. We need to remove all the filter objects. In this case, IMHO, it is necessary to release the unhandled packets. Thanks. > Thanks > >> + packet_destroy(pkt, NULL); >> + } >> + while (!g_queue_is_empty(&conn->secondary_list)) { >> + pkt = g_queue_pop_head(&conn->secondary_list); >> + packet_destroy(pkt, NULL); >> + } >> +} >> + >> static void colo_compare_class_init(ObjectClass *oc, void *data) >> { >> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); >> @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) >> g_main_loop_quit(s->compare_loop); >> qemu_thread_join(&s->thread); >> >> + /* Release all unhandled packets after compare thead exited */ >> + g_queue_foreach(&s->conn_list, colo_release_packets, s); >> >> g_queue_free(&s->conn_list); >> >> + g_hash_table_destroy(s->connection_track_table); >> g_free(s->pri_indev); >> g_free(s->sec_indev); >> g_free(s->outdev); > > > . >
On 2017年02月16日 10:43, Hailiang Zhang wrote: > On 2017/2/16 10:34, Jason Wang wrote: >> >> >> On 2017年02月15日 16:34, zhanghailiang wrote: >>> We should release all unhandled packets before finalize colo compare. >>> Besides, we need to free connection_track_table, or there will be >>> a memory leak bug. >>> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> --- >>> net/colo-compare.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>> index a16e2d5..809bad3 100644 >>> --- a/net/colo-compare.c >>> +++ b/net/colo-compare.c >>> @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable >>> *uc, Error **errp) >>> return; >>> } >>> >>> +static void colo_release_packets(void *opaque, void *user_data) >>> +{ >>> + CompareState *s = user_data; >>> + Connection *conn = opaque; >>> + Packet *pkt = NULL; >>> + >>> + while (!g_queue_is_empty(&conn->primary_list)) { >>> + pkt = g_queue_pop_head(&conn->primary_list); >>> + compare_chr_send(&s->chr_out, pkt->data, pkt->size); >> >> Any reason to send packets here? >> > > Yes, considering the usage case which we shut COLO for > the VM to make it as a normal VM without FT. > We need to remove all the filter objects. In this case, > IMHO, it is necessary to release the unhandled packets. > > > Thanks. Right, I see. All other patches looks good let's squash this into 2. Thanks > >> Thanks >> >>> + packet_destroy(pkt, NULL); >>> + } >>> + while (!g_queue_is_empty(&conn->secondary_list)) { >>> + pkt = g_queue_pop_head(&conn->secondary_list); >>> + packet_destroy(pkt, NULL); >>> + } >>> +} >>> + >>> static void colo_compare_class_init(ObjectClass *oc, void *data) >>> { >>> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); >>> @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) >>> g_main_loop_quit(s->compare_loop); >>> qemu_thread_join(&s->thread); >>> >>> + /* Release all unhandled packets after compare thead exited */ >>> + g_queue_foreach(&s->conn_list, colo_release_packets, s); >>> >>> g_queue_free(&s->conn_list); >>> >>> + g_hash_table_destroy(s->connection_track_table); >>> g_free(s->pri_indev); >>> g_free(s->sec_indev); >>> g_free(s->outdev); >> >> >> . >> >
On 2017/2/16 10:27, Zhang Chen wrote: > > > On 02/15/2017 04:34 PM, zhanghailiang wrote: >> We should release all unhandled packets before finalize colo compare. >> Besides, we need to free connection_track_table, or there will be >> a memory leak bug. >> >> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com> >> --- >> net/colo-compare.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index a16e2d5..809bad3 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) >> return; >> } >> > > This function in my patch "colo-compare and filter-rewriter work with > colo-frame " > Named 'colo_flush_connection', I think use 'flush' instead of 'release' > is better, > OK, i will fix it in next version, thanks. > Thanks > Zhang Chen > > >> +static void colo_release_packets(void *opaque, void *user_data) >> +{ >> + CompareState *s = user_data; >> + Connection *conn = opaque; >> + Packet *pkt = NULL; >> + >> + while (!g_queue_is_empty(&conn->primary_list)) { >> + pkt = g_queue_pop_head(&conn->primary_list); >> + compare_chr_send(&s->chr_out, pkt->data, pkt->size); >> + packet_destroy(pkt, NULL); >> + } >> + while (!g_queue_is_empty(&conn->secondary_list)) { >> + pkt = g_queue_pop_head(&conn->secondary_list); >> + packet_destroy(pkt, NULL); >> + } >> +} >> + >> static void colo_compare_class_init(ObjectClass *oc, void *data) >> { >> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); >> @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) >> g_main_loop_quit(s->compare_loop); >> qemu_thread_join(&s->thread); >> >> + /* Release all unhandled packets after compare thead exited */ >> + g_queue_foreach(&s->conn_list, colo_release_packets, s); >> >> g_queue_free(&s->conn_list); >> >> + g_hash_table_destroy(s->connection_track_table); >> g_free(s->pri_indev); >> g_free(s->sec_indev); >> g_free(s->outdev); >
diff --git a/net/colo-compare.c b/net/colo-compare.c index a16e2d5..809bad3 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -676,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) return; } +static void colo_release_packets(void *opaque, void *user_data) +{ + CompareState *s = user_data; + Connection *conn = opaque; + Packet *pkt = NULL; + + while (!g_queue_is_empty(&conn->primary_list)) { + pkt = g_queue_pop_head(&conn->primary_list); + compare_chr_send(&s->chr_out, pkt->data, pkt->size); + packet_destroy(pkt, NULL); + } + while (!g_queue_is_empty(&conn->secondary_list)) { + pkt = g_queue_pop_head(&conn->secondary_list); + packet_destroy(pkt, NULL); + } +} + static void colo_compare_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); @@ -707,9 +724,12 @@ static void colo_compare_finalize(Object *obj) g_main_loop_quit(s->compare_loop); qemu_thread_join(&s->thread); + /* Release all unhandled packets after compare thead exited */ + g_queue_foreach(&s->conn_list, colo_release_packets, s); g_queue_free(&s->conn_list); + g_hash_table_destroy(s->connection_track_table); g_free(s->pri_indev); g_free(s->sec_indev); g_free(s->outdev);
We should release all unhandled packets before finalize colo compare. Besides, we need to free connection_track_table, or there will be a memory leak bug. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- net/colo-compare.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)