qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c
@ 2013-11-05 11:00 Amos Kong
  2013-11-15 15:27 ` Stefan Hajnoczi
  0 siblings, 1 reply; 4+ messages in thread
From: Amos Kong @ 2013-11-05 11:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: jasowang, stefanha, mst

rxfilter_notify() is a generic function for all nics, not only for
virtio_net, so move it to net.c

Signed-off-by: Amos Kong <akong@redhat.com>
---
 hw/net/virtio-net.c | 32 +++++---------------------------
 include/net/net.h   |  1 +
 net/net.c           | 20 ++++++++++++++++++++
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 22dbd05..d34c1c7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -194,28 +194,6 @@ static void virtio_net_set_link_status(NetClientState *nc)
     virtio_net_set_status(vdev, vdev->status);
 }
 
-static void rxfilter_notify(NetClientState *nc)
-{
-    QObject *event_data;
-    VirtIONet *n = qemu_get_nic_opaque(nc);
-
-    if (nc->rxfilter_notify_enabled) {
-        if (n->netclient_name) {
-            event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
-                                    n->netclient_name,
-                                    object_get_canonical_path(OBJECT(n->qdev)));
-        } else {
-            event_data = qobject_from_jsonf("{ 'path': %s }",
-                                    object_get_canonical_path(OBJECT(n->qdev)));
-        }
-        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
-        qobject_decref(event_data);
-
-        /* disable event notification to avoid events flooding */
-        nc->rxfilter_notify_enabled = 0;
-    }
-}
-
 static char *mac_strdup_printf(const uint8_t *mac)
 {
     return g_strdup_printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x", mac[0],
@@ -545,7 +523,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
         return VIRTIO_NET_ERR;
     }
 
-    rxfilter_notify(nc);
+    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
     return VIRTIO_NET_OK;
 }
@@ -601,7 +579,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
         s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
         assert(s == sizeof(n->mac));
         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
-        rxfilter_notify(nc);
+        rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
         return VIRTIO_NET_OK;
     }
@@ -667,12 +645,12 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
         n->mac_table.multi_overflow = 1;
     }
 
-    rxfilter_notify(nc);
+    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
     return VIRTIO_NET_OK;
 
 error:
-    rxfilter_notify(nc);
+    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
     return VIRTIO_NET_ERR;
 }
 
@@ -699,7 +677,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
     else
         return VIRTIO_NET_ERR;
 
-    rxfilter_notify(nc);
+    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
     return VIRTIO_NET_OK;
 }
diff --git a/include/net/net.h b/include/net/net.h
index 11e1468..a8192a1 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -138,6 +138,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
                             void *opaque);
 
 void print_net_client(Monitor *mon, NetClientState *nc);
+void rxfilter_notify(NetClientState *nc, const char *path);
 void do_info_network(Monitor *mon, const QDict *qdict);
 
 /* NIC info */
diff --git a/net/net.c b/net/net.c
index c330c9a..f41a457 100644
--- a/net/net.c
+++ b/net/net.c
@@ -40,6 +40,7 @@
 #include "qapi-visit.h"
 #include "qapi/opts-visitor.h"
 #include "qapi/dealloc-visitor.h"
+#include "qapi/qmp/qjson.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -962,6 +963,25 @@ void print_net_client(Monitor *mon, NetClientState *nc)
                    nc->info_str);
 }
 
+void rxfilter_notify(NetClientState *nc, const char *path)
+{
+    QObject *event_data;
+
+    if (nc->rxfilter_notify_enabled) {
+        if (nc->name) {
+            event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
+					    nc->name, path);
+        } else {
+            event_data = qobject_from_jsonf("{ 'path': %s }", path);
+        }
+        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
+        qobject_decref(event_data);
+
+        /* disable event notification to avoid events flooding */
+        nc->rxfilter_notify_enabled = 0;
+    }
+}
+
 RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
                                       Error **errp)
 {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c
  2013-11-05 11:00 [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c Amos Kong
@ 2013-11-15 15:27 ` Stefan Hajnoczi
  2013-11-15 16:49   ` Vlad Yasevich
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-11-15 15:27 UTC (permalink / raw)
  To: Amos Kong; +Cc: jasowang, qemu-devel, stefanha, mst

On Tue, Nov 05, 2013 at 07:00:48PM +0800, Amos Kong wrote:
> @@ -545,7 +523,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>          return VIRTIO_NET_ERR;
>      }
>  
> -    rxfilter_notify(nc);
> +    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
>  
>      return VIRTIO_NET_OK;
>  }
[...]
> diff --git a/net/net.c b/net/net.c
> index c330c9a..f41a457 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -40,6 +40,7 @@
>  #include "qapi-visit.h"
>  #include "qapi/opts-visitor.h"
>  #include "qapi/dealloc-visitor.h"
> +#include "qapi/qmp/qjson.h"
>  
>  /* Net bridge is currently not supported for W32. */
>  #if !defined(_WIN32)
> @@ -962,6 +963,25 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>                     nc->info_str);
>  }
>  
> +void rxfilter_notify(NetClientState *nc, const char *path)
> +{
> +    QObject *event_data;
> +
> +    if (nc->rxfilter_notify_enabled) {
> +        if (nc->name) {
> +            event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
> +					    nc->name, path);
> +        } else {
> +            event_data = qobject_from_jsonf("{ 'path': %s }", path);
> +        }
> +        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
> +        qobject_decref(event_data);
> +
> +        /* disable event notification to avoid events flooding */
> +        nc->rxfilter_notify_enabled = 0;
> +    }
> +}

Please fix the memory leak:
object_get_canonical_path() returns a gchar* that the caller must free.

Stefan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c
  2013-11-15 15:27 ` Stefan Hajnoczi
