Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Jun 28, 2017 at 08:00:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > When we receive a GET_VRING_BASE message set enable = false
> > to stop any new received packets modifying the ring.
> >
> > Signed-off-by: Dr. David Alan Gilbert
>
> I think I already reviewed a similar patch.
> Spec says:
> Client must only process each ring when it is started.
>
> IMHO the real fix is to fix client to check the started
> flag before processing the ring.
Done, I added a vu_queue_started to match vu_queue_enabled
and then used it.
Dave
> > ---
> > contrib/libvhost-user/libvhost-user.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c
> > b/contrib/libvhost-user/libvhost-user.c
> > index ceddeac74f..d37052b7b0 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> > vmsg->size = sizeof(vmsg->payload.state);
> >
> > dev->vq[index].started = false;
> > +dev->vq[index].enable = false;
> > if (dev->iface->queue_set_started) {
> > dev->iface->queue_set_started(dev, index, false);
> > }
> > --
> > 2.13.0
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
On Wed, Jul 05, 2017 at 06:16:17PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > On Wed, Jun 28, 2017 at 08:00:43PM +0100, Dr. David Alan Gilbert (git)
> > wrote:
> > > From: "Dr. David Alan Gilbert"
> > >
> > > When we receive a GET_VRING_BASE message set enable = false
> > > to stop any new received packets modifying the ring.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert
> >
> > I think I already reviewed a similar patch.
>
> Yes you replied to my off-list mail; I hadn't got
> around to fixing it yet.
>
> > Spec says:
> > Client must only process each ring when it is started.
>
> but in that reply you said the spec said
>
> Client must only pass data between the ring and the
> backend, when the ring is enabled.
>
> So does the spec say 'started' or 'enabled'
Both. Ring processing is limited by ring being started.
Passing actual data - by ring also being enabled.
With ring started but disabled you drop all packets.
> (Pointer to the spec?)
It's part of QEMU source:
docs/interop/vhost-user.txt
> > IMHO the real fix is to fix client to check the started
> > flag before processing the ring.
>
> Yep I can do that. I was curious however whether it was
> specified as 'started' or 'enabled' or both.
>
> Dave
>
> > > ---
> > > contrib/libvhost-user/libvhost-user.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/contrib/libvhost-user/libvhost-user.c
> > > b/contrib/libvhost-user/libvhost-user.c
> > > index ceddeac74f..d37052b7b0 100644
> > > --- a/contrib/libvhost-user/libvhost-user.c
> > > +++ b/contrib/libvhost-user/libvhost-user.c
> > > @@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> > > vmsg->size = sizeof(vmsg->payload.state);
> > >
> > > dev->vq[index].started = false;
> > > +dev->vq[index].enable = false;
> > > if (dev->iface->queue_set_started) {
> > > dev->iface->queue_set_started(dev, index, false);
> > > }
> > > --
> > > 2.13.0
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Jun 28, 2017 at 08:00:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > When we receive a GET_VRING_BASE message set enable = false
> > to stop any new received packets modifying the ring.
> >
> > Signed-off-by: Dr. David Alan Gilbert
>
> I think I already reviewed a similar patch.
Yes you replied to my off-list mail; I hadn't got
around to fixing it yet.
> Spec says:
> Client must only process each ring when it is started.
but in that reply you said the spec said
Client must only pass data between the ring and the
backend, when the ring is enabled.
So does the spec say 'started' or 'enabled'
(Pointer to the spec?)
> IMHO the real fix is to fix client to check the started
> flag before processing the ring.
Yep I can do that. I was curious however whether it was
specified as 'started' or 'enabled' or both.
Dave
> > ---
> > contrib/libvhost-user/libvhost-user.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c
> > b/contrib/libvhost-user/libvhost-user.c
> > index ceddeac74f..d37052b7b0 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> > vmsg->size = sizeof(vmsg->payload.state);
> >
> > dev->vq[index].started = false;
> > +dev->vq[index].enable = false;
> > if (dev->iface->queue_set_started) {
> > dev->iface->queue_set_started(dev, index, false);
> > }
> > --
> > 2.13.0
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
On Wed, Jun 28, 2017 at 08:00:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> When we receive a GET_VRING_BASE message set enable = false
> to stop any new received packets modifying the ring.
>
> Signed-off-by: Dr. David Alan Gilbert
I think I already reviewed a similar patch.
Spec says:
Client must only process each ring when it is started.
IMHO the real fix is to fix client to check the started
flag before processing the ring.
> ---
> contrib/libvhost-user/libvhost-user.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index ceddeac74f..d37052b7b0 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> vmsg->size = sizeof(vmsg->payload.state);
>
> dev->vq[index].started = false;
> +dev->vq[index].enable = false;
> if (dev->iface->queue_set_started) {
> dev->iface->queue_set_started(dev, index, false);
> }
> --
> 2.13.0
Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
On 06/28/2017 09:00 PM, Dr. David Alan Gilbert (git) wrote:
From: "Dr. David Alan Gilbert"
When we receive a GET_VRING_BASE message set enable = false
to stop any new received packets modifying the ring.
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Maxime Coquelin
Maxime
---
contrib/libvhost-user/libvhost-user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index ceddeac74f..d37052b7b0 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
vmsg->size = sizeof(vmsg->payload.state);
dev->vq[index].started = false;
+dev->vq[index].enable = false;
if (dev->iface->queue_set_started) {
dev->iface->queue_set_started(dev, index, false);
}
[Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base
From: "Dr. David Alan Gilbert"
When we receive a GET_VRING_BASE message set enable = false
to stop any new received packets modifying the ring.
Signed-off-by: Dr. David Alan Gilbert
---
contrib/libvhost-user/libvhost-user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index ceddeac74f..d37052b7b0 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
vmsg->size = sizeof(vmsg->payload.state);
dev->vq[index].started = false;
+dev->vq[index].enable = false;
if (dev->iface->queue_set_started) {
dev->iface->queue_set_started(dev, index, false);
}
--
2.13.0
