lkml.org 
[lkml]   [2009]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
    Date
    On Tuesday 25 August 2009, Michael S. Tsirkin wrote:
    > > I'd like to avoid that here,
    > > though it's kind of ugly. We'd need VHOST_GET_FEATURES (and ACK) to take a
    > > struct like:
    > >
    > > u32 feature_size;
    > > u32 features[];

    Hmm, variable length ioctl arguments, I'd rather not go there.
    The ioctl infrastructure already has a length argument encoded
    in the ioctl number. We can use that if we need more, e.g.

    /* now */
    #define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
    /*
    * uncomment if we run out of feature bits:

    struct vhost_get_features2 {
    __u64 bits[2];
    };
    #define VHOST_GET_FEATURES2 _IOR(VHOST_VIRTIO, 0x00, \
    struct vhost_get_features2)
    */

    > Thinking about this proposal some more, how will the guest
    > determine the size to supply the GET_FEATURES ioctl?

    Wait, the *guest*?

    Maybe I misunderstood something in a major way here, but
    I expected the features to be negotiated between host
    user space (qemu) and host kernel, as well as between
    guest and qemu (as they are already), but never between
    guest and kernel.

    I would certainly expect the bits to be distinct from
    the virtio-net feature bits. E.g. stuff like TAP frame
    format opposed to TCP socket frame format (length+date)
    is something we need to negotiate here but that the
    guest does not care about.

    > Since we are a bit tight in 32 bit space already,
    > let's just use a 64 bit integer and be done with it?

    Can't hurt, but don't use a struct unless you think
    we are going to need more than 64 bits.

    Arnd <><


    \
     
     \ /
      Last update: 2009-08-26 15:45    [W:2.361 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site