[Qemu-devel] Re: [PATCH 02/17] vrtio-9p: Implement P9_TVERSION for 9P

[Aneesh Kumar K. V]

On Thu, 4 Mar 2010 11:23:24 +0200, "Michael S. Tsirkin" <address@hidden> wrote:
> On Wed, Mar 03, 2010 at 01:00:59PM -0600, Anthony Liguori wrote:
> > address@hidden: malloc to qemu_malloc coversion]
> > 
> > Signed-off-by: Anthony Liguori <address@hidden>
> > Signed-off-by: Aneesh Kumar K.V <address@hidden>
> > ---
> >  hw/virtio-9p.c |  263 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 262 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> > index 93402c5..a057fbb 100644
> > --- a/hw/virtio-9p.c
> > +++ b/hw/virtio-9p.c
> > @@ -111,10 +111,271 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
> >      }
> >  }
> >  
> > -static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
> > +static void v9fs_string_free(V9fsString *str)
> > +{
> > +    free(str->data);
> > +    str->data = NULL;
> > +    str->size = 0;
> > +}
> > +
> > +static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t 
> > size)
> > +{
> > +    struct iovec *sg = pdu->elem.out_sg;
> > +    BUG_ON((offset + size) > sg[0].iov_len);
> > +    memcpy(dst, sg[0].iov_base + offset, size);
> > +    return size;
> > +}
> > +
> > +/* FIXME i can do this with less variables */
> > +static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, 
> > size_t size)
> 
> Is the point of this functuion to copy size bytes starting
> at offset? Maybe generalize this to work on any iovec?

The 9p debug code also need a similar function. I have a bug fix patch
that make sure we use all the elem.out_num elements in elem.out_sg
array. That patch already does abstract this out for any iovec. But
still keep the function in virtio-9p.c.


> 
> > +{
> > +    struct iovec *sg = pdu->elem.in_sg;
> > +    size_t off = 0;
> > +    size_t copied = 0;
> > +    int i = 0;
> > +
> > +    for (i = 0; size && i < pdu->elem.in_num; i++) {
> > +   size_t len;
> 
> indentation by tabs.

Fixed

> 
> > +
> > +   if (offset >= off && offset < (off + sg[i].iov_len)) {
> 
> The above math might overflow. Not sure what the result will be.
> 
> > +       len = MIN(sg[i].iov_len - (offset - off), size);
> > +       memcpy(sg[i].iov_base + (offset - off), src, len);
> > +       size -= len;
> > +       offset += len;
> > +       off = offset;
> > +       copied += len;
> > +       src += len;
> > +   } else
> > +       off += sg[i].iov_len;
> 
> {}

Fixed 

> 
> > +    }
> > +
> > +    return copied;
> > +}
> > +
> > +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec 
> > *sg)
> > +{
> 
> Maybe generalize this to work on any iovec?
> 
> > +    size_t pos = 0;
> > +    int i, j;
> > +    struct iovec *src_sg;
> > +    unsigned int num;
> > +
> > +    if (rx) {
> > +       src_sg = pdu->elem.in_sg;
> > +       num = pdu->elem.in_num;
> > +    } else {
> > +       src_sg = pdu->elem.out_sg;
> > +       num = pdu->elem.out_num;
> > +    }
> > +
> > +    j = 0;