On Mon, Oct 19, 2015 at 5:39 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
> 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 <m...@redhat.com> 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
>> > > <thibaut.col...@6wind.com>
>> > > wrote:
>> > >
>> > >
>> > >
>> > > On Fri, Oct 9, 2015 at 5:17 PM, <marcandre.lur...@redhat.com> wrote:
>> > >
>> > > From: Marc-André Lureau <marcandre.lur...@redhat.com>
>> > >
>> > > 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 <thibaut.col...@6wind.com>
>> > >
>> > >
>> > >
>> > >
>> >
>> > > From bd39e9efb7034077f5516debd8b504ffcc10c780 Mon Sep 17 00:00:00 2001
>> > > From: Thibaut Collet <thibaut.col...@6wind.com>
>> > > 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 <thibaut.col...@6wind.com>
>> >
>> > 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.
>
Sorry the fixup was not attached (and it is different from the first one)
I have sent the fixup by this way as the issue appears only on your
tree on the branch for_thibaut. This fixup is mainly just for you to
update the for_thibaut branch.
With the rebase of a patch from live migration series the function
vhost_kernel_get_vq_index appears twice in the file
hw/virtio/vhost-backend.c
> I'm not sure which commit were you testing.
> My tree does not have the lines you removed and it does build.
I am testing the head of the for_thibaut branch
>
> Can you pls check refs/heads/for_thibaut?
> It should have your patch as the latest commit.
I do not see yet my patch on the for_thibaut branch
>
>
>> >
>> > > ---
>> > > 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
>> > >
>> >
From 71fff859db63dc8332854d826e97142706825fef Mon Sep 17 00:00:00 2001
From: Thibaut Collet <thibaut.col...@6wind.com>
Date: Mon, 19 Oct 2015 10:52:43 +0200
Subject: [PATCH] fixup! vhost: use a function for each call
---
hw/virtio/vhost-backend.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 34e26cd..f358017 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -166,13 +166,6 @@ static int vhost_kernel_reset_device(struct vhost_dev *dev)
return vhost_kernel_call(dev, VHOST_RESET_DEVICE, NULL);
}
-static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
-{
- assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
-
- return idx - dev->vq_index;
-}
-
static const VhostOps kernel_ops = {
.backend_type = VHOST_BACKEND_TYPE_KERNEL,
.vhost_backend_init = vhost_kernel_init,
--
2.1.4
