From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NwGJd-0004Py-IJ for qemu-devel@nongnu.org; Mon, 29 Mar 2010 10:51:45 -0400 Received: from [140.186.70.92] (port=45335 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NwGJZ-0004Ph-7R for qemu-devel@nongnu.org; Mon, 29 Mar 2010 10:51:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NwGJW-0002NP-Vv for qemu-devel@nongnu.org; Mon, 29 Mar 2010 10:51:40 -0400 Received: from e2.ny.us.ibm.com ([32.97.182.142]:51244) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1NwGJW-0002Mw-AX for qemu-devel@nongnu.org; Mon, 29 Mar 2010 10:51:38 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by e2.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id o2TEedoM026478 for ; Mon, 29 Mar 2010 10:40:39 -0400 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o2TEpWhg181406 for ; Mon, 29 Mar 2010 10:51:33 -0400 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o2TEpIZo022504 for ; Mon, 29 Mar 2010 08:51:20 -0600 Message-ID: <4BB0BE63.2050508@linux.vnet.ibm.com> Date: Mon, 29 Mar 2010 09:51:15 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH -v2 02/22] vrtio-9p: Implement P9_TVERSION for 9P 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> <87mxxrhar4.fsf@linux.vnet.ibm.com> In-Reply-To: <87mxxrhar4.fsf@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K. V" Cc: ericvh@gmail.com, Anthony Liguori , qemu-devel@nongnu.org On 03/29/2010 02:01 AM, Aneesh Kumar K. V wrote: > 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. > Something like (void)pdu_marshal; should keep GCC happy. Regards, Anthony Liguori > Any suggestions how to do the above ? > > -aneesh >