[Qemu-devel] [PATCH v4 0/1] Add live migration for vhost user

[Qemu-devel] [PATCH v4 0/1] Add live migration for vhost user

[Thibaut Collet]

v3->v4
1. The first patch is updated by:
   - removing the warning trace
   - setting the error trace inside a static bool flag to only print this once
   - removing the vhost_net_inject_rarp function (no more useful)
2. The second patch is temporarly removed.
   vhost user backend is responsible to send the RARP for guest that does not
   support VIRTIO_NET_F_GUEST_ANNOUNCE. More tricks will be delivered later
   ([PATCH RFC]) to help vhost user backend to send RARP at the best time (today
   RARP is sent when the virtual ring is kicked and can occur late).

Thibaut Collet (1):
  vhost user: add support of live migration

 hw/net/vhost_net.c |    2 ++
 net/vhost-user.c   |   21 +++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Thibaut Collet]

Some vhost client/backend are able to support live migration.
To provide this service the following features must be added:
1. Add the VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
   backend is vhost-user.
2. Provide a nop receive callback to vhost-user. This callback is for RARP
   packets automatically send by qemu_announce_self after a migration.
   These packets are useless for vhost user and just discarded.

Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
---
 hw/net/vhost_net.c |    2 ++
 net/vhost-user.c   |   21 +++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9bd360b..668c422 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -85,6 +85,8 @@ static const int user_feature_bits[] = {
     VIRTIO_NET_F_CTRL_MAC_ADDR,
     VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
 
+    VIRTIO_NET_F_GUEST_ANNOUNCE,
+
     VIRTIO_NET_F_MQ,
 
     VHOST_INVALID_FEATURE_BIT
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b51bc04..20778a1 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -65,6 +65,24 @@ static void vhost_user_stop(VhostUserState *s)
     s->vhost_net = 0;
 }
 
