Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-06-26 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 05/23/2013 03:07 AM, Amos Kong wrote:
 Introduce this new QMP event to notify management after guest changes
 rx-filter configuration.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  QMP/qmp-events.txt| 14 ++
  include/monitor/monitor.h |  1 +
  monitor.c |  1 +
  3 files changed, 16 insertions(+)
 
 diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
 index 92fe5fb..ad6612b 100644
 --- a/QMP/qmp-events.txt
 +++ b/QMP/qmp-events.txt
 @@ -154,6 +154,20 @@ Data:
  path: /machine/peripheral/virtio-net-pci-0 },
timestamp: { seconds: 1265044230, microseconds: 450486 } }
  
 +RX_FILTER_CHANGED
 +-

 +
  DEVICE_TRAY_MOVED
  -

 Isn't this file supposed to be kept in sorted order, to minimize merge
 conflicts when backporting events?

Yes, please.  Also helps humans navigate.

GUEST_PANICKED is out of order, needs fixing.



Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-06-26 Thread Luiz Capitulino
On Wed, 26 Jun 2013 08:54:46 +0200
Markus Armbruster arm...@redhat.com wrote:

 Eric Blake ebl...@redhat.com writes:
 
  On 05/23/2013 03:07 AM, Amos Kong wrote:
  Introduce this new QMP event to notify management after guest changes
  rx-filter configuration.
  
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
   QMP/qmp-events.txt| 14 ++
   include/monitor/monitor.h |  1 +
   monitor.c |  1 +
   3 files changed, 16 insertions(+)
  
  diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
  index 92fe5fb..ad6612b 100644
  --- a/QMP/qmp-events.txt
  +++ b/QMP/qmp-events.txt
  @@ -154,6 +154,20 @@ Data:
   path: /machine/peripheral/virtio-net-pci-0 },
 timestamp: { seconds: 1265044230, microseconds: 450486 } }
   
  +RX_FILTER_CHANGED
  +-
 
  +
   DEVICE_TRAY_MOVED
   -
 
  Isn't this file supposed to be kept in sorted order, to minimize merge
  conflicts when backporting events?
 
 Yes, please.  Also helps humans navigate.
 
 GUEST_PANICKED is out of order, needs fixing.

The ultimate fix would be to add event support to the qapi.



[Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Amos Kong
Introduce this new QMP event to notify management after guest changes
rx-filter configuration.

Signed-off-by: Amos Kong ak...@redhat.com
---
 QMP/qmp-events.txt| 14 ++
 include/monitor/monitor.h |  1 +
 monitor.c |  1 +
 3 files changed, 16 insertions(+)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 92fe5fb..ad6612b 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -154,6 +154,20 @@ Data:
 path: /machine/peripheral/virtio-net-pci-0 },
   timestamp: { seconds: 1265044230, microseconds: 450486 } }
 
