Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c

2013-11-18 Thread Amos Kong 
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.

Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c

2013-11-15 Thread Stefan Hajnoczi 
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

Re: [Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c

2013-11-15 Thread Vlad Yasevich 

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

[Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c

2013-11-05 Thread Amos Kong 
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 ak...@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