Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-08-01 Thread Michael S. Tsirkin
On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
 netclient 'name' entry in event is useful for management to know
 which device is changed. n-netclient_name is not always set.
 This patch changes to use nc-name. If we don't assign 'id',
 qemu will set a generated name to nc-name.
 
 Signed-off-by: Amos Kong ak...@redhat.com

Just making sure - you don't think this patch is necessary, correct?

 ---
  hw/net/virtio-net.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)
 
 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index c88403a..e4d9752 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -200,14 +200,9 @@ static void rxfilter_notify(NetClientState *nc)
  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)));
 -}
 +event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
 +   nc-name,
 +   object_get_canonical_path(OBJECT(n-qdev)));
  monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
  qobject_decref(event_data);
  
 -- 
 1.8.1.4



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-08-01 Thread Amos Kong
On Thu, Aug 01, 2013 at 11:59:14AM +0300, Michael S. Tsirkin wrote:
 On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
  netclient 'name' entry in event is useful for management to know
  which device is changed. n-netclient_name is not always set.
  This patch changes to use nc-name. If we don't assign 'id',
  qemu will set a generated name to nc-name.
  
  Signed-off-by: Amos Kong ak...@redhat.com
 
 Just making sure - you don't think this patch is necessary, correct?
 
Correct.




Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-08-01 Thread Andreas Färber
Am 01.07.2013 04:55, schrieb Amos Kong:
 On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote:
 Amos Kong ak...@redhat.com writes:

 On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
 netclient 'name' entry in event is useful for management to know
 which device is changed. n-netclient_name is not always set.
 This patch changes to use nc-name. If we don't assign 'id',
 qemu will set a generated name to nc-name.


 IRC: mst akong, what do other events include? name or id?

 I just checked QMP/qmp-event.txt, they all use 'device name'.
 (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)

 If we assign 'id' for -device, device name will be set to id.
 Otherwise, a generated device name will set to some device.

 DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).

 For reasons I don't understand, it sets path only when the device has
 no qdev ID.  That should be cleaned up.
 
 The path are alwasy set.
 
 example:
 (have id)
   path: /machine/peripheral-anon/vnet0/virtio-backend

You hopefully meant /machine/peripheral/vnet0/virtio-backend?
Otherwise we have a bug somewhere.

Andreas

 
 (no id)
   path: /machine/peripheral-anon/device[0]/virtio-backend
 
 It's enough to just use path to distinguish the changed device.
 So we ignore this patch.
 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-08-01 Thread Amos Kong
On Thu, Aug 01, 2013 at 03:30:53PM +0200, Andreas Färber wrote:
 Am 01.07.2013 04:55, schrieb Amos Kong:
  On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote:
  Amos Kong ak...@redhat.com writes:
 
  On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
  netclient 'name' entry in event is useful for management to know
  which device is changed. n-netclient_name is not always set.
  This patch changes to use nc-name. If we don't assign 'id',
  qemu will set a generated name to nc-name.
 
 
  IRC: mst akong, what do other events include? name or id?
 
  I just checked QMP/qmp-event.txt, they all use 'device name'.
  (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)
 
  If we assign 'id' for -device, device name will be set to id.
  Otherwise, a generated device name will set to some device.
 
  DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).
 
  For reasons I don't understand, it sets path only when the device has
  no qdev ID.  That should be cleaned up.
  
  The path are alwasy set.
  
  example:
  (have id)
path: /machine/peripheral-anon/vnet0/virtio-backend
 
 You hopefully meant /machine/peripheral/vnet0/virtio-backend?
 Otherwise we have a bug somewhere.
 
Just my typo in mail.


// -device virtio-net-pci,netdev=h1,id=vnet0 -netdev tap,id=h1

{
timestamp: {
seconds: 1375366321, 
microseconds: 595727
}, 
event: NIC_RX_FILTER_CHANGED, 
data: {
name: vnet0, 
path: /machine/peripheral/vnet0/virtio-backend
}
}


 Andreas
 
  
  (no id)
path: /machine/peripheral-anon/device[0]/virtio-backend
  
  It's enough to just use path to distinguish the changed device.
  So we ignore this patch.
  
 
 
 -- 
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Amos.



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-07-01 Thread Markus Armbruster
Amos Kong ak...@redhat.com writes:

 On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote:
 Amos Kong ak...@redhat.com writes:
 
  On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
  netclient 'name' entry in event is useful for management to know
  which device is changed. n-netclient_name is not always set.
  This patch changes to use nc-name. If we don't assign 'id',
  qemu will set a generated name to nc-name.
 
 
  IRC: mst akong, what do other events include? name or id?
 
  I just checked QMP/qmp-event.txt, they all use 'device name'.
  (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)
 
  If we assign 'id' for -device, device name will be set to id.
  Otherwise, a generated device name will set to some device.
 
 DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).
 
 For reasons I don't understand, it sets path only when the device has
 no qdev ID.  That should be cleaned up.

 The path are alwasy set.

You're right; I misread device_unparent().

