| Message ID | CABUUfwM=2XVd_FLBzsK1xAgYa4GM+8QObY0mfH+AcvsWNAtk5g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 19, 2015 at 06:42:43PM +0200, Thibaut Collet wrote: > > > > 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 Ouch. I meant refs/tags/for_thibaut. Sorry about that.
On Mon, Oct 19, 2015 at 11:12 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Oct 19, 2015 at 06:42:43PM +0200, Thibaut Collet wrote: >> > >> > 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 > > Ouch. I meant refs/tags/for_thibaut. > Sorry about that. > > Sorry for the incorrect wording (I write to quickly my email and use the word branch rather than tag). I use the for_thibaut tag for my live migration test. The fixup for the double definition of vhost_kernel_get_vq_index function id for this tag. To do successfully live migration in any conditions I have removed this double definition and apply the recent sending patch "vhost: set the correct queue index in case of migration with multiqueue" When you say " It should have your patch as the latest commit." you think about which patch ? The "0001-FIXUP-vhost-user-add-support-of-live-migration.patch" one or the "vhost: set the correct queue index in case of migration with multiqueue" one ? Regards. Thibaut.
On Tue, Oct 20, 2015 at 08:30:49AM +0200, Thibaut Collet wrote: > On Mon, Oct 19, 2015 at 11:12 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Oct 19, 2015 at 06:42:43PM +0200, Thibaut Collet wrote: > >> > > >> > 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 > > > > Ouch. I meant refs/tags/for_thibaut. > > Sorry about that. > > > > > > Sorry for the incorrect wording (I write to quickly my email and use > the word branch rather than tag). I use the for_thibaut tag for my > live migration test. The fixup for the double definition of > vhost_kernel_get_vq_index function id for this tag. > To do successfully live migration in any conditions I have removed > this double definition and apply the recent sending patch "vhost: set > the correct queue index in case of migration with multiqueue" > > When you say " It should have your patch as the latest commit." you > think about which patch ? The > "0001-FIXUP-vhost-user-add-support-of-live-migration.patch" one or the > "vhost: set the correct queue index in case of migration with > multiqueue" one ? > > Regards. > > Thibaut. This is where for_thibaut points at for me: commit bf6830e2416f67571ee2e7196f3625725adec170 Author: Thibaut Collet <thibaut.collet@6wind.com> Date: Mon Oct 19 14:59:27 2015 +0200 vhost: set the correct queue index in case of migration with multiqueue When a live migration is started the log address to mark dirty pages is provided to the vhost backend through the vhost_dev_set_log function. This function is called for each queue pairs but the queue index is wrongly set: always set to the first queue pair. Then vhost backend lost descriptor addresses of the queue pairs greater than 1 and behaviour of the vhost backend is unpredictable. The queue index is computed by taking account of the vq_index (to retrieve the queue pair index) and calling the vhost_get_vq_index method of the backend. Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com> hash might change if I find any issues. If this is not what you see, you need to re-fetch the tag. Please let me know whether this tag works for you.
On Tue, Oct 20, 2015 at 12:21 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Tue, Oct 20, 2015 at 08:30:49AM +0200, Thibaut Collet wrote: >> On Mon, Oct 19, 2015 at 11:12 PM, Michael S. Tsirkin <mst@redhat.com> wrote: >> > On Mon, Oct 19, 2015 at 06:42:43PM +0200, Thibaut Collet wrote: >> >> > >> >> > 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 >> > >> > Ouch. I meant refs/tags/for_thibaut. >> > Sorry about that. >> > >> > >> >> Sorry for the incorrect wording (I write to quickly my email and use >> the word branch rather than tag). I use the for_thibaut tag for my >> live migration test. The fixup for the double definition of >> vhost_kernel_get_vq_index function id for this tag. >> To do successfully live migration in any conditions I have removed >> this double definition and apply the recent sending patch "vhost: set >> the correct queue index in case of migration with multiqueue" >> >> When you say " It should have your patch as the latest commit." you >> think about which patch ? The >> "0001-FIXUP-vhost-user-add-support-of-live-migration.patch" one or the >> "vhost: set the correct queue index in case of migration with >> multiqueue" one ? >> >> Regards. >> >> Thibaut. > > > This is where for_thibaut points at for me: > > commit bf6830e2416f67571ee2e7196f3625725adec170 > Author: Thibaut Collet <thibaut.collet@6wind.com> > Date: Mon Oct 19 14:59:27 2015 +0200 > > vhost: set the correct queue index in case of migration with multiqueue > > When a live migration is started the log address to mark dirty pages is provided > to the vhost backend through the vhost_dev_set_log function. > This function is called for each queue pairs but the queue index is wrongly set: > always set to the first queue pair. Then vhost backend lost descriptor addresses > of the queue pairs greater than 1 and behaviour of the vhost backend is > unpredictable. > > The queue index is computed by taking account of the vq_index (to retrieve the > queue pair index) and calling the vhost_get_vq_index method of the backend. > > Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com> > > > hash might change if I find any issues. > > If this is not what you see, you need to re-fetch the tag. Ok I got and tested the updated branch. The problem, with my previous attempt, is there are a tag and a branch called for_thibaut. This morning after re-fetch operation I do a "git checkout for_thibaut" that takes the tag and not the branch. I realize it with the commit sha-1 of the "vhost: set the correct queue index in case of migration with multiqueue" patch (sha-1 of commit of for_thibaut tag is e20cff854) > > Please let me know whether this tag works for you. > I have tested this version (commit bf6830e24) with live migration and everything is OK. tested-by: Thibaut Collet <thibaut.collet@6wind.com> > -- > MST
From 71fff859db63dc8332854d826e97142706825fef Mon Sep 17 00:00:00 2001 From: Thibaut Collet <thibaut.collet@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