Patchwork [-v2,02/22] vrtio-9p: Implement P9_TVERSION for 9P

login
register
mail settings
Submitter Aneesh Kumar K.V
Date March 16, 2010, 9:15 a.m.
Message ID <1268730920-14584-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/47815/
State New
Headers show

Comments

Aneesh Kumar K.V - March 16, 2010, 9:15 a.m.
From: Anthony Liguori <aliguori@us.ibm.com>

[kiran@linux.vnet.ibm.com: malloc to qemu_malloc coversion]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 hw/virtio-9p.c |  263 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 261 insertions(+), 2 deletions(-)
Anthony Liguori - March 26, 2010, 4:15 p.m.
On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote:
> From: Anthony Liguori<aliguori@us.ibm.com>
>
> [kiran@linux.vnet.ibm.com: malloc to qemu_malloc coversion]
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> ---
>   hw/virtio-9p.c |  263 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 261 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 115c93b..53b3d78 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -111,10 +111,269 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
>       }
>   }
>
> +static void v9fs_string_free(V9fsString *str)
> +{
> +    free(str->data);
>    

qemu_free.
> +    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)
> +{
> +    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;
> +
> +        if (offset>= off&&  offset<  (off + sg[i].iov_len)) {
> +            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;
> +        }
> +    }
> +
> +    return copied;
> +}
> +
> +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
> +{
> +    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;
> +    for (i = 0; i<  num; i++) {
> +        if (offset<= pos) {
> +            sg[j].iov_base = src_sg[i].iov_base;
> +            sg[j].iov_len = src_sg[i].iov_len;
> +            j++;
> +        } else if (offset<  (src_sg[i].iov_len + pos)) {
> +            sg[j].iov_base = src_sg[i].iov_base;
> +            sg[j].iov_len = src_sg[i].iov_len;
> +            sg[j].iov_base += (offset - pos);
> +            sg[j].iov_len -= (offset - pos);
> +            j++;
> +        }
> +        pos += src_sg[i].iov_len;
> +    }
> +
> +    return j;
> +}
> +
> +static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
> +{
> +    size_t old_offset = offset;
> +    va_list ap;
> +    int i;
> +
> +    va_start(ap, fmt);
> +    for (i = 0; fmt[i]; i++) {
> +	switch (fmt[i]) {
> +	case 'b': {
> +	    int8_t *valp = va_arg(ap, int8_t *);
> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> +	    break;
> +	}
> +	case 'w': {
> +	    int16_t *valp = va_arg(ap, int16_t *);
> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> +	    break;
> +	}
> +	case 'd': {
> +	    int32_t *valp = va_arg(ap, int32_t *);
> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> +	    break;
> +	}
> +	case 'q': {
> +	    int64_t *valp = va_arg(ap, int64_t *);
> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> +	    break;
> +	}
> +	case 'v': {
> +	    struct iovec *iov = va_arg(ap, struct iovec *);
> +	    int *iovcnt = va_arg(ap, int *);
> +	    *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
> +	    break;
> +	}
> +	case 's': {
> +	    V9fsString *str = va_arg(ap, V9fsString *);
> +	    offset += pdu_unmarshal(pdu, offset, "w",&str->size);
> +	    /* FIXME: sanity check str->size */
> +	    str->data = qemu_malloc(str->size + 1);
> +	    offset += pdu_unpack(str->data, pdu, offset, str->size);
> +	    str->data[str->size] = 0;
> +	    break;
> +	}
> +	case 'Q': {
> +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
> +	    offset += pdu_unmarshal(pdu, offset, "bdq",
> +				&qidp->type,&qidp->version,&qidp->path);
> +	    break;
> +	}
> +	case 'S': {
> +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
> +	    offset += pdu_unmarshal(pdu, offset, "wwdQdddqsssssddd",
> +				&statp->size,&statp->type,&statp->dev,
> +				&statp->qid,&statp->mode,&statp->atime,
> +				&statp->mtime,&statp->length,
> +				&statp->name,&statp->uid,&statp->gid,
> +				&statp->muid,&statp->extension,
> +				&statp->n_uid,&statp->n_gid,
> +				&statp->n_muid);
> +	    break;
> +	}
> +	default:
> +	    break;
> +	}
> +    }
> +
> +    va_end(ap);
> +
> +    return offset - old_offset;
> +}
> +
> +static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
> +{
> +    size_t old_offset = offset;
> +    va_list ap;
> +    int i;
> +
> +    va_start(ap, fmt);
> +    for (i = 0; fmt[i]; i++) {
> +	switch (fmt[i]) {
> +	case 'b': {
> +	    int8_t val = va_arg(ap, int);
> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> +	    break;
> +	}
> +	case 'w': {
> +	    int16_t val = va_arg(ap, int);
> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> +	    break;
> +	}
> +	case 'd': {
> +	    int32_t val = va_arg(ap, int);
> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> +	    break;
> +	}
> +	case 'q': {
> +	    int64_t val = va_arg(ap, int64_t);
> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> +	    break;
> +	}
> +	case 'v': {
> +	    struct iovec *iov = va_arg(ap, struct iovec *);
> +	    int *iovcnt = va_arg(ap, int *);
> +	    *iovcnt = pdu_copy_sg(pdu, offset, 1, iov);
> +	    break;
> +	}
> +	case 's': {
> +	    V9fsString *str = va_arg(ap, V9fsString *);
> +	    offset += pdu_marshal(pdu, offset, "w", str->size);
> +	    offset += pdu_pack(pdu, offset, str->data, str->size);
> +	    break;
> +	}
> +	case 'Q': {
> +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
> +	    offset += pdu_marshal(pdu, offset, "bdq",
> +				  qidp->type, qidp->version, qidp->path);
> +	    break;
> +	}
> +	case 'S': {
> +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
> +	    offset += pdu_marshal(pdu, offset, "wwdQdddqsssssddd",
> +				  statp->size, statp->type, statp->dev,
> +				&statp->qid, statp->mode, statp->atime,
> +				  statp->mtime, statp->length,&statp->name,
> +				&statp->uid,&statp->gid,&statp->muid,
> +				&statp->extension, statp->n_uid,
> +				  statp->n_gid, statp->n_muid);
> +	    break;
> +	}
> +	default:
> +	    break;
> +	}
> +    }
> +    va_end(ap);
> +
> +    return offset - old_offset;
> +}
>    