+RX_FILTER_CHANGED
+-
+
+Emitted when rx-filter configuration is changed by the guest.
+
+Data:
+
+- name: net client name (json-string)
+
+{ event: RX_FILTER_CHANGED,
+  data: { name: vnet0 },
+  timestamp: { seconds: 1368697518, microseconds: 326866 }}
+}
+
 DEVICE_TRAY_MOVED
 -
 
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1a6cfcf..c495a67 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -40,6 +40,7 @@ typedef enum MonitorEvent {
 QEVENT_BLOCK_JOB_ERROR,
 QEVENT_BLOCK_JOB_READY,
 QEVENT_DEVICE_DELETED,
+QEVENT_RX_FILTER_CHANGED,
 QEVENT_DEVICE_TRAY_MOVED,
 QEVENT_SUSPEND,
 QEVENT_SUSPEND_DISK,
diff --git a/monitor.c b/monitor.c
index 6ce2a4e..4f7bd48 100644
--- a/monitor.c
+++ b/monitor.c
@@ -489,6 +489,7 @@ static const char *monitor_event_names[] = {
 [QEVENT_BLOCK_JOB_ERROR] = BLOCK_JOB_ERROR,
 [QEVENT_BLOCK_JOB_READY] = BLOCK_JOB_READY,
 [QEVENT_DEVICE_DELETED] = DEVICE_DELETED,
+[QEVENT_RX_FILTER_CHANGED] = RX_FILTER_CHANGED,
 [QEVENT_DEVICE_TRAY_MOVED] = DEVICE_TRAY_MOVED,
 [QEVENT_SUSPEND] = SUSPEND,
 [QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Michael S. Tsirkin
On Thu, May 23, 2013 at 05:07:59PM +0800, Amos Kong wrote:
 Introduce this new QMP event to notify management after guest changes
 rx-filter configuration.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  QMP/qmp-events.txt| 14 ++
  include/monitor/monitor.h |  1 +
  monitor.c |  1 +
  3 files changed, 16 insertions(+)
 
 diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
 index 92fe5fb..ad6612b 100644
 --- a/QMP/qmp-events.txt
 +++ b/QMP/qmp-events.txt
 @@ -154,6 +154,20 @@ Data:
  path: /machine/peripheral/virtio-net-pci-0 },
timestamp: { seconds: 1265044230, microseconds: 450486 } }
  
 +RX_FILTER_CHANGED
 +-
 +
 +Emitted when rx-filter configuration is changed by the guest.

Please stress this is only for the NIC. It does not apply
to non-NIC netclients.

 +
 +Data:
 +
 +- name: net client name (json-string)

Maybe a device path here as well?

 +
 +{ event: RX_FILTER_CHANGED,
 +  data: { name: vnet0 },
 +  timestamp: { seconds: 1368697518, microseconds: 326866 }}
 +}
 +
  DEVICE_TRAY_MOVED
  -
  
 diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
 index 1a6cfcf..c495a67 100644
 --- a/include/monitor/monitor.h
 +++ b/include/monitor/monitor.h
 @@ -40,6 +40,7 @@ typedef enum MonitorEvent {
  QEVENT_BLOCK_JOB_ERROR,
  QEVENT_BLOCK_JOB_READY,
  QEVENT_DEVICE_DELETED,
 +QEVENT_RX_FILTER_CHANGED,
  QEVENT_DEVICE_TRAY_MOVED,
  QEVENT_SUSPEND,
  QEVENT_SUSPEND_DISK,
 diff --git a/monitor.c b/monitor.c
 index 6ce2a4e..4f7bd48 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -489,6 +489,7 @@ static const char *monitor_event_names[] = {
  [QEVENT_BLOCK_JOB_ERROR] = BLOCK_JOB_ERROR,
  [QEVENT_BLOCK_JOB_READY] = BLOCK_JOB_READY,
  [QEVENT_DEVICE_DELETED] = DEVICE_DELETED,
 +[QEVENT_RX_FILTER_CHANGED] = RX_FILTER_CHANGED,
  [QEVENT_DEVICE_TRAY_MOVED] = DEVICE_TRAY_MOVED,
  [QEVENT_SUSPEND] = SUSPEND,
  [QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
 -- 
 1.8.1.4



Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Eric Blake
On 05/23/2013 03:07 AM, Amos Kong wrote:
 Introduce this new QMP event to notify management after guest changes
 rx-filter configuration.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  QMP/qmp-events.txt| 14 ++
  include/monitor/monitor.h |  1 +
  monitor.c |  1 +
  3 files changed, 16 insertions(+)
 
 diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
 index 92fe5fb..ad6612b 100644
 --- a/QMP/qmp-events.txt
 +++ b/QMP/qmp-events.txt
 @@ -154,6 +154,20 @@ Data:
  path: /machine/peripheral/virtio-net-pci-0 },
timestamp: { seconds: 1265044230, microseconds: 450486 } }
  
 +RX_FILTER_CHANGED
 +-

 +
  DEVICE_TRAY_MOVED
  -

Isn't this file supposed to be kept in sorted order, to minimize merge
conflicts when backporting events?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Luiz Capitulino
On Thu, 23 May 2013 13:24:59 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, May 23, 2013 at 05:07:59PM +0800, Amos Kong wrote:
  Introduce this new QMP event to notify management after guest changes
  rx-filter configuration.
  
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
   QMP/qmp-events.txt| 14 ++
   include/monitor/monitor.h |  1 +
   monitor.c |  1 +
   3 files changed, 16 insertions(+)
  
  diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
  index 92fe5fb..ad6612b 100644
  --- a/QMP/qmp-events.txt
  +++ b/QMP/qmp-events.txt
  @@ -154,6 +154,20 @@ Data:
   path: /machine/peripheral/virtio-net-pci-0 },
 timestamp: { seconds: 1265044230, microseconds: 450486 } }
   
  +RX_FILTER_CHANGED
  +-
  +
  +Emitted when rx-filter configuration is changed by the guest.
 
 Please stress this is only for the NIC. It does not apply
 to non-NIC netclients.

Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
generic.

