On Tue, Jun 14, 2022 at 02:18:41PM +0300, Andrey Ryabinin wrote:
> Hi
> 
> These couple patches aims to  make possible local migration (within one host)
> on the same TAP device used by source and destination QEMU
> 
> The scenario looks like this
>  1. Create TAP devices and pass file descriptors to source QEMU
>  2. Launch destination QEMU (-incoming defer) and pass same descriptors to it.
>  3. Start migration
> 
> 
> Regarding the first patch: It makes possible to receive file descriptor in non-blocking
> state. But I probably didn't cover all FD users which might need to set blocking state after
> the patch. So I'm hopping for the hints where else, besides fd_start_incoming_migration()
> I need to put qemu_socket_set_block() calls.

Nice feature. I am worried that these patches are unsafe/incomplete
though.

Tap local migration isn't explicitly visible in the code. How will other
developers know the feature is there and how to avoid breaking it when
modifying the code? Maybe a migration test case, comments that explain
the rules about accessing the tap fd, and/or assertions?

How does this interact with hw/net/vhost_net.c, which uses tap_get_fd()
to borrow the fd? I guess the idea is that the source VM is paused and
no tap activity is expected. Then migration handover happens and the
destination VM starts running and is allowed to access the tap fd.
However, the source VM still has vhost_net with the tap fd set up. I
wonder if there is any issue with interference between the two vhost_net
instances?

These kinds of questions should be answered, mostly in the code but also
in the cover letter. It should be clear why this approach is correct.

Thanks,
Stefan

> 
> 
> Andrey Ryabinin (2):
>   chardev: don't set O_NONBLOCK on SCM_RIGHTS file descriptors.
>   tap: initialize TAPState->enabled according to the actual state of
>     queue
> 
>  chardev/char-socket.c |  3 ---
>  io/channel-socket.c   |  3 ---
>  migration/fd.c        |  2 ++
>  net/tap-bsd.c         |  5 +++++
>  net/tap-linux.c       | 12 ++++++++++++
>  net/tap-solaris.c     |  5 +++++
>  net/tap.c             |  2 +-
>  net/tap_int.h         |  1 +
>  8 files changed, 26 insertions(+), 7 deletions(-)
> 
> -- 
> 2.35.1
>