+static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
+                                  size_t size)
+{
+    /* A live migration is done. Display an error if the packet is not a RARP.
+     * RARP are just discarded: guest is already notified of live migration
+     * by the virtio-net NIC or by the vhost-user backend */
+    if (size != 60) {
+        static int display_trace = 1;
+
+        if (display_trace) {
+            fprintf(stderr,"Vhost user receives unexpected packets\n");
+            fflush(stderr);
+            display_trace = 0;
+        }
+    }
+    return size;
+}
+
 static void vhost_user_cleanup(NetClientState *nc)
 {
     VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -90,6 +108,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
 static NetClientInfo net_vhost_user_info = {
         .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
         .size = sizeof(VhostUserState),
+        .receive = vhost_user_receive,
         .cleanup = vhost_user_cleanup,
         .has_vnet_hdr = vhost_user_has_vnet_hdr,
         .has_ufo = vhost_user_has_ufo,
@@ -146,8 +165,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
 
         s = DO_UPCAST(VhostUserState, nc, nc);
 
-        /* We don't provide a receive callback */
-        s->nc.receive_disabled = 1;
         s->chr = chr;
         s->nc.queue_index = i;
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Paolo Bonzini]

On 26/06/2015 11:22, Thibaut Collet wrote:
> Some vhost client/backend are able to support live migration.
> To provide this service the following features must be added:
> 1. Add the VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
>    backend is vhost-user.
> 2. Provide a nop receive callback to vhost-user. This callback is for RARP
>    packets automatically send by qemu_announce_self after a migration.
>    These packets are useless for vhost user and just discarded.

When a packet is received by vhost-user, the vhost-user writes the
packet in guest memory.  QEMU must then copy that page of guest memory
from source to destination; it uses a dirty bitmap for this purpose.

How does vhost-user do this?  I can see this patch providing enough
support for *non*live migration.  However, it cannot be enough for live
migration unless I'm missing something obvious.

Paolo

> Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
> ---
>  hw/net/vhost_net.c |    2 ++
>  net/vhost-user.c   |   21 +++++++++++++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 9bd360b..668c422 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -85,6 +85,8 @@ static const int user_feature_bits[] = {
>      VIRTIO_NET_F_CTRL_MAC_ADDR,
>      VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
>  
> +    VIRTIO_NET_F_GUEST_ANNOUNCE,
> +
>      VIRTIO_NET_F_MQ,
>  
>      VHOST_INVALID_FEATURE_BIT
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index b51bc04..20778a1 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -65,6 +65,24 @@ static void vhost_user_stop(VhostUserState *s)
>      s->vhost_net = 0;
>  }
>  
> +static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
> +                                  size_t size)
> +{
> +    /* A live migration is done. Display an error if the packet is not a RARP.
> +     * RARP are just discarded: guest is already notified of live migration
> +     * by the virtio-net NIC or by the vhost-user backend */
> +    if (size != 60) {
> +        static int display_trace = 1;
> +
> +        if (display_trace) {
> +            fprintf(stderr,"Vhost user receives unexpected packets\n");
> +            fflush(stderr);
> +            display_trace = 0;
> +        }
> +    }
> +    return size;
> +}
> +
>  static void vhost_user_cleanup(NetClientState *nc)
>  {
>      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> @@ -90,6 +108,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  static NetClientInfo net_vhost_user_info = {
>          .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>          .size = sizeof(VhostUserState),
> +        .receive = vhost_user_receive,
>          .cleanup = vhost_user_cleanup,
>          .has_vnet_hdr = vhost_user_has_vnet_hdr,
>          .has_ufo = vhost_user_has_ufo,
> @@ -146,8 +165,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>  
>          s = DO_UPCAST(VhostUserState, nc, nc);
>  
> -        /* We don't provide a receive callback */
> -        s->nc.receive_disabled = 1;
>          s->chr = chr;
>          s->nc.queue_index = i;
>  
> 

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Linhaifeng]

On 2015/7/10 21:05, Paolo Bonzini wrote:
>
> On 26/06/2015 11:22, Thibaut Collet wrote:
>> Some vhost client/backend are able to support live migration.
>> To provide this service the following features must be added:
>> 1. Add the VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
>>     backend is vhost-user.
>> 2. Provide a nop receive callback to vhost-user. This callback is for RARP
>>     packets automatically send by qemu_announce_self after a migration.
>>     These packets are useless for vhost user and just discarded.
> When a packet is received by vhost-user, the vhost-user writes the
> packet in guest memory.  QEMU must then copy that page of guest memory
> from source to destination; it uses a dirty bitmap for this purpose.
>
> How does vhost-user do this?  I can see this patch providing enough
> support for *non*live migration.  However, it cannot be enough for live
> migration unless I'm missing something obvious.
>
> Paolo
Agree. vhost-user should mmap the log memory and mark dirty pages when 
send or receive packets.
>> Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
>> ---
>>   hw/net/vhost_net.c |    2 ++
>>   net/vhost-user.c   |   21 +++++++++++++++++++--
>>   2 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index 9bd360b..668c422 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -85,6 +85,8 @@ static const int user_feature_bits[] = {
>>       VIRTIO_NET_F_CTRL_MAC_ADDR,
>>       VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
>>   
>> +    VIRTIO_NET_F_GUEST_ANNOUNCE,
>> +
>>       VIRTIO_NET_F_MQ,
>>   
>>       VHOST_INVALID_FEATURE_BIT
>> diff --git a/net/vhost-user.c b/net/vhost-user.c
>> index b51bc04..20778a1 100644
>> --- a/net/vhost-user.c
>> +++ b/net/vhost-user.c
>> @@ -65,6 +65,24 @@ static void vhost_user_stop(VhostUserState *s)
>>       s->vhost_net = 0;
>>   }
>>   
>> +static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
>> +                                  size_t size)
>> +{
>> +    /* A live migration is done. Display an error if the packet is not a RARP.
>> +     * RARP are just discarded: guest is already notified of live migration
>> +     * by the virtio-net NIC or by the vhost-user backend */
>> +    if (size != 60) {
>> +        static int display_trace = 1;
>> +
>> +        if (display_trace) {
>> +            fprintf(stderr,"Vhost user receives unexpected packets\n");
>> +            fflush(stderr);
>> +            display_trace = 0;
>> +        }
>> +    }
>> +    return size;
>> +}
>> +
>>   static void vhost_user_cleanup(NetClientState *nc)
>>   {
>>       VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
>> @@ -90,6 +108,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>>   static NetClientInfo net_vhost_user_info = {
>>           .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>>           .size = sizeof(VhostUserState),
>> +        .receive = vhost_user_receive,
>>           .cleanup = vhost_user_cleanup,
>>           .has_vnet_hdr = vhost_user_has_vnet_hdr,
>>           .has_ufo = vhost_user_has_ufo,
>> @@ -146,8 +165,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>>   
>>           s = DO_UPCAST(VhostUserState, nc, nc);
>>   
>> -        /* We don't provide a receive callback */
>> -        s->nc.receive_disabled = 1;
>>           s->chr = chr;
>>           s->nc.queue_index = i;
>>   
>>
>
>

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Marc-André Lureau]

Hi

On Mon, Jul 13, 2015 at 4:27 AM, Linhaifeng <haifeng.lin@huawei.com> wrote:
>> When a packet is received by vhost-user, the vhost-user writes the
>> packet in guest memory.  QEMU must then copy that page of guest memory
>> from source to destination; it uses a dirty bitmap for this purpose.
>>
>> How does vhost-user do this?  I can see this patch providing enough
>> support for *non*live migration.  However, it cannot be enough for live
>> migration unless I'm missing something obvious.
>>
>> Paolo
>
> Agree. vhost-user should mmap the log memory and mark dirty pages when send
> or receive packets.

This is already supported by vhost-user protocol, isn't it? The
LOG_BASE/FD and vring log_guest_addr are provided. I can't find any
vhost-user backend implementing dirty bitmaps yet though, but it looks
like it should work. I suppose the backend should stop all IO after
RESET_OWNER is received.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 0/1] Add live migration for vhost user

