Re: [Qemu-devel] [PATCH] net: synchronize net_host_device_remove with host_net_remove_completion

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/02/2015 14:54, Stefan Hajnoczi wrote:
> On Mon, Jan 19, 2015 at 12:27:11PM +0100, Paolo Bonzini wrote:
>> On 02/01/2015 17:20, Paolo Bonzini wrote:
>>>>>
>>>>> The assert can be dropped completely since the code already has an
>>>>> equivalent assert:
>>>>>
>>>>>   queues = qemu_find_net_clients_except(nc->name, ncs,
>>>>>                                         NET_CLIENT_OPTIONS_KIND_NIC,
>>>>>                                         MAX_QUEUE_NUM);
>>>>>   assert(queues != 0); <-- fail if type == NET_CLIENT_OPTIONS_KIND_NIC
>>> I left it on purpose for documentation, but I'll send v2 next week that
>>> removes it.
>>
>> Actually it's not the same.  If you have "-netdev user,id=e1000 -device
>> e1000,netdev=e1000" you will be able to call qemu_del_net_client on the
>> NIC, and it will _not_ fail if the assertion is removed.
> 
> I don't follow.
> 
> If you call qemu_del_net_client(e1000_nic) then
> qemu_find_net_clients_except(nc->name, ncs, NET_CLIENT_OPTIONS_KIND_NIC,
> MAX_QUEUE_NUM) returns 0.  This causes the assert(queues != 0) to fail.

NICs and other clients are in separate namespaces.  So if you do

   -netdev user,id=e1000
   -device e1000,netdev=e1000,id=e1000

you have two NetClients named "e1000".  If you call (by mistake)
qemu_del_net_client(e1000_nic), qemu_find_net_clients_except will return
the SLIRP client and the assertion will not fail.

So you need a separate assertion.

Paolo