Adding the marshalling in  patch to implement P9_TVERSION is probably 
not ideal.

Regards,

Anthony Liguori

> +static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
> +{
> +    int8_t id = pdu->id + 1; /* Response */
> +
> +    if (len<  0) {
> +        V9fsString str;
> +        int err = -len;
> +
> +        str.data = strerror(err);
> +        str.size = strlen(str.data);
> +
> +        len = 7;
> +        len += pdu_marshal(pdu, len, "s",&str);
> +        if (dotu)
> +            len += pdu_marshal(pdu, len, "d", err);
> +
> +        id = P9_RERROR;
> +    }
> +
> +    /* fill out the header */
> +    pdu_marshal(pdu, 0, "dbw", (int32_t)len, id, pdu->tag);
> +
> +    /* keep these in sync */
> +    pdu->size = len;
> +    pdu->id = id;
> +
> +    /* push onto queue and notify */
> +    virtqueue_push(s->vq,&pdu->elem, len);
> +
> +    /* FIXME: we should batch these completions */
> +    virtio_notify(&s->vdev, s->vq);
> +
> +    free_pdu(s, pdu);
> +}
> +
>   static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
>   {
> -    if (debug_9p_pdu)
> -        pprint_pdu(pdu);
> +    int32_t msize;
> +    V9fsString version;
> +    size_t offset = 7;
> +
> +    pdu_unmarshal(pdu, offset, "ds",&msize,&version);
> +    BUG_ON(strcmp(version.data, "9P2000.u") != 0);
> +
> +    offset += pdu_marshal(pdu, offset, "ds", msize,&version);
> +    complete_pdu(s, pdu, offset);
> +
> +    v9fs_string_free(&version);
>   }
>
>   static void v9fs_attach(V9fsState *s, V9fsPDU *pdu)
>
Aneesh Kumar K.V - March 29, 2010, 7:01 a.m.
On Fri, 26 Mar 2010 11:15:47 -0500, Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
> On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote:
> > From: Anthony Liguori<aliguori@us.ibm.com>
> >
> > [kiran@linux.vnet.ibm.com: malloc to qemu_malloc coversion]
> >
> > Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> > Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >   hw/virtio-9p.c |  263 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 files changed, 261 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> > index 115c93b..53b3d78 100644
> > --- a/hw/virtio-9p.c
> > +++ b/hw/virtio-9p.c
> > @@ -111,10 +111,269 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
> >       }
> >   }
> >
> > +static void v9fs_string_free(V9fsString *str)
> > +{
> > +    free(str->data);
> >    
> 
> qemu_free.
> > +    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)
> > +{
> > +    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;
> > +
> > +        if (offset>= off&&  offset<  (off + sg[i].iov_len)) {
> > +            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;
> > +        }
> > +    }
> > +
> > +    return copied;
> > +}
> > +
> > +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
> > +{
> > +    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;
> > +    for (i = 0; i<  num; i++) {
> > +        if (offset<= pos) {
> > +            sg[j].iov_base = src_sg[i].iov_base;
> > +            sg[j].iov_len = src_sg[i].iov_len;
> > +            j++;
> > +        } else if (offset<  (src_sg[i].iov_len + pos)) {
> > +            sg[j].iov_base = src_sg[i].iov_base;
> > +            sg[j].iov_len = src_sg[i].iov_len;
> > +            sg[j].iov_base += (offset - pos);
> > +            sg[j].iov_len -= (offset - pos);
> > +            j++;
> > +        }
> > +        pos += src_sg[i].iov_len;
> > +    }
> > +
> > +    return j;
> > +}
> > +
> > +static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
> > +{
> > +    size_t old_offset = offset;
> > +    va_list ap;
> > +    int i;
> > +
> > +    va_start(ap, fmt);
> > +    for (i = 0; fmt[i]; i++) {
> > +	switch (fmt[i]) {
> > +	case 'b': {
> > +	    int8_t *valp = va_arg(ap, int8_t *);
> > +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +	    break;
> > +	}
> > +	case 'w': {
> > +	    int16_t *valp = va_arg(ap, int16_t *);
> > +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +	    break;
> > +	}
> > +	case 'd': {
> > +	    int32_t *valp = va_arg(ap, int32_t *);
> > +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +	    break;
> > +	}
> > +	case 'q': {
> > +	    int64_t *valp = va_arg(ap, int64_t *);
> > +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +	    break;
> > +	}
> > +	case 'v': {
> > +	    struct iovec *iov = va_arg(ap, struct iovec *);
> > +	    int *iovcnt = va_arg(ap, int *);
> > +	    *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
> > +	    break;
> > +	}
> > +	case 's': {
> > +	    V9fsString *str = va_arg(ap, V9fsString *);
> > +	    offset += pdu_unmarshal(pdu, offset, "w",&str->size);
> > +	    /* FIXME: sanity check str->size */
> > +	    str->data = qemu_malloc(str->size + 1);
> > +	    offset += pdu_unpack(str->data, pdu, offset, str->size);
> > +	    str->data[str->size] = 0;
> > +	    break;
> > +	}
> > +	case 'Q': {
> > +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
> > +	    offset += pdu_unmarshal(pdu, offset, "bdq",
> > +				&qidp->type,&qidp->version,&qidp->path);
> > +	    break;
> > +	}
> > +	case 'S': {
> > +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
> > +	    offset += pdu_unmarshal(pdu, offset, "wwdQdddqsssssddd",
> > +				&statp->size,&statp->type,&statp->dev,
> > +				&statp->qid,&statp->mode,&statp->atime,
> > +				&statp->mtime,&statp->length,
> > +				&statp->name,&statp->uid,&statp->gid,
> > +				&statp->muid,&statp->extension,
> > +				&statp->n_uid,&statp->n_gid,
> > +				&statp->n_muid);
> > +	    break;
> > +	}
> > +	default:
> > +	    break;
> > +	}
> > +    }
> > +
> > +    va_end(ap);
> > +
> > +    return offset - old_offset;
> > +}
> > +
> > +static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
> > +{
> > +    size_t old_offset = offset;
> > +    va_list ap;
> > +    int i;
> > +
> > +    va_start(ap, fmt);
> > +    for (i = 0; fmt[i]; i++) {
> > +	switch (fmt[i]) {
> > +	case 'b': {
> > +	    int8_t val = va_arg(ap, int);
> > +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> > +	    break;
> > +	}
> > +	case 'w': {
> > +	    int16_t val = va_arg(ap, int);
> > +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> > +	    break;
> > +	}
> > +	case 'd': {
> > +	    int32_t val = va_arg(ap, int);
> > +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> > +	    break;
> > +	}
> > +	case 'q': {
> > +	    int64_t val = va_arg(ap, int64_t);
> > +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
> > +	    break;
> > +	}
> > +	case 'v': {
> > +	    struct iovec *iov = va_arg(ap, struct iovec *);
> > +	    int *iovcnt = va_arg(ap, int *);
> > +	    *iovcnt = pdu_copy_sg(pdu, offset, 1, iov);
> > +	    break;
> > +	}
> > +	case 's': {
> > +	    V9fsString *str = va_arg(ap, V9fsString *);
> > +	    offset += pdu_marshal(pdu, offset, "w", str->size);
> > +	    offset += pdu_pack(pdu, offset, str->data, str->size);
> > +	    break;
> > +	}
> > +	case 'Q': {
> > +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
> > +	    offset += pdu_marshal(pdu, offset, "bdq",
> > +				  qidp->type, qidp->version, qidp->path);
> > +	    break;
> > +	}
> > +	case 'S': {
> > +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
> > +	    offset += pdu_marshal(pdu, offset, "wwdQdddqsssssddd",
> > +				  statp->size, statp->type, statp->dev,
> > +				&statp->qid, statp->mode, statp->atime,
> > +				  statp->mtime, statp->length,&statp->name,
> > +				&statp->uid,&statp->gid,&statp->muid,
> > +				&statp->extension, statp->n_uid,
> > +				  statp->n_gid, statp->n_muid);
> > +	    break;
> > +	}
> > +	default:
> > +	    break;
> > +	}
> > +    }
> > +    va_end(ap);
> > +
> > +    return offset - old_offset;
> > +}
> >    
> 
> Adding the marshalling in  patch to implement P9_TVERSION is probably 
> not ideal.
> 