Also, although I haven't reviewed the next patch yet, I think you
should move the event trigger to this patch.

 
  +
  +Data:
  +
  +- name: net client name (json-string)
 
 Maybe a device path here as well?
 
  +
  +{ event: RX_FILTER_CHANGED,
  +  data: { name: vnet0 },
  +  timestamp: { seconds: 1368697518, microseconds: 326866 }}
  +}
  +
   DEVICE_TRAY_MOVED
   -
   
  diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
  index 1a6cfcf..c495a67 100644
  --- a/include/monitor/monitor.h
  +++ b/include/monitor/monitor.h
  @@ -40,6 +40,7 @@ typedef enum MonitorEvent {
   QEVENT_BLOCK_JOB_ERROR,
   QEVENT_BLOCK_JOB_READY,
   QEVENT_DEVICE_DELETED,
  +QEVENT_RX_FILTER_CHANGED,
   QEVENT_DEVICE_TRAY_MOVED,
   QEVENT_SUSPEND,
   QEVENT_SUSPEND_DISK,
  diff --git a/monitor.c b/monitor.c
  index 6ce2a4e..4f7bd48 100644
  --- a/monitor.c
  +++ b/monitor.c
  @@ -489,6 +489,7 @@ static const char *monitor_event_names[] = {
   [QEVENT_BLOCK_JOB_ERROR] = BLOCK_JOB_ERROR,
   [QEVENT_BLOCK_JOB_READY] = BLOCK_JOB_READY,
   [QEVENT_DEVICE_DELETED] = DEVICE_DELETED,
  +[QEVENT_RX_FILTER_CHANGED] = RX_FILTER_CHANGED,
   [QEVENT_DEVICE_TRAY_MOVED] = DEVICE_TRAY_MOVED,
   [QEVENT_SUSPEND] = SUSPEND,
   [QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
  -- 
  1.8.1.4
 




Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Michael S. Tsirkin
On Thu, May 23, 2013 at 10:28:42AM -0400, Luiz Capitulino wrote:
 On Thu, 23 May 2013 13:24:59 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, May 23, 2013 at 05:07:59PM +0800, Amos Kong wrote:
   Introduce this new QMP event to notify management after guest changes
   rx-filter configuration.
   
   Signed-off-by: Amos Kong ak...@redhat.com
   ---
QMP/qmp-events.txt| 14 ++
include/monitor/monitor.h |  1 +
monitor.c |  1 +
3 files changed, 16 insertions(+)
   
   diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
   index 92fe5fb..ad6612b 100644
   --- a/QMP/qmp-events.txt
   +++ b/QMP/qmp-events.txt
   @@ -154,6 +154,20 @@ Data:
path: /machine/peripheral/virtio-net-pci-0 },
  timestamp: { seconds: 1265044230, microseconds: 450486 } }

   +RX_FILTER_CHANGED
   +-
   +
   +Emitted when rx-filter configuration is changed by the guest.
  
  Please stress this is only for the NIC. It does not apply
  to non-NIC netclients.
 
 Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
 generic.

What do you suggest?
NIC_RX_FILTER_CHANGED ?

 Also, although I haven't reviewed the next patch yet, I think you
 should move the event trigger to this patch.

That's hard because of the flag that's shared with the query
command.

  
   +
   +Data:
   +
   +- name: net client name (json-string)
  
  Maybe a device path here as well?
  
   +
   +{ event: RX_FILTER_CHANGED,
   +  data: { name: vnet0 },
   +  timestamp: { seconds: 1368697518, microseconds: 326866 }}
   +}
   +
