Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration
> > 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
Re: [Qemu-devel] [PATCH v4 1/1] vhost user: add support of live migration
Hi On Fri, Jul 17, 2015 at 3:50 PM, Paolo Bonzini 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
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
Hi On Fri, Jul 17, 2015 at 2:57 PM, Paolo Bonzini 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
On 17/07/2015 12:34, Marc-André Lureau wrote: > On Fri, Jul 17, 2015 at 4:25 AM, Paolo Bonzini 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
Hi On Fri, Jul 17, 2015 at 4:25 AM, Paolo Bonzini 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
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
Hi On Mon, Jul 13, 2015 at 4:27 AM, Linhaifeng 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 1/1] vhost user: add support of live migration
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
---
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
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
> ---
> 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;
>
>