@ 2013-11-15 16:49   ` Vlad Yasevich
  2013-11-18 12:59     ` Amos Kong
  0 siblings, 1 reply; 4+ messages in thread
From: Vlad Yasevich @ 2013-11-15 16:49 UTC (permalink / raw)
  To: qemu-devel

On 11/15/2013 10:27 AM, Stefan Hajnoczi wrote:
> On Tue, Nov 05, 2013 at 07:00:48PM +0800, Amos Kong wrote:
>> @@ -545,7 +523,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
>>           return VIRTIO_NET_ERR;
>>       }
>>
>> -    rxfilter_notify(nc);
>> +    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
>>
>>       return VIRTIO_NET_OK;
>>   }
> [...]
>> diff --git a/net/net.c b/net/net.c
>> index c330c9a..f41a457 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -40,6 +40,7 @@
>>   #include "qapi-visit.h"
>>   #include "qapi/opts-visitor.h"
>>   #include "qapi/dealloc-visitor.h"
>> +#include "qapi/qmp/qjson.h"
>>
>>   /* Net bridge is currently not supported for W32. */
>>   #if !defined(_WIN32)
>> @@ -962,6 +963,25 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>>                      nc->info_str);
>>   }
>>
>> +void rxfilter_notify(NetClientState *nc, const char *path)
>> +{
>> +    QObject *event_data;
>> +
>> +    if (nc->rxfilter_notify_enabled) {
>> +        if (nc->name) {
>> +            event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
>> +					    nc->name, path);
>> +        } else {
>> +            event_data = qobject_from_jsonf("{ 'path': %s }", path);
>> +        }
>> +        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
>> +        qobject_decref(event_data);
>> +
>> +        /* disable event notification to avoid events flooding */
>> +        nc->rxfilter_notify_enabled = 0;
>> +    }
>> +}
>
> Please fix the memory leak:
> object_get_canonical_path() returns a gchar* that the caller must free.
>

Wow, this memory leak is all over the place and not just in this patch.

-vlad

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c
  2013-11-15 16:49   ` Vlad Yasevich
@ 2013-11-18 12:59     ` Amos Kong
  0 siblings, 0 replies; 4+ messages in thread
From: Amos Kong @ 2013-11-18 12:59 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: qemu-devel

On Fri, Nov 15, 2013 at 11:49:27AM -0500, Vlad Yasevich wrote:
> On 11/15/2013 10:27 AM, Stefan Hajnoczi wrote:
> >On Tue, Nov 05, 2013 at 07:00:48PM +0800, Amos Kong wrote:
> >>@@ -545,7 +523,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
> >>          return VIRTIO_NET_ERR;
> >>      }
> >>
> >>-    rxfilter_notify(nc);
> >>+    rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
> >>
> >>      return VIRTIO_NET_OK;
> >>  }
> >[...]
> >>diff --git a/net/net.c b/net/net.c
> >>index c330c9a..f41a457 100644
> >>--- a/net/net.c
> >>+++ b/net/net.c
> >>@@ -40,6 +40,7 @@
> >>  #include "qapi-visit.h"
> >>  #include "qapi/opts-visitor.h"
> >>  #include "qapi/dealloc-visitor.h"
> >>+#include "qapi/qmp/qjson.h"
> >>
> >>  /* Net bridge is currently not supported for W32. */
> >>  #if !defined(_WIN32)
> >>@@ -962,6 +963,25 @@ void print_net_client(Monitor *mon, NetClientState *nc)
> >>                     nc->info_str);
> >>  }
> >>
> >>+void rxfilter_notify(NetClientState *nc, const char *path)
> >>+{
> >>+    QObject *event_data;
> >>+
> >>+    if (nc->rxfilter_notify_enabled) {
> >>+        if (nc->name) {
> >>+            event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
> >>+					    nc->name, path);
> >>+        } else {
> >>+            event_data = qobject_from_jsonf("{ 'path': %s }", path);
> >>+        }
> >>+        monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
> >>+        qobject_decref(event_data);
> >>+
> >>+        /* disable event notification to avoid events flooding */
> >>+        nc->rxfilter_notify_enabled = 0;
> >>+    }
> >>+}
> >
> >Please fix the memory leak:
> >object_get_canonical_path() returns a gchar* that the caller must free.

> Wow, this memory leak is all over the place and not just in this patch.

Yes, my v2 fix the memory leak in rxfilter_notify().
I just check the code, your patch fixed _the last_ leak
of object_get_canonical_path(). Thanks.
 
> -vlad

-- 
			Amos.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-11-18 13:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-05 11:00 [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c Amos Kong
2013-11-15 15:27 ` Stefan Hajnoczi
2013-11-15 16:49   ` Vlad Yasevich
2013-11-18 12:59     ` Amos Kong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).