DEVICE_TRAY_MOVED
-

   diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
   index 1a6cfcf..c495a67 100644
   --- a/include/monitor/monitor.h
   +++ b/include/monitor/monitor.h
   @@ -40,6 +40,7 @@ typedef enum MonitorEvent {
QEVENT_BLOCK_JOB_ERROR,
QEVENT_BLOCK_JOB_READY,
QEVENT_DEVICE_DELETED,
   +QEVENT_RX_FILTER_CHANGED,
QEVENT_DEVICE_TRAY_MOVED,
QEVENT_SUSPEND,
QEVENT_SUSPEND_DISK,
   diff --git a/monitor.c b/monitor.c
   index 6ce2a4e..4f7bd48 100644
   --- a/monitor.c
   +++ b/monitor.c
   @@ -489,6 +489,7 @@ static const char *monitor_event_names[] = {
[QEVENT_BLOCK_JOB_ERROR] = BLOCK_JOB_ERROR,
[QEVENT_BLOCK_JOB_READY] = BLOCK_JOB_READY,
[QEVENT_DEVICE_DELETED] = DEVICE_DELETED,
   +[QEVENT_RX_FILTER_CHANGED] = RX_FILTER_CHANGED,
[QEVENT_DEVICE_TRAY_MOVED] = DEVICE_TRAY_MOVED,
[QEVENT_SUSPEND] = SUSPEND,
[QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
   -- 
   1.8.1.4
  



Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Eric Blake
On 05/23/2013 08:43 AM, Michael S. Tsirkin wrote:
 Please stress this is only for the NIC. It does not apply
 to non-NIC netclients.

 Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
 generic.
 
 What do you suggest?
 NIC_RX_FILTER_CHANGED ?

Yes, that might work.  (And whatever name we bikeshed, insert it in the
correct sorted order in the file)

 
 Also, although I haven't reviewed the next patch yet, I think you
 should move the event trigger to this patch.
 
 That's hard because of the flag that's shared with the query
 command.

It's fine if this patch declares and sets the flag, but the event is
one-shot until the next patch adds the query to clear the flag.  And
again, the flag should be per-device, not global.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Michael S. Tsirkin
On Thu, May 23, 2013 at 08:52:16AM -0600, Eric Blake wrote:
 On 05/23/2013 08:43 AM, Michael S. Tsirkin wrote:
  Please stress this is only for the NIC. It does not apply
  to non-NIC netclients.
 
  Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
  generic.
  
  What do you suggest?
  NIC_RX_FILTER_CHANGED ?
 
 Yes, that might work.  (And whatever name we bikeshed, insert it in the
 correct sorted order in the file)
 
  
  Also, although I haven't reviewed the next patch yet, I think you
  should move the event trigger to this patch.
  
  That's hard because of the flag that's shared with the query
  command.
 
 It's fine if this patch declares and sets the flag, but the event is
 one-shot until the next patch adds the query to clear the flag.  And
 again, the flag should be per-device, not global.

Maybe just merge both patches together. They are pretty small anyway.

 -- 
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 





Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Luiz Capitulino
On Thu, 23 May 2013 17:57:56 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, May 23, 2013 at 08:52:16AM -0600, Eric Blake wrote:
  On 05/23/2013 08:43 AM, Michael S. Tsirkin wrote:
   Please stress this is only for the NIC. It does not apply
   to non-NIC netclients.
  
   Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
   generic.
   
   What do you suggest?
   NIC_RX_FILTER_CHANGED ?
  
  Yes, that might work.  (And whatever name we bikeshed, insert it in the
  correct sorted order in the file)
  
   
   Also, although I haven't reviewed the next patch yet, I think you
   should move the event trigger to this patch.
   
   That's hard because of the flag that's shared with the query
   command.
  
  It's fine if this patch declares and sets the flag, but the event is
  one-shot until the next patch adds the query to clear the flag.  And
  again, the flag should be per-device, not global.
 
 Maybe just merge both patches together. They are pretty small anyway.

That's what I was going to suggest if it's really hard to move the
event to this patch.



Re: [Qemu-devel] [PATCH v3 1/2] net: introduce RX_FILTER_CHANGED event

2013-05-23 Thread Amos Kong
On Thu, May 23, 2013 at 11:33:37AM -0400, Luiz Capitulino wrote:
 On Thu, 23 May 2013 17:57:56 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, May 23, 2013 at 08:52:16AM -0600, Eric Blake wrote:
   On 05/23/2013 08:43 AM, Michael S. Tsirkin wrote:
Please stress this is only for the NIC. It does not apply
to non-NIC netclients.
   
Stress it in the event name too, please. I find RX_FILTER_CHANGED a bit
generic.

What do you suggest?
NIC_RX_FILTER_CHANGED ?
   
   Yes, that might work.  (And whatever name we bikeshed, insert it in the
   correct sorted order in the file)

Reasonable.

Also, although I haven't reviewed the next patch yet, I think you
should move the event trigger to this patch.

That's hard because of the flag that's shared with the query
command.
   
   It's fine if this patch declares and sets the flag, but the event is
   one-shot until the next patch adds the query to clear the flag.  And
   again, the flag should be per-device, not global.

It works.

  Maybe just merge both patches together. They are pretty small anyway.
 
 That's what I was going to suggest if it's really hard to move the
 event to this patch.

Ok, I will merge those two patches together, and give a detail/whole
description in one patch.

-- 
Amos.