Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support

[Thibaut Collet]

On Sun, Oct 18, 2015 at 10:21 AM, Michael S. Tsirkin <address@hidden> wrote:
>
> On Tue, Oct 13, 2015 at 02:19:41PM +0200, Thibaut Collet wrote:
> > Hi,
> >
> > I have still a comment on this serie. During rebase operation with 
> > multiqueue a
> > modification has been lost.
> > This lost impact only guest without GUEST_ANNOUNCE capabilities: the 
> > backend is
> > not notified to send a fake RARP to reduce VM outage.
> >
> > Sorry for the late notice but I have tested only recent guest to give my ack
> > yesterday.
> >
> > Marc Andre and Michael could you apply the attached fixup to the patch 
> > "vhost
> > user: add support of live migration" on the pull request ?
> >
> > Thanks
> >
> > Best regards.
>
> The way to post fixups is just like a regular patch, but prepend
> fixup! on the subject line.
> Comments can come after the --- line.
>
>
>
> > On Mon, Oct 12, 2015 at 5:56 PM, Thibaut Collet <address@hidden>
> > wrote:
> >
> >
> >
> >     On Fri, Oct 9, 2015 at 5:17 PM, <address@hidden> wrote:
> >
> >         From: Marc-André Lureau <address@hidden>
> >
> >         Hi,
> >
> >         The following series implement shareable log for vhost-user to 
> > support
> >         memory tracking during live migration. On qemu-side, the solution is
> >         fairly straightfoward since vhost already supports the dirty log, 
> > only
> >         vhost-user couldn't access the log memory until then.
> >
> >         The series includes "vhost user: Add live migration" patches from
> >         Thibaut Collet.
> >
> >         v7->v8:
> >         - rebased
> >         - fix build on osx & aarch64
> >         - add seccomp patch from Eduardo
> >         - fix enum usage and MQ (squashed Thibaut fix)
> >         - fixed vhost_net_notify_migration_done() fallback
> >         - split util-obj- on multi-lines in seperate patch
> >
> >         v6->v7:
> >         - add migration blocker if memfd failed
> >         - add doc about the qemu_memfd_alloc() helper
> >         - (rebase on top of Michael pci branch)
> >
> >         v5->v6:
> >         - rebased
> >         - fix protocol feature mask update
> >         - add a patch from Thibault Collet using enum for features, and
> >           compute mask
> >         - add unistd.h linux headers to help building memfd if missing on
> >           build host. Hopefully will be useful for other syscalls some day
> >         - reorder/merge patches to share-allocate the log only if needed
> >         - split the memfd helper to allow to drop the fallback code
> >         - allow to call qemu_memfd_free() even if alloc failed
> >         - add some missing spaces
> >
> >         v4->v5:
> >         - rebase on top of last Michael S. Tsirkin PULL request
> >         - block live migration if !PROTOCOL_F_LOG_SHMFD
> >         - wait for a reply after SET_LOG_BASE
> >         - split vhost_set_log_base from the rest of vhost_call refactoring
> >         - use a seperate global vhost_log_shm
> >
> >         v3->v4:
> >         - add the proto negotiation & the migration series
> >         - replace the varargs vhost_call() approach for callbacks
> >         - only share-allocate when the backend needs it
> >
> >         v2->v3:
> >         - changed some patch summary
> >         - added migration tests
> >         - added a patch to replace error message with a trace
> >
> >         The development branch I used is:
> >         https://github.com/elmarco/qemu branch "vhost-user"
> >
> >         Eduardo Otubo (1):
> >           seccomp: add memfd_create to whitelist
> >
> >         Marc-André Lureau (22):
> >           configure: probe for memfd
> >           linux-headers: add unistd.h
> >           build-sys: split util-obj- on multi-lines
> >           util: add linux-only memfd fallback
> >           util: add memfd helpers
> >           util: add fallback for qemu_memfd_alloc()
> >           vhost: document log resizing
> >           vhost: add vhost_set_log_base op
> >           vhost-user: add vhost_user_requires_shm_log()
> >           vhost: alloc shareable log
> >           vhost-user: send log shm fd along with log_base
> >           vhost-user: add a migration blocker
> >           vhost: use a function for each call
> >           vhost-user: document migration log
> >           net: add trace_vhost_user_event
> >           vhost: add migration block if memfd failed
> >           vhost-user-test: move wait_for_fds() out
> >           vhost-user-test: remove useless static check
> >           vhost-user-test: wrap server in TestServer struct
> >           vhost-user-test: learn to tweak various qemu arguments
> >           vhost-user-test: add live-migration test
> >           vhost-user-test: check ownership during migration
> >
> >         Michael S. Tsirkin (1):
> >           exec: factor out duplicate mmap code
> >
> >         Thibaut Collet (3):
> >           vhost user: add support of live migration
> >           vhost user: add rarp sending after live migration for legacy guest
> >           vhost-user: use an enum helper for features mask
> >
> >          configure                          |   19 +
> >          docs/specs/vhost-user.txt          |   63 ++-
> >          exec.c                             |   47 +-
> >          hw/net/vhost_net.c                 |   35 +-
> >          hw/scsi/vhost-scsi.c               |    7 +-
> >          hw/virtio/vhost-backend.c          |  121 +++-
> >          hw/virtio/vhost-user.c             |  576 ++++++++++++-------
> >          hw/virtio/vhost.c                  |  121 ++--
> >          include/hw/virtio/vhost-backend.h  |   77 ++-
> >          include/hw/virtio/vhost.h          |   15 +-
> >          include/net/vhost_net.h            |    1 +
> >          include/qemu/memfd.h               |   26 +
> >          include/qemu/mmap-alloc.h          |   10 +
> >          linux-headers/asm-arm/unistd.h     |  448 +++++++++++++++
> >          linux-headers/asm-arm64/kvm.h      |   37 +-
> >          linux-headers/asm-arm64/unistd.h   |   16 +
> >          linux-headers/asm-mips/unistd.h    | 1063
> >         ++++++++++++++++++++++++++++++++++++
> >          linux-headers/asm-powerpc/unistd.h |  392 +++++++++++++
> >          linux-headers/asm-s390/unistd.h    |  404 ++++++++++++++
> >          linux-headers/asm-x86/unistd.h     |   15 +
> >          linux-headers/asm-x86/unistd_32.h  |  377 +++++++++++++
> >          linux-headers/asm-x86/unistd_64.h  |  330 +++++++++++
> >          linux-headers/asm-x86/unistd_x32.h |  319 +++++++++++
> >          net/vhost-user.c                   |   34 +-
> >          qemu-seccomp.c                     |    3 +-
> >          scripts/update-linux-headers.sh    |    7 +-
> >          tests/vhost-user-test.c            |  372 +++++++++++--
> >          trace-events                       |    3 +
> >          util/Makefile.objs                 |   13 +-
> >          util/memfd.c                       |  162 ++++++
> >          util/mmap-alloc.c                  |   71 +++
> >          util/oslib-posix.c                 |   28 +-
> >          32 files changed, 4814 insertions(+), 398 deletions(-)
> >          create mode 100644 include/qemu/memfd.h
> >          create mode 100644 include/qemu/mmap-alloc.h
> >          create mode 100644 linux-headers/asm-arm/unistd.h
> >          create mode 100644 linux-headers/asm-arm64/unistd.h
> >          create mode 100644 linux-headers/asm-mips/unistd.h
> >          create mode 100644 linux-headers/asm-powerpc/unistd.h
> >          create mode 100644 linux-headers/asm-s390/unistd.h
> >          create mode 100644 linux-headers/asm-x86/unistd.h
> >          create mode 100644 linux-headers/asm-x86/unistd_32.h
> >          create mode 100644 linux-headers/asm-x86/unistd_64.h
> >          create mode 100644 linux-headers/asm-x86/unistd_x32.h
> >          create mode 100644 util/memfd.c
> >          create mode 100644 util/mmap-alloc.c
> >
> >         --
> >         2.4.3
> >
> >
> >
> >     Acked-by: Thibaut Collet <address@hidden>
> >
> >
> >
> >
>
> > From bd39e9efb7034077f5516debd8b504ffcc10c780 Mon Sep 17 00:00:00 2001
> > From: Thibaut Collet <address@hidden>
> > Date: Tue, 13 Oct 2015 14:01:44 +0200
> > Subject: [PATCH] FIXUP: vhost user: add support of live migration
> >
> > Removal of receive disable has been lost during rebase with multiqueue 
> > feature.
> > Without this fixup migration of old guest (without GUEST_ANNOUNCE) is not
> > notified as the RARP request is not sent to the vhost user backend.
> >
> > Signed-off-by: Thibaut Collet <address@hidden>
>
> I picked this up, but I had to do a lot of tweaking to even
> get the patch series to apply.
>
> If you want all this to go upstream, please test it here:
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git refs/tags/for_thibaut
>
> And send a tested-by tag on list.
>

