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

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

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.

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

	--yliu