Re: [PATCH v2 0/2] Move net backend cleanup to NIC cleanup

[Jonah Palmer <jonah.palmer@oracle.com>]

On 12/23/24 11:33 AM, Eugenio Perez Martin wrote:
> On Fri, Dec 20, 2024 at 10:02 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
>>
>> 12.09.2024 19:54, Eugenio Pérez wrote:
>>> Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
>>> structures if peer nic is present") effectively delayed the backend
>>> cleanup, allowing the frontend or the guest to access it resources as
>>> long as the frontend NIC is still visible to the guest.
>>>
>>> However it does not clean up the resources until the qemu process is
>>> over.  This causes an effective leak if the device is deleted with
>>> device_del, as there is no way to close the vdpa device.  This makes
>>> impossible to re-add that device to this or other QEMU instances until
>>> the first instance of QEMU is finished.
>>>
>>> Move the cleanup from qemu_cleanup to the NIC deletion.
>>>
>>> v2:
>>> Remove NIC peer also at net_cleanup. vhost-user trust all the
>>> backends are clean before qemu removes char devices.
>>>
>>> This is not a requisite introduced by this commit as
>>> system/runstate.c:qemu_cleanup shows.
>>>
>>> Eugenio Pérez (2):
>>>     net: parameterize the removing client from nc list
>>>     net: move backend cleanup to NIC cleanup
>>>
>>>    net/net.c        | 44 ++++++++++++++++++++++++++++++++++----------
>>>    net/vhost-vdpa.c |  8 --------
>>>    2 files changed, 34 insertions(+), 18 deletions(-)
>> Hi!
>>
>> It looks like this series has been forgotten.  Is it still needed?
>>
>> In order for it to build, a single line in patch 2 needs to be
>> changed (in net_cleanup()), from:
>>
>> +                for (int i = 0; i < queues; i++) {
>> +                    NetClientState *nc = qemu_get_subqueue(nic, i);
>> +                    qemu_cleanup_net_client(nc->peer, false);
>> +                }
>>
>> to
>>
>> +                for (int i = 0; i < queues; i++) {
>> +                    nc = qemu_get_subqueue(nic, i);
>> +                    qemu_cleanup_net_client(nc->peer, false);
>> +                }
>>
>> so there's no variable shadowing anymore.
>>
>> Should this series be resent (a v3), or can this be fixed at apply time?
>>
> 
> I guess it is better to resend a v3, so we don't have a compiler
> warning (or error). Jonah, maybe do you want to move this one forward?
> 
> Thanks!
> 
Just sent out the v3 with the quick fix.

Ugh, and I just realized I forgot to CC Michael Tokarev in the v3 
series. My apologies Michael!

Jonah