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

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 03:22:12PM +0200, Thibaut Collet wrote:
> 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.

I don't see any fixup - is this the part below?
Please post patches in the regular way.

I'm not sure which commit were you testing.
My tree does not have the lines you removed and it does build.

Can you pls check refs/heads/for_thibaut?
It should have your patch as the latest commit.


> >
> > > ---
> > >  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
> > >
> >