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

2013-11-18 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
---
v2: fix the memory leak (Stefan)
---
 hw/net/virtio-net.c | 32 +---
 include/net/net.h   |  2 ++
 net/net.c   | 22 ++
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 613f144..cee52ff 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(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(n-qdev));
 
 return VIRTIO_NET_OK;
 }
@@ -668,12 +646,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(n-qdev));
 
 return VIRTIO_NET_OK;
 
 error:
-rxfilter_notify(nc);
+rxfilter_notify(nc, OBJECT(n-qdev));
 return VIRTIO_NET_ERR;
 }
 
@@ -700,7 +678,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(n-qdev));
 
 return VIRTIO_NET_OK;
 }
diff --git a/include/net/net.h b/include/net/net.h
index 11e1468..178db62 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -8,6 +8,7 @@
 #include net/queue.h
 #include migration/vmstate.h
 #include qapi-types.h
+#include qom/object.h
 
 #define MAX_QUEUE_NUM 1024
 
@@ -138,6 +139,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
 void *opaque);
 
 void print_net_client(Monitor *mon, NetClientState *nc);
+void rxfilter_notify(NetClientState *nc, Object *obj);
 void do_info_network(Monitor *mon, const QDict *qdict);
 
 /* NIC info */
diff --git a/net/net.c b/net/net.c
index 0a88e68..341dd2b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -41,6 +41,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)
@@ -967,6 +968,27 @@ void print_net_client(Monitor *mon, NetClientState *nc)
nc-info_str);
 }
 
+void rxfilter_notify(NetClientState *nc, Object *obj)
+{
+QObject *event_data;
+gchar *path = object_get_canonical_path(obj);
+
+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;
+}
+g_free(path);
+}
+
 RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
   Error **errp)
 {
-- 
1.8.3.1




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

2013-11-18 Thread Stefan Hajnoczi
On Mon, Nov 18, 2013 at 04:20:12PM +0800, Amos Kong wrote:
 @@ -967,6 +968,27 @@ void print_net_client(Monitor *mon, NetClientState *nc)
 nc-info_str);
  }
  
 +void rxfilter_notify(NetClientState *nc, Object *obj)
 +{
 +QObject *event_data;
 +gchar *path = object_get_canonical_path(obj);
 +
 +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;

Only hw/net/virtio-net.c uses nc-rxfilter_notify_enabled.  This
function isn't reusable in its current form so I'm left wondering what
the point of this patch is?

If you have patches that invoke rxfilter_notify() from other NICs then
please submit them together in a series.

Otherwise, let's not move things around just for the sake of it,
especially when the refactoring is not done correctly.

Stefan



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

2013-11-18 Thread Amos Kong
On Mon, Nov 18, 2013 at 02:25:40PM +0100, Stefan Hajnoczi wrote:
 On Mon, Nov 18, 2013 at 04:20:12PM +0800, Amos Kong wrote:
  @@ -967,6 +968,27 @@ void print_net_client(Monitor *mon, NetClientState *nc)
  nc-info_str);
   }
   
  +void rxfilter_notify(NetClientState *nc, Object *obj)
  +{
  +QObject *event_data;
  +gchar *path = object_get_canonical_path(obj);
  +
  +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;
 
 Only hw/net/virtio-net.c uses nc-rxfilter_notify_enabled.  This
 function isn't reusable in its current form so I'm left wondering what
 the point of this patch is?
 
 If you have patches that invoke rxfilter_notify() from other NICs then
 please submit them together in a series.

I don't have patch to change other NICs to send rxfilter notify right
now.

So I will send a patch to fix the leak. We can move the function to
net.c in future.
 
 Otherwise, let's not move things around just for the sake of it,
 especially when the refactoring is not done correctly.
 
 Stefan

-- 
Amos.



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

2013-11-18 Thread Stefan Hajnoczi
On Mon, Nov 18, 2013 at 09:39:26PM +0800, Amos Kong wrote:
 On Mon, Nov 18, 2013 at 02:25:40PM +0100, Stefan Hajnoczi wrote:
  On Mon, Nov 18, 2013 at 04:20:12PM +0800, Amos Kong wrote:
   @@ -967,6 +968,27 @@ void print_net_client(Monitor *mon, NetClientState 
   *nc)
   nc-info_str);
}

   +void rxfilter_notify(NetClientState *nc, Object *obj)
   +{
   +QObject *event_data;
   +gchar *path = object_get_canonical_path(obj);
   +
   +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;
  
  Only hw/net/virtio-net.c uses nc-rxfilter_notify_enabled.  This
  function isn't reusable in its current form so I'm left wondering what
  the point of this patch is?
  
  If you have patches that invoke rxfilter_notify() from other NICs then
  please submit them together in a series.
 
 I don't have patch to change other NICs to send rxfilter notify right
 now.
 
 So I will send a patch to fix the leak. We can move the function to
 net.c in future.

Okay, sounds good.

Stefan