Re: [Qemu-devel] [PATCH 3/5] colo-compare: release all unhandled packets in finalize function

[Hailiang Zhang]

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
---
   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(>primary_list)) {
+pkt = g_queue_pop_head(>primary_list);
+compare_chr_send(>chr_out, pkt->data, pkt->size);
+packet_destroy(pkt, NULL);
+}
+while (!g_queue_is_empty(>secondary_list)) {
+pkt = g_queue_pop_head(>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(>thread);

+/* Release all unhandled packets after compare thead exited */
+g_queue_foreach(>conn_list, colo_release_packets, s);

   g_queue_free(>conn_list);

+g_hash_table_destroy(s->connection_track_table);
   g_free(s->pri_indev);
   g_free(s->sec_indev);
   g_free(s->outdev);

Re: [Qemu-devel] [PATCH 3/5] colo-compare: release all unhandled packets in finalize function

[Jason Wang]

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 
---
   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(>primary_list)) {
+pkt = g_queue_pop_head(>primary_list);
+compare_chr_send(>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(>secondary_list)) {
+pkt = g_queue_pop_head(>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(>thread);

+/* Release all unhandled packets after compare thead exited */
+g_queue_foreach(>conn_list, colo_release_packets, s);

   g_queue_free(>conn_list);

+g_hash_table_destroy(s->connection_track_table);
   g_free(s->pri_indev);
   g_free(s->sec_indev);
   g_free(s->outdev);



.

Re: [Qemu-devel] [PATCH 3/5] colo-compare: release all unhandled packets in finalize function

[Hailiang Zhang]

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 
---
   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(>primary_list)) {
+pkt = g_queue_pop_head(>primary_list);
+compare_chr_send(>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(>secondary_list)) {
+pkt = g_queue_pop_head(>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(>thread);

+/* Release all unhandled packets after compare thead exited */
+g_queue_foreach(>conn_list, colo_release_packets, s);

   g_queue_free(>conn_list);

+g_hash_table_destroy(s->connection_track_table);
   g_free(s->pri_indev);
   g_free(s->sec_indev);
   g_free(s->outdev);



.

Re: [Qemu-devel] [PATCH 3/5] colo-compare: release all unhandled packets in finalize function

[Jason Wang]

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 
---
  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(>primary_list)) {
+pkt = g_queue_pop_head(>primary_list);
+compare_chr_send(>chr_out, pkt->data, pkt->size);


Any reason to send packets here?

Thanks


+packet_destroy(pkt, NULL);
+}
+while (!g_queue_is_empty(>secondary_list)) {
+pkt = g_queue_pop_head(>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(>thread);
  
+/* Release all unhandled packets after compare thead exited */

+g_queue_foreach(>conn_list, colo_release_packets, s);
  
  g_queue_free(>conn_list);
  
+g_hash_table_destroy(s->connection_track_table);

  g_free(s->pri_indev);
  g_free(s->sec_indev);
  g_free(s->outdev);

Re: [Qemu-devel] [PATCH 3/5] colo-compare: release all unhandled packets in finalize function

[Zhang Chen]

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
---
  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(>primary_list)) {
+pkt = g_queue_pop_head(>primary_list);
+compare_chr_send(>chr_out, pkt->data, pkt->size);
+packet_destroy(pkt, NULL);
+}
+while (!g_queue_is_empty(>secondary_list)) {
+pkt = g_queue_pop_head(>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(>thread);
  
+/* Release all unhandled packets after compare thead exited */

+g_queue_foreach(>conn_list, colo_release_packets, s);
  
  g_queue_free(>conn_list);
  
+g_hash_table_destroy(s->connection_track_table);

  g_free(s->pri_indev);
  g_free(s->sec_indev);
  g_free(s->outdev);


--
Thanks
Zhang Chen