Re: [PATCH 0/2] Make local migration with TAP network device possible

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3040 bytes --]

Thanks! I will be offline for the next week.

Feel free to progress this series without my review.

Stefan

On Fri, Jun 24, 2022, 11:53 Andrey Ryabinin <arbn@yandex-team.com> wrote:

>
>
> On 6/15/22 16:10, Stefan Hajnoczi wrote:
> > 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?
> >
>
> Yeah, I'm writing avocado test case, will add it in future next iterations.
>
> > 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?
> >
>
> I'll try to take a closer look at code, let's see if I can find any
> possible issues.
> But I did some brief testing with vhost=on net device, and it find any
> problems.
> The test was
> 1. launch pings from source VM to host
> 2. Migrate
> 3. Check at dest VM that there is no lost packets.
>
>
> > 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
> >>
>
>

[-- Attachment #2: Type: text/html, Size: 3992 bytes --]