[Marc-André Lureau]

Hi Thibaut

On Fri, Jun 26, 2015 at 11:22 AM, Thibaut Collet
<thibaut.collet@6wind.com> wrote:
> v3->v4
> 1. The first patch is updated by:
>    - removing the warning trace
>    - setting the error trace inside a static bool flag to only print this once
>    - removing the vhost_net_inject_rarp function (no more useful)
> 2. The second patch is temporarly removed.
>    vhost user backend is responsible to send the RARP for guest that does not
>    support VIRTIO_NET_F_GUEST_ANNOUNCE. More tricks will be delivered later
>    ([PATCH RFC]) to help vhost user backend to send RARP at the best time (today
>    RARP is sent when the virtual ring is kicked and can occur late).

Are you still working on this RFC?

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Paolo Bonzini]

On 17/07/2015 02:19, Marc-André Lureau wrote:
>>> >> How does vhost-user do this?  I can see this patch providing enough
>>> >> support for *non*live migration.  However, it cannot be enough for live
>>> >> migration unless I'm missing something obvious.
>>> >>
>>> >> Paolo
>> >
>> > Agree. vhost-user should mmap the log memory and mark dirty pages when send
>> > or receive packets.
> This is already supported by vhost-user protocol, isn't it? The
> LOG_BASE/FD and vring log_guest_addr are provided. I can't find any
> vhost-user backend implementing dirty bitmaps yet though, but it looks
> like it should work. I suppose the backend should stop all IO after
> RESET_OWNER is received.

But LOG_BASE makes little sense across processes, and LOG_FD is unused
in QEMU, isn't it?  So this patch is not enough to add support of live
migration.

Paolo

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Marc-André Lureau]

Hi

On Fri, Jul 17, 2015 at 4:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> But LOG_BASE makes little sense across processes, and LOG_FD is unused
> in QEMU, isn't it?  So this patch is not enough to add support of live
> migration.

You are right, LOG_BASE doesn't make much sense, and LOG_FD isn't
used, despite some code already there. Furthermore, the log is
allocated with regular malloc, hardly shareable.


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Paolo Bonzini]

On 17/07/2015 12:34, Marc-André Lureau wrote:
> On Fri, Jul 17, 2015 at 4:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > But LOG_BASE makes little sense across processes, and LOG_FD is unused
> > in QEMU, isn't it?  So this patch is not enough to add support of live
> > migration.
>
> You are right, LOG_BASE doesn't make much sense, and LOG_FD isn't
> used, despite some code already there. Furthermore, the log is
> allocated with regular malloc, hardly shareable.