Thanks for your work.
I have just finished to test migration with your branch with the
following conditions:
 - Ubuntu14, multiqueue not set, VM with GUEST_ANNOUNCE feature
 - Ubuntu14, multiqueue not set, VM without GUEST_ANNOUNCE feature
 - Ubuntu14, multiqueue set (8 queue pairs), VM with GUEST_ANNOUNCE feature
 - Ubuntu14, multiqueue set (8 queue pairs), VM without GUEST_ANNOUNCE feature

The VM is just pinged during the migration to have small traffic.

After my test I have two remarks:
1. The branch does not compile due to a rebase issue (for commit
ed2ae0bee5). Please could you apply the attached fixup on your branch
to solve it (I hope the fixup is correctly written) ?
2. Migration test with multiqueue set has an incorrect behaviour. I
have just send a patch on the qemu mailing list to solve it ([PATCH
0/1] vhost-user: support of live migration with multiqueue and [PATCH
1/1] vhost: set the correct queue index in case of migration with
multiqueue)

With the attached fixup and "vhost: set the correct queue index in
case of migration with multiqueue" patch the migration works in the 4
described cases.
If you agree and integrate the fixup and the patch on your branch I
can give you my tested-by

Best regards.

Thibaut.

>
> > ---
> >  net/vhost-user.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index cfe11b8..17b5c2a 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -212,8 +212,6 @@ static int net_vhost_user_init(NetClientState *peer, 
> > const char *device,
> >          snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to %s",
> >                   i, chr->label);
> >
> > -        /* We don't provide a receive callback */
> > -        nc->receive_disabled = 1;
> >          nc->queue_index = i;
> >
> >          s = DO_UPCAST(VhostUserState, nc, nc);
> > --
> > 2.1.4
> >
>