From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nw8z7-0007Ym-55 for qemu-devel@nongnu.org; Mon, 29 Mar 2010 03:02:05 -0400 Received: from [140.186.70.92] (port=54256 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nw8z5-0007Xv-0G for qemu-devel@nongnu.org; Mon, 29 Mar 2010 03:02:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Nw8yz-0001qq-4t for qemu-devel@nongnu.org; Mon, 29 Mar 2010 03:02:02 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:57630) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Nw8yu-0001pD-2b for qemu-devel@nongnu.org; Mon, 29 Mar 2010 03:01:56 -0400 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [202.81.31.246]) by e23smtp09.au.ibm.com (8.14.3/8.13.1) with ESMTP id o2T71gNf024818 for ; Mon, 29 Mar 2010 18:01:42 +1100 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o2T6tZJR1396882 for ; Mon, 29 Mar 2010 17:55:36 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o2T71ft8019490 for ; Mon, 29 Mar 2010 18:01:41 +1100 From: "Aneesh Kumar K. V" Subject: Re: [Qemu-devel] [PATCH -v2 02/22] vrtio-9p: Implement P9_TVERSION for 9P In-Reply-To: <4BACDDB3.9070909@linux.vnet.ibm.com> References: <1268730920-14584-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1268730920-14584-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <4BACDDB3.9070909@linux.vnet.ibm.com> Date: Mon, 29 Mar 2010 12:31:35 +0530 Message-ID: <87mxxrhar4.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: ericvh@gmail.com, aliguori@us.ibm.com, qemu-devel@nongnu.org On Fri, 26 Mar 2010 11:15:47 -0500, Anthony Liguori wrote: > On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote: > > From: Anthony Liguori > > > > [kiran@linux.vnet.ibm.com: malloc to qemu_malloc coversion] > > > > Signed-off-by: Anthony Liguori > > Signed-off-by: Aneesh Kumar K.V > > --- > > 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