[Qemu-devel] VhostUserRequest # 20

[Qemu-devel] VhostUserRequest # 20

[Dr. David Alan Gilbert]

Hi,
  libvhost-user.h defines:
   VHOST_USER_INPUT_GET_CONFIG = 20,
while
  vhost-user.c defines:
   VHOST_USER_NET_SET_MTU = 20,

who wins?

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] VhostUserRequest # 20

[Marc-André Lureau]

HI

----- Original Message -----
> Hi,
>   libvhost-user.h defines:
>    VHOST_USER_INPUT_GET_CONFIG = 20,

That slipped by mistake from my vhost-user-input series WIP, please send a fix.
(luckily, this is only internal header for now)

thanks

> while
>   vhost-user.c defines:
>    VHOST_USER_NET_SET_MTU = 20,
> 
> who wins?
> 
> Dave
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] VhostUserRequest # 20

[Dr. David Alan Gilbert]

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> HI
> 
> ----- Original Message -----
> > Hi,
> >   libvhost-user.h defines:
> >    VHOST_USER_INPUT_GET_CONFIG = 20,
> 
> That slipped by mistake from my vhost-user-input series WIP, please send a fix.
> (luckily, this is only internal header for now)

OK, just posted.

However, I just spotted, or should I say gcc just spotted another issue:

  CC      tests/vhost-user-bridge.o
/home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: variables 'front' and 'iov' used in loop condition not modified in loop body [-Wfor-loop-analysis]
    for (cur = front; front != iov; cur++) {
                      ^~~~~    ~~~
1 warning generated.


static void
iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes)
{
    struct iovec *cur;

    for (cur = front; front != iov; cur++) {
        bytes -= cur->iov_len;
    }

    cur->iov_base -= bytes;
    cur->iov_len += bytes;
}

What's that actually intending to do?

Dave

> thanks
> 
> > while
> >   vhost-user.c defines:
> >    VHOST_USER_NET_SET_MTU = 20,
> > 
> > who wins?
> > 
> > Dave
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] VhostUserRequest # 20

[Marc-André Lureau]

Hi

----- Original Message -----
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > HI
> > 
> > ----- Original Message -----
> > > Hi,
> > >   libvhost-user.h defines:
> > >    VHOST_USER_INPUT_GET_CONFIG = 20,
> > 
> > That slipped by mistake from my vhost-user-input series WIP, please send a
> > fix.
> > (luckily, this is only internal header for now)
> 
> OK, just posted.
> 
> However, I just spotted, or should I say gcc just spotted another issue:
> 
>   CC      tests/vhost-user-bridge.o
> /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning:
> variables 'front' and 'iov' used in loop condition not modified in loop body
> [-Wfor-loop-analysis]
>     for (cur = front; front != iov; cur++) {
>                       ^~~~~    ~~~
> 1 warning generated.
> 
> 
> static void
> iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes)
> {
>     struct iovec *cur;
> 
>     for (cur = front; front != iov; cur++) {
>         bytes -= cur->iov_len;
>     }
> 
>     cur->iov_base -= bytes;
>     cur->iov_len += bytes;
> }
> 
> What's that actually intending to do?

It was meant to revert the effect of iov_discard_front()

The code should read:

for (cur = front; cur != iov; cur++) {

In practice, it doesn't reach the loop since the front sg buffer is big enough to discard the header..

Does that looks correct to you?


thanks

> 
> Dave
> 
> > thanks
> > 
> > > while
> > >   vhost-user.c defines:
> > >    VHOST_USER_NET_SET_MTU = 20,
> > > 
> > > who wins?
> > > 
> > > Dave
> > > --
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] VhostUserRequest # 20

[Dr. David Alan Gilbert]

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Hi
> 
> ----- Original Message -----
> > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > HI
> > > 
> > > ----- Original Message -----
> > > > Hi,
> > > >   libvhost-user.h defines:
> > > >    VHOST_USER_INPUT_GET_CONFIG = 20,
> > > 
> > > That slipped by mistake from my vhost-user-input series WIP, please send a
> > > fix.
> > > (luckily, this is only internal header for now)
> > 
> > OK, just posted.
> > 
> > However, I just spotted, or should I say gcc just spotted another issue:
> > 
> >   CC      tests/vhost-user-bridge.o
> > /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning:
> > variables 'front' and 'iov' used in loop condition not modified in loop body
> > [-Wfor-loop-analysis]
> >     for (cur = front; front != iov; cur++) {
> >                       ^~~~~    ~~~
> > 1 warning generated.
> > 
> > 
> > static void
> > iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes)
> > {
> >     struct iovec *cur;
> > 
> >     for (cur = front; front != iov; cur++) {
> >         bytes -= cur->iov_len;
> >     }
> > 
> >     cur->iov_base -= bytes;
> >     cur->iov_len += bytes;
> > }
> > 
> > What's that actually intending to do?
> 
> It was meant to revert the effect of iov_discard_front()
> 
> The code should read:
> 
> for (cur = front; cur != iov; cur++) {
> 
> In practice, it doesn't reach the loop since the front sg buffer is big enough to discard the header..
> 
> Does that looks correct to you?

OK, so let me check if I understand;  it's supposed to be called with
'front' being the original struct iovec *, and 'bytes' being the number
of bytes you shaved off using iov_discard_front?
If so then yes I think that makes sense; a comment to explain that
would probably help.
A sanity check for bytes being >=0 after the loop might be worth it as
well.

Dave

> 
> 
> thanks
> 
> > 
> > Dave
> > 
> > > thanks
> > > 
> > > > while
> > > >   vhost-user.c defines:
> > > >    VHOST_USER_NET_SET_MTU = 20,
> > > > 
> > > > who wins?
> > > > 
> > > > Dave
> > > > --
> > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK