Re: [dpdk-dev] [PATCH] vhost: fix leak of fds and mmaps

["Xie, Huawei" <huawei.xie@intel.com>]

On 1/7/2016 10:27 AM, Yuanhan Liu wrote:
> On Tue, Jan 05, 2016 at 02:14:09PM -0800, Rich Lane wrote:
>> The common vhost code only supported a single mmap per device. vhost-user
>> worked around this by saving the address/length/fd of each mmap after the end
>> of the rte_virtio_memory struct. This only works if the vhost-user code frees
>> dev->mem, since the common code is unaware of the extra info. The
>> VHOST_USER_RESET_OWNER message is one situation where the common code frees
>> dev->mem and leaks the fds and mappings. This happens every time I shut down a
>> VM.
> That is a good catch! But I don't think it needs the complexity your
> patch has to fix it. Besides that, your patch has ABI change, which
> is not acceptable at this stage.
>
> Back to the issue, the thing is that, mmap/unmap is vhost-user/vhost-cuse
> specific, thus we'd better to handle them inside vhost-user/vhost-cuse
> differently, but not in the common path, virtio-net.c: let the common
> path handle common things only. That would make the logic clear, and
> hence less error prone.
>
> Note that we have already handled the mmap inside vhost-user/vhost-cuse,
> thinking of that way, there is no reason to handle unmap at virtio-net.c:
> it should be a historic issue while we added vhost-cuse first, which will
> not be an issue until we added vhost-user.

Agree. Let vhost-user part handle its specific cleanup and let the
virtio-net.c handle the common logic.

>
> Another note is that you should not name an internal function with "rte_"
> prefix; it should be reserved for public functions.
>
> 	--yliu
>