Re: [Qemu-devel] [PATCH MST/PCI] additional fixes for mac-programming feature

[Markus Armbruster]

Amos Kong <address@hidden> writes:

> Markus added some comments on old patchset, this patch contains
> some additional fixes, it's based on MST's PCI tree.
>
> * Fix typos (missed 1.6, NIC)
> * Don't initialize list point at its declaration
> * Always notify QMP client if mactable is changed
> * Returns NULL list if no net client supports rx-filter querying.
>
> BTW, we can also use e1000 with macvtap device, so we can implement
> a query_rx_filter function for e1000 in future.
>
> Signed-off-by: Amos Kong <address@hidden>

Addresses those parts of my review that have made it into the PCI tree,
thus:

Reviewed-by: Markus Armbruster <address@hidden>

One suggestion inline.

> ---
>  hw/net/virtio-net.c | 24 ++++++++++++++----------
>  net/net.c           | 19 ++++++++++---------
>  qapi-schema.json    | 11 ++++++-----
>  qmp-commands.hx     | 10 +++++-----
>  4 files changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index c88403a..679f50c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -226,10 +226,8 @@ static RxFilterInfo 
> *virtio_net_query_rxfilter(NetClientState *nc)
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
>      RxFilterInfo *info;
> -    strList *str_list = NULL;
> -    strList *entry;
> -    intList *int_list = NULL;
> -    intList *int_entry;
> +    strList *str_list, *entry;
> +    intList *int_list, *int_entry;
>      int i, j;
>  
>      info = g_malloc0(sizeof(*info));
> @@ -258,6 +256,7 @@ static RxFilterInfo 
> *virtio_net_query_rxfilter(NetClientState *nc)
>  
>      info->main_mac = mac_strdup_printf(n->mac);
>  
> +    str_list = NULL;
>      for (i = 0; i < n->mac_table.first_multi; i++) {
>          entry = g_malloc0(sizeof(*entry));
>          entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
           entry->next = str_list;
           str_list = entry;
       }
       info->unicast_table = str_list;

       str_list = NULL;
       for (i = n->mac_table.first_multi; i < n->mac_table.in_use; i++) {
           entry = g_malloc0(sizeof(*entry));
           entry->value = mac_strdup_printf(n->mac_table.macs + i * ETH_ALEN);
           entry->next = str_list;
           str_list = entry;
       }
       info->multicast_table = str_list;

Suggest a helper function to build str_list.  Can be done as a follow-up
patch, no need to respin this one.

[...]