Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event
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
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
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
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
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
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
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
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
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
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.