Re: [Qemu-devel] [PATCH] Revert "vhost: send SET_VRING_ENABLE at start/stop"
On Wed, Nov 25, 2015 at 1:42 PM, Michael S. Tsirkin wrote:
> This reverts commit 3a12f32229a046f4d4ab0a3a52fb01d2d5a1ab76.
>
> In case of live migration several queues can be enabled and not only the
> first one. So informing backend that only the first queue is enabled is
> wrong.
>
> Reported-by: Thibaut Collet
> Cc: Yuanhan Liu
> Signed-off-by: Michael S. Tsirkin
> ---
> hw/virtio/vhost.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1794f0d..de29968 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev,
> VirtIODevice *vdev)
> }
> }
>
> -if (hdev->vhost_ops->vhost_set_vring_enable) {
> -/* only enable first vq pair by default */
> -hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index == 0);
> -}
> -
> return 0;
> fail_log:
> vhost_log_put(hdev, false);
> @@ -1261,10 +1256,6 @@ void vhost_dev_stop(struct vhost_dev *hdev,
> VirtIODevice *vdev)
> hdev->vq_index + i);
> }
>
> -if (hdev->vhost_ops->vhost_set_vring_enable) {
> -hdev->vhost_ops->vhost_set_vring_enable(hdev, 0);
> -}
> -
> vhost_log_put(hdev, true);
> hdev->started = false;
> hdev->log = NULL;
> --
Ack
> MST
Re: [Qemu-devel] [PATCH for-2.5 1/1] vhost-user: do not send SET_VRING_ENABLE at start
On Wed, Nov 25, 2015 at 12:04 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 25, 2015 at 09:32:15AM +0800, Yuanhan Liu wrote:
>> On Tue, Nov 24, 2015 at 11:23:34PM +0200, Michael S. Tsirkin wrote:
>> > On Tue, Nov 24, 2015 at 10:05:27PM +0100, Thibaut Collet wrote:
>> > > On Tue, Nov 24, 2015 at 9:52 PM, Michael S. Tsirkin
>> > > wrote:
>> > > > On Tue, Nov 24, 2015 at 05:10:36PM +0100, Thibaut Collet wrote:
>> > > >> This patch reverts partially commit 3a12f32229a.
>> > > >>
>> > > >> In case of live migration several queues can be enabled and not only
>> > > >> the first
>> > > >> one. So inform backend that only the first queue is enabled is wrong.
>> > > >>
>> > > >> Since commit 7263a0ad7899 backend is already notified of the state of
>> > > >> the vring
>> > > >> through the vring attach operation. This function, called during the
>> > > >> startup
>> > > >> sequence, provides the correct state of the vring, even in case of
>> > > >> live
>> > > >> migration.
>> > > >>
>> > > >> So nothing has to be added to give the vring state to the backend at
>> > > >> the startup.
>> > > >>
>> > > >> Signed-off-by: Thibaut Collet
>> > > >> ---
>> > > >> hw/virtio/vhost.c | 5 -
>> > > >> 1 file changed, 5 deletions(-)
>> > > >>
>> > > >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> > > >> index 1794f0d..870cd12 100644
>> > > >> --- a/hw/virtio/vhost.c
>> > > >> +++ b/hw/virtio/vhost.c
>> > > >> @@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev,
>> > > >> VirtIODevice *vdev)
>> > > >> }
>> > > >> }
>> > > >>
>> > > >> -if (hdev->vhost_ops->vhost_set_vring_enable) {
>> > > >> -/* only enable first vq pair by default */
>> > > >> -hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index
>> > > >> == 0);
>> > > >> -}
>> > > >> -
>> > > >> return 0;
>> > > >> fail_log:
>> > > >> vhost_log_put(hdev, false);
>> > > >> --
>> > > >> 2.1.4
>> > > >
>> > > > Yes - and I'm beginning to think that maybe we should revert
>> > > > all of 3a12f32229a then, for symmetry.
>> > > >
>> > >
>> > > Keep the disable vring on the stop can be useful. For example if the
>> > > VM is rebooted all the vring will be disabled and backend will avoid
>> > > to send packet to the VM in this case (I am not sure the virtio ring
>> > > address is always valid during a reboot and writingg data in this
>> > > memory can cause unexpected behaviour in this case).
>> >
>> > I think there's still some confusion:
>> > writing memory can still happen even if you disable the ring
>> > since the TX ring is still processed so we write into the used ring.
>> >
>> > We call GET_VRING_BASE on stop and that ensures rings are
>> > stopped.
>>
>> Yes, that's what I suggested first, which also makes the logic quite
>> simple: we use GET_VRING_BASE as the sign of vring stop. Intead of
>> GET_VRING_BASE when protocol not negotiated, and SET_VRING_ENABLE
>> when protocol is negotiated.
>>
>> Michael, should I submit a revert patch, or you could do it directly?
>>
>> --yliu
>
> I can handle it.
>
The patch must be completly reverted. There are too an issue with
suspend/resume operations.
On the suspend the dev_stop is called that will disable all the vrings.
On the resume the dev_start is called but not the peer_attach (vring
is already attached) and state of the vring is not provided to the
backend.
>> >
>> >
>> > > > Yunnan, Victor - what do you think?
>> > > >
>> > > > --
>> > > > MST
Re: [Qemu-devel] [PATCH for-2.5 1/1] vhost-user: do not send SET_VRING_ENABLE at start
On Tue, Nov 24, 2015 at 9:52 PM, Michael S. Tsirkin wrote:
> On Tue, Nov 24, 2015 at 05:10:36PM +0100, Thibaut Collet wrote:
>> This patch reverts partially commit 3a12f32229a.
>>
>> In case of live migration several queues can be enabled and not only the
>> first
>> one. So inform backend that only the first queue is enabled is wrong.
>>
>> Since commit 7263a0ad7899 backend is already notified of the state of the
>> vring
>> through the vring attach operation. This function, called during the startup
>> sequence, provides the correct state of the vring, even in case of live
>> migration.
>>
>> So nothing has to be added to give the vring state to the backend at the
>> startup.
>>
>> Signed-off-by: Thibaut Collet
>> ---
>> hw/virtio/vhost.c | 5 -
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 1794f0d..870cd12 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev,
>> VirtIODevice *vdev)
>> }
>> }
>>
>> -if (hdev->vhost_ops->vhost_set_vring_enable) {
>> -/* only enable first vq pair by default */
>> -hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index == 0);
>> -}
>> -
>> return 0;
>> fail_log:
>> vhost_log_put(hdev, false);
>> --
>> 2.1.4
>
> Yes - and I'm beginning to think that maybe we should revert
> all of 3a12f32229a then, for symmetry.
>
Keep the disable vring on the stop can be useful. For example if the
VM is rebooted all the vring will be disabled and backend will avoid
to send packet to the VM in this case (I am not sure the virtio ring
address is always valid during a reboot and writingg data in this
memory can cause unexpected behaviour in this case).
> Yunnan, Victor - what do you think?
>
> --
> MST
[Qemu-devel] [PATCH for-2.5 1/1] vhost-user: do not send SET_VRING_ENABLE at start
This patch reverts partially commit 3a12f32229a.
In case of live migration several queues can be enabled and not only the first
one. So inform backend that only the first queue is enabled is wrong.
Since commit 7263a0ad7899 backend is already notified of the state of the vring
through the vring attach operation. This function, called during the startup
sequence, provides the correct state of the vring, even in case of live
migration.
So nothing has to be added to give the vring state to the backend at the
startup.
Signed-off-by: Thibaut Collet
---
hw/virtio/vhost.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1794f0d..870cd12 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice
*vdev)
}
}
-if (hdev->vhost_ops->vhost_set_vring_enable) {
-/* only enable first vq pair by default */
-hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index == 0);
-}
-
return 0;
fail_log:
vhost_log_put(hdev, false);
--
2.1.4
[Qemu-devel] [PATCH for-2.5 0/1] vhost-user: live migration with multiqueue fails
Since commit 3a12f32229a live migration of vhost user, if multiqueue is set, fails: The state of the vring are lost, only the first queue pair is set to enable even if there are several queue pairs enabled. Thibaut Collet (1): vhost-user: do not send SET_VRING_ENABLE at start hw/virtio/vhost.c | 5 - 1 file changed, 5 deletions(-) -- 2.1.4
Re: [Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set
On Mon, Nov 16, 2015 at 6:09 PM, Michael S. Tsirkin wrote:
> On Mon, Nov 16, 2015 at 05:53:10PM +0100, Thibaut Collet wrote:
>> On Mon, Nov 16, 2015 at 5:21 PM, Michael S. Tsirkin wrote:
>> > On Mon, Nov 16, 2015 at 05:14:37PM +0100, Thibaut Collet wrote:
>> >> Fixes: 2b8819c6eee5 ("vhost-user: modify SET_LOG_BASE to pass mmap size
>> >> and
>> >> offset")
>> >>
>> >> For compatibility with old vhost backend content of the SET_LOG_BASE
>> >> message
>> >> can not be modified.
>> >
>> > Hmm that's true. Interesting. But this only happens on migration,
>> > right? And if VHOST_USER_PROTOCOL_F_LOG_SHMFD is not set
>> > then we block migration. So how come the old message
>> > is ever sent?
>> >
>>
>> Agree. With the migration blocker on VHOST_USER_PROTOCOL_F_LOG_SHMFD
>> message with the new payload can not be sent to old vhost backend.
>> The documentation is a little bit confusing. The message payload for
>> SET_LOG_BASE still indicates a u64 whereas it is no more the case.
>>
>> Modification on the vhost-user.c file is not really needed but maybe
>> we can keep the patch on the doc ?
>
> Absolutely but let's say something like
> Historically migration would still happen when
> VHOST_USER_PROTOCOL_F_LOG_SHMFD was not negotiated.
> In that case, the value sent was u64 with no file
> descriptors. This message should be ignored.
>
OK.
Regarding the code of the function vhost_user_set_log_base any test on
the shmfd boolean can be removed, regarding the design it is always
true, and it will avoid any doubt if this function can be called with
other conditions than migration. A safety check can be added to log
this unexpected case i.e something like that:
@@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
.size = sizeof(msg.payload.log),
};
+if (!shmfd) {
+error_report("SET_LOG_BASE can not occur if "
+ "VHOST_USER_PROTOCOL_F_LOG_SHMFD is not set ");
+return -1;
+}
-if (shmfd && log->fd != -1) {
+if (log->fd != -1) {
fds[fd_num++] = log->fd;
}
vhost_user_write(dev, &msg, fds, fd_num);
-if (shmfd) {
[snip]
-}
return 0;
}
>> >> The SET_LOG_BASE message payload is modified only if the
>> >> VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature has been negociated.
>> >>
>> >> The documentation has been updated accordingly with remarks from Marc
>> >> André
>> >> Lureau.
>> >>
>> >> Signed-off-by: Thibaut Collet
>> >> ---
>> >> docs/specs/vhost-user.txt | 16 ++--
>> >> hw/virtio/vhost-user.c| 12 +---
>> >> 2 files changed, 23 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> >> index 26dde2e..da4bf9c 100644
>> >> --- a/docs/specs/vhost-user.txt
>> >> +++ b/docs/specs/vhost-user.txt
>> >> @@ -87,6 +87,14 @@ Depending on the request type, payload can be:
>> >> User address: a 64-bit user address
>> >> mmap offset: 64-bit offset where region starts in the mapped memory
>> >>
>> >> + * vhost user log description
>> >> + -
>> >> + | size | offset |
>> >> + -
>> >> +
>> >> + size: a 64-bit size
>> >> + Offset: a 64-bit offset where log starts in the mapped memory
>> >> +
>> >> In QEMU the vhost-user message is implemented with the following struct:
>> >>
>> >> typedef struct VhostUserMsg {
>> >> @@ -280,14 +288,18 @@ Message types
>> >>
>> >>Id: 6
>> >>Equivalent ioctl: VHOST_SET_LOG_BASE
>> >> - Master payload: u64
>> >> + Master payload: - u64 if slave has not the
>> >> VHOST_USER_PROTOCOL_F_LOG_SHMFD
>> >> +protocol feature
>> >> + - vhost user log if slave has the
>> >> +VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature
>> >>Slave payload: N/A
>> >>
>> >>Sets logging shared memory space.
>> >>When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol
>> >>feature, the log memory fd is provided in the ancillary data of
>> >>VHOS
Re: [Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set
On Mon, Nov 16, 2015 at 5:21 PM, Michael S. Tsirkin wrote:
> On Mon, Nov 16, 2015 at 05:14:37PM +0100, Thibaut Collet wrote:
>> Fixes: 2b8819c6eee5 ("vhost-user: modify SET_LOG_BASE to pass mmap size and
>> offset")
>>
>> For compatibility with old vhost backend content of the SET_LOG_BASE message
>> can not be modified.
>
> Hmm that's true. Interesting. But this only happens on migration,
> right? And if VHOST_USER_PROTOCOL_F_LOG_SHMFD is not set
> then we block migration. So how come the old message
> is ever sent?
>
Agree. With the migration blocker on VHOST_USER_PROTOCOL_F_LOG_SHMFD
message with the new payload can not be sent to old vhost backend.
The documentation is a little bit confusing. The message payload for
SET_LOG_BASE still indicates a u64 whereas it is no more the case.
Modification on the vhost-user.c file is not really needed but maybe
we can keep the patch on the doc ?
>> The SET_LOG_BASE message payload is modified only if the
>> VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature has been negociated.
>>
>> The documentation has been updated accordingly with remarks from Marc André
>> Lureau.
>>
>> Signed-off-by: Thibaut Collet
>> ---
>> docs/specs/vhost-user.txt | 16 ++--
>> hw/virtio/vhost-user.c| 12 +---
>> 2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> index 26dde2e..da4bf9c 100644
>> --- a/docs/specs/vhost-user.txt
>> +++ b/docs/specs/vhost-user.txt
>> @@ -87,6 +87,14 @@ Depending on the request type, payload can be:
>> User address: a 64-bit user address
>> mmap offset: 64-bit offset where region starts in the mapped memory
>>
>> + * vhost user log description
>> + -
>> + | size | offset |
>> + -
>> +
>> + size: a 64-bit size
>> + Offset: a 64-bit offset where log starts in the mapped memory
>> +
>> In QEMU the vhost-user message is implemented with the following struct:
>>
>> typedef struct VhostUserMsg {
>> @@ -280,14 +288,18 @@ Message types
>>
>>Id: 6
>>Equivalent ioctl: VHOST_SET_LOG_BASE
>> - Master payload: u64
>> + Master payload: - u64 if slave has not the
>> VHOST_USER_PROTOCOL_F_LOG_SHMFD
>> +protocol feature
>> + - vhost user log if slave has the
>> +VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature
>>Slave payload: N/A
>>
>>Sets logging shared memory space.
>>When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol
>>feature, the log memory fd is provided in the ancillary data of
>>VHOST_USER_SET_LOG_BASE message, the size and offset of shared
>> - memory area provided in the message.
>> + memory area provided in the message and the message reply is an
>> + empty message (size of 0).
>>
>>
>> * VHOST_USER_SET_LOG_FD
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index c443602..dcdfd40 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -206,11 +206,17 @@ static int vhost_user_set_log_base(struct vhost_dev
>> *dev, uint64_t base,
>> VhostUserMsg msg = {
>> .request = VHOST_USER_SET_LOG_BASE,
>> .flags = VHOST_USER_VERSION,
>> -.payload.log.mmap_size = log->size,
>> -.payload.log.mmap_offset = 0,
>> -.size = sizeof(msg.payload.log),
>> };
>>
>> +if (shmfd) {
>> +msg.payload.log.mmap_size = log->size;
>> +msg.payload.log.mmap_offset = 0;
>> +msg.size = sizeof(msg.payload.log);
>> +} else {
>> +msg.payload.u64 = base;
>> +msg.size = sizeof(msg.payload.u64);
>> +}
>> +
>> if (shmfd && log->fd != -1) {
>> fds[fd_num++] = log->fd;
>> }
>> --
>> 2.1.4
[Qemu-devel] [PATCH 0/1] vhost-user: Adapt payload of SET_LOG_BASE regarding support of VHOST_USER_PROTOCOL_F_LOG_SHMFD
For compatibility with old vhost backend (as vapp) payload of SET_LOG_BASE can not be modified. New payload (log size and offset) of this message is sent only for vhost backend that supports the VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature. Thibaut Collet (1): vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set docs/specs/vhost-user.txt | 16 ++-- hw/virtio/vhost-user.c| 12 +--- 2 files changed, 23 insertions(+), 5 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set
Fixes: 2b8819c6eee5 ("vhost-user: modify SET_LOG_BASE to pass mmap size and
offset")
For compatibility with old vhost backend content of the SET_LOG_BASE message
can not be modified.
The SET_LOG_BASE message payload is modified only if the
VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature has been negociated.
The documentation has been updated accordingly with remarks from Marc André
Lureau.
Signed-off-by: Thibaut Collet
---
docs/specs/vhost-user.txt | 16 ++--
hw/virtio/vhost-user.c| 12 +---
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 26dde2e..da4bf9c 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -87,6 +87,14 @@ Depending on the request type, payload can be:
User address: a 64-bit user address
mmap offset: 64-bit offset where region starts in the mapped memory
+ * vhost user log description
+ -
+ | size | offset |
+ -
+
+ size: a 64-bit size
+ Offset: a 64-bit offset where log starts in the mapped memory
+
In QEMU the vhost-user message is implemented with the following struct:
typedef struct VhostUserMsg {
@@ -280,14 +288,18 @@ Message types
Id: 6
Equivalent ioctl: VHOST_SET_LOG_BASE
- Master payload: u64
+ Master payload: - u64 if slave has not the
VHOST_USER_PROTOCOL_F_LOG_SHMFD
+protocol feature
+ - vhost user log if slave has the
+VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature
Slave payload: N/A
Sets logging shared memory space.
When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol
feature, the log memory fd is provided in the ancillary data of
VHOST_USER_SET_LOG_BASE message, the size and offset of shared
- memory area provided in the message.
+ memory area provided in the message and the message reply is an
+ empty message (size of 0).
* VHOST_USER_SET_LOG_FD
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c443602..dcdfd40 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -206,11 +206,17 @@ static int vhost_user_set_log_base(struct vhost_dev *dev,
uint64_t base,
VhostUserMsg msg = {
.request = VHOST_USER_SET_LOG_BASE,
.flags = VHOST_USER_VERSION,
-.payload.log.mmap_size = log->size,
-.payload.log.mmap_offset = 0,
-.size = sizeof(msg.payload.log),
};
+if (shmfd) {
+msg.payload.log.mmap_size = log->size;
+msg.payload.log.mmap_offset = 0;
+msg.size = sizeof(msg.payload.log);
+} else {
+msg.payload.u64 = base;
+msg.size = sizeof(msg.payload.u64);
+}
+
if (shmfd && log->fd != -1) {
fds[fd_num++] = log->fd;
}
--
2.1.4
[Qemu-devel] [PATCH v2 1/1] 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 provided to the
vhost_virtqueue_set_addr function 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 vhost_dev_set_log is modified to provide the correct queue index to the
vhost_virtqueue_set_addr function that calls internally the vhost_get_vq_index
to compute the expected vhost_vq_index for vhost kernel and vhost user.
This change implies a modification of the vhost_virtqueue_start function to
provide the index and not the vhost_vq_index.
Signed-off-by: Thibaut Collet
---
hw/virtio/vhost.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index feeaaa4..9311832 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -628,8 +628,9 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
struct vhost_virtqueue *vq,
unsigned idx, bool enable_log)
{
+int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
struct vhost_vring_addr addr = {
-.index = idx,
+.index = vhost_vq_index,
.desc_user_addr = (uint64_t)(unsigned long)vq->desc,
.avail_user_addr = (uint64_t)(unsigned long)vq->avail,
.used_user_addr = (uint64_t)(unsigned long)vq->used,
@@ -662,7 +663,7 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool
enable_log)
goto err_features;
}
for (i = 0; i < dev->nvqs; ++i) {
-r = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+r = vhost_virtqueue_set_addr(dev, dev->vqs + i, dev->vq_index + i,
enable_log);
if (r < 0) {
goto err_vq;
@@ -671,7 +672,7 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool
enable_log)
return 0;
err_vq:
for (; i >= 0; --i) {
-t = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+t = vhost_virtqueue_set_addr(dev, dev->vqs + i, dev->vq_index + i,
dev->log_enabled);
assert(t >= 0);
}
@@ -836,7 +837,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
goto fail_alloc_ring;
}
-r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
+r = vhost_virtqueue_set_addr(dev, vq, idx, dev->log_enabled);
if (r < 0) {
r = -errno;
goto fail_alloc;
--
2.1.4
[Qemu-devel] [PATCH v2 0/1] vhost-user: support of live migration with multiqueue
Patches serie for vhost-user live migration from Marc-Andre Lureau [PATCH v8 00/27] vhost-user: add migration support (http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg02452.html) does not work if multiqueue is set. This patch correct an issue of queue index when a migration is started with multiqueue v1->v2: - call vhost_get_vq_index internally in vhost_virtqueue_set_addr Thibaut Collet (1): vhost: set the correct queue index in case of migration with multiqueue hw/virtio/vhost.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.1.4
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
On Tue, Oct 20, 2015 at 12:21 PM, Michael S. Tsirkin 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 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 > 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 > > > 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 > -- > MST
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
On Mon, Oct 19, 2015 at 11:12 PM, Michael S. Tsirkin 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.
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
On Mon, Oct 19, 2015 at 5:39 PM, Michael S. Tsirkin 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 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 >> > > >> > > wrote: >> > > >> > > >> > > >> > > On Fri, Oct 9, 2015 at 5:17 PM, wrote: >> > > >> > > From: Marc-André Lureau >> > > >> > > 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: >> >
Re: [Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case of migration with multiqueue
On Mon, Oct 19, 2015 at 5:41 PM, Michael S. Tsirkin wrote:
> On Mon, Oct 19, 2015 at 02:59:27PM +0200, Thibaut Collet wrote:
>> 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
>
> This needs some thought to make sure we don't break the kernel vhost.
For kernel vhost my patch does nothing has vhost_get_vq_index method
for vhost kernel subtract dev->vq_index (that was just added before)
and idx is still equal to i.
>
> I queued this temporarily to enable your testing but I think it would be
> preferable to make vhost_virtqueue_set_addr for vhost_user call
> vhost_get_vq_index internally.
>
If I call the vhost_get_vq_index internally by vhost_user when
vhost_virtqueue_set_addr is called I will break the
vhost_virtqueue_start: this function calls the vhost_get_vq_index
function for vhost user and vhost kernel to initializes the queue.
>
>
>> ---
>> hw/virtio/vhost.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index feeaaa4..de29968 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -656,13 +656,14 @@ static int vhost_dev_set_features(struct vhost_dev
>> *dev, bool enable_log)
>>
>> static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
>> {
>> -int r, t, i;
>> +int r, t, i, idx;
>> r = vhost_dev_set_features(dev, enable_log);
>> if (r < 0) {
>> goto err_features;
>> }
>> for (i = 0; i < dev->nvqs; ++i) {
>> -r = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
>> +idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
>> +r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
>> enable_log);
>> if (r < 0) {
>> goto err_vq;
>> @@ -671,7 +672,8 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool
>> enable_log)
>> return 0;
>> err_vq:
>> for (; i >= 0; --i) {
>> -t = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
>> +idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
>> +t = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
>> dev->log_enabled);
>> assert(t >= 0);
>> }
>> --
>> 2.1.4
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
On Sun, Oct 18, 2015 at 10:21 AM, Michael S. Tsirkin 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 > > wrote: > > > > > > > > On Fri, Oct 9, 2015 at 5:17 PM, wrote: > > > > From: Marc-André Lureau > > > > 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 > >
[Qemu-devel] [PATCH 0/1] vhost-user: support of live migration with multiqueue
Patches serie for vhost-user live migration from Marc-Andre Lureau [PATCH v8 00/27] vhost-user: add migration support (http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg02452.html) does not work if multiqueue is set. This patch correct an issue of queue index when a migration is started with multiqueue Thibaut Collet (1): vhost: set the correct queue index in case of migration with multiqueue hw/virtio/vhost.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH 1/1] 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
---
hw/virtio/vhost.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index feeaaa4..de29968 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -656,13 +656,14 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
bool enable_log)
static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
{
-int r, t, i;
+int r, t, i, idx;
r = vhost_dev_set_features(dev, enable_log);
if (r < 0) {
goto err_features;
}
for (i = 0; i < dev->nvqs; ++i) {
-r = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
+r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
enable_log);
if (r < 0) {
goto err_vq;
@@ -671,7 +672,8 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool
enable_log)
return 0;
err_vq:
for (; i >= 0; --i) {
-t = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
+t = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
dev->log_enabled);
assert(t >= 0);
}
--
2.1.4
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
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. On Mon, Oct 12, 2015 at 5:56 PM, Thibaut Collet wrote: > > > On Fri, Oct 9, 2015 at 5:17 PM, wrote: > >> From: Marc-André Lureau >> >> 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
Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support
On Fri, Oct 9, 2015 at 5:17 PM, wrote: > From: Marc-André Lureau > > 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
Re: [Qemu-devel] [PULL 12/25] vhost: use a function for each call
RING_ENDIAN, &s)) {
> +if (!dev->vhost_ops->vhost_set_vring_endian(dev, &s)) {
> return 0;
> }
>
> @@ -757,7 +757,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> {
> hwaddr s, l, a;
> int r;
> -int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev,
> idx);
> +int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
> struct vhost_vring_file file = {
> .index = vhost_vq_index
> };
> @@ -768,13 +768,13 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>
>
> vq->num = state.num = virtio_queue_get_num(vdev, idx);
> -r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_NUM, &state);
> +r = dev->vhost_ops->vhost_set_vring_num(dev, &state);
> if (r) {
> return -errno;
> }
>
> state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> -r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_BASE, &state);
> +r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
> if (r) {
> return -errno;
> }
> @@ -826,7 +826,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> }
>
> file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
> -r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_KICK, &file);
> +r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
> if (r) {
> r = -errno;
> goto fail_kick;
> @@ -859,13 +859,13 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
> struct vhost_virtqueue *vq,
> unsigned idx)
> {
> -int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev,
> idx);
> +int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
> struct vhost_vring_state state = {
> .index = vhost_vq_index,
> };
> int r;
>
> -r = dev->vhost_ops->vhost_call(dev, VHOST_GET_VRING_BASE, &state);
> +r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
> if (r < 0) {
>
Re: [Qemu-devel] [PATCH v7 12/24] vhost: use a function for each call
On Wed, Oct 7, 2015 at 5:58 PM, Marc-André Lureau wrote:
> Hi Thibaut
>
> - Original Message -
>> On Thu, Oct 1, 2015 at 7:23 PM, wrote:
>> > From: Marc-André Lureau
>> >
>> > Replace the generic vhost_call() by specific functions for each
>> > function call to help with type safety and changing arguments.
>> >
>> > While doing this, I found that "unsigned long long" and "uint64_t" were
>> > used interchangeably and causing compilation warnings, using uint64_t
>> > instead, as the vhost & protocol specifies.
>> >
>> > Signed-off-by: Marc-André Lureau
>> > ---
>> > hw/net/vhost_net.c| 16 +-
>> > hw/scsi/vhost-scsi.c | 7 +-
>> > hw/virtio/vhost-backend.c | 124 -
>> > hw/virtio/vhost-user.c| 518
>> > ++
>> > hw/virtio/vhost.c | 36 +--
>> > include/hw/virtio/vhost-backend.h | 63 -
>> > include/hw/virtio/vhost.h | 12 +-
>> > 7 files changed, 501 insertions(+), 275 deletions(-)
>> >
>> [snip]
>> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> > index f1edd04..cd84f0c 100644
>> > --- a/hw/virtio/vhost-user.c
>> > +++ b/hw/virtio/vhost-user.c
>> [snip]
>> > @@ -190,231 +182,317 @@ static int vhost_user_write(struct vhost_dev *dev,
>> > VhostUserMsg *msg,
>> > 0 : -1;
>> > }
>> >
>> > -static bool vhost_user_one_time_request(VhostUserRequest request)
>> > +static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
>> > + struct vhost_log *log)
>> > {
>> > -switch (request) {
>> > -case VHOST_USER_SET_OWNER:
>> > -case VHOST_USER_RESET_DEVICE:
>> > -case VHOST_USER_SET_MEM_TABLE:
>> > -case VHOST_USER_GET_QUEUE_NUM:
>> > -return true;
>> > -default:
>> > -return false;
>> > +int fds[VHOST_MEMORY_MAX_NREGIONS];
>> > +size_t fd_num = 0;
>> > +bool shmfd = virtio_has_feature(dev->protocol_features,
>> > +VHOST_USER_PROTOCOL_F_LOG_SHMFD);
>> > +VhostUserMsg msg = {
>> > +.request = VHOST_USER_SET_LOG_BASE,
>> > +.flags = VHOST_USER_VERSION,
>> > +.u64 = base,
>> > +.size = sizeof(m.u64),
>> > +};
>> > +
>> > +if (shmfd && log->fd != -1) {
>> > +fds[fd_num++] = log->fd;
>> > }
>> > +
>> > +vhost_user_write(dev, &msg, fds, fd_num);
>> > +
>> > +if (shmfd) {
>> > +msg.size = 0;
>> > +if (vhost_user_read(dev, &msg) < 0) {
>> > +return 0;
>> > +}
>> > +
>> > +if (msg.request != VHOST_USER_SET_LOG_BASE) {
>> > +error_report("Received unexpected msg type. "
>> > + "Expected %d received %d",
>> > + VHOST_USER_SET_LOG_BASE, msg.request);
>> > +return -1;
>> > +}
>> > +}
>> > +
>> > +return 0;
>> > }
>> >
>> > -static int vhost_user_call(struct vhost_dev *dev, unsigned long int
>> > request,
>> > -void *arg)
>> > +static int vhost_user_set_mem_table(struct vhost_dev *dev,
>> > +struct vhost_memory *mem)
>> > {
>> > -VhostUserMsg msg;
>> > -VhostUserRequest msg_request;
>> > -struct vhost_vring_file *file = 0;
>> > -int need_reply = 0;
>> > int fds[VHOST_MEMORY_MAX_NREGIONS];
>> > int i, fd;
>> > size_t fd_num = 0;
>> > +VhostUserMsg msg = {
>> > +.request = VHOST_USER_SET_MEM_TABLE,
>> > +.flags = VHOST_USER_VERSION,
>> > +};
>> >
>> > -assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>> > -
>> > -/* only translate vhost ioctl requests */
>> > -if (request > VHOST_USER_MAX) {
>> > -msg_request = vhost_user_request_translate(request);
>> > -} else {
>> > -msg_request = request;
>> > +for (i = 0; i < dev->mem->nregions; ++i) {
>> > +struct vhost_memory_region *reg = dev->mem->regions + i;
>> > +ram_addr_t ram_addr;
>> > +
>> > +assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
>> > +qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
>> > +&ram_addr);
>> > +fd = qemu_get_ram_fd(ram_addr);
>> > +if (fd > 0) {
>> > +msg.memory.regions[fd_num].userspace_addr =
>> > reg->userspace_addr;
>> > +msg.memory.regions[fd_num].memory_size = reg->memory_size;
>> > +msg.memory.regions[fd_num].guest_phys_addr =
>> > reg->guest_phys_addr;
>> > +msg.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
>> > +(uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
>> > +assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
>> > +fds[fd_num++] = fd;
>> > +}
>> > }
>> >
>> > -/*
>> > - * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
>> > - * we just need send it once in the
Re: [Qemu-devel] [PATCH v7 12/24] vhost: use a function for each call
,
> -.u64 = base,
> +.u64 = file->index & VHOST_USER_VRING_IDX_MASK,
> .size = sizeof(m.u64),
> };
>
> -if (shmfd && log->fd != -1) {
> -fds[fd_num++] = log->fd;
> +if (ioeventfd_enabled() && file->fd > 0) {
> +fds[fd_num++] = file->fd;
> +} else {
> +msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
> }
>
> vhost_user_write(dev, &msg, fds, fd_num);
>
> -if (shmfd) {
> -msg.size = 0;
> -if (vhost_user_read(dev, &msg) < 0) {
> -return 0;
> -}
> +return 0;
> +}
>
> -if (msg.request != VHOST_USER_SET_LOG_BASE) {
> -error_report("Received unexpected msg type. "
> - "Expected %d received %d",
> - VHOST_USER_SET_LOG_BASE, msg.request);
> -return -1;
> -}
> +static int vhost_user_set_vring_kick(struct vhost_dev *dev,
> + struct vhost_vring_file *file)
> +{
> +return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_KICK, file);
> +}
> +
> +static int vhost_user_set_vring_call(struct vhost_dev *dev,
> + struct vhost_vring_file *file)
> +{
> +return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_CALL, file);
> +}
> +
> +static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t
> u64)
> +{
> +VhostUserMsg msg = {
> +.request = request,
> +.flags = VHOST_USER_VERSION,
> +.u64 = u64,
> +.size = sizeof(m.u64),
> +};
> +
> +vhost_user_write(dev, &msg, NULL, 0);
> +
> +return 0;
> +}
> +
> +static int vhost_user_set_features(struct vhost_dev *dev,
> + uint64_t features)
> +{
> +return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features);
> +}
> +
> +static int vhost_user_set_protocol_features(struct vhost_dev *dev,
> +uint64_t features)
> +{
> +return vhost_user_set_u64(dev, VHOST_USER_SET_PROTOCOL_FEATURES,
> features);
> +}
> +
> +static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t
> *u64)
> +{
> +VhostUserMsg msg = {
> +.request = request,
> +.flags = VHOST_USER_VERSION,
> +};
> +
With multiqueue there are an issue with this implementation. The
VHOST_USER_GET_QUEUE_NUM message is sent through this function and it
is a one time message.
For the queue index different from 0 the vhost_user_write returns
immediately without sending the request to the backend.
Then the vhost_user_read never returns and QEMU is blocked.
I suggest to add the following test before calling the
vhost_user_write function to avoid this issue:
+if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
+return 0;
+}
+
> +vhost_user_write(dev, &msg, NULL, 0);
> +
> +if (vhost_user_read(dev, &msg) < 0) {
> +return 0;
> +}
> +
> +if (msg.request != request) {
> +error_report("Received unexpected msg type. Expected %d received %d",
> + request, msg.request);
> +return -1;
> +}
> +
> +if (msg.size != sizeof(m.u64)) {
> +error_report("Received bad msg size.");
> +return -1;
> }
>
[snip]
The attached file is the fixup to apply to this patch.
Regards.
Thibaut.
From effefe37d6ffa929657d788873311e026a5c0c80 Mon Sep 17 00:00:00 2001
From: Thibaut Collet
Date: Wed, 7 Oct 2015 17:41:42 +0200
Subject: [PATCH] FIXUP for vhost: use a function for each call
Signed-off-by: Thibaut Collet
---
hw/virtio/vhost-user.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index bbf5eeb..8b4ea0f 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -315,13 +315,13 @@ static int vhost_set_vring(struct vhost_dev *dev,
static int vhost_user_set_vring_num(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
-return vhost_set_vring(dev, VHOST_SET_VRING_NUM, ring);
+return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
}
static int vhost_user_set_vring_base(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
-return vhost_set_vring(dev, VHOST_SET_VRING_BASE, ring);
+return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
}
static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
@@ -440,6 +440,10 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
.flags = VHOST_USER_VERSION,
};
+if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
+return 0;
+}
+
vhost_user_write(dev, &msg, NULL, 0);
if (vhost_user_read(dev, &msg) < 0) {
--
2.1.4
Re: [Qemu-devel] [PATCH v7 16/24] vhost user: add rarp sending after live migration for legacy guest
On Fri, Oct 2, 2015 at 4:02 PM, Michael S. Tsirkin wrote:
> On Fri, Oct 02, 2015 at 09:55:01AM -0400, Marc-André Lureau wrote:
>>
>>
>> - Original Message -
>> > On Thu, Oct 01, 2015 at 07:24:00PM +0200, marcandre.lur...@redhat.com
>> > wrote:
>> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> > > index 840f443..da66b64 100644
>> > > --- a/hw/net/vhost_net.c
>> > > +++ b/hw/net/vhost_net.c
>> > > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
>> > > g_free(net);
>> > > }
>> > >
>> > > +int vhost_net_notify_migration_done(struct vhost_net *net, char*
>> > > mac_addr)
>> > > +{
>> > > +const VhostOps *vhost_ops = net->dev.vhost_ops;
>> > > +int r = -1;
>> > > +
>> > > +if (vhost_ops->vhost_migration_done) {
>> > > +r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
>> > > +}
>> > > +
>> > > +return r;
>> > > +}
>> > > +
>> > > bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>> > > {
>> > > return vhost_virtqueue_pending(&net->dev, idx);
>> > > @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
>> > > VirtIODevice *dev,
>> > > {
>> > > }
>> > >
>> > > +int vhost_net_notify_migration_done(struct vhost_net *net)
>> > > +{
>> > > +return -1;
>> > > +}
>> > > +
>> > > VHostNetState *get_vhost_net(NetClientState *nc)
>> > > {
>> > > return 0;
>> >
>> > This signature does not fit the one above.
>> > How was this tested?
>> >
>>
>> Good question, I totally missed that. It has been there since Thibaut v6
>> series.
>>
>> I guess we all compile with CONFIG_VHOST_NET, and adding the missing char*
>> mac_addr is enough to fix this.
>
> You will catch this if you build and test all targets.
>
> --
> MST
Sorry for the omission.
I have tested some QEMU configuration but all of them to test my feature.
The add of the missing char* mac_addr is sufficient : in this case the
function will be compliant with its definition set in the
include/net/vhost_net.h file.
Next time I will build all targets to avoid this kind of errors.
Thibaut.
Re: [Qemu-devel] [PATCH v5 15/21] vhost user: add rarp sending after live migration for legacy guest
On Sun, Sep 27, 2015 at 5:13 PM, Marc-André Lureau
wrote:
> Hi
>
> On Sun, Sep 27, 2015 at 3:12 PM, Michael S. Tsirkin wrote:
>> On Thu, Sep 24, 2015 at 05:53:05PM -0400, Marc-André Lureau wrote:
>>> Hi
>>>
>>> - Original Message -
>>> > On Thu, Sep 24, 2015 at 6:22 PM, wrote:
>>> > > From: Thibaut Collet
>>> > >
>>> > > A new vhost user message is added to allow QEMU to ask to vhost user
>>> > > backend to
>>> > > broadcast a fake RARP after live migration for guest without
>>> > > GUEST_ANNOUNCE
>>> > > capability.
>>> > >
>>> > > This new message is sent only if the backend supports the new
>>> > > VHOST_USER_PROTOCOL_F_RARP protocol feature.
>>> > > The payload of this new message is the MAC address of the guest (not
>>> > > known
>>> > > by
>>> > > the backend). The MAC address is copied in the first 6 bytes of a u64 to
>>> > > avoid
>>> > > to create a new payload message type.
>>> > >
>>> > > This new message has no equivalent ioctl so a new callback is added in
>>> > > the
>>> > > userOps structure to send the request.
>>> > >
>>> > > Upon reception of this new message the vhost user backend must generate
>>> > > and
>>> > > broadcast a fake RARP request to notify the migration is terminated.
>>> > >
>>> > > Signed-off-by: Thibaut Collet
>>> > > [Rebased and fixed checkpatch errors - Marc-André]
>>> > > Signed-off-by: Marc-André Lureau
>>> > > ---
>>> > > docs/specs/vhost-user.txt | 15 +++
>>> > > hw/net/vhost_net.c| 17 +
>>> > > hw/virtio/vhost-user.c| 30 ++
>>> > > include/hw/virtio/vhost-backend.h | 3 +++
>>> > > include/net/vhost_net.h | 1 +
>>> > > net/vhost-user.c | 24 ++--
>>> > > 6 files changed, 88 insertions(+), 2 deletions(-)
>>> > >
>>> > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>> > > index e0292a0..e0d71e2 100644
>>> > > --- a/docs/specs/vhost-user.txt
>>> > > +++ b/docs/specs/vhost-user.txt
>>> > > @@ -194,6 +194,7 @@ Protocol features
>>> > >
>>> > > #define VHOST_USER_PROTOCOL_F_MQ 0
>>> > > #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
>>> > > +#define VHOST_USER_PROTOCOL_F_RARP 2
>>> > >
>>> > > Message types
>>> > > -
>>> > > @@ -381,3 +382,17 @@ Message types
>>> > >Master payload: vring state description
>>> > >
>>> > >Signal slave to enable or disable corresponding vring.
>>> > > +
>>> > > + * VHOST_USER_SEND_RARP
>>> > > +
>>> > > + Id: 19
>>> > > + Equivalent ioctl: N/A
>>> > > + Master payload: u64
>>> > > +
>>> > > + Ask vhost user backend to broadcast a fake RARP to notify the
>>> > > migration
>>> > > + is terminated for guest that does not support GUEST_ANNOUNCE.
>>> > > + Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is
>>> > > present
>>> > > in
>>> > > + VHOST_USER_GET_FEATURES and protocol feature bit
>>> > > VHOST_USER_PROTOCOL_F_RARP
>>> > > + is present in VHOST_USER_GET_PROTOCOL_FEATURES.
>>> > > + The first 6 bytes of the payload contain the mac address of the
>>> > > guest to
>>> > > + allow the vhost user backend to construct and broadcast the fake
>>> > > RARP.
>>> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>>> > > index 840f443..da66b64 100644
>>> > > --- a/hw/net/vhost_net.c
>>> > > +++ b/hw/net/vhost_net.c
>>> > > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
>>> > > g_free(net);
>>> > > }
>>> > >
>>> > > +int vhost_net_notify_migration_done(struct vhost_net *net, char*
>>> > > mac_addr)
>>> > > +{
>
Re: [Qemu-devel] [PATCH v5 15/21] vhost user: add rarp sending after live migration for legacy guest
On Thu, Sep 24, 2015 at 6:22 PM, wrote:
> From: Thibaut Collet
>
> A new vhost user message is added to allow QEMU to ask to vhost user backend
> to
> broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
> capability.
>
> This new message is sent only if the backend supports the new
> VHOST_USER_PROTOCOL_F_RARP protocol feature.
> The payload of this new message is the MAC address of the guest (not known by
> the backend). The MAC address is copied in the first 6 bytes of a u64 to avoid
> to create a new payload message type.
>
> This new message has no equivalent ioctl so a new callback is added in the
> userOps structure to send the request.
>
> Upon reception of this new message the vhost user backend must generate and
> broadcast a fake RARP request to notify the migration is terminated.
>
> Signed-off-by: Thibaut Collet
> [Rebased and fixed checkpatch errors - Marc-André]
> Signed-off-by: Marc-André Lureau
> ---
> docs/specs/vhost-user.txt | 15 +++
> hw/net/vhost_net.c| 17 +
> hw/virtio/vhost-user.c| 30 ++
> include/hw/virtio/vhost-backend.h | 3 +++
> include/net/vhost_net.h | 1 +
> net/vhost-user.c | 24 ++--
> 6 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index e0292a0..e0d71e2 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -194,6 +194,7 @@ Protocol features
>
> #define VHOST_USER_PROTOCOL_F_MQ 0
> #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
> +#define VHOST_USER_PROTOCOL_F_RARP 2
>
> Message types
> -
> @@ -381,3 +382,17 @@ Message types
>Master payload: vring state description
>
>Signal slave to enable or disable corresponding vring.
> +
> + * VHOST_USER_SEND_RARP
> +
> + Id: 19
> + Equivalent ioctl: N/A
> + Master payload: u64
> +
> + Ask vhost user backend to broadcast a fake RARP to notify the migration
> + is terminated for guest that does not support GUEST_ANNOUNCE.
> + Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
> + VHOST_USER_GET_FEATURES and protocol feature bit
> VHOST_USER_PROTOCOL_F_RARP
> + is present in VHOST_USER_GET_PROTOCOL_FEATURES.
> + The first 6 bytes of the payload contain the mac address of the guest
> to
> + allow the vhost user backend to construct and broadcast the fake RARP.
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 840f443..da66b64 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
> g_free(net);
> }
>
> +int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
> +{
> +const VhostOps *vhost_ops = net->dev.vhost_ops;
> +int r = -1;
> +
> +if (vhost_ops->vhost_migration_done) {
> +r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
> +}
> +
> +return r;
> +}
> +
> bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
> {
> return vhost_virtqueue_pending(&net->dev, idx);
> @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
> VirtIODevice *dev,
> {
> }
>
> +int vhost_net_notify_migration_done(struct vhost_net *net)
> +{
> +return -1;
> +}
> +
> VHostNetState *get_vhost_net(NetClientState *nc)
> {
> return 0;
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 455caba..b7f3699 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -10,6 +10,7 @@
>
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-backend.h"
> +#include "hw/virtio/virtio-net.h"
> #include "sysemu/char.h"
> #include "sysemu/kvm.h"
> #include "qemu/error-report.h"
> @@ -30,6 +31,7 @@
> #define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
> #define VHOST_USER_PROTOCOL_F_MQ 0
> #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
> +#define VHOST_USER_PROTOCOL_F_RARP 2
The VHOST_USER_PROTOCOL_FEATURE_MASK must be changed and set to 0x7ULL
>
> typedef enum VhostUserRequest {
> VHOST_USER_NONE = 0,
> @@ -51,6 +53,7 @@ typedef enum VhostUserRequest {
> VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> VHOST_USER_GET_QUEUE_NUM = 17,
> VHOST_USER_SET_VRING_ENABLE = 18,
> +VHOST_USER_SEND_RARP = 19,
> VHOST_USER_MAX
> } VhostUserRequest;
>
> @@ -561,6 +56
Re: [Qemu-devel] [PATCH v4 09/22] vhost: use a function for each call
On Sat, Sep 19, 2015 at 12:12 PM, wrote:
> From: Marc-André Lureau
>
> Replace the generic vhost_call() by specific functions for each
> function call to help with type safety and changing arguments.
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/net/vhost_net.c| 12 +-
> hw/scsi/vhost-scsi.c | 7 +-
> hw/virtio/vhost-backend.c | 140 +++--
> hw/virtio/vhost-user.c| 402
> ++
> hw/virtio/vhost.c | 34 ++--
> include/hw/virtio/vhost-backend.h | 59 +-
> 6 files changed, 484 insertions(+), 170 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 9d32d76..d116fb3 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -243,8 +243,7 @@ static int vhost_net_start_one(struct vhost_net *net,
> file.fd = net->backend;
> for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
> const VhostOps *vhost_ops = net->dev.vhost_ops;
> -r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> - &file);
> +r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
> if (r < 0) {
> r = -errno;
> goto fail;
> @@ -257,8 +256,7 @@ fail:
> if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
> while (file.index-- > 0) {
> const VhostOps *vhost_ops = net->dev.vhost_ops;
> -int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> - &file);
> +int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
> assert(r >= 0);
> }
> }
> @@ -280,15 +278,13 @@ static void vhost_net_stop_one(struct vhost_net *net,
> if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
> for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
> const VhostOps *vhost_ops = net->dev.vhost_ops;
> -int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> - &file);
> +int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
> assert(r >= 0);
> }
> } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
> for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
> const VhostOps *vhost_ops = net->dev.vhost_ops;
> -int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
> - NULL);
> +int r = vhost_ops->vhost_reset_owner(&net->dev);
> assert(r >= 0);
> }
> }
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index bac9ddb..a0034ab 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -45,7 +45,7 @@ static int vhost_scsi_set_endpoint(VHostSCSI *s)
>
> memset(&backend, 0, sizeof(backend));
> pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
> -ret = vhost_ops->vhost_call(&s->dev, VHOST_SCSI_SET_ENDPOINT, &backend);
> +ret = vhost_ops->vhost_scsi_set_endpoint(&s->dev, &backend);
> if (ret < 0) {
> return -errno;
> }
> @@ -60,7 +60,7 @@ static void vhost_scsi_clear_endpoint(VHostSCSI *s)
>
> memset(&backend, 0, sizeof(backend));
> pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
> -vhost_ops->vhost_call(&s->dev, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +vhost_ops->vhost_scsi_clear_endpoint(&s->dev, &backend);
> }
>
> static int vhost_scsi_start(VHostSCSI *s)
> @@ -76,8 +76,7 @@ static int vhost_scsi_start(VHostSCSI *s)
> return -ENOSYS;
> }
>
> -ret = vhost_ops->vhost_call(&s->dev,
> -VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +ret = vhost_ops->vhost_scsi_get_abi_version(&s->dev, &abi_version);
> if (ret < 0) {
> return -errno;
> }
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 4d68a27..bf2d1d4 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -11,19 +11,10 @@
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-backend.h"
> #include "qemu/error-report.h"
> +#include "linux/vhost.h"
>
> #include
>
> -static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int
> request,
> - void *arg)
> -{
> -int fd = (uintptr_t) dev->opaque;
> -
> -assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);
> -
> -return ioctl(fd, request, arg);
> -}
> -
> static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
> {
> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);
> @@ -42,11 +33,136 @@ static int vhost_kernel_cleanup(struct vhost_dev *dev)
> return close(fd);
> }
>
> +static int vhost_kernel_call(struct vh
[Qemu-devel] [PATCH v6 2/2] vhost user: add rarp sending after live migration for legacy guest
A new vhost user message is added to allow QEMU to ask to vhost user backend to
broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
capability.
This new message is sent only if the backend supports the new
VHOST_USER_PROTOCOL_F_RARP protocol feature.
The payload of this new message is the MAC address of the guest (not known by
the backend). The MAC address is copied in the first 6 bytes of a u64 to avoid
to create a new payload message type.
This new message has no equivalent ioctl so a new callback is added in the
userOps structure to send the request.
Upon reception of this new message the vhost user backend must generate and
broadcast a fake RARP request to notify the migration is terminated.
Signed-off-by: Thibaut Collet
---
docs/specs/vhost-user.txt | 15 +++
hw/net/vhost_net.c| 16
hw/virtio/vhost-backend.c |3 ++-
hw/virtio/vhost-user.c| 32 ++--
include/hw/virtio/vhost-backend.h |2 ++
include/net/vhost_net.h |1 +
net/vhost-user.c | 23 +--
7 files changed, 87 insertions(+), 5 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index c2d2e2a..8c05301 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -140,6 +140,7 @@ Protocol features
-
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP 1
Message types
-
@@ -308,6 +309,20 @@ Message types
invalid FD flag. This flag is set when there is no file descriptor
in the ancillary data.
+ * VHOST_USER_SEND_RARP
+
+ Id: 17
+ Equivalent ioctl: N/A
+ Master payload: u64
+
+ Ask vhost user backend to broadcast a fake RARP to notify the migration
+ is terminated for guest that does not support GUEST_ANNOUNCE.
+ Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+ VHOST_USER_GET_FEATURES and protocol feature bit
VHOST_USER_PROTOCOL_F_RARP
+ is present in VHOST_USER_GET_PROTOCOL_FEATURES.
+ The first 6 bytes of the payload contain the mac address of the guest to
+ allow the vhost user backend to construct and broadcast the fake RARP.
+
Migration
-
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9850520..cbcb6dd 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -383,6 +383,17 @@ void vhost_net_cleanup(struct vhost_net *net)
g_free(net);
}
+int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
+{
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+int r = -1;
+
+if (vhost_ops->vhost_backend_migration_done)
+r = vhost_ops->vhost_backend_migration_done(&net->dev, mac_addr);
+
+return r;
+}
+
bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
{
return vhost_virtqueue_pending(&net->dev, idx);
@@ -456,6 +467,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
VirtIODevice *dev,
{
}
+int vhost_net_notify_migration_done(struct vhost_net *net)
+{
+return -1;
+}
+
VHostNetState *get_vhost_net(NetClientState *nc)
{
return 0;
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 4d68a27..09a5d67 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -46,7 +46,8 @@ static const VhostOps kernel_ops = {
.backend_type = VHOST_BACKEND_TYPE_KERNEL,
.vhost_call = vhost_kernel_call,
.vhost_backend_init = vhost_kernel_init,
-.vhost_backend_cleanup = vhost_kernel_cleanup
+.vhost_backend_cleanup = vhost_kernel_cleanup,
+.vhost_backend_migration_done = NULL
};
int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType
backend_type)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index fe75618..aa65cd5 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-backend.h"
+#include "hw/virtio/virtio-net.h"
#include "sysemu/char.h"
#include "sysemu/kvm.h"
#include "qemu/error-report.h"
@@ -27,8 +28,9 @@
#define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP 1
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
@@ -48,6 +50,7 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_VRING_ERR = 14,
VHOST_USER_GET_PROTOCOL_FEATURES = 15,
VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+VHOST_USER_SEND_RARP = 17,
VHOST_USER_MAX
} VhostUserRequest;
@@ -409,9 +412,34 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
return 0;
}
+static int vhost_user_migration_done(struct vhost_dev *dev, char* mac
[Qemu-devel] [PATCH v6 0/2] vhost user: Add live migration
v5->v6 1. First patch: remove a warning log 2. Second patch: rename some functions to be more explicit on the purpose of these functions. The first patch provides limited live migration: - guest without GUEST_ANNOUNCE capabilities does not announce migration ending and peers talking to the migrated guest can suffer important network outage. - Some packets sent by remote peers to the guest can be lost during migration. The second patch fixes limitation for guest without GUEST_ANNOUNCE capabilities and patches from Marc Andre Lureau fix potential packet's lost during migration. Thibaut Collet (2): vhost user: add support of live migration vhost user: add rarp sending after live migration for legacy guest docs/specs/vhost-user.txt | 15 +++ hw/net/vhost_net.c| 18 ++ hw/virtio/vhost-backend.c |3 ++- hw/virtio/vhost-user.c| 32 ++-- include/hw/virtio/vhost-backend.h |2 ++ include/net/vhost_net.h |1 + net/vhost-user.c | 31 +-- 7 files changed, 97 insertions(+), 5 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v6 1/2] vhost user: add support of live migration
Some vhost user backends 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 called by:
* qemu_announce_self after a migration to send fake RARP to avoid network
outage for peers talking to the migrated guest.
- For guest with GUEST_ANNOUNCE capabilities, guest already sends GARP
when the bit VIRTIO_NET_S_ANNOUNCE is set.
=> These packets must be discarded.
- For guest without GUEST_ANNOUNCE capabilities, migration termination
is notified when the guest sends packets.
=> These packets can be discarded.
* virtio_net_tx_bh with a dummy boot to send fake bootp/dhcp request.
BIOS guest manages virtio driver to send 4 bootp/dhcp request in case of
dummy boot.
=> These packets must be discarded.
Signed-off-by: Thibaut Collet
---
hw/net/vhost_net.c |2 ++
net/vhost-user.c | 12 ++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index c864237..9850520 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 93dcecd..1b1c626 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -65,6 +65,15 @@ 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)
+{
+/* Discard the request that is received and managed by backend
+ * by an other way.
+ */
+return size;
+}
+
static void vhost_user_cleanup(NetClientState *nc)
{
VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -90,6 +99,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,
@@ -143,8 +153,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;
qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
--
1.7.10.4
Re: [Qemu-devel] [PATCH v5 1/2] vhost user: add support of live migration
On Tue, Aug 4, 2015 at 7:50 PM, Marc-André Lureau
wrote:
> Hi Thibaut
>
> On Mon, Aug 3, 2015 at 11:22 AM, Thibaut Collet
> wrote:
>> Some vhost user backends 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 used only by qemu_announce_self after a migration to send
>>fake RARP to avoid network outage for peers talking to the migrated guest.
>>- For guest with GUEST_ANNOUNCE capabilities these packets must be
>> discarded,
>> GARP are sent by the guest thanks the GUEST_ANNOUNCE.
>>- For guest without GUEST_ANNOUNCE capabilities these packets are
>> discarded
>> too, migration termination is notified when the guest sends packets.
>>
>> Signed-off-by: Thibaut Collet
>> ---
>> hw/net/vhost_net.c |2 ++
>> net/vhost-user.c | 25 +++--
>> 2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index c864237..9850520 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 93dcecd..2290271 100644
>> --- a/net/vhost-user.c
>> +++ b/net/vhost-user.c
>> @@ -65,6 +65,28 @@ 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 about 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 expects only RARP (size 60)."
>> +"Receives unexpected packets with size %lu\n",
>> +size);
>> +fflush(stderr);
>> +display_trace = 0;
>> +}
>> +}
>> +return size;
>> +}
>
> This warning appears during a dummy boot with vapp: qemu-system-x86_64
> -m 512 -object
> memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs/,share=on
> -numa node,memdev=mem -netdev
> vhost-user,id=net0,chardev=chr0,vhostforce -device
> virtio-net-pci,netdev=net0 -chardev
> socket,id=chr0,path=/tmp/vapp.sock.
>
> I think silentely returning 0 (without warning) is the right way to
> keep the packets queued in the ring instead. But I must say I am quite
> confused as this is triggered by a VIRTIO_PCI_QUEUE_NOTIFY ioport
> write, and I fail to see how vhost-user handles it then without
> eventfd kick...
>
>> static void vhost_user_cleanup(NetClientState *nc)
>> {
>> VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
>> @@ -90,6 +112,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,
>> @@ -143,8 +166,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;
>>
>> qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
>> --
>> 1.7.10.4
>>
>
>
>
> --
> Marc-André Lureau
Hi,
I have reproduced the issue and the message receive by vhost-user is
quite strange: this message is a virtio header plus a bootp/dhcp
request (I do not understand why there are the virtio header)
With a dummy boot, vhost user backend already receives 4 bootp/dhcp
requests form the bios guest so I do not see any interest to transmit
this request to vhostuser backend.
I suggest to remove the warning and discard the dhcp/bootp request (it
is useless to duplicate bootp/dhcp request)
Thibaut.
Re: [Qemu-devel] [PATCH v5 2/2] vhost user: add rarp sending after live migration for legacy guest
On Tue, Aug 4, 2015 at 8:22 AM, Jason Wang wrote:
>
>
> On 08/03/2015 05:22 PM, Thibaut Collet wrote:
>> A new vhost user message is added to allow QEMU to ask to vhost user backend
>> to
>> broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
>> capability.
>>
>> This new message is sent only if the backend supports the new
>> VHOST_USER_PROTOCOL_F_RARP protocol feature.
>> The payload of this new message is the MAC address of the guest (not known by
>> the backend). The MAC address is copied in the first 6 bytes of a u64 to
>> avoid
>> to create a new payload message type.
>>
>> This new message has no equivalent ioctl so a new callback is added in the
>> userOps structure to send the request.
>>
>> Upon reception of this new message the vhost user backend must generate and
>> broadcast a fake RARP request to notify the migration is terminated.
>>
>> Signed-off-by: Thibaut Collet
>> ---
>> docs/specs/vhost-user.txt | 15 +++
>> hw/net/vhost_net.c| 16
>> hw/virtio/vhost-user.c| 32 ++--
>> include/hw/virtio/vhost-backend.h |2 ++
>> include/net/vhost_net.h |1 +
>> net/vhost-user.c | 18 ++
>> 6 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> index c2d2e2a..8c05301 100644
>> --- a/docs/specs/vhost-user.txt
>> +++ b/docs/specs/vhost-user.txt
>> @@ -140,6 +140,7 @@ Protocol features
>> -
>>
>> #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
>> +#define VHOST_USER_PROTOCOL_F_RARP 1
>>
>> Message types
>> -
>> @@ -308,6 +309,20 @@ Message types
>>invalid FD flag. This flag is set when there is no file descriptor
>>in the ancillary data.
>>
>> + * VHOST_USER_SEND_RARP
>> +
>> + Id: 17
>> + Equivalent ioctl: N/A
>> + Master payload: u64
>> +
>> + Ask vhost user backend to broadcast a fake RARP to notify the
>> migration
>> + is terminated for guest that does not support GUEST_ANNOUNCE.
>> + Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
>> + VHOST_USER_GET_FEATURES and protocol feature bit
>> VHOST_USER_PROTOCOL_F_RARP
>> + is present in VHOST_USER_GET_PROTOCOL_FEATURES.
>> + The first 6 bytes of the payload contain the mac address of the guest
>> to
>> + allow the vhost user backend to construct and broadcast the fake RARP.
>> +
>> Migration
>
> If for some reason the announce packet was changed in the future, we may
> then need another kind of message type. Just wonder whether allowing
> qemu to inject packet directly to vhost-user is better.
>
Purpose of the announce packet is to notified switches that MAC
address of the guest has moved to avoid network outage.
So only knowledge of the MAC address is needed to construct the
announce packet (whatever is format)
The new message provides the MAC address. Vhost user backend can
create an announce packet that is not necessary the same of QEMU
Providing the announce packet directly to vhost user is possible but
needs a lot of additional change:
- The announce packet can not be copy in the master payload (in the
future if the announce packet changed its size can changed too)
The master payload can be a u64 that gives the size of the message.
The message will be stored in a shared memory between QEMU and backend
and a fd to this shared memory must be provided too.
That can be interesting if QEMU has several type of packets to inject
to vhost user but it is not the case and I prefer to keep my solution
(less complex and risk of bugs or memory leakage is less)
>> -
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index 9850520..c3b9664 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -383,6 +383,17 @@ void vhost_net_cleanup(struct vhost_net *net)
>> g_free(net);
>> }
>>
>> +int vhost_net_migrate(struct vhost_net *net, char* mac_addr)
>> +{
>> +const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +int r = -1;
>> +
>> +if (vhost_ops->vhost_backend_migrate)
>> +r = vhost_ops->vhost_backend_migrate(&net->dev, mac_addr);
>> +
>> +return r;
>> +}
>> +
>> bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>> {
>> return vhost_virtqueue_pending(&net->dev,
[Qemu-devel] [PATCH v5 2/2] vhost user: add rarp sending after live migration for legacy guest
A new vhost user message is added to allow QEMU to ask to vhost user backend to
broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
capability.
This new message is sent only if the backend supports the new
VHOST_USER_PROTOCOL_F_RARP protocol feature.
The payload of this new message is the MAC address of the guest (not known by
the backend). The MAC address is copied in the first 6 bytes of a u64 to avoid
to create a new payload message type.
This new message has no equivalent ioctl so a new callback is added in the
userOps structure to send the request.
Upon reception of this new message the vhost user backend must generate and
broadcast a fake RARP request to notify the migration is terminated.
Signed-off-by: Thibaut Collet
---
docs/specs/vhost-user.txt | 15 +++
hw/net/vhost_net.c| 16
hw/virtio/vhost-user.c| 32 ++--
include/hw/virtio/vhost-backend.h |2 ++
include/net/vhost_net.h |1 +
net/vhost-user.c | 18 ++
6 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index c2d2e2a..8c05301 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -140,6 +140,7 @@ Protocol features
-
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP 1
Message types
-
@@ -308,6 +309,20 @@ Message types
invalid FD flag. This flag is set when there is no file descriptor
in the ancillary data.
+ * VHOST_USER_SEND_RARP
+
+ Id: 17
+ Equivalent ioctl: N/A
+ Master payload: u64
+
+ Ask vhost user backend to broadcast a fake RARP to notify the migration
+ is terminated for guest that does not support GUEST_ANNOUNCE.
+ Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+ VHOST_USER_GET_FEATURES and protocol feature bit
VHOST_USER_PROTOCOL_F_RARP
+ is present in VHOST_USER_GET_PROTOCOL_FEATURES.
+ The first 6 bytes of the payload contain the mac address of the guest to
+ allow the vhost user backend to construct and broadcast the fake RARP.
+
Migration
-
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9850520..c3b9664 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -383,6 +383,17 @@ void vhost_net_cleanup(struct vhost_net *net)
g_free(net);
}
+int vhost_net_migrate(struct vhost_net *net, char* mac_addr)
+{
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+int r = -1;
+
+if (vhost_ops->vhost_backend_migrate)
+r = vhost_ops->vhost_backend_migrate(&net->dev, mac_addr);
+
+return r;
+}
+
bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
{
return vhost_virtqueue_pending(&net->dev, idx);
@@ -456,6 +467,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
VirtIODevice *dev,
{
}
+int vhost_net_migrate(struct vhost_net *net)
+{
+return -1;
+}
+
VHostNetState *get_vhost_net(NetClientState *nc)
{
return 0;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index fe75618..693840f 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-backend.h"
+#include "hw/virtio/virtio-net.h"
#include "sysemu/char.h"
#include "sysemu/kvm.h"
#include "qemu/error-report.h"
@@ -27,8 +28,9 @@
#define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP 1
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
@@ -48,6 +50,7 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_VRING_ERR = 14,
VHOST_USER_GET_PROTOCOL_FEATURES = 15,
VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+VHOST_USER_SEND_RARP = 17,
VHOST_USER_MAX
} VhostUserRequest;
@@ -409,9 +412,34 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
return 0;
}
+static int vhost_user_migrate(struct vhost_dev *dev, char* mac_addr)
+{
+VhostUserMsg msg = { 0 };
+int err;
+
+assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+/* If guest supports GUEST_ANNOUNCE do nothing */
+if (__virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE))
+return 0;
+
+/* if backend supports VHOST_USER_PROTOCOL_F_RARP ask it to send the RARP
*/
+if (__virtio_has_feature(dev->protocol_features,
VHOST_USER_PROTOCOL_F_RARP)) {
+msg.request = VHOST_USER_SEND_RARP;
+msg.flags = VHOST_USER_VERSION;
+memcpy((char*) &msg.u64, mac_addr, 6);
+msg.size = sizeof(m.u64);
+
+err = vhost_user_write(dev, &msg, NULL, 0
[Qemu-devel] [PATCH v5 0/2] vhost user: Add live migration
v4->v5 1. The first patch is unchanged 2. The second patch is based on top of "vhost-user: protocol updates" series proposed earlier by Michael S. Tsirkin and "vhost-user: add migration log support" series proposed earlier by Marc Andre Lureau. The first patch provides limited live migration: - guest without GUEST_ANNOUNCE capabilities does not announce migration ending and peers talking to the migrated guest can suffer important network outage. - Some packets sent by remote peers to the guest can be lost during migration. The second patch fixes limitation for guest without GUEST_ANNOUNCE capabilities and patches from Marc Andre Lureau fix potential packet's lost during migration. Thibaut Collet (2): vhost user: add support of live migration vhost user: add rarp sending after live migration for legacy guest docs/specs/vhost-user.txt | 15 + hw/net/vhost_net.c| 18 hw/virtio/vhost-user.c| 32 +-- include/hw/virtio/vhost-backend.h |2 ++ include/net/vhost_net.h |1 + net/vhost-user.c | 43 +++-- 6 files changed, 107 insertions(+), 4 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v5 1/2] vhost user: add support of live migration
Some vhost user backends 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 used only by qemu_announce_self after a migration to send
fake RARP to avoid network outage for peers talking to the migrated guest.
- For guest with GUEST_ANNOUNCE capabilities these packets must be discarded,
GARP are sent by the guest thanks the GUEST_ANNOUNCE.
- For guest without GUEST_ANNOUNCE capabilities these packets are discarded
too, migration termination is notified when the guest sends packets.
Signed-off-by: Thibaut Collet
---
hw/net/vhost_net.c |2 ++
net/vhost-user.c | 25 +++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index c864237..9850520 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 93dcecd..2290271 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -65,6 +65,28 @@ 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 about 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 expects only RARP (size 60)."
+"Receives unexpected packets with size %lu\n",
+size);
+fflush(stderr);
+display_trace = 0;
+}
+}
+return size;
+}
+
static void vhost_user_cleanup(NetClientState *nc)
{
VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -90,6 +112,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,
@@ -143,8 +166,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;
qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
--
1.7.10.4
Re: [Qemu-devel] [PATCH 0/4] vhost-user: protocol updates
On Fri, Jul 17, 2015 at 4:09 PM, Michael S. Tsirkin wrote: > This patchset sets the stage for extending the vhost user > protocol, with full backwards compatibility. > > The approach is to negotiate feature bits queried and > acknowledged during device setup. > > For now, there's no new functionality: two new messages > are added that will allow negotiating new messages > required for functionality such as MQ and migration. > > For now, I used the feature bit 30 to signal support for these new messages, > and we now have 64 more bits to play. > > The patches can be found in my tree, branch vhost-user. > > Only patch 1 is intended for 2.4. > > Posting early so people working on extensions such as > migration can review this - but please note > the protocol is not set in stone yet. > > Michael S. Tsirkin (4): > Revert "vhost-user: add multi queue support" > vhost-user: refactor ioctl translation > vhost-user: add protocol feature negotiation > vhost-user: unit test for new messages > > qapi-schema.json | 6 +- > include/hw/virtio/vhost.h | 1 + > hw/net/vhost_net.c| 5 +- > hw/virtio/vhost-user.c| 150 > ++ > net/vhost-user.c | 37 > tests/vhost-user-test.c | 19 ++ > docs/specs/vhost-user.txt | 40 +++-- > qemu-options.hx | 5 +- > 8 files changed, 174 insertions(+), 89 deletions(-) > > -- > MST > Great job. I will reuse the protocol extension to rewrite my patch for live migration with vhost user and legacy guest that does not support GUEST_ANNOUNCE. I will rebase my work on top of this patch and Marc André one about "add migration log support". I hope to finalize my patch next week and provide a complete live migration with vapp as backend. Thanks. Thibaut.
[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
---
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
[Qemu-devel] [PATCH v4 0/1] Add live migration for vhost user
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
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
On Thu, Jun 25, 2015 at 2:53 PM, Michael S. Tsirkin wrote: > On Thu, Jun 25, 2015 at 01:01:29PM +0200, Thibaut Collet wrote: >> On Thu, Jun 25, 2015 at 11:59 AM, Jason Wang wrote: >> > >> > >> > >> > On 06/24/2015 07:05 PM, Michael S. Tsirkin wrote: >> > > On Wed, Jun 24, 2015 at 04:31:15PM +0800, Jason Wang wrote: >> > >> >> > >> On 06/23/2015 01:49 PM, Michael S. Tsirkin wrote: >> > >>> On Tue, Jun 23, 2015 at 10:12:17AM +0800, Jason Wang wrote: >> > >>>>> >> > >>>>> On 06/18/2015 11:16 PM, Thibaut Collet wrote: >> > >>>>>>> On Tue, Jun 16, 2015 at 10:05 AM, Jason Wang >> > >>>>>>> wrote: >> > >>>>>>>>> On 06/16/2015 03:24 PM, Thibaut Collet wrote: >> > >>>>>>>>>>> If my understanding is correct, on a resume operation, we have >> > >>>>>>>>>>> the >> > >>>>>>>>>>> following callback trace: >> > >>>>>>>>>>> 1. virtio_pci_restore function that calls all restore call >> > >>>>>>>>>>> back of >> > >>>>>>>>>>> virtio devices >> > >>>>>>>>>>> 2. virtnet_restore that calls try_fill_recv function for each >> > >>>>>>>>>>> virtual queues >> > >>>>>>>>>>> 3. try_fill_recv function kicks the virtual queue (through >> > >>>>>>>>>>> virtqueue_kick function) >> > >>>>>>>>> Yes, but this happens only after pm resume not migration. >> > >>>>>>>>> Migration is >> > >>>>>>>>> totally transparent to guest. >> > >>>>>>>>> >> > >>>>>>> Hi Jason, >> > >>>>>>> >> > >>>>>>> After a deeper look in the migration code of QEMU a resume event is >> > >>>>>>> always sent when the live migration is finished. >> > >>>>>>> On a live migration we have the following callback trace: >> > >>>>>>> 1. The VM on the new host is set to the state RUN_STATE_INMIGRATE, >> > >>>>>>> the >> > >>>>>>> autostart boolean to 1 and calls the qemu_start_incoming_migration >> > >>>>>>> function (see function main of vl.c) >> > >>>>>>> . >> > >>>>>>> 2. call of process_incoming_migration function in >> > >>>>>>> migration/migration.c file whatever the way to do the live >> > >>>>>>> migration >> > >>>>>>> (tcp:, fd:, unix:, exec: ...) >> > >>>>>>> 3. call of process_incoming_migration_co function in >> > >>>>>>> migration/migration.c >> > >>>>>>> 4. call of vm_start function in vl.c (otherwise the migrated VM >> > >>>>>>> stay >> > >>>>>>> in the pause state, the autostart boolean is set to 1 by the main >> > >>>>>>> function in vl.c) >> > >>>>>>> 5. call of vm_start function that sets the VM is the >> > >>>>>>> RUN_STATE_RUNNING state. >> > >>>>>>> 6. call of qapi_event_send_resume function that ends a resume >> > >>>>>>> event to the VM >> > >>>>> AFAIK, this function sends resume event to qemu monitor not VM. >> > >>>>> >> > >>>>>>> So when a live migration is ended: >> > >>>>>>> 1. a resume event is sent to the guest >> > >>>>>>> 2. On the reception of this resume event the virtual queue are >> > >>>>>>> kicked >> > >>>>>>> by the guest >> > >>>>>>> 3. Backend vhost user catches this kick and can emit a RARP to >> > >>>>>>> guest >> > >>>>>>> that does not support GUEST_ANNOUNCE >> > >>>>>>> >> > >>>>>>> This solution, as solution based on detection of DRIVER_OK status >> > >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
On Thu, Jun 25, 2015 at 11:59 AM, Jason Wang wrote: > > > > On 06/24/2015 07:05 PM, Michael S. Tsirkin wrote: > > On Wed, Jun 24, 2015 at 04:31:15PM +0800, Jason Wang wrote: > >> > >> On 06/23/2015 01:49 PM, Michael S. Tsirkin wrote: > >>> On Tue, Jun 23, 2015 at 10:12:17AM +0800, Jason Wang wrote: > >>>>> > >>>>> On 06/18/2015 11:16 PM, Thibaut Collet wrote: > >>>>>>> On Tue, Jun 16, 2015 at 10:05 AM, Jason Wang > >>>>>>> wrote: > >>>>>>>>> On 06/16/2015 03:24 PM, Thibaut Collet wrote: > >>>>>>>>>>> If my understanding is correct, on a resume operation, we have the > >>>>>>>>>>> following callback trace: > >>>>>>>>>>> 1. virtio_pci_restore function that calls all restore call back of > >>>>>>>>>>> virtio devices > >>>>>>>>>>> 2. virtnet_restore that calls try_fill_recv function for each > >>>>>>>>>>> virtual queues > >>>>>>>>>>> 3. try_fill_recv function kicks the virtual queue (through > >>>>>>>>>>> virtqueue_kick function) > >>>>>>>>> Yes, but this happens only after pm resume not migration. Migration > >>>>>>>>> is > >>>>>>>>> totally transparent to guest. > >>>>>>>>> > >>>>>>> Hi Jason, > >>>>>>> > >>>>>>> After a deeper look in the migration code of QEMU a resume event is > >>>>>>> always sent when the live migration is finished. > >>>>>>> On a live migration we have the following callback trace: > >>>>>>> 1. The VM on the new host is set to the state RUN_STATE_INMIGRATE, the > >>>>>>> autostart boolean to 1 and calls the qemu_start_incoming_migration > >>>>>>> function (see function main of vl.c) > >>>>>>> . > >>>>>>> 2. call of process_incoming_migration function in > >>>>>>> migration/migration.c file whatever the way to do the live migration > >>>>>>> (tcp:, fd:, unix:, exec: ...) > >>>>>>> 3. call of process_incoming_migration_co function in > >>>>>>> migration/migration.c > >>>>>>> 4. call of vm_start function in vl.c (otherwise the migrated VM stay > >>>>>>> in the pause state, the autostart boolean is set to 1 by the main > >>>>>>> function in vl.c) > >>>>>>> 5. call of vm_start function that sets the VM is the > >>>>>>> RUN_STATE_RUNNING state. > >>>>>>> 6. call of qapi_event_send_resume function that ends a resume event > >>>>>>> to the VM > >>>>> AFAIK, this function sends resume event to qemu monitor not VM. > >>>>> > >>>>>>> So when a live migration is ended: > >>>>>>> 1. a resume event is sent to the guest > >>>>>>> 2. On the reception of this resume event the virtual queue are kicked > >>>>>>> by the guest > >>>>>>> 3. Backend vhost user catches this kick and can emit a RARP to guest > >>>>>>> that does not support GUEST_ANNOUNCE > >>>>>>> > >>>>>>> This solution, as solution based on detection of DRIVER_OK status > >>>>>>> suggested by Michael, allows backend to send the RARP to legacy guest > >>>>>>> without involving QEMU and add ioctl to vhost-user. > >>>>> A question here is did vhost-user code pass status to the backend? If > >>>>> not, how can userspace backend detect DRIVER_OK? > >>> Sorry, I must have been unclear. > >>> vhost core calls VHOST_NET_SET_BACKEND on DRIVER_OK. > >>> Unfortunately vhost user currently translates it to VHOST_USER_NONE. > >> Looks like VHOST_NET_SET_BACKEND was only used for tap backend. > >> > >>> As a work around, I think kicking ioeventfds once you get > >>> VHOST_NET_SET_BACKEND will work. > >> Maybe just a eventfd_set() in vhost_net_start(). But is this > >> "workaround" elegant enough to be documented? Is it better to do this > >> ex
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
On Tue, Jun 16, 2015 at 10:05 AM, Jason Wang wrote: > > > On 06/16/2015 03:24 PM, Thibaut Collet wrote: >> If my understanding is correct, on a resume operation, we have the >> following callback trace: >> 1. virtio_pci_restore function that calls all restore call back of >> virtio devices >> 2. virtnet_restore that calls try_fill_recv function for each virtual queues >> 3. try_fill_recv function kicks the virtual queue (through >> virtqueue_kick function) > > Yes, but this happens only after pm resume not migration. Migration is > totally transparent to guest. > Hi Jason, After a deeper look in the migration code of QEMU a resume event is always sent when the live migration is finished. On a live migration we have the following callback trace: 1. The VM on the new host is set to the state RUN_STATE_INMIGRATE, the autostart boolean to 1 and calls the qemu_start_incoming_migration function (see function main of vl.c) . 2. call of process_incoming_migration function in migration/migration.c file whatever the way to do the live migration (tcp:, fd:, unix:, exec: ...) 3. call of process_incoming_migration_co function in migration/migration.c 4. call of vm_start function in vl.c (otherwise the migrated VM stay in the pause state, the autostart boolean is set to 1 by the main function in vl.c) 5. call of vm_start function that sets the VM is the RUN_STATE_RUNNING state. 6. call of qapi_event_send_resume function that ends a resume event to the VM So when a live migration is ended: 1. a resume event is sent to the guest 2. On the reception of this resume event the virtual queue are kicked by the guest 3. Backend vhost user catches this kick and can emit a RARP to guest that does not support GUEST_ANNOUNCE This solution, as solution based on detection of DRIVER_OK status suggested by Michael, allows backend to send the RARP to legacy guest without involving QEMU and add ioctl to vhost-user. Vhost user backend is free to implement one of this two solutions. The single drawback of these two solutions is useless RARP sending in some case (reboot, ...). These messages are harmless and probably not blocking for a "light" patch to support live migration with vhost user. If you agree 1. The first patch must be 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 can be removed. Management of legacy guest for rarp sending can be done by modifications in backend without any change in QEMU. Best regards. Thibaut.
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
On Wed, Jun 17, 2015 at 8:42 AM, Michael S. Tsirkin wrote: > On Wed, Jun 17, 2015 at 12:16:09PM +0800, Jason Wang wrote: >> >> >> On 06/16/2015 04:16 PM, Thibaut Collet wrote: >> > For a live migration my understanding is there are a suspend resume >> > operation: >> > - The VM image is regularly copied from the old host to the new one >> > (modified pages due to VM operation can be copied several time) >> > - As soon as there are only few pages to copy the VM is suspended on >> > the old host, the last pages are copied, and the VM is resumed on the >> > new host. Migration is not totally transparent to guest that has a >> > small period of unavailability. >> >> But guest is not involved in the any of the migration process. Unless >> the performance drop, guest should not notice the migration at all. > > It is easy for backend to detect that guest started using the device, > e.g. detect DRIVER_OK status set. > I think that's enough to send RARPs. > >> Btw, please use bottom-posting which is easier for reader to understand >> the context. >> >> Thanks > > I agree, please don't top-post. > > -- > MST Sorry for top-posting rather than bottom-posting. Regarding the use of DRIVER_OK, in your opinion, when is the best time to test it ?
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
For a live migration my understanding is there are a suspend resume operation: - The VM image is regularly copied from the old host to the new one (modified pages due to VM operation can be copied several time) - As soon as there are only few pages to copy the VM is suspended on the old host, the last pages are copied, and the VM is resumed on the new host. Migration is not totally transparent to guest that has a small period of unavailability. On Tue, Jun 16, 2015 at 10:05 AM, Jason Wang wrote: > > > On 06/16/2015 03:24 PM, Thibaut Collet wrote: >> If my understanding is correct, on a resume operation, we have the >> following callback trace: >> 1. virtio_pci_restore function that calls all restore call back of >> virtio devices >> 2. virtnet_restore that calls try_fill_recv function for each virtual queues >> 3. try_fill_recv function kicks the virtual queue (through >> virtqueue_kick function) > > Yes, but this happens only after pm resume not migration. Migration is > totally transparent to guest. > >> >> >> On Tue, Jun 16, 2015 at 7:29 AM, Jason Wang wrote: >>> >>> On 06/15/2015 08:12 PM, Thibaut Collet wrote: >>>> After a resume operation the guest always kicks the backend for each >>>> virtual queues. >>>> A live migration does a suspend operation on the old host and a resume >>>> operation on the new host. So the backend has a kick after migration. >>>> >>>> I have checked this point with a legacy guest (redhat 6-5 with kernel >>>> version 2.6.32-431.29.2) and the kick occurs after migration or >>>> resume. >>>> >>>> Jason have you an example of legacy guest that will not kick the >>>> virtual queue after a resume ? >>> I must miss something but migration should be transparent to guest. >>> Could you show me the code that guest does the kick after migration? >>> >>>> On Mon, Jun 15, 2015 at 10:44 AM, Michael S. Tsirkin >>>> wrote: >>>>> On Mon, Jun 15, 2015 at 03:43:13PM +0800, Jason Wang wrote: >>>>>> On 06/12/2015 10:28 PM, Michael S. Tsirkin wrote: >>>>>>> On Fri, Jun 12, 2015 at 03:55:33PM +0800, Jason Wang wrote: >>>>>>>> On 06/11/2015 08:13 PM, Michael S. Tsirkin wrote: >>>>>>>>> On Thu, Jun 11, 2015 at 02:10:48PM +0200, Thibaut Collet wrote: >>>>>>>>>> I am not sure to understand your remark: >>>>>>>>>> >>>>>>>>>>> It needs to be sent when backend is activated by guest kick >>>>>>>>>>> (in case of virtio 1, it's possible to use DRIVER_OK for this). >>>>>>>>>>> This does not happen when VM still runs on source. >>>>>>>>>> Could you confirm rarp can be sent by backend when the >>>>>>>>>> VHOST_USER_SET_VRING_KICK message is received by the backend ? >>>>>>>>> No - the time to send pakets is when you start processing >>>>>>>>> the rings. >>>>>>>>> >>>>>>>>> And the time to do that is when you detect a kick on >>>>>>>>> an eventfd, not when said fd is set. >>>>>>>>> >>>>>>>> Probably not. What if guest is only doing receiving? >>>>>>> Clarification: the kick can be on any VQs. >>>>>>> In your example, guest kicks after adding receive buffers. >>>>>> Yes, but refill only happens on we are lacking of receive buffers. It is >>>>>> not guaranteed to happen just after migration, we may have still have >>>>>> enough rx buffers for device to receive. >>>>> I think we also kick the backend after migration, do we not? >>>>> Further, DRIVER_OK can be used as a signal to start backend too. >>>>> >>>>>>>> In this case, you >>>>>>>> won't detect any kick if you don't send the rarp first. >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
If my understanding is correct, on a resume operation, we have the following callback trace: 1. virtio_pci_restore function that calls all restore call back of virtio devices 2. virtnet_restore that calls try_fill_recv function for each virtual queues 3. try_fill_recv function kicks the virtual queue (through virtqueue_kick function) On Tue, Jun 16, 2015 at 7:29 AM, Jason Wang wrote: > > > On 06/15/2015 08:12 PM, Thibaut Collet wrote: >> After a resume operation the guest always kicks the backend for each >> virtual queues. >> A live migration does a suspend operation on the old host and a resume >> operation on the new host. So the backend has a kick after migration. >> >> I have checked this point with a legacy guest (redhat 6-5 with kernel >> version 2.6.32-431.29.2) and the kick occurs after migration or >> resume. >> >> Jason have you an example of legacy guest that will not kick the >> virtual queue after a resume ? > > I must miss something but migration should be transparent to guest. > Could you show me the code that guest does the kick after migration? > >> >> On Mon, Jun 15, 2015 at 10:44 AM, Michael S. Tsirkin wrote: >>> On Mon, Jun 15, 2015 at 03:43:13PM +0800, Jason Wang wrote: >>>> >>>> On 06/12/2015 10:28 PM, Michael S. Tsirkin wrote: >>>>> On Fri, Jun 12, 2015 at 03:55:33PM +0800, Jason Wang wrote: >>>>>> On 06/11/2015 08:13 PM, Michael S. Tsirkin wrote: >>>>>>> On Thu, Jun 11, 2015 at 02:10:48PM +0200, Thibaut Collet wrote: >>>>>>>> I am not sure to understand your remark: >>>>>>>> >>>>>>>>> It needs to be sent when backend is activated by guest kick >>>>>>>>> (in case of virtio 1, it's possible to use DRIVER_OK for this). >>>>>>>>> This does not happen when VM still runs on source. >>>>>>>> Could you confirm rarp can be sent by backend when the >>>>>>>> VHOST_USER_SET_VRING_KICK message is received by the backend ? >>>>>>> No - the time to send pakets is when you start processing >>>>>>> the rings. >>>>>>> >>>>>>> And the time to do that is when you detect a kick on >>>>>>> an eventfd, not when said fd is set. >>>>>>> >>>>>> Probably not. What if guest is only doing receiving? >>>>> Clarification: the kick can be on any VQs. >>>>> In your example, guest kicks after adding receive buffers. >>>> Yes, but refill only happens on we are lacking of receive buffers. It is >>>> not guaranteed to happen just after migration, we may have still have >>>> enough rx buffers for device to receive. >>> I think we also kick the backend after migration, do we not? >>> Further, DRIVER_OK can be used as a signal to start backend too. >>> >>>>>> In this case, you >>>>>> won't detect any kick if you don't send the rarp first. >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
If we use DRIVER_OK status bit to send the RARP by the backend I am afraid that some legacy guest are not supported. Moreover the vhost user backend is not aware of the change of the DRIVER_OK status bit. If this solution is chosen as event to send the RARP a message between QEMU and vhost user backend must be added. If there is a solution to avoid to add a message, i.e a IOCTL, to vhost user backend it will be better (no kernel update). Your solution with the kick on virtual queue seems OK in any case and does not need the add of a message. Except if there are a case where this solution does not work (today I have not found a case) I will prefer to implement this solution. On Mon, Jun 15, 2015 at 2:45 PM, Michael S. Tsirkin wrote: > On Mon, Jun 15, 2015 at 02:12:40PM +0200, Thibaut Collet wrote: >> After a resume operation the guest always kicks the backend for each >> virtual queues. >> A live migration does a suspend operation on the old host and a resume >> operation on the new host. So the backend has a kick after migration. >> >> I have checked this point with a legacy guest (redhat 6-5 with kernel >> version 2.6.32-431.29.2) and the kick occurs after migration or >> resume. >> >> Jason have you an example of legacy guest that will not kick the >> virtual queue after a resume ? > > In practice with sufficiently modern guests you can look > at DRIVER_OK status bit. Process rings if that bit is set. > > -- > MST
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
After a resume operation the guest always kicks the backend for each virtual queues. A live migration does a suspend operation on the old host and a resume operation on the new host. So the backend has a kick after migration. I have checked this point with a legacy guest (redhat 6-5 with kernel version 2.6.32-431.29.2) and the kick occurs after migration or resume. Jason have you an example of legacy guest that will not kick the virtual queue after a resume ? On Mon, Jun 15, 2015 at 10:44 AM, Michael S. Tsirkin wrote: > On Mon, Jun 15, 2015 at 03:43:13PM +0800, Jason Wang wrote: >> >> >> On 06/12/2015 10:28 PM, Michael S. Tsirkin wrote: >> > On Fri, Jun 12, 2015 at 03:55:33PM +0800, Jason Wang wrote: >> >> >> >> On 06/11/2015 08:13 PM, Michael S. Tsirkin wrote: >> >>> On Thu, Jun 11, 2015 at 02:10:48PM +0200, Thibaut Collet wrote: >> >>>> I am not sure to understand your remark: >> >>>> >> >>>>> It needs to be sent when backend is activated by guest kick >> >>>>> (in case of virtio 1, it's possible to use DRIVER_OK for this). >> >>>>> This does not happen when VM still runs on source. >> >>>> Could you confirm rarp can be sent by backend when the >> >>>> VHOST_USER_SET_VRING_KICK message is received by the backend ? >> >>> No - the time to send pakets is when you start processing >> >>> the rings. >> >>> >> >>> And the time to do that is when you detect a kick on >> >>> an eventfd, not when said fd is set. >> >>> >> >> Probably not. What if guest is only doing receiving? >> > Clarification: the kick can be on any VQs. >> > In your example, guest kicks after adding receive buffers. >> >> Yes, but refill only happens on we are lacking of receive buffers. It is >> not guaranteed to happen just after migration, we may have still have >> enough rx buffers for device to receive. > > I think we also kick the backend after migration, do we not? > Further, DRIVER_OK can be used as a signal to start backend too. > >> > >> >> In this case, you >> >> won't detect any kick if you don't send the rarp first.
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
If I correctly understand how vhost user / virtio works the solution proposed by Michael is OK: - Rings to exchange data between host and guest are allocated by the guest. - As soon as the guest add rings in a queue (for RX or TX) a kick is done on the eventfd associated to the queue - On a live migration (as for a first startup), the guest virtio-net pre-allocates rings for each queue and so send a kick on the eventfd (for RX and TX) So even if the guest is only doing receiving, there is a kick on any queues. --> Vhost-user backend knows when send the rarp in any conditions without involving QEMU. Michael, could you confirm that my analysis is correct? On Fri, Jun 12, 2015 at 9:55 AM, Jason Wang wrote: > > > On 06/11/2015 08:13 PM, Michael S. Tsirkin wrote: >> On Thu, Jun 11, 2015 at 02:10:48PM +0200, Thibaut Collet wrote: >>> I am not sure to understand your remark: >>> >>>> It needs to be sent when backend is activated by guest kick >>>> (in case of virtio 1, it's possible to use DRIVER_OK for this). >>>> This does not happen when VM still runs on source. >>> Could you confirm rarp can be sent by backend when the >>> VHOST_USER_SET_VRING_KICK message is received by the backend ? >> No - the time to send pakets is when you start processing >> the rings. >> >> And the time to do that is when you detect a kick on >> an eventfd, not when said fd is set. >> > > Probably not. What if guest is only doing receiving? In this case, you > won't detect any kick if you don't send the rarp first.
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
Ok. backend is able to know when the eventfd is kick the first time and then can send a rarp for legacy guest. With this backend modification the issue point by Jason is no more a QEMU problem. Full support of live migration for vhost user: - Need my first patch. - For legacy guest the backend must be updated to send a rarp when the eventfd is kick the first time. To summarize the status: 1. The first patch must be 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 must be removed. Management of legacy guest for rarp sending can be done by modifications in backend without any change in QEMU. Does everyone agree with this summary? On Thu, Jun 11, 2015 at 2:13 PM, Michael S. Tsirkin wrote: > On Thu, Jun 11, 2015 at 02:10:48PM +0200, Thibaut Collet wrote: > > I am not sure to understand your remark: > > > > > It needs to be sent when backend is activated by guest kick > > > (in case of virtio 1, it's possible to use DRIVER_OK for this). > > > This does not happen when VM still runs on source. > > > > Could you confirm rarp can be sent by backend when the > > VHOST_USER_SET_VRING_KICK message is received by the backend ? > > No - the time to send pakets is when you start processing > the rings. > > And the time to do that is when you detect a kick on > an eventfd, not when said fd is set. > > > > At this time the migration is completed and there is no risk of > confusing the > > switch. > > In this case: > > - there are nothing to do in QEMU to manage legacy guest with > > no GUEST_ANNOUNCE. > > - All the job is done by the backend on the VHOST_USER_SET_VRING_KICK > > reception. Maybe switch notification of live migration is done with a > small > > delay but it works > > - This patch can be discarded. > > > > > > On Thu, Jun 11, 2015 at 12:38 PM, Michael S. Tsirkin > wrote: > > > > On Thu, Jun 11, 2015 at 01:54:22PM +0800, Jason Wang wrote: > > > > > > > > > On 06/11/2015 01:49 PM, Thibaut Collet wrote: > > > > > Yes, but still need a mechanism to notify the backend of > migration > > > > > completion from qemu side if GUEST_ANNOUNCE is not negotiated. > > > > > > > > backend is aware of a connection with the guest (with the feature > > > > negociation) and can send a rarp. This rarp will be always sent > by the > > > > backend when a VM is launched (first start or live migration > > > > completion) if the GUEST_ANOUNCE is not supported. > > > > In this case the issue is solved without done everything by QEMU. > > > > > > The issue is during migration guest network is still active. So > sending > > > rarp too early in the destination (e.g during VM is launched) may > > > confuse the switch. We want it to be sent exactly when the > migration is > > > completed in destination. > > > > It needs to be sent when backend is activated by guest kick > > (in case of virtio 1, it's possible to use DRIVER_OK for this). > > This does not happen when VM still runs on source. > > > > > > If sending a rarp message on the start of te VM is not > accceptable, we > > > > must provide a mechanism similar of the one I have implemented. > The > > > > message content can be empty as the backend is able to create > the rarp > > > > message. > > > > > > Yes. > > > > >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
I am not sure to understand your remark: > It needs to be sent when backend is activated by guest kick > (in case of virtio 1, it's possible to use DRIVER_OK for this). > This does not happen when VM still runs on source. Could you confirm rarp can be sent by backend when the VHOST_USER_SET_VRING_KICK message is received by the backend ? At this time the migration is completed and there is no risk of confusing the switch. In this case: - there are nothing to do in QEMU to manage legacy guest with no GUEST_ANNOUNCE. - All the job is done by the backend on the VHOST_USER_SET_VRING_KICK reception. Maybe switch notification of live migration is done with a small delay but it works - This patch can be discarded. On Thu, Jun 11, 2015 at 12:38 PM, Michael S. Tsirkin wrote: > On Thu, Jun 11, 2015 at 01:54:22PM +0800, Jason Wang wrote: > > > > > > On 06/11/2015 01:49 PM, Thibaut Collet wrote: > > > > Yes, but still need a mechanism to notify the backend of migration > > > > completion from qemu side if GUEST_ANNOUNCE is not negotiated. > > > > > > backend is aware of a connection with the guest (with the feature > > > negociation) and can send a rarp. This rarp will be always sent by the > > > backend when a VM is launched (first start or live migration > > > completion) if the GUEST_ANOUNCE is not supported. > > > In this case the issue is solved without done everything by QEMU. > > > > The issue is during migration guest network is still active. So sending > > rarp too early in the destination (e.g during VM is launched) may > > confuse the switch. We want it to be sent exactly when the migration is > > completed in destination. > > It needs to be sent when backend is activated by guest kick > (in case of virtio 1, it's possible to use DRIVER_OK for this). > This does not happen when VM still runs on source. > > > > If sending a rarp message on the start of te VM is not accceptable, we > > > must provide a mechanism similar of the one I have implemented. The > > > message content can be empty as the backend is able to create the rarp > > > message. > > > > Yes. >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
> Yes, but still need a mechanism to notify the backend of migration > completion from qemu side if GUEST_ANNOUNCE is not negotiated. backend is aware of a connection with the guest (with the feature negociation) and can send a rarp. This rarp will be always sent by the backend when a VM is launched (first start or live migration completion) if the GUEST_ANOUNCE is not supported. In this case the issue is solved without done everything by QEMU. If sending a rarp message on the start of te VM is not accceptable, we must provide a mechanism similar of the one I have implemented. The message content can be empty as the backend is able to create the rarp message.
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
The warning message is not really necessary is just a reminder.
On Wed, Jun 10, 2015 at 10:50 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2015 at 10:25:57PM +0200, Thibaut Collet wrote:
> > Yes backend can save everything to be able to send the rarp alone after
> a live
> > migration.
> > Main purpose of this patch is to answer to the point raise by Jason on
> the
> > previous version of my patch:
> > > Yes, your patch works well for recent drivers. But the problem is
> legacy
> > > guest/driver without VIRTIO_NET_F_GUEST_ANNOUNCE. In this case there
> > > will be no GARP sent after migration.
> >
> > If Jason is OK with this solution this patch can be forgotten.
> >
> > Maybe, to warn user of this issue if the backend does not send the rarp,
> it can
> > be useful to keep the warn message of the previous patch:
> > > +fprintf(stderr,
> > > +"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support.
> > RARP must be sent by vhost-user backend\n");
> > > +fflush(stderr);
> > with a static bool to display this message only one time to limit the
> number of
> > message ?
>
> I don't see why it's necessary.
>
> > On Wed, Jun 10, 2015 at 6:00 PM, Michael S. Tsirkin
> wrote:
> >
> > On Wed, Jun 10, 2015 at 05:48:47PM +0200, Thibaut Collet wrote:
> > > I have involved QEMU because QEMU prepares the rarp. I agree that
> backend
> > has
> > > probably all the information to do that.
> > > But backend does not know if the guest supports
> > the VIRTIO_NET_F_GUEST_ANNOUNCE
> >
> > Why not? Backend has the acked feature mask.
> >
> >
> > > and will send a useless rarp.
> > > Maybe this duplication of requests is not very important and in
> this case
> > this
> > > patch is not mandatory.
> > >
> > > On Wed, Jun 10, 2015 at 5:34 PM, Michael S. Tsirkin <
> m...@redhat.com>
> > wrote:
> > >
> > > On Wed, Jun 10, 2015 at 03:43:03PM +0200, Thibaut Collet wrote:
> > > > In case of live migration with legacy guest (without
> > > VIRTIO_NET_F_GUEST_ANNOUNCE)
> > > > a message is added between QEMU and the vhost client/backend.
> > > > This message provides the RARP content, prepared by QEMU, to
> the
> > vhost
> > > > client/backend.
> > > > The vhost client/backend is responsible to send the RARP.
> > > >
> > > > Signed-off-by: Thibaut Collet
> > > > ---
> > > > docs/specs/vhost-user.txt | 16
> > > > hw/net/vhost_net.c |8
> > > > hw/virtio/vhost-user.c | 11 ++-
> > > > linux-headers/linux/vhost.h |9 +
> > > > 4 files changed, 43 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/docs/specs/vhost-user.txt
> b/docs/specs/vhost-user.txt
> > > > index 2c8e934..ef5d475 100644
> > > > --- a/docs/specs/vhost-user.txt
> > > > +++ b/docs/specs/vhost-user.txt
> > > > @@ -97,6 +97,7 @@ typedef struct VhostUserMsg {
> > > > uint64_t u64;
> > > > struct vhost_vring_state state;
> > > > struct vhost_vring_addr addr;
> > > > +struct vhost_inject_rarp rarp;
> > > > VhostUserMemory memory;
> > > > };
> > > > } QEMU_PACKED VhostUserMsg;
> > > > @@ -132,6 +133,12 @@ Multi queue support
> > > > The protocol supports multiple queues by setting all index
> fields
> > in the
> > > sent
> > > > messages to a properly calculated value.
> > > >
> > > > +Live migration support
> > > > +--
> > > > +The protocol supports live migration. GARP from the
> migrated guest
> > is
> > > done
> > > > +through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism for guest
> that
> > > supports it or
> > > > +through RARP.
> > > > +
> > > > Message types
> > > > -
> >
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
Yes backend can save everything to be able to send the rarp alone after a
live migration.
Main purpose of this patch is to answer to the point raise by Jason on the
previous version of my patch:
> Yes, your patch works well for recent drivers. But the problem is legacy
> guest/driver without VIRTIO_NET_F_GUEST_ANNOUNCE. In this case there
> will be no GARP sent after migration.
If Jason is OK with this solution this patch can be forgotten.
Maybe, to warn user of this issue if the backend does not send the rarp, it
can be useful to keep the warn message of the previous patch:
> +fprintf(stderr,
> +"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
support. RARP must be sent by vhost-user backend\n");
> +fflush(stderr);
with a static bool to display this message only one time to limit the
number of message ?
On Wed, Jun 10, 2015 at 6:00 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2015 at 05:48:47PM +0200, Thibaut Collet wrote:
> > I have involved QEMU because QEMU prepares the rarp. I agree that
> backend has
> > probably all the information to do that.
> > But backend does not know if the guest supports
> the VIRTIO_NET_F_GUEST_ANNOUNCE
>
> Why not? Backend has the acked feature mask.
>
>
> > and will send a useless rarp.
> > Maybe this duplication of requests is not very important and in this
> case this
> > patch is not mandatory.
> >
> > On Wed, Jun 10, 2015 at 5:34 PM, Michael S. Tsirkin
> wrote:
> >
> > On Wed, Jun 10, 2015 at 03:43:03PM +0200, Thibaut Collet wrote:
> > > In case of live migration with legacy guest (without
> > VIRTIO_NET_F_GUEST_ANNOUNCE)
> > > a message is added between QEMU and the vhost client/backend.
> > > This message provides the RARP content, prepared by QEMU, to the
> vhost
> > > client/backend.
> > > The vhost client/backend is responsible to send the RARP.
> > >
> > > Signed-off-by: Thibaut Collet
> > > ---
> > > docs/specs/vhost-user.txt | 16
> > > hw/net/vhost_net.c |8
> > > hw/virtio/vhost-user.c | 11 ++-
> > > linux-headers/linux/vhost.h |9 +
> > > 4 files changed, 43 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > index 2c8e934..ef5d475 100644
> > > --- a/docs/specs/vhost-user.txt
> > > +++ b/docs/specs/vhost-user.txt
> > > @@ -97,6 +97,7 @@ typedef struct VhostUserMsg {
> > > uint64_t u64;
> > > struct vhost_vring_state state;
> > > struct vhost_vring_addr addr;
> > > +struct vhost_inject_rarp rarp;
> > > VhostUserMemory memory;
> > > };
> > > } QEMU_PACKED VhostUserMsg;
> > > @@ -132,6 +133,12 @@ Multi queue support
> > > The protocol supports multiple queues by setting all index fields
> in the
> > sent
> > > messages to a properly calculated value.
> > >
> > > +Live migration support
> > > +--
> > > +The protocol supports live migration. GARP from the migrated
> guest is
> > done
> > > +through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism for guest that
> > supports it or
> > > +through RARP.
> > > +
> > > Message types
> > > -
> > >
> > > @@ -269,3 +276,12 @@ Message types
> > >Bits (0-7) of the payload contain the vring index. Bit 8 is
> the
> > >invalid FD flag. This flag is set when there is no file
> descriptor
> > >in the ancillary data.
> > > +
> > > + * VHOST_USER_NET_INJECT_RARP
> > > +
> > > + Id: 15
> > > + Master payload: rarp content
> > > +
> > > + Provide the RARP message to send to the guest after a live
> > migration. This
> > > + message is sent only for guest that does not support
> > > + VIRTIO_NET_F_GUEST_ANNOUNCE.
> >
> > I don't see why this is needed.
> > Can't backend simply send rarp itself?
> > Why do we need to involve QEMU?
> >
> >
> >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 4a42325..f66d48d 100644
> > > --- a/hw/net/vhost_net.c
> > &g
Re: [Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
I have involved QEMU because QEMU prepares the rarp. I agree that backend
has probably all the information to do that.
But backend does not know if the guest supports the VIRTIO_NET_F_GUEST_ANNOUNCE
and will send a useless rarp.
Maybe this duplication of requests is not very important and in this case
this patch is not mandatory.
On Wed, Jun 10, 2015 at 5:34 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2015 at 03:43:03PM +0200, Thibaut Collet wrote:
> > In case of live migration with legacy guest (without
> VIRTIO_NET_F_GUEST_ANNOUNCE)
> > a message is added between QEMU and the vhost client/backend.
> > This message provides the RARP content, prepared by QEMU, to the vhost
> > client/backend.
> > The vhost client/backend is responsible to send the RARP.
> >
> > Signed-off-by: Thibaut Collet
> > ---
> > docs/specs/vhost-user.txt | 16
> > hw/net/vhost_net.c |8
> > hw/virtio/vhost-user.c | 11 ++-
> > linux-headers/linux/vhost.h |9 +
> > 4 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > index 2c8e934..ef5d475 100644
> > --- a/docs/specs/vhost-user.txt
> > +++ b/docs/specs/vhost-user.txt
> > @@ -97,6 +97,7 @@ typedef struct VhostUserMsg {
> > uint64_t u64;
> > struct vhost_vring_state state;
> > struct vhost_vring_addr addr;
> > +struct vhost_inject_rarp rarp;
> > VhostUserMemory memory;
> > };
> > } QEMU_PACKED VhostUserMsg;
> > @@ -132,6 +133,12 @@ Multi queue support
> > The protocol supports multiple queues by setting all index fields in
> the sent
> > messages to a properly calculated value.
> >
> > +Live migration support
> > +--
> > +The protocol supports live migration. GARP from the migrated guest is
> done
> > +through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism for guest that
> supports it or
> > +through RARP.
> > +
> > Message types
> > -
> >
> > @@ -269,3 +276,12 @@ Message types
> >Bits (0-7) of the payload contain the vring index. Bit 8 is the
> >invalid FD flag. This flag is set when there is no file descriptor
> >in the ancillary data.
> > +
> > + * VHOST_USER_NET_INJECT_RARP
> > +
> > + Id: 15
> > + Master payload: rarp content
> > +
> > + Provide the RARP message to send to the guest after a live
> migration. This
> > + message is sent only for guest that does not support
> > + VIRTIO_NET_F_GUEST_ANNOUNCE.
>
> I don't see why this is needed.
> Can't backend simply send rarp itself?
> Why do we need to involve QEMU?
>
>
>
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 4a42325..f66d48d 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -369,10 +369,18 @@ void vhost_net_stop(VirtIODevice *dev,
> NetClientState *ncs,
> >
> > void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf,
> size_t size)
> > {
> > +struct vhost_inject_rarp inject_rarp;
> > +memcpy(&inject_rarp.rarp, buf, size);
> > +
> > if ((net->dev.acked_features & (1 << VIRTIO_NET_F_GUEST_ANNOUNCE))
> == 0) {
> > +const VhostOps *vhost_ops = net->dev.vhost_ops;
> > +int r;
> > +
> > fprintf(stderr,
> > "Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support. RARP must be sent by vhost-user backend\n");
> > fflush(stderr);
> > +r = vhost_ops->vhost_call(&net->dev, VHOST_NET_INJECT_RARP,
> &inject_rarp);
> > +assert(r >= 0);
> > }
> > }
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index d6f2163..2e752ab 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -41,6 +41,7 @@ typedef enum VhostUserRequest {
> > VHOST_USER_SET_VRING_KICK = 12,
> > VHOST_USER_SET_VRING_CALL = 13,
> > VHOST_USER_SET_VRING_ERR = 14,
> > +VHOST_USER_NET_INJECT_RARP = 15,
> > VHOST_USER_MAX
> > } VhostUserRequest;
> >
> > @@ -70,6 +71,7 @@ typedef struct VhostUserMsg {
> > uint64_t u64;
> > struct vhost_vring_state state;
> > struct vhost_vring_addr addr;
> > +struct vhost_inject_rarp rarp;
> > VhostUserMemory memory;
> > };
> > }
Re: [Qemu-devel] [PATCH v3 1/2] vhost user: add support of live migration
> vhost_net_inject_rarp is vhost-user specific.
For this function test on VIRTIO_NET_F_GUEST_ANNOUNCE bit is important to
take a decision and call a new vhost ioctl (see next patch) if the guest
does not support this feature and allow the vhost client/backend send the
RARP packet.
Use of the VIRTIO_NET_F_GUEST_ANNOUNCE field and call to a vhost ioctl does
not seem relevant in the net/vhost-user.c file
Do you suggest to create a new hw/net/vhost_user_net.c file to manage this
new feature ?
On Wed, Jun 10, 2015 at 4:27 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2015 at 04:22:10PM +0200, Thibaut Collet wrote:
> > I am not sure to understand your comments.
> >
> > Could you confirm that the useless warning is:
> > +fprintf(stderr,
> > +"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support.
> > RARP must be sent by vhost-user backend\n");
>
> Yes.
>
> > > Can you move this out to vhost-user? It's not a generic function, is
> it?
> >
> > Which part of the code do you think it must be moved in a specific file ?
>
> vhost_net_inject_rarp is vhost-user specific.
>
> >
> > On Wed, Jun 10, 2015 at 3:52 PM, Michael S. Tsirkin
> wrote:
> >
> > On Wed, Jun 10, 2015 at 03:43:02PM +0200, Thibaut Collet wrote:
> > > Some vhost client/backend are able to support live migration.
> > > To provide this service the following features must be added:
> > > 1. GARP from guest after live migration:
> > >the VIRTIO_NET_F_GUEST_ANNOUNCE capability is added to
> vhost-net when
> > netdev
> > >backend is vhost-user to let virtio-net NIC manages GARP after
> > migration.
> > > 2. RARP on vhost-user:
> > >After a live migration RARP are automatically sent to any
> interfaces.
> > A
> > >receive callback is added to vhost-user to catch this packet.
> If guest
> > >supports VIRTIO_NET_F_GUEST_ANNOUNCE this packet is discarded
> to avoid
> > >message duplication (already done by virtio-net NIC). For
> legacy guest
> > >without VIRTIO_NET_F_GUEST_ANNOUNCE a warning message is
> displayed to
> > >alert the user. RARP must be sent to the vhost client/backend.
> > >
> > > Signed-off-by: Thibaut Collet
> > > ---
> > > hw/net/vhost_net.c | 15 +++
> > > include/net/vhost_net.h |1 +
> > > net/vhost-user.c| 21 +++--
> > > 3 files changed, 35 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 426b23e..4a42325 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -82,6 +82,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
> > > @@ -365,6 +367,15 @@ void vhost_net_stop(VirtIODevice *dev,
> > NetClientState *ncs,
> > > assert(r >= 0);
> > > }
> > >
> > > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t
> *buf,
> > size_t size)
> > > +{
> > > +if ((net->dev.acked_features & (1 <<
> VIRTIO_NET_F_GUEST_ANNOUNCE)) =
> > = 0) {
> > > +fprintf(stderr,
> > > +"Warning: Guest with no
> VIRTIO_NET_F_GUEST_ANNOUNCE
> > support. RARP must be sent by vhost-user backend\n");
> > > +fflush(stderr);
> >
> > And maybe it does, then you are just filling log up with useless
> > warnings. Pls add some ifdef so this isn't normally compiled in.
> >
> > > +}
> >
> > Can you move this out to vhost-user? It's not a generic function, is
> it?
> >
> > > +}
> > > +
> > > void vhost_net_cleanup(struct vhost_net *net)
> > > {
> > > vhost_dev_cleanup(&net->dev);
> > > @@ -427,6 +438,10 @@ void vhost_net_stop(VirtIODevice *dev,
> > > {
> > > }
> > >
> > > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t
> *buf,
> > size_t size);
> > > +{
Re: [Qemu-devel] [PATCH v3 1/2] vhost user: add support of live migration
I am not sure to understand your comments.
Could you confirm that the useless warning is:
+fprintf(stderr,
+"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
support. RARP must be sent by vhost-user backend\n");
> Can you move this out to vhost-user? It's not a generic function, is it?
Which part of the code do you think it must be moved in a specific file ?
On Wed, Jun 10, 2015 at 3:52 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2015 at 03:43:02PM +0200, Thibaut Collet wrote:
> > Some vhost client/backend are able to support live migration.
> > To provide this service the following features must be added:
> > 1. GARP from guest after live migration:
> >the VIRTIO_NET_F_GUEST_ANNOUNCE capability is added to vhost-net when
> netdev
> >backend is vhost-user to let virtio-net NIC manages GARP after
> migration.
> > 2. RARP on vhost-user:
> >After a live migration RARP are automatically sent to any interfaces.
> A
> >receive callback is added to vhost-user to catch this packet. If guest
> >supports VIRTIO_NET_F_GUEST_ANNOUNCE this packet is discarded to avoid
> >message duplication (already done by virtio-net NIC). For legacy guest
> >without VIRTIO_NET_F_GUEST_ANNOUNCE a warning message is displayed to
> > alert the user. RARP must be sent to the vhost client/backend.
> >
> > Signed-off-by: Thibaut Collet
> > ---
> > hw/net/vhost_net.c | 15 +++
> > include/net/vhost_net.h |1 +
> > net/vhost-user.c| 21 +++--
> > 3 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 426b23e..4a42325 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -82,6 +82,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
> > @@ -365,6 +367,15 @@ void vhost_net_stop(VirtIODevice *dev,
> NetClientState *ncs,
> > assert(r >= 0);
> > }
> >
> > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf,
> size_t size)
> > +{
> > +if ((net->dev.acked_features & (1 << VIRTIO_NET_F_GUEST_ANNOUNCE))
> == 0) {
> > +fprintf(stderr,
> > +"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE
> support. RARP must be sent by vhost-user backend\n");
> > +fflush(stderr);
>
> And maybe it does, then you are just filling log up with useless
> warnings. Pls add some ifdef so this isn't normally compiled in.
>
> > +}
>
> Can you move this out to vhost-user? It's not a generic function, is it?
>
> > +}
> > +
> > void vhost_net_cleanup(struct vhost_net *net)
> > {
> > vhost_dev_cleanup(&net->dev);
> > @@ -427,6 +438,10 @@ void vhost_net_stop(VirtIODevice *dev,
> > {
> > }
> >
> > +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf,
> size_t size);
> > +{
> > +}
> > +
> > void vhost_net_cleanup(struct vhost_net *net)
> > {
> > }
> > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > index b1c18a3..e82a0f9 100644
> > --- a/include/net/vhost_net.h
> > +++ b/include/net/vhost_net.h
> > @@ -20,6 +20,7 @@ bool vhost_net_query(VHostNetState *net, VirtIODevice
> *dev);
> > int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int
> total_queues);
> > void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int
> total_queues);
> >
> > +void vhost_net_inject_rarp(VHostNetState *net, const uint8_t *buf,
> size_t size);
> > void vhost_net_cleanup(VHostNetState *net);
> >
> > unsigned vhost_net_get_features(VHostNetState *net, unsigned features);
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 8d26728..69c2313 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -66,6 +66,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)
> > +{
> > +if (size == 60) {
> > +/* Assume it is a RARP request sent automatically after a
> > + * live migration */
> > +/* The RARP must be sent if guest does not support
>
[Qemu-devel] [PATCH v3 2/2] vhost user: Add RARP injection for legacy guest
In case of live migration with legacy guest (without
VIRTIO_NET_F_GUEST_ANNOUNCE)
a message is added between QEMU and the vhost client/backend.
This message provides the RARP content, prepared by QEMU, to the vhost
client/backend.
The vhost client/backend is responsible to send the RARP.
Signed-off-by: Thibaut Collet
---
docs/specs/vhost-user.txt | 16
hw/net/vhost_net.c |8
hw/virtio/vhost-user.c | 11 ++-
linux-headers/linux/vhost.h |9 +
4 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 2c8e934..ef5d475 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -97,6 +97,7 @@ typedef struct VhostUserMsg {
uint64_t u64;
struct vhost_vring_state state;
struct vhost_vring_addr addr;
+struct vhost_inject_rarp rarp;
VhostUserMemory memory;
};
} QEMU_PACKED VhostUserMsg;
@@ -132,6 +133,12 @@ Multi queue support
The protocol supports multiple queues by setting all index fields in the sent
messages to a properly calculated value.
+Live migration support
+--
+The protocol supports live migration. GARP from the migrated guest is done
+through the VIRTIO_NET_F_GUEST_ANNOUNCE mechanism for guest that supports it or
+through RARP.
+
Message types
-
@@ -269,3 +276,12 @@ Message types
Bits (0-7) of the payload contain the vring index. Bit 8 is the
invalid FD flag. This flag is set when there is no file descriptor
in the ancillary data.
+
+ * VHOST_USER_NET_INJECT_RARP
+
+ Id: 15
+ Master payload: rarp content
+
+ Provide the RARP message to send to the guest after a live migration.
This
+ message is sent only for guest that does not support
+ VIRTIO_NET_F_GUEST_ANNOUNCE.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 4a42325..f66d48d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -369,10 +369,18 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState
*ncs,
void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf, size_t
size)
{
+struct vhost_inject_rarp inject_rarp;
+memcpy(&inject_rarp.rarp, buf, size);
+
if ((net->dev.acked_features & (1 << VIRTIO_NET_F_GUEST_ANNOUNCE)) == 0) {
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+int r;
+
fprintf(stderr,
"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE support.
RARP must be sent by vhost-user backend\n");
fflush(stderr);
+r = vhost_ops->vhost_call(&net->dev, VHOST_NET_INJECT_RARP,
&inject_rarp);
+assert(r >= 0);
}
}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d6f2163..2e752ab 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -41,6 +41,7 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_VRING_KICK = 12,
VHOST_USER_SET_VRING_CALL = 13,
VHOST_USER_SET_VRING_ERR = 14,
+VHOST_USER_NET_INJECT_RARP = 15,
VHOST_USER_MAX
} VhostUserRequest;
@@ -70,6 +71,7 @@ typedef struct VhostUserMsg {
uint64_t u64;
struct vhost_vring_state state;
struct vhost_vring_addr addr;
+struct vhost_inject_rarp rarp;
VhostUserMemory memory;
};
} QEMU_PACKED VhostUserMsg;
@@ -104,7 +106,8 @@ static unsigned long int
ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
VHOST_GET_VRING_BASE, /* VHOST_USER_GET_VRING_BASE */
VHOST_SET_VRING_KICK, /* VHOST_USER_SET_VRING_KICK */
VHOST_SET_VRING_CALL, /* VHOST_USER_SET_VRING_CALL */
-VHOST_SET_VRING_ERR /* VHOST_USER_SET_VRING_ERR */
+VHOST_SET_VRING_ERR,/* VHOST_USER_SET_VRING_ERR */
+VHOST_NET_INJECT_RARP /* VHOST_USER_NET_INJECT_RARP */
};
static VhostUserRequest vhost_user_request_translate(unsigned long int request)
@@ -287,6 +290,12 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned
long int request,
msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
}
break;
+
+case VHOST_NET_INJECT_RARP:
+memcpy(&msg.rarp, arg, sizeof(struct vhost_inject_rarp));
+msg.size = sizeof(struct vhost_inject_rarp);
+break;
+
default:
error_report("vhost-user trying to send unhandled ioctl");
return -1;
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index c656f61..1920134 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -63,6 +63,10 @@ struct vhost_memory {
struct vhost_memory_region regions[0];
};
+struct vhost_inject_rarp {
+ __u8 rarp[60];
+};
+
/* ioctls */
#define VHOST_VIRTIO 0xAF
@@ -121,6 +125,11 @@ struct vhost_memory {
* device. This can be used to stop the ring (e.g. for migration). */
#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_
[Qemu-devel] [PATCH v3 1/2] 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. GARP from guest after live migration:
the VIRTIO_NET_F_GUEST_ANNOUNCE capability is added to vhost-net when netdev
backend is vhost-user to let virtio-net NIC manages GARP after migration.
2. RARP on vhost-user:
After a live migration RARP are automatically sent to any interfaces. A
receive callback is added to vhost-user to catch this packet. If guest
supports VIRTIO_NET_F_GUEST_ANNOUNCE this packet is discarded to avoid
message duplication (already done by virtio-net NIC). For legacy guest
without VIRTIO_NET_F_GUEST_ANNOUNCE a warning message is displayed to
alert the user. RARP must be sent to the vhost client/backend.
Signed-off-by: Thibaut Collet
---
hw/net/vhost_net.c | 15 +++
include/net/vhost_net.h |1 +
net/vhost-user.c| 21 +++--
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 426b23e..4a42325 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -82,6 +82,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
@@ -365,6 +367,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
assert(r >= 0);
}
+void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf, size_t
size)
+{
+if ((net->dev.acked_features & (1 << VIRTIO_NET_F_GUEST_ANNOUNCE)) == 0) {
+fprintf(stderr,
+"Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE support.
RARP must be sent by vhost-user backend\n");
+fflush(stderr);
+}
+}
+
void vhost_net_cleanup(struct vhost_net *net)
{
vhost_dev_cleanup(&net->dev);
@@ -427,6 +438,10 @@ void vhost_net_stop(VirtIODevice *dev,
{
}
+void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf, size_t
size);
+{
+}
+
void vhost_net_cleanup(struct vhost_net *net)
{
}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index b1c18a3..e82a0f9 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -20,6 +20,7 @@ bool vhost_net_query(VHostNetState *net, VirtIODevice *dev);
int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int total_queues);
void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int total_queues);
+void vhost_net_inject_rarp(VHostNetState *net, const uint8_t *buf, size_t
size);
void vhost_net_cleanup(VHostNetState *net);
unsigned vhost_net_get_features(VHostNetState *net, unsigned features);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8d26728..69c2313 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -66,6 +66,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)
+{
+if (size == 60) {
+/* Assume it is a RARP request sent automatically after a
+ * live migration */
+/* The RARP must be sent if guest does not support
+ * VIRTIO_NET_F_GUEST_ANNOUNCE */
+VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+
+vhost_net_inject_rarp(s->vhost_net, buf, size);
+} else {
+fprintf(stderr,"Vhost user receives unexpected packets\n");
+fflush(stderr);
+}
+return size;
+}
+
static void vhost_user_cleanup(NetClientState *nc)
{
VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -91,6 +109,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,
@@ -147,8 +166,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
[Qemu-devel] [PATCH v3 0/2] Add live migration for vhost user
This patch is an update of [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user v2->v3 Define a receive function to vhost-user to manage RARP packet send after a live migration. If the guest supports VIRTIO_NET_F_GUEST_ANNOUNCE the packet is discarded (GARP request is managed by the virtio-net NIC). Otherwise the RARP packet is transmitted to the vhost client/backend that is responsible to send the RARP Thibaut Collet (2): vhost user: add support of live migration vhost user: Add RARP injection for legacy guest docs/specs/vhost-user.txt | 16 hw/net/vhost_net.c | 23 +++ hw/virtio/vhost-user.c | 11 ++- include/net/vhost_net.h |1 + linux-headers/linux/vhost.h |9 + net/vhost-user.c| 21 +++-- 6 files changed, 78 insertions(+), 3 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
> 3. The easiest solution - nop .receive() The solution 3 is similar of my implementation and does not solve issue pointed by Jason: legacy guest do not send a gratuitous ARP. > 2. Fail negotiation when VIRTIO_NET_F_GUEST_ANNOUNCE is not accepted > file:///home/stefanha/virtio/trunk/virtio-v1.0-cs02.html#x1-450001 Could you send me the virtio-v1.0-cs02.html#x1-450001 if you think it can help me ? Nevertheless it seems that only solution 1 can provide a solution to the issue pointed by Jason. On Mon, Jun 8, 2015 at 5:13 PM, Stefan Hajnoczi wrote: > On Mon, Jun 8, 2015 at 3:05 PM, Thibaut Collet > wrote: > >> I think Jason is pointing out that your patch lacks support for guests > >> that do not negotiate VIRTIO_NET_F_GUEST_ANNOUNCE. > > > > I have understood the issue with old guest pointed by Jason. > > I have thinking about the best way to do solve it and any advices are > > welcome. > > 1. Add vhost-user virtio-net command to inject packets > > Add a new VhostUserRequest VHOST_USER_NET_INJECT_PACKET allowing QEMU > to hand a packet to the vhost-user process for injection. This > command is virtio-net specific and fails if called on a different > device type. > > 2. Fail negotiation when VIRTIO_NET_F_GUEST_ANNOUNCE is not accepted > > This is only really possible in VIRTIO 1.0 and later. Pre-1.0 does > not allow the device to reject due to disabled features. > > file:///home/stefanha/virtio/trunk/virtio-v1.0-cs02.html#x1-450001 > > Therefore this is not a great solution. > > 3. The easiest solution - nop .receive() > > Just implement a nop .receive() which drops the packet and prints a > warning message to stderr. This way VIRTIO_NET_F_GUEST_ANNOUNCE > guests work while legacy guests don't send a gratuitous ARP packet. > > Stefan >
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
> I've asked several times whether vhost-user support live migration? vhost-user can support live migration but it needs such fixes of Qemu. I have found the issue and develop my patch by trying live migration with vhost user. > I think Jason is pointing out that your patch lacks support for guests > that do not negotiate VIRTIO_NET_F_GUEST_ANNOUNCE. I have understood the issue with old guest pointed by Jason. I have thinking about the best way to do solve it and any advices are welcome. > Please send each patch revision as a new email thread. This makes it > easy to see your new patches in threaded email clients. I have used the same email thread as it is a reworked of my first patch. I will create a new email thread for the next version. On Mon, Jun 8, 2015 at 3:32 PM, Stefan Hajnoczi wrote: > On Mon, Jun 08, 2015 at 10:21:38AM +0200, Thibaut Collet wrote: > > Hi, > > > > My understanding of gratuitous packet with virtio for any backend (vhost > > user or other): > > - When the VM is loaded (first start or migration) the virtio net > > interfaces are loaded ( virtio_net_load_device function in > > hw/net/virtio-net.c) > > - If the guest has the VIRTIO_NET_F_GUEST_ANNOUNCE capability, request to > > send gratuitous packet is done. > > > > 1. To enable gratuitous packet through this mechanism I have added > > VIRTIO_NET_F_GUEST_ANNOUNCE > > capability to hw/net/vhost_net.c. So host and guest can negotiate this > > feature when vhost-user is used. > > > > 2. self announce occurs in case of live migration. During a live > migration > > a GARP is sent to all net backend through a queue dedicated to the net > > backend. > >But for vhost-user: > >- this operation is not possible (vhost-user has no queue) > >- it is already done with the previous mechanism. > >Rather to define a queue to vhost user and notify twice the guest to > > send gratuitous packet I have disable GARP from self announce and use > only > > the first mechanism for that. > > > > I have tested my modifications with guest that supports > > VIRTIO_NET_F_GUEST_ANNOUNCE and vhost-user on the host. After a live > > migration I have the GARP from the guest. > > I think Jason is pointing out that your patch lacks support for guests > that do not negotiate VIRTIO_NET_F_GUEST_ANNOUNCE. > > If the guest does not set the feature bit then packets might continue to > get forwarded to the old host. > > Perhaps the correct place to implement this is in the virtio-net.c > device instead of in vhost-user.c. The non-vhost-user case should also > skip sending two ARP packets. > > BUT before we go any further: > > I've asked several times whether vhost-user support live migration? > > You didn't answer that question. Fixing this issue only makes sense if > vhost-user live migration is supported. > > Stefan >
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
> How about implementing a receive function that discards all packets > then? Implementing a receive function that discards all packets is an other solution. I have not chosen this one to avoid to mask other potential issues: Normally vhost-user never receives packets. By keeping a NULL function as received callback we can detect easily case where packets are sent (qemu crashes) and solve this issue. > Can't vhost-user backend sends this automatically? > Why do we need to do anything in QEMU? My explanations are maybe unclear. For old driver we have to send the RARP to the guest through the network interface (and to implement a receive function for vhost-user). My question is: where is the best location to do that: 1. In the receive function of vhost-user (in the file net/vhost-user.c) 2. In a self announce function (called when vhost-user receives a RARP, analysis of the packet content is needed in this case) in the file hw/net/vhost-net.c 3. In the vhost-user backend (file hw/virtio/vhost-user.c). In this case a new message must be defined between hw/net/vhost-net.c and hw/virtio/vhost-user.c. 4. In the vhost client/backend (vapp or other) On Mon, Jun 8, 2015 at 2:45 PM, Michael S. Tsirkin wrote: > On Mon, Jun 08, 2015 at 01:29:39PM +0200, Thibaut Collet wrote: > > Hi, > > > > > I don't think qemu_send_packet_raw can ever work for vhost user. > > > What happens if you merely add VIRTIO_NET_F_GUEST_ANNOUNCE > > > to the feature list, and drop the rest of the patch? > > > > If you merely add VIRTIO_NET_F_GUEST_ANNOUNCE you have the GARP with > recent > > guest after a live migration. > > The rest of the patch (can be set in a separate commit) avoid a qemu > crashes > > with live migration when vhost-user is present: > > - When a live migration migration is complete a RARP is automatically > send to > > any net backend (self announce mechanism) > > - vhost user does not provide any receive function and RARP request is > stored > > in the vhost-user queue > > - When a migration back is done all the net backend queues are purged. > The > > stored RARP request for vhost-user is then sent to the register receive > > callback of vhost-user that is NULL. > > > > Support of live migration for vhost user needs the whole patch. (and > maore if > > we want support legacy guest with no support > of VIRTIO_NET_F_GUEST_ANNOUNCE) > > How about implementing a receive function that discards all packets > then? > > > > > > I don't get it. Why do you need any extra messages for old drivers? > To > > > detect old drivers, simply have backend check whether > > > VIRTIO_NET_F_GUEST_ANNOUNCE is set. > > > > For old driver we can not use the mechanism implemented in virtio-net > that > > manages VIRTIO_NET_F_GUEST_ANNOUNCE. In this case we must send the RARP > on the > > network interface created by vhost-user. > > My first idea to do that was to add a message between QEMU and vhost > client/ > > backend (vapp or other) to let the vhost client/backend send the RARP. > > But maybe it will be easier to let QEMU send directly the RARP message > on the > > network interface created by vhost user. > > This point can be done later if it is needed. > > > > Regards. > > Can't vhost-user backend sends this automatically? > Why do we need to do anything in QEMU? > > > > > On Mon, Jun 8, 2015 at 12:12 PM, Michael S. Tsirkin > wrote: > > > > On Fri, Jun 05, 2015 at 03:24:12PM +0200, Thibaut Collet wrote: > > > Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev > > backend is > > > vhost-user. > > > > > > For netdev backend using virtio-net NIC the self announce is > managed > > directly > > > by the virtio-net NIC and not by the netdev backend itself. > > > To avoid duplication of announces (once from the guest and once > from > > QEMU) a > > > bitfield is added in the NetClientState structure. > > > If this bit is set self announce does not send message to the > guest to > > request > > > gratuitous ARP but let virtio-net NIC set the > VIRTIO_NET_S_ANNOUNCE for > > > gratuitous ARP. > > > > > > Signed-off-by: Thibaut Collet > > > --- > > > v2: do not discard anymore packets send to vhost-user: it is GARP > request > > after > > > live migration. > > > As suggested by S. Hajnoczi qemu_announce_self skips > virtio-net NIC > > that > > >
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
Hi,
> I don't think qemu_send_packet_raw can ever work for vhost user.
> What happens if you merely add VIRTIO_NET_F_GUEST_ANNOUNCE
> to the feature list, and drop the rest of the patch?
If you merely add VIRTIO_NET_F_GUEST_ANNOUNCE you have the GARP with recent
guest after a live migration.
The rest of the patch (can be set in a separate commit) avoid a qemu
crashes with live migration when vhost-user is present:
- When a live migration migration is complete a RARP is automatically send
to any net backend (self announce mechanism)
- vhost user does not provide any receive function and RARP request is
stored in the vhost-user queue
- When a migration back is done all the net backend queues are purged. The
stored RARP request for vhost-user is then sent to the register receive
callback of vhost-user that is NULL.
Support of live migration for vhost user needs the whole patch. (and maore
if we want support legacy guest with no support of
VIRTIO_NET_F_GUEST_ANNOUNCE)
> I don't get it. Why do you need any extra messages for old drivers? To
> detect old drivers, simply have backend check whether
> VIRTIO_NET_F_GUEST_ANNOUNCE is set.
For old driver we can not use the mechanism implemented in virtio-net that
manages VIRTIO_NET_F_GUEST_ANNOUNCE. In this case we must send the RARP on
the network interface created by vhost-user.
My first idea to do that was to add a message between QEMU and vhost
client/backend (vapp or other) to let the vhost client/backend send the
RARP.
But maybe it will be easier to let QEMU send directly the RARP message on
the network interface created by vhost user.
This point can be done later if it is needed.
Regards.
On Mon, Jun 8, 2015 at 12:12 PM, Michael S. Tsirkin wrote:
> On Fri, Jun 05, 2015 at 03:24:12PM +0200, Thibaut Collet wrote:
> > Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
> backend is
> > vhost-user.
> >
> > For netdev backend using virtio-net NIC the self announce is managed
> directly
> > by the virtio-net NIC and not by the netdev backend itself.
> > To avoid duplication of announces (once from the guest and once from
> QEMU) a
> > bitfield is added in the NetClientState structure.
> > If this bit is set self announce does not send message to the guest to
> request
> > gratuitous ARP but let virtio-net NIC set the VIRTIO_NET_S_ANNOUNCE for
> > gratuitous ARP.
> >
> > Signed-off-by: Thibaut Collet
> > ---
> > v2: do not discard anymore packets send to vhost-user: it is GARP
> request after
> > live migration.
> > As suggested by S. Hajnoczi qemu_announce_self skips virtio-net NIC
> that
> > already send GARP.
> >
> > hw/net/vhost_net.c |2 ++
> > include/net/net.h |1 +
> > net/vhost-user.c |2 ++
> > savevm.c | 11 ---
> > 4 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 426b23e..a745f97 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -82,6 +82,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/include/net/net.h b/include/net/net.h
> > index e66ca03..a78e9df 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -85,6 +85,7 @@ struct NetClientState {
> > char *name;
> > char info_str[256];
> > unsigned receive_disabled : 1;
> > +unsigned self_announce_disabled : 1;
> > NetClientDestructor *destructor;
> > unsigned int queue_index;
> > unsigned rxfilter_notify_enabled:1;
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 8d26728..b345446 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -147,6 +147,8 @@ static int net_vhost_user_init(NetClientState *peer,
> const char *device,
> >
> > s = DO_UPCAST(VhostUserState, nc, nc);
> >
> > +/* Self announce is managed directly by virtio-net NIC */
> > +s->nc.self_announce_disabled = 1;
> > /* We don't provide a receive callback */
> > s->nc.receive_disabled = 1;
> > s->chr = chr;
> > diff --git a/savevm.c b/savevm.c
> > index 3b0e222..7a134b1 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -80,11 +80,16 @@ static void qemu_announce_self_iter(NICState *nic,
> void *opaque)
> > {
> > uint8_t buf[60];
>
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
Hi, I agree that my patch is OK only for recent drivers. To have a simple patch with only few modifications I do not implement a solution for old driver but I can be done later. For legacy guest without VIRTIO_NET_F_GUEST_ANNOUNCE the problem is more complex. The RARP must be sent by the vhost client/backend. This component is outside QEMU (the reference implementation is snabbswitch: http://www.virtualopensystems.com/en/solutions/guides/snabbswitch-qemu/). To do that: - a receive function must be defined for the vhost user. - a message must be added between QEMU and vapp. This message will be sent only for old guest driver to avoid GARP duplication. - the added self_announce_disabled must be removed (decision to send or not the RARP is done later by the backend and not by the generic migration method) Do you agree with this solution ? Regards. On Mon, Jun 8, 2015 at 11:14 AM, Jason Wang wrote: > > > On 06/08/2015 04:21 PM, Thibaut Collet wrote: > > Hi, > > > > My understanding of gratuitous packet with virtio for any backend > > (vhost user or other): > > - When the VM is loaded (first start or migration) the virtio net > > interfaces are loaded ( virtio_net_load_device function in > > hw/net/virtio-net.c) > > - If the guest has the VIRTIO_NET_F_GUEST_ANNOUNCE capability, request > > to send gratuitous packet is done. > > > > 1. To enable gratuitous packet through this mechanism I have > > added VIRTIO_NET_F_GUEST_ANNOUNCE capability to hw/net/vhost_net.c. So > > host and guest can negotiate this feature when vhost-user is used. > > > > 2. self announce occurs in case of live migration. During a live > > migration a GARP is sent to all net backend through a queue dedicated > > to the net backend. > >But for vhost-user: > >- this operation is not possible (vhost-user has no queue) > >- it is already done with the previous mechanism. > >Rather to define a queue to vhost user and notify twice the guest > > to send gratuitous packet I have disable GARP from self announce and > > use only the first mechanism for that. > > > > I have tested my modifications with guest that supports > > VIRTIO_NET_F_GUEST_ANNOUNCE and vhost-user on the host. After a live > > migration I have the GARP from the guest. > > Yes, your patch works well for recent drivers. But the problem is legacy > guest/driver without VIRTIO_NET_F_GUEST_ANNOUNCE. In this case there > will be no GARP sent after migration. > > Thanks >
Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
Hi,
My understanding of gratuitous packet with virtio for any backend (vhost
user or other):
- When the VM is loaded (first start or migration) the virtio net
interfaces are loaded ( virtio_net_load_device function in
hw/net/virtio-net.c)
- If the guest has the VIRTIO_NET_F_GUEST_ANNOUNCE capability, request to
send gratuitous packet is done.
1. To enable gratuitous packet through this mechanism I have added
VIRTIO_NET_F_GUEST_ANNOUNCE
capability to hw/net/vhost_net.c. So host and guest can negotiate this
feature when vhost-user is used.
2. self announce occurs in case of live migration. During a live migration
a GARP is sent to all net backend through a queue dedicated to the net
backend.
But for vhost-user:
- this operation is not possible (vhost-user has no queue)
- it is already done with the previous mechanism.
Rather to define a queue to vhost user and notify twice the guest to
send gratuitous packet I have disable GARP from self announce and use only
the first mechanism for that.
I have tested my modifications with guest that supports
VIRTIO_NET_F_GUEST_ANNOUNCE and vhost-user on the host. After a live
migration I have the GARP from the guest.
Regards.
On Mon, Jun 8, 2015 at 7:55 AM, Jason Wang wrote:
>
>
> On 06/05/2015 09:24 PM, Thibaut Collet wrote:
> > Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
> backend is
> > vhost-user.
> >
> > For netdev backend using virtio-net NIC the self announce is managed
> directly
> > by the virtio-net NIC and not by the netdev backend itself.
> > To avoid duplication of announces (once from the guest and once from
> QEMU) a
> > bitfield is added in the NetClientState structure.
> > If this bit is set self announce does not send message to the guest to
> request
> > gratuitous ARP but let virtio-net NIC set the VIRTIO_NET_S_ANNOUNCE for
> > gratuitous ARP.
> >
> > Signed-off-by: Thibaut Collet
> > ---
> > v2: do not discard anymore packets send to vhost-user: it is GARP
> request after
> > live migration.
> > As suggested by S. Hajnoczi qemu_announce_self skips virtio-net NIC
> that
> > already send GARP.
> >
> > hw/net/vhost_net.c |2 ++
> > include/net/net.h |1 +
> > net/vhost-user.c |2 ++
> > savevm.c | 11 ---
> > 4 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 426b23e..a745f97 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -82,6 +82,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/include/net/net.h b/include/net/net.h
> > index e66ca03..a78e9df 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -85,6 +85,7 @@ struct NetClientState {
> > char *name;
> > char info_str[256];
> > unsigned receive_disabled : 1;
> > +unsigned self_announce_disabled : 1;
> > NetClientDestructor *destructor;
> > unsigned int queue_index;
> > unsigned rxfilter_notify_enabled:1;
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 8d26728..b345446 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -147,6 +147,8 @@ static int net_vhost_user_init(NetClientState *peer,
> const char *device,
> >
> > s = DO_UPCAST(VhostUserState, nc, nc);
> >
> > +/* Self announce is managed directly by virtio-net NIC */
> > +s->nc.self_announce_disabled = 1;
>
> Shouldn't we do this in set_features consider it needs guest support or
> you want to disable gratuitous packet for vhost-user at all?
>
> > /* We don't provide a receive callback */
> > s->nc.receive_disabled = 1;
> > s->chr = chr;
> > diff --git a/savevm.c b/savevm.c
> > index 3b0e222..7a134b1 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -80,11 +80,16 @@ static void qemu_announce_self_iter(NICState *nic,
> void *opaque)
> > {
> > uint8_t buf[60];
> > int len;
> > +NetClientState *nc;
> >
> > -trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
> > -len = announce_self_create(buf, nic->conf->macaddr.a);
> > +nc = qemu_get_queue(nic);
> >
> > -qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
> > +if (!nc->peer->self_announce_disabled) {
> > +
> trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
> > +len = announce_self_create(buf, nic->conf->macaddr.a);
> > +
> > +qemu_send_packet_raw(nc, buf, len);
> > +}
> > }
> >
> >
>
>
[Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user
Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev backend is
vhost-user.
For netdev backend using virtio-net NIC the self announce is managed directly
by the virtio-net NIC and not by the netdev backend itself.
To avoid duplication of announces (once from the guest and once from QEMU) a
bitfield is added in the NetClientState structure.
If this bit is set self announce does not send message to the guest to request
gratuitous ARP but let virtio-net NIC set the VIRTIO_NET_S_ANNOUNCE for
gratuitous ARP.
Signed-off-by: Thibaut Collet
---
v2: do not discard anymore packets send to vhost-user: it is GARP request after
live migration.
As suggested by S. Hajnoczi qemu_announce_self skips virtio-net NIC that
already send GARP.
hw/net/vhost_net.c |2 ++
include/net/net.h |1 +
net/vhost-user.c |2 ++
savevm.c | 11 ---
4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 426b23e..a745f97 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -82,6 +82,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/include/net/net.h b/include/net/net.h
index e66ca03..a78e9df 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -85,6 +85,7 @@ struct NetClientState {
char *name;
char info_str[256];
unsigned receive_disabled : 1;
+unsigned self_announce_disabled : 1;
NetClientDestructor *destructor;
unsigned int queue_index;
unsigned rxfilter_notify_enabled:1;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8d26728..b345446 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -147,6 +147,8 @@ static int net_vhost_user_init(NetClientState *peer, const
char *device,
s = DO_UPCAST(VhostUserState, nc, nc);
+/* Self announce is managed directly by virtio-net NIC */
+s->nc.self_announce_disabled = 1;
/* We don't provide a receive callback */
s->nc.receive_disabled = 1;
s->chr = chr;
diff --git a/savevm.c b/savevm.c
index 3b0e222..7a134b1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -80,11 +80,16 @@ static void qemu_announce_self_iter(NICState *nic, void
*opaque)
{
uint8_t buf[60];
int len;
+NetClientState *nc;
-trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
-len = announce_self_create(buf, nic->conf->macaddr.a);
+nc = qemu_get_queue(nic);
-qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
+if (!nc->peer->self_announce_disabled) {
+trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
+len = announce_self_create(buf, nic->conf->macaddr.a);
+
+qemu_send_packet_raw(nc, buf, len);
+}
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH 1/1] net: fix queue's purge on VM stop
Hi, thanks for your point, I did not notice the problem of the lost of ARP announce by discarding the message. So I must rewrite my patch to define a queue to transmit the ARP announce to the guest. Is it the proper solution? Thanks. Best regards. Thibaut. On Tue, Jun 2, 2015 at 12:34 PM, Stefan Hajnoczi wrote: > On Fri, May 29, 2015 at 04:28:48PM +0200, Thibaut Collet wrote: > > I agree that virtio-net NIC never enqueues packet to vhost-user > > but qemu_announce_self function (savevm.c file) can do it through > > the qemu_announce_self_iter / qemu_send_packet_raw sequence. > > Does vhost-user support live migration? > > If no, then qemu_announce_self is never called. > > If yes, then the packet should not be discarded since the ARP announce > has a purpose. > > Stefan >
Re: [Qemu-devel] [PATCH 1/1] net: fix queue's purge on VM stop
Hi, >- vhost-user set received_disabled to true to prevent this from >happening. but looks like qemu_flush_or_purge_queue_packets() change it >to false unconditionally. And I don't quite understand this, probably we >can just remove this and the issue is fixed? I am afraid that solution can cause issues with other netdev backends. If for any reason a netdev backend is no more able to treat packets it is unvalidated by setting the received_disabled to true. This boolean is reset to false only by the qemu_flush_or_purge_queue_packets function. This way allows to restart communication with a netdev backend that will be temporary done. I do not know if this case can occur but I do not want to break this mechanism. >- If not, maybe you just need another flag like receive_disabled. This >seems can save a lot of codes. Idea of my fix is to avoid enqueuing packet. This solution has the advantage to provide dynamic configuration of queue size for future use but modified several files. Your suggestion is more vhost user centric, and has the disadvantage to enqueue packet that will be never send, nor free during the flush operation. Memory will be normally freed by the system when qemu stops but it can increase the risk of memory leak. Regards. Thibaut. On Mon, Jun 1, 2015 at 11:17 AM, Jason Wang wrote: > > > On 05/28/2015 04:03 PM, Thibaut Collet wrote: > > For netdev backend with no receive callback, packets must not be > enqueued. When > > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That > causes a > > qemu segfault due to a call of a NULL pointer function. > > > > Function to create net client is modified to allow backend to specify > the size > > of the queue. > > For netdev backend with no receive callback, the queue size is set to 0 > to > > prevent enqueuing of packets. > > For other netdev backend, a default queue size is provided. This value > (set to > > 1) is the value previously set to any queue. > > > > Signed-off-by: Thibaut Collet > > --- > > include/net/net.h |3 ++- > > include/net/queue.h |5 - > > net/dump.c |3 ++- > > net/hub.c |3 ++- > > net/l2tpv3.c|3 ++- > > net/net.c | 10 ++ > > net/netmap.c|3 ++- > > net/queue.c |4 ++-- > > net/slirp.c |3 ++- > > net/socket.c|9 ++--- > > net/tap-win32.c |3 ++- > > net/tap.c |3 ++- > > net/vde.c |3 ++- > > net/vhost-user.c|4 +++- > > 14 files changed, 39 insertions(+), 20 deletions(-) > > > > diff --git a/include/net/net.h b/include/net/net.h > > index e66ca03..cb3e2df 100644 > > --- a/include/net/net.h > > +++ b/include/net/net.h > > @@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char *id, > NetClientState **ncs, > > NetClientState *qemu_new_net_client(NetClientInfo *info, > > NetClientState *peer, > > const char *model, > > -const char *name); > > +const char *name, > > +uint32_t queue_size); > > NICState *qemu_new_nic(NetClientInfo *info, > > NICConf *conf, > > const char *model, > > diff --git a/include/net/queue.h b/include/net/queue.h > > index fc02b33..4659c5a 100644 > > --- a/include/net/queue.h > > +++ b/include/net/queue.h > > @@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState *sender, > ssize_t ret); > > #define QEMU_NET_PACKET_FLAG_NONE 0 > > #define QEMU_NET_PACKET_FLAG_RAW (1<<0) > > > > -NetQueue *qemu_new_net_queue(void *opaque); > > +#define QEMU_DEFAULT_QUEUE_SIZE 1 > > +#define QEMU_EMPTY_QUEUE 0 > > + > > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size); > > > > void qemu_del_net_queue(NetQueue *queue); > > > > diff --git a/net/dump.c b/net/dump.c > > index 9d3a09e..299b09e 100644 > > --- a/net/dump.c > > +++ b/net/dump.c > > @@ -129,7 +129,8 @@ static int net_dump_init(NetClientState *peer, const > char *device, > > return -1; > > } > > > > -nc = qemu_new_net_client(&net_dump_info, peer, device, name); > > +nc = qemu_new_net_client(&net_dump_info, peer, device, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), > >
Re: [Qemu-devel] [PATCH 1/1] net: fix queue's purge on VM stop
Hi, I agree that virtio-net NIC never enqueues packet to vhost-user but qemu_announce_self function (savevm.c file) can do it through the qemu_announce_self_iter / qemu_send_packet_raw sequence. Regards Thibaut. On Fri, May 29, 2015 at 3:12 PM, Stefan Hajnoczi wrote: > On Thu, May 28, 2015 at 10:03:14AM +0200, Thibaut Collet wrote: > > For netdev backend with no receive callback, packets must not be > enqueued. When > > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That > causes a > > qemu segfault due to a call of a NULL pointer function. > > virtio-net NIC <-> vhost-user should bypass the QEMU net subsystem since > rx/tx virtqueues are handled outside the QEMU process. Who is enqueuing > packets on vhost-user's incoming_queue? >
[Qemu-devel] [PATCH 1/1] net: fix queue's purge on VM stop
For netdev backend with no receive callback, packets must not be enqueued. When
VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That causes a
qemu segfault due to a call of a NULL pointer function.
Function to create net client is modified to allow backend to specify the size
of the queue.
For netdev backend with no receive callback, the queue size is set to 0 to
prevent enqueuing of packets.
For other netdev backend, a default queue size is provided. This value (set to
1) is the value previously set to any queue.
Signed-off-by: Thibaut Collet
---
include/net/net.h |3 ++-
include/net/queue.h |5 -
net/dump.c |3 ++-
net/hub.c |3 ++-
net/l2tpv3.c|3 ++-
net/net.c | 10 ++
net/netmap.c|3 ++-
net/queue.c |4 ++--
net/slirp.c |3 ++-
net/socket.c|9 ++---
net/tap-win32.c |3 ++-
net/tap.c |3 ++-
net/vde.c |3 ++-
net/vhost-user.c|4 +++-
14 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index e66ca03..cb3e2df 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char *id,
NetClientState **ncs,
NetClientState *qemu_new_net_client(NetClientInfo *info,
NetClientState *peer,
const char *model,
-const char *name);
+const char *name,
+uint32_t queue_size);
NICState *qemu_new_nic(NetClientInfo *info,
NICConf *conf,
const char *model,
diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..4659c5a 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState *sender,
ssize_t ret);
#define QEMU_NET_PACKET_FLAG_NONE 0
#define QEMU_NET_PACKET_FLAG_RAW (1<<0)
-NetQueue *qemu_new_net_queue(void *opaque);
+#define QEMU_DEFAULT_QUEUE_SIZE 1
+#define QEMU_EMPTY_QUEUE 0
+
+NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size);
void qemu_del_net_queue(NetQueue *queue);
diff --git a/net/dump.c b/net/dump.c
index 9d3a09e..299b09e 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -129,7 +129,8 @@ static int net_dump_init(NetClientState *peer, const char
*device,
return -1;
}
-nc = qemu_new_net_client(&net_dump_info, peer, device, name);
+nc = qemu_new_net_client(&net_dump_info, peer, device, name,
+ QEMU_DEFAULT_QUEUE_SIZE);
snprintf(nc->info_str, sizeof(nc->info_str),
"dump to %s (len=%d)", filename, len);
diff --git a/net/hub.c b/net/hub.c
index 2b60ab9..a42cac3 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -151,7 +151,8 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const char
*name)
name = default_name;
}
-nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name);
+nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name,
+ QEMU_DEFAULT_QUEUE_SIZE);
port = DO_UPCAST(NetHubPort, nc, nc);
port->id = id;
port->hub = hub;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8c598b0..1d2e4e5 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -548,7 +548,8 @@ int net_init_l2tpv3(const NetClientOptions *opts,
struct addrinfo *result = NULL;
char *srcport, *dstport;
-nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name);
+nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name,
+ QEMU_DEFAULT_QUEUE_SIZE);
s = DO_UPCAST(NetL2TPV3State, nc, nc);
diff --git a/net/net.c b/net/net.c
index 7427f6a..bcf69da 100644
--- a/net/net.c
+++ b/net/net.c
@@ -214,6 +214,7 @@ static void qemu_net_client_setup(NetClientState *nc,
NetClientState *peer,
const char *model,
const char *name,
+ uint32_t queue_size,
NetClientDestructor *destructor)
{
nc->info = info;
@@ -231,21 +232,22 @@ static void qemu_net_client_setup(NetClientState *nc,
}
QTAILQ_INSERT_TAIL(&net_clients, nc, next);
-nc->incoming_queue = qemu_new_net_queue(nc);
+nc->incoming_queue = qemu_new_net_queue(nc, queue_size);
nc->destructor = destructor;
}
NetClientState *qemu_new_net_client(NetClientInfo *info,
NetClientState *peer,
const char *model,
-const char *name)
+const char *na