Qemu Makefile rules won't allow to add functions that are not used. The
error checking is strict. So there is not an easy way i can add these
functional logically and let each of the changeset successfully build.
Any suggestions how to do the above ?

-aneesh
Anthony Liguori - March 29, 2010, 2:51 p.m.
On 03/29/2010 02:01 AM, Aneesh Kumar K. V wrote:
> On Fri, 26 Mar 2010 11:15:47 -0500, Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>    
>> On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote:
>>      
>>> From: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>> [kiran@linux.vnet.ibm.com: malloc to qemu_malloc coversion]
>>>
>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>>    hw/virtio-9p.c |  263 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>    1 files changed, 261 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
>>> index 115c93b..53b3d78 100644
>>> --- a/hw/virtio-9p.c
>>> +++ b/hw/virtio-9p.c
>>> @@ -111,10 +111,269 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
>>>        }
>>>    }
>>>
>>> +static void v9fs_string_free(V9fsString *str)
>>> +{
>>> +    free(str->data);
>>>
>>>        
>> qemu_free.
>>      
>>> +    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)
>>> +{
>>> +    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;
>>> +
>>> +        if (offset>= off&&   offset<   (off + sg[i].iov_len)) {
>>> +            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;
>>> +        }
>>> +    }
>>> +
>>> +    return copied;
>>> +}
>>> +
>>> +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
>>> +{
>>> +    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;
>>> +    for (i = 0; i<   num; i++) {
>>> +        if (offset<= pos) {
>>> +            sg[j].iov_base = src_sg[i].iov_base;
>>> +            sg[j].iov_len = src_sg[i].iov_len;
>>> +            j++;
>>> +        } else if (offset<   (src_sg[i].iov_len + pos)) {
>>> +            sg[j].iov_base = src_sg[i].iov_base;
>>> +            sg[j].iov_len = src_sg[i].iov_len;
>>> +            sg[j].iov_base += (offset - pos);
>>> +            sg[j].iov_len -= (offset - pos);
>>> +            j++;
>>> +        }
>>> +        pos += src_sg[i].iov_len;
>>> +    }
>>> +
>>> +    return j;
>>> +}
>>> +
>>> +static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>>> +{
>>> +    size_t old_offset = offset;
>>> +    va_list ap;
>>> +    int i;
>>> +
>>> +    va_start(ap, fmt);
>>> +    for (i = 0; fmt[i]; i++) {
>>> +	switch (fmt[i]) {
>>> +	case 'b': {
>>> +	    int8_t *valp = va_arg(ap, int8_t *);
>>> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
>>> +	    break;
>>> +	}
>>> +	case 'w': {
>>> +	    int16_t *valp = va_arg(ap, int16_t *);
>>> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
>>> +	    break;
>>> +	}
>>> +	case 'd': {
>>> +	    int32_t *valp = va_arg(ap, int32_t *);
>>> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
>>> +	    break;
>>> +	}
>>> +	case 'q': {
>>> +	    int64_t *valp = va_arg(ap, int64_t *);
>>> +	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
>>> +	    break;
>>> +	}
>>> +	case 'v': {
>>> +	    struct iovec *iov = va_arg(ap, struct iovec *);
>>> +	    int *iovcnt = va_arg(ap, int *);
>>> +	    *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
>>> +	    break;
>>> +	}
>>> +	case 's': {
>>> +	    V9fsString *str = va_arg(ap, V9fsString *);
>>> +	    offset += pdu_unmarshal(pdu, offset, "w",&str->size);
>>> +	    /* FIXME: sanity check str->size */
>>> +	    str->data = qemu_malloc(str->size + 1);
>>> +	    offset += pdu_unpack(str->data, pdu, offset, str->size);
>>> +	    str->data[str->size] = 0;
>>> +	    break;
>>> +	}
>>> +	case 'Q': {
>>> +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
>>> +	    offset += pdu_unmarshal(pdu, offset, "bdq",
>>> +				&qidp->type,&qidp->version,&qidp->path);
>>> +	    break;
>>> +	}
>>> +	case 'S': {
>>> +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
>>> +	    offset += pdu_unmarshal(pdu, offset, "wwdQdddqsssssddd",
>>> +				&statp->size,&statp->type,&statp->dev,
>>> +				&statp->qid,&statp->mode,&statp->atime,
>>> +				&statp->mtime,&statp->length,
>>> +				&statp->name,&statp->uid,&statp->gid,
>>> +				&statp->muid,&statp->extension,
>>> +				&statp->n_uid,&statp->n_gid,
>>> +				&statp->n_muid);
>>> +	    break;
>>> +	}
>>> +	default:
>>> +	    break;
>>> +	}
>>> +    }
>>> +
>>> +    va_end(ap);
>>> +
>>> +    return offset - old_offset;
>>> +}
>>> +
>>> +static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>>> +{
>>> +    size_t old_offset = offset;
>>> +    va_list ap;
>>> +    int i;
>>> +
>>> +    va_start(ap, fmt);
>>> +    for (i = 0; fmt[i]; i++) {
>>> +	switch (fmt[i]) {
>>> +	case 'b': {
>>> +	    int8_t val = va_arg(ap, int);
>>> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
>>> +	    break;
>>> +	}
>>> +	case 'w': {
>>> +	    int16_t val = va_arg(ap, int);
>>> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
>>> +	    break;
>>> +	}
>>> +	case 'd': {
>>> +	    int32_t val = va_arg(ap, int);
>>> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
>>> +	    break;
>>> +	}
>>> +	case 'q': {
>>> +	    int64_t val = va_arg(ap, int64_t);
>>> +	    offset += pdu_pack(pdu, offset,&val, sizeof(val));
>>> +	    break;
>>> +	}
>>> +	case 'v': {
>>> +	    struct iovec *iov = va_arg(ap, struct iovec *);
>>> +	    int *iovcnt = va_arg(ap, int *);
>>> +	    *iovcnt = pdu_copy_sg(pdu, offset, 1, iov);
>>> +	    break;
>>> +	}
>>> +	case 's': {
>>> +	    V9fsString *str = va_arg(ap, V9fsString *);
>>> +	    offset += pdu_marshal(pdu, offset, "w", str->size);
>>> +	    offset += pdu_pack(pdu, offset, str->data, str->size);
>>> +	    break;
>>> +	}
>>> +	case 'Q': {
>>> +	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
>>> +	    offset += pdu_marshal(pdu, offset, "bdq",
>>> +				  qidp->type, qidp->version, qidp->path);
>>> +	    break;
>>> +	}
>>> +	case 'S': {
>>> +	    V9fsStat *statp = va_arg(ap, V9fsStat *);
>>> +	    offset += pdu_marshal(pdu, offset, "wwdQdddqsssssddd",
>>> +				  statp->size, statp->type, statp->dev,
>>> +				&statp->qid, statp->mode, statp->atime,
>>> +				  statp->mtime, statp->length,&statp->name,
>>> +				&statp->uid,&statp->gid,&statp->muid,
>>> +				&statp->extension, statp->n_uid,
>>> +				  statp->n_gid, statp->n_muid);
>>> +	    break;
>>> +	}
>>> +	default:
>>> +	    break;
>>> +	}
>>> +    }
>>> +    va_end(ap);
>>> +
>>> +    return offset - old_offset;
>>> +}
>>>
>>>        
>> Adding the marshalling in  patch to implement P9_TVERSION is probably
>> not ideal.
>>
>>      
> Qemu Makefile rules won't allow to add functions that are not used. The
> error checking is strict. So there is not an easy way i can add these
> functional logically and let each of the changeset successfully build.
>    