[...]



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-06-30 Thread Amos Kong
On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote:
 Amos Kong ak...@redhat.com writes:
 
  On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
  netclient 'name' entry in event is useful for management to know
  which device is changed. n-netclient_name is not always set.
  This patch changes to use nc-name. If we don't assign 'id',
  qemu will set a generated name to nc-name.
 
 
  IRC: mst akong, what do other events include? name or id?
 
  I just checked QMP/qmp-event.txt, they all use 'device name'.
  (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)
 
  If we assign 'id' for -device, device name will be set to id.
  Otherwise, a generated device name will set to some device.
 
 DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).
 
 For reasons I don't understand, it sets path only when the device has
 no qdev ID.  That should be cleaned up.

The path are alwasy set.

example:
(have id)
  path: /machine/peripheral-anon/vnet0/virtio-backend

(no id)
  path: /machine/peripheral-anon/device[0]/virtio-backend

It's enough to just use path to distinguish the changed device.
So we ignore this patch.

-- 
Amos.



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-06-30 Thread Amos Kong
On Wed, Jun 26, 2013 at 12:02:45PM +0200, Markus Armbruster wrote:
 Amos Kong ak...@redhat.com writes:
 
  netclient 'name' entry in event is useful for management to know
  which device is changed. n-netclient_name is not always set.
  This patch changes to use nc-name. If we don't assign 'id',
  qemu will set a generated name to nc-name.
 
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
   hw/net/virtio-net.c | 11 +++
   1 file changed, 3 insertions(+), 8 deletions(-)
 
  diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
  index c88403a..e4d9752 100644
  --- a/hw/net/virtio-net.c
  +++ b/hw/net/virtio-net.c
  @@ -200,14 +200,9 @@ static void rxfilter_notify(NetClientState *nc)
   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)));
  -}
  +event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
  +   nc-name,
  +   object_get_canonical_path(OBJECT(n-qdev)));
   monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
   qobject_decref(event_data);
 
 Is this on top of [PATCH v3 0/2] mac programming over macvtap?

It's based on mst's pci tree:

  https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/log/?h=pci
  
https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/commit/?h=pciid=1c0fa6b709d02fe4f98d4ce7b55a6cc3c925791c
 
 Yes, qdev IDs are optional, and therefore can serve as reliable
 identifier only when the user / management application always specifies
 one, and even then, you're still screwed for auto-created devices.
 Easily avoided for NICs, but yes, the problem is real.
 
 However, I don't agree with the solution use NetCientState name,
 because that's too ad hoc, and not general.  Can we please use QOM
 paths?

path is always provided in event, libvirt can use it.

-- 
Amos.



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-06-26 Thread Markus Armbruster
Amos Kong ak...@redhat.com writes:

 netclient 'name' entry in event is useful for management to know
 which device is changed. n-netclient_name is not always set.
 This patch changes to use nc-name. If we don't assign 'id',
 qemu will set a generated name to nc-name.

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  hw/net/virtio-net.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index c88403a..e4d9752 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -200,14 +200,9 @@ static void rxfilter_notify(NetClientState *nc)
  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)));
 -}
 +event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
 +   nc-name,
 +   object_get_canonical_path(OBJECT(n-qdev)));
  monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
  qobject_decref(event_data);

Is this on top of [PATCH v3 0/2] mac programming over macvtap?

Yes, qdev IDs are optional, and therefore can serve as reliable
identifier only when the user / management application always specifies
one, and even then, you're still screwed for auto-created devices.
Easily avoided for NICs, but yes, the problem is real.

However, I don't agree with the solution use NetCientState name,
because that's too ad hoc, and not general.  Can we please use QOM
paths?



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-06-26 Thread Markus Armbruster
Amos Kong ak...@redhat.com writes:

 On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
 netclient 'name' entry in event is useful for management to know
 which device is changed. n-netclient_name is not always set.
 This patch changes to use nc-name. If we don't assign 'id',
 qemu will set a generated name to nc-name.


 IRC: mst akong, what do other events include? name or id?

 I just checked QMP/qmp-event.txt, they all use 'device name'.
 (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)

 If we assign 'id' for -device, device name will be set to id.
 Otherwise, a generated device name will set to some device.

DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).

For reasons I don't understand, it sets path only when the device has
no qdev ID.  That should be cleaned up.



Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event

2013-06-25 Thread Amos Kong
On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
 netclient 'name' entry in event is useful for management to know
 which device is changed. n-netclient_name is not always set.
 This patch changes to use nc-name. If we don't assign 'id',
 qemu will set a generated name to nc-name.


IRC: mst akong, what do other events include? name or id?

I just checked QMP/qmp-event.txt, they all use 'device name'.
(eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)

If we assign 'id' for -device, device name will be set to id.
Otherwise, a generated device name will set to some device.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  hw/net/virtio-net.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)
 
 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index c88403a..e4d9752 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -200,14 +200,9 @@ static void rxfilter_notify(NetClientState *nc)
  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)));
 -}
 +event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
 +   nc-name,
 +   object_get_canonical_path(OBJECT(n-qdev)));
  monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
  qobject_decref(event_data);
  
 -- 
 1.8.1.4

-- 
Amos.