On Tue, Jun 18, 2013 at 8:41 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Thu, Jun 13, 2013 at 05:03:03PM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>>
>> 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 <pingf...@linux.vnet.ibm.com>
>> ---
>> 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
