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