Re: [Qemu-devel] [PATCH v2 3/6] net: make netclient re-entrant with refcnt

[liu ping fan]

On Tue, Jun 18, 2013 at 8:41 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Thu, Jun 13, 2013 at 05:03:03PM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <address@hidden>
>>
>> With refcnt, NetClientState's user can run agaist deleter.
>
> Please split this into two patches:
>
> 1. net_clients lock
> 2. NetClientState refcount
>
Ok.
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ---
>>  hw/core/qdev-properties-system.c | 14 ++++++++++++
>>  include/net/net.h                |  3 +++
>>  net/hub.c                        |  3 +++
>>  net/net.c                        | 47 
>> +++++++++++++++++++++++++++++++++++++---
>>  net/slirp.c                      |  3 ++-
>>  5 files changed, 66 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/core/qdev-properties-system.c 
>> b/hw/core/qdev-properties-system.c
>> index 0eada32..41cc7e6 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -302,6 +302,7 @@ static void set_vlan(Object *obj, Visitor *v, void 
>> *opaque,
>>          return;
>>      }
>>
>> +    /* inc ref, released when unset property */
>>      hubport = net_hub_port_find(id);
>>      if (!hubport) {
>>          error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> @@ -311,11 +312,24 @@ static void set_vlan(Object *obj, Visitor *v, void 
>> *opaque,
>>      *ptr = hubport;
>>  }
>>
>> +static void release_vlan(Object *obj, const char *name, void *opaque)
>> +{
>> +    DeviceState *dev = DEVICE(obj);
>> +    Property *prop = opaque;
>> +    NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
>> +    NetClientState **ptr = &peers_ptr->ncs[0];
>> +
>> +    if (*ptr) {
>> +        netclient_unref(*ptr);
>> +    }
>> +}
>> +
>>  PropertyInfo qdev_prop_vlan = {
>>      .name  = "vlan",
>>      .print = print_vlan,
>>      .get   = get_vlan,
>>      .set   = set_vlan,
>> +    .release = release_vlan,
>>  };
>>
>>  int qdev_prop_set_drive(DeviceState *dev, const char *name,
>
> What about the netdev property?  I don't see any refcount code there.
>
Yes, the release of netdev and vlan property should all free its
backend. Will add the code.
>> @@ -1109,6 +1146,7 @@ void net_cleanup(void)
>>              qemu_del_net_client(nc);
>>          }
>>      }
>> +    qemu_mutex_destroy(&net_clients_lock);
>
> Why is it okay to iterate over net_clients here without the lock?

 atexit(&net_cleanup); So no other racers exist.

Thx & Regards,
Pingfan