Something like (void)pdu_marshal; should keep GCC happy.

Regards,

Anthony Liguori

> Any suggestions how to do the above ?
>
> -aneesh
>

Patch

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 115c93b..53b3d78 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -111,10 +111,269 @@  static void free_pdu(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)
+{
+    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;
+
+        if (offset >= off && offset < (off + sg[i].iov_len)) {
+            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;
+        }
+    }
+
+    return copied;
+}
+
+static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
+{
+    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;
+    for (i = 0; i < num; i++) {
+        if (offset <= pos) {
+            sg[j].iov_base = src_sg[i].iov_base;
+            sg[j].iov_len = src_sg[i].iov_len;
+            j++;
+        } else if (offset < (src_sg[i].iov_len + pos)) {
+            sg[j].iov_base = src_sg[i].iov_base;
+            sg[j].iov_len = src_sg[i].iov_len;
+            sg[j].iov_base += (offset - pos);
+            sg[j].iov_len -= (offset - pos);
+            j++;
+        }
+        pos += src_sg[i].iov_len;
+    }
+
+    return j;
+}
+
+static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
+{
+    size_t old_offset = offset;
+    va_list ap;
+    int i;
+
+    va_start(ap, fmt);
+    for (i = 0; fmt[i]; i++) {
+	switch (fmt[i]) {
+	case 'b': {
+	    int8_t *valp = va_arg(ap, int8_t *);
+	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+	    break;
+	}
+	case 'w': {
+	    int16_t *valp = va_arg(ap, int16_t *);
+	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+	    break;
+	}
+	case 'd': {
+	    int32_t *valp = va_arg(ap, int32_t *);
+	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+	    break;
+	}
+	case 'q': {
+	    int64_t *valp = va_arg(ap, int64_t *);
+	    offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+	    break;
+	}
+	case 'v': {
+	    struct iovec *iov = va_arg(ap, struct iovec *);
+	    int *iovcnt = va_arg(ap, int *);
+	    *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
+	    break;
+	}
+	case 's': {
+	    V9fsString *str = va_arg(ap, V9fsString *);
+	    offset += pdu_unmarshal(pdu, offset, "w", &str->size);
+	    /* FIXME: sanity check str->size */
+	    str->data = qemu_malloc(str->size + 1);
+	    offset += pdu_unpack(str->data, pdu, offset, str->size);
+	    str->data[str->size] = 0;
+	    break;
+	}
+	case 'Q': {
+	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
+	    offset += pdu_unmarshal(pdu, offset, "bdq",
+				    &qidp->type, &qidp->version, &qidp->path);
+	    break;
+	}
+	case 'S': {
+	    V9fsStat *statp = va_arg(ap, V9fsStat *);
+	    offset += pdu_unmarshal(pdu, offset, "wwdQdddqsssssddd",
+				    &statp->size, &statp->type, &statp->dev,
+				    &statp->qid, &statp->mode, &statp->atime,
+				    &statp->mtime, &statp->length,
+				    &statp->name, &statp->uid, &statp->gid,
+				    &statp->muid, &statp->extension,
+				    &statp->n_uid, &statp->n_gid,
+				    &statp->n_muid);
+	    break;
+	}
+	default:
+	    break;
+	}
+    }
+
+    va_end(ap);
+
+    return offset - old_offset;
+}
+
+static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
+{
+    size_t old_offset = offset;
+    va_list ap;
+    int i;
+
+    va_start(ap, fmt);
+    for (i = 0; fmt[i]; i++) {
+	switch (fmt[i]) {
+	case 'b': {
+	    int8_t val = va_arg(ap, int);
+	    offset += pdu_pack(pdu, offset, &val, sizeof(val));
+	    break;
+	}
+	case 'w': {
+	    int16_t val = va_arg(ap, int);
+	    offset += pdu_pack(pdu, offset, &val, sizeof(val));
+	    break;
+	}
+	case 'd': {
+	    int32_t val = va_arg(ap, int);
+	    offset += pdu_pack(pdu, offset, &val, sizeof(val));
+	    break;
+	}
+	case 'q': {
+	    int64_t val = va_arg(ap, int64_t);
+	    offset += pdu_pack(pdu, offset, &val, sizeof(val));
+	    break;
+	}
+	case 'v': {
+	    struct iovec *iov = va_arg(ap, struct iovec *);
+	    int *iovcnt = va_arg(ap, int *);
+	    *iovcnt = pdu_copy_sg(pdu, offset, 1, iov);
+	    break;
+	}
+	case 's': {
+	    V9fsString *str = va_arg(ap, V9fsString *);
+	    offset += pdu_marshal(pdu, offset, "w", str->size);
+	    offset += pdu_pack(pdu, offset, str->data, str->size);
+	    break;
+	}
+	case 'Q': {
+	    V9fsQID *qidp = va_arg(ap, V9fsQID *);
+	    offset += pdu_marshal(pdu, offset, "bdq",
+				  qidp->type, qidp->version, qidp->path);
+	    break;
+	}
+	case 'S': {
+	    V9fsStat *statp = va_arg(ap, V9fsStat *);
+	    offset += pdu_marshal(pdu, offset, "wwdQdddqsssssddd",
+				  statp->size, statp->type, statp->dev,
+				  &statp->qid, statp->mode, statp->atime,
+				  statp->mtime, statp->length, &statp->name,
+				  &statp->uid, &statp->gid, &statp->muid,
+				  &statp->extension, statp->n_uid,
+				  statp->n_gid, statp->n_muid);
+	    break;
+	}
+	default:
+	    break;
+	}
+    }
+    va_end(ap);
+
+    return offset - old_offset;
+}
+
+static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
+{
+    int8_t id = pdu->id + 1; /* Response */
+
+    if (len < 0) {
+        V9fsString str;
+        int err = -len;
+
+        str.data = strerror(err);
+        str.size = strlen(str.data);
+
+        len = 7;
+        len += pdu_marshal(pdu, len, "s", &str);
+        if (dotu)
+            len += pdu_marshal(pdu, len, "d", err);
+
+        id = P9_RERROR;
+    }
+
+    /* fill out the header */
+    pdu_marshal(pdu, 0, "dbw", (int32_t)len, id, pdu->tag);
+
+    /* keep these in sync */
+    pdu->size = len;
+    pdu->id = id;
+
+    /* push onto queue and notify */
+    virtqueue_push(s->vq, &pdu->elem, len);
+
+    /* FIXME: we should batch these completions */
+    virtio_notify(&s->vdev, s->vq);
+
+    free_pdu(s, pdu);
+}
+
 static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
 {
-    if (debug_9p_pdu)
-        pprint_pdu(pdu);
+    int32_t msize;
+    V9fsString version;
+    size_t offset = 7;
+
+    pdu_unmarshal(pdu, offset, "ds", &msize, &version);
+    BUG_ON(strcmp(version.data, "9P2000.u") != 0);
+
+    offset += pdu_marshal(pdu, offset, "ds", msize, &version);
+    complete_pdu(s, pdu, offset);
+
+    v9fs_string_free(&version);
 }
 
 static void v9fs_attach(V9fsState *s, V9fsPDU *pdu)