LOG_FD is implemented in the kernel drivers/vhost/vhost.c.  It seems to
be an eventfd-like mechanism to save on dirty bitmap scans.  However,
it's not well documented how to implement it in a correct way.

In any case, live migration needs a new message type (like LOG_MMAP_FD)
in the vhost-user protocol.

Paolo

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Marc-André Lureau]

Hi

On Fri, Jul 17, 2015 at 2:57 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> LOG_FD is implemented in the kernel drivers/vhost/vhost.c.  It seems to
> be an eventfd-like mechanism to save on dirty bitmap scans.  However,
> it's not well documented how to implement it in a correct way.

and it's not used by qemu, so hard to say if it actually work well.

> In any case, live migration needs a new message type (like LOG_MMAP_FD)
> in the vhost-user protocol.

Yes, perhaps with size and offset. I am looking at this.


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Paolo Bonzini]

On 17/07/2015 15:35, Marc-André Lureau wrote:
>> > LOG_FD is implemented in the kernel drivers/vhost/vhost.c.  It seems to
>> > be an eventfd-like mechanism to save on dirty bitmap scans.  However,
>> > it's not well documented how to implement it in a correct way.
> and it's not used by qemu, so hard to say if it actually work well.
> 
> > In any case, live migration needs a new message type (like LOG_MMAP_FD)
> > in the vhost-user protocol.
>
> Yes, perhaps with size and offset. I am looking at this.

The offset should be 0...

Do you know the size of the ram_addr_t space from
VHOST_USER_SET_MEM_TABLE's user address and size fields?

If the size isn't needed, you can reuse LOG_BASE, ignoring the content
of the payload and adding the SCM_RIGHTS file descriptor.

Paolo

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Marc-André Lureau]

Hi

On Fri, Jul 17, 2015 at 3:50 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The offset should be 0...

Yeah, except if we want to prepend other kind of data in the same
allocation, or we want to split somehow usage of this region. I think
it's more future-proof to have it if we introduce a new message.

> Do you know the size of the ram_addr_t space from
> VHOST_USER_SET_MEM_TABLE's user address and size fields?

For some reason, vhost_get_log_size() also takes pc-bios region. I
think it's quite unnecessary given that the backend will not have
access to this region. I can provide a simple fix for that I suppose.

get_log_size() also computes the size of the rings, I wonder if it's
necessary since the dev->mem->regions should already contain the
rings, isn't it?

>
> If the size isn't needed, you can reuse LOG_BASE, ignoring the content
> of the payload and adding the SCM_RIGHTS file descriptor.

That's possible, if we assume that existing backends won't leak the
passed fds (vapp does for ex), perhaps it needs a new flag (vhost-user
features flags are common with vhost, not sure we want to add that
here, but where else)

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration

[Paolo Bonzini]

> > Do you know the size of the ram_addr_t space from
> > VHOST_USER_SET_MEM_TABLE's user address and size fields?
> 
> For some reason, vhost_get_log_size() also takes pc-bios region. I
> think it's quite unnecessary given that the backend will not have
> access to this region.

That's by design.  The pc-bios is mapped into the guest memory, so it
is taken into account.  It shouldn't be a problem, it's just wasting
memory for <4GB VMs but it's not more expensive at run time.

> get_log_size() also computes the size of the rings, I wonder if it's
> necessary since the dev->mem->regions should already contain the
> rings, isn't it?

Indeed.

> > If the size isn't needed, you can reuse LOG_BASE, ignoring the content
> > of the payload and adding the SCM_RIGHTS file descriptor.
> 
> That's possible, if we assume that existing backends won't leak the
> passed fds (vapp does for ex), perhaps it needs a new flag (vhost-user
> features flags are common with vhost, not sure we want to add that
> here, but where else)

Ok, that's for mst to answer.  I think the leak wouldn't be too bad
because right now vhost-user live migration corrupts data so nobody
must be using it.  But if you want to introduce a new message, that
would also be okay of course.

Paolo