From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NuuDu-0003hx-Hv for qemu-devel@nongnu.org; Thu, 25 Mar 2010 17:04:14 -0400 Received: from [140.186.70.92] (port=39114 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NuuDs-0003gL-F9 for qemu-devel@nongnu.org; Thu, 25 Mar 2010 17:04:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NuuDp-0001t5-4W for qemu-devel@nongnu.org; Thu, 25 Mar 2010 17:04:12 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:60730) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1NuuDo-0001sa-VT for qemu-devel@nongnu.org; Thu, 25 Mar 2010 17:04:09 -0400 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by e6.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id o2PL1Khk015298 for ; Thu, 25 Mar 2010 17:01:20 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o2PL44DW163722 for ; Thu, 25 Mar 2010 17:04:05 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o2PL44aZ018950 for ; Thu, 25 Mar 2010 17:04:04 -0400 Message-ID: <4BABCFC3.2020306@linux.vnet.ibm.com> Date: Thu, 25 Mar 2010 16:04:03 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH -V3 01/32] vitio-9p: Add a virtio 9p device to qemu References: <1269535420-31206-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1269535420-31206-2-git-send-email-aneesh.kumar@linux.vnet.ibm.com> In-Reply-To: <1269535420-31206-2-git-send-email-aneesh.kumar@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, aliguori@us.ibm.com, qemu-devel@nongnu.org On 03/25/2010 11:43 AM, Aneesh Kumar K.V wrote: > From: Anthony Liguori > > This patch doesn't implement the 9p protocol handling > code. It add a simple device which dump the protocl data > > Signed-off-by: Anthony Liguori > Signed-off-by: Aneesh Kumar K.V > --- > Makefile.target | 1 + > hw/virtio-9p-debug.c | 442 ++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/virtio-9p.c | 275 +++++++++++++++++++++++++++++++ > hw/virtio-9p.h | 70 ++++++++ > hw/virtio-pci.c | 25 +++ > hw/virtio.h | 1 + > 6 files changed, 814 insertions(+), 0 deletions(-) > create mode 100644 hw/virtio-9p-debug.c > create mode 100644 hw/virtio-9p.c > create mode 100644 hw/virtio-9p.h > > diff --git a/Makefile.target b/Makefile.target > index eb4d010..178ddce 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -166,6 +166,7 @@ obj-y += qemu-timer.o > # virtio has to be here due to weird dependency between PCI and virtio-net. > # need to fix this properly > obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o > +obj-y += virtio-9p.o virtio-9p-debug.o > obj-y += rwhandler.o > obj-$(CONFIG_KVM) += kvm.o kvm-all.o > LIBS+=-lz > diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c > new file mode 100644 > index 0000000..9230659 > --- /dev/null > +++ b/hw/virtio-9p-debug.c > @@ -0,0 +1,442 @@ > +/* > + * Virtio 9p PDU debug > + * > + * Copyright IBM, Corp. 2010 > + * > + * Authors: > + * Anthony Liguori > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > +#include "virtio.h" > +#include "pc.h" > +#include "virtio-9p.h" > + > +#include > +#include > Can't directly include sys/uio here (doesn't exist on windows). qemu-common.h will do the right thing. > + > +#define BUG_ON(cond) assert(!(cond)) > + > +extern int dotu; > +static FILE *llogfile; > + > +static struct iovec *get_sg(V9fsPDU *pdu, int rx) > +{ > + if (rx) > + return pdu->elem.in_sg; > Coding style. > + return pdu->elem.out_sg; > +} > + > +static void pprint_int8(V9fsPDU *pdu, int rx, size_t *offsetp, > + const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int8_t value; > + > + BUG_ON((offset + sizeof(value))> sg[0].iov_len); > + > + memcpy(&value, sg[0].iov_base + offset, sizeof(value)); > + offset += sizeof(value); > + > + fprintf(llogfile, "%s=0x%x", name, value); > + > + *offsetp = offset; > +} > + > +static void pprint_int16(V9fsPDU *pdu, int rx, size_t *offsetp, > + const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int16_t value; > + > + BUG_ON((offset + sizeof(value))> sg[0].iov_len); > + > + memcpy(&value, sg[0].iov_base + offset, sizeof(value)); > + offset += sizeof(value); > + > + fprintf(llogfile, "%s=0x%x", name, value); > + > + *offsetp = offset; > +} > + > +static void pprint_int32(V9fsPDU *pdu, int rx, size_t *offsetp, > + const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int32_t value; > + > + BUG_ON((offset + sizeof(value))> sg[0].iov_len); > + > + memcpy(&value, sg[0].iov_base + offset, sizeof(value)); > + offset += sizeof(value); > + > + fprintf(llogfile, "%s=0x%x", name, value); > + > + *offsetp = offset; > +} > + > +static void pprint_int64(V9fsPDU *pdu, int rx, size_t *offsetp, > + const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int64_t value; > + > + BUG_ON((offset + sizeof(value))> sg[0].iov_len); > + > + memcpy(&value, sg[0].iov_base + offset, sizeof(value)); > + offset += sizeof(value); > + > + fprintf(llogfile, "%s=0x%" PRIx64, name, value); > + > + *offsetp = offset; > +} > + > +static void pprint_str(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int16_t size; > + size_t result; > + > + BUG_ON((offset + 2)> sg[0].iov_len); > + memcpy(&size, sg[0].iov_base + offset, 2); > + offset += 2; > + > + BUG_ON((offset + size)> sg[0].iov_len); > + fprintf(llogfile, "%s=", name); > + result = fwrite(sg[0].iov_base + offset, 1, size, llogfile); > + BUG_ON(result != size); > + offset += size; > + > + *offsetp = offset; > +} > + > +static void pprint_qid(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + fprintf(llogfile, "%s={", name); > + pprint_int8(pdu, rx, offsetp, "type"); > + pprint_int32(pdu, rx, offsetp, ", version"); > + pprint_int64(pdu, rx, offsetp, ", path"); > + fprintf(llogfile, "}"); > +} > + > +static void pprint_stat(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + fprintf(llogfile, "%s={", name); > + pprint_int16(pdu, rx, offsetp, "size"); > + pprint_int16(pdu, rx, offsetp, ", type"); > + pprint_int32(pdu, rx, offsetp, ", dev"); > + pprint_qid(pdu, rx, offsetp, ", qid"); > + pprint_int32(pdu, rx, offsetp, ", mode"); > + pprint_int32(pdu, rx, offsetp, ", atime"); > + pprint_int32(pdu, rx, offsetp, ", mtime"); > + pprint_int64(pdu, rx, offsetp, ", length"); > + pprint_str(pdu, rx, offsetp, ", name"); > + pprint_str(pdu, rx, offsetp, ", uid"); > + pprint_str(pdu, rx, offsetp, ", gid"); > + pprint_str(pdu, rx, offsetp, ", muid"); > + if (dotu) { > + pprint_str(pdu, rx, offsetp, ", extension"); > + pprint_int32(pdu, rx, offsetp, ", uid"); > + pprint_int32(pdu, rx, offsetp, ", gid"); > + pprint_int32(pdu, rx, offsetp, ", muid"); > + } > + fprintf(llogfile, "}"); > +} > + > +static void pprint_strs(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int16_t count, i; > + > + fprintf(llogfile, "%s={", name); > + > + BUG_ON((offset + 2)> sg[0].iov_len); > + memcpy(&count, sg[0].iov_base + offset, 2); > + offset += 2; > + > + for (i = 0; i< count; i++) { > + char str[512]; > + if (i) > + fprintf(llogfile, ", "); > + snprintf(str, sizeof(str), "[%d]", i); > + pprint_str(pdu, rx,&offset, str); > + } > + > + fprintf(llogfile, "}"); > + > + *offsetp = offset; > +} > + > +static void pprint_qids(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + int16_t count, i; > + > + fprintf(llogfile, "%s={", name); > + > + BUG_ON((offset + 2)> sg[0].iov_len); > + memcpy(&count, sg[0].iov_base + offset, 2); > + offset += 2; > + > + for (i = 0; i< count; i++) { > + char str[512]; > + if (i) > + fprintf(llogfile, ", "); > + snprintf(str, sizeof(str), "[%d]", i); > + pprint_qid(pdu, rx,&offset, str); > + } > + > + fprintf(llogfile, "}"); > + > + *offsetp = offset; > +} > + > +static void pprint_sg(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + unsigned int count; > + int i; > + > + if (rx) > + count = pdu->elem.in_num; > + else > + count = pdu->elem.out_num; > + > + fprintf(llogfile, "%s={", name); > + for (i = 0; i< count; i++) { > + if (i) > + fprintf(llogfile, ", "); > + fprintf(llogfile, "(%p, 0x%zx)", sg[i].iov_base, sg[i].iov_len); > + } > + fprintf(llogfile, "}"); > +} > + > +/* FIXME: read from a directory fid returns serialized stat_t's */ > +#ifdef DEBUG_DATA > +static void pprint_data(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > +{ > + struct iovec *sg = get_sg(pdu, rx); > + size_t offset = *offsetp; > + unsigned int count; > + int32_t size; > + int total, i, j; > + ssize_t len; > + > + if (rx) > + count = pdu->elem.in_num; > + else > + count = pdu->elem.out_num; > Coding style.diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > new file mode 100644 > index 0000000..115c93b > --- /dev/null > +++ b/hw/virtio-9p.c > @@ -0,0 +1,275 @@ > +/* > + * Virtio 9p backend > + * > + * Copyright IBM, Corp. 2010 > + * > + * Authors: > + * Anthony Liguori > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#include "virtio.h" > +#include "pc.h" > +#include "qemu_socket.h" > +#include "virtio-9p.h" > + > +#include > + > +/* from Linux's linux/virtio_9p.h */ > + > +/* The ID for virtio console */ > +#define VIRTIO_ID_9P 9 > +/* Maximum number of virtio channels per partition (1 for now) */ > +#define MAX_9P_CHAN 1 > + > +#define MAX_REQ 128 > + > +#define BUG_ON(cond) assert(!(cond)) > + > +typedef struct V9fsFidState V9fsFidState; > + > +typedef struct V9fsString > +{ > + int16_t size; > + char *data; > +} V9fsString; > + > +typedef struct V9fsQID > +{ > + int8_t type; > + int32_t version; > + int64_t path; > +} V9fsQID; > + > +typedef struct V9fsStat > +{ > + int16_t size; > + int16_t type; > + int32_t dev; > + V9fsQID qid; > + int32_t mode; > + int32_t atime; > + int32_t mtime; > + int64_t length; > + V9fsString name; > + V9fsString uid; > + V9fsString gid; > + V9fsString muid; > + /* 9p2000.u */ > + V9fsString extension; > + int32_t n_uid; > + int32_t n_gid; > + int32_t n_muid; > +} V9fsStat; > + > +struct V9fsFidState > +{ > + int32_t fid; > + V9fsString path; > + int fd; > + DIR *dir; > + uid_t uid; > + V9fsFidState *next; > +}; > + > +typedef struct V9fsState > +{ > + VirtIODevice vdev; > + VirtQueue *vq; > + V9fsPDU pdus[MAX_REQ]; > + V9fsPDU *free_pdu; > + V9fsFidState *fid_list; > + char *root; > + uid_t uid; > +} V9fsState; > + > +int dotu = 1; > +int debug_9p_pdu = 1; > This shouldn't be enabled by default. > + > +extern void pprint_pdu(V9fsPDU *pdu); > + > +static V9fsPDU *alloc_pdu(V9fsState *s) > +{ > + V9fsPDU *pdu = NULL; > + > + if (s->free_pdu) { > + pdu = s->free_pdu; > + s->free_pdu = pdu->next; > + } > + > + return pdu; > +} > + > +static void free_pdu(V9fsState *s, V9fsPDU *pdu) > +{ > + if (pdu) { > + pdu->next = s->free_pdu; > + s->free_pdu = pdu; > + } > +} > + > +static void v9fs_version(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_attach(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_stat(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_walk(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > + static void v9fs_open(V9fsState *s, V9fsPDU *pdu) > Whitespace is off. > +{ if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_read(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_write(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_create(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_flush(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_remove(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > + > +static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu) > +{ > + if (debug_9p_pdu) > + pprint_pdu(pdu); > +} > Coding style is consistently off. > + > +typedef void (pdu_handler_t)(V9fsState *s, V9fsPDU *pdu); > + > +static pdu_handler_t *pdu_handlers[] = { > + [P9_TVERSION] = v9fs_version, > + [P9_TATTACH] = v9fs_attach, > + [P9_TSTAT] = v9fs_stat, > + [P9_TWALK] = v9fs_walk, > + [P9_TCLUNK] = v9fs_clunk, > + [P9_TOPEN] = v9fs_open, > + [P9_TREAD] = v9fs_read, > +#if 0 > + [P9_TAUTH] = v9fs_auth, > +#endif > + [P9_TFLUSH] = v9fs_flush, > + [P9_TCREATE] = v9fs_create, > + [P9_TWRITE] = v9fs_write, > + [P9_TWSTAT] = v9fs_wstat, > + [P9_TREMOVE] = v9fs_remove, > +}; > + > +static void submit_pdu(V9fsState *s, V9fsPDU *pdu) > +{ > + pdu_handler_t *handler; > + > + if (debug_9p_pdu) > + pprint_pdu(pdu); > + > + BUG_ON(pdu->id>= ARRAY_SIZE(pdu_handlers)); > + > + handler = pdu_handlers[pdu->id]; > + BUG_ON(handler == NULL); > + > + handler(s, pdu); > +} > + > +static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) > +{ > + V9fsState *s = (V9fsState *)vdev; > + V9fsPDU *pdu; > + ssize_t len; > + > + while ((pdu = alloc_pdu(s))&& > + (len = virtqueue_pop(vq,&pdu->elem)) != 0) { > + uint8_t *ptr; > + > + BUG_ON(pdu->elem.out_num == 0 || pdu->elem.in_num == 0); > + BUG_ON(pdu->elem.out_sg[0].iov_len< 7); > We shouldn't assume that everything is in the first sg element.diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h > static PCIDeviceInfo virtio_info[] = { > { > .qdev.name = "virtio-blk-pci", > @@ -606,6 +622,15 @@ static PCIDeviceInfo virtio_info[] = { > }, > .qdev.reset = virtio_pci_reset, > },{ > + .qdev.name = "virtio-9p-pci", > + .qdev.size = sizeof(VirtIOPCIProxy), > + .init = virtio_9p_init_pci, > + .qdev.props = (Property[]) { > + DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > + DEFINE_PROP_STRING("share_path", VirtIOPCIProxy, share_path), > + DEFINE_PROP_END_OF_LIST(), > + }, > It's odd to introduce -fsdev after this and have a transient qdev property. Regards, Anthony Liguori > + },{ > /* end of list */ > } > }; > diff --git a/hw/virtio.h b/hw/virtio.h > index 3baa2a3..4032a96 100644 > --- a/hw/virtio.h > +++ b/hw/virtio.h > @@ -174,6 +174,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf); > VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf); > VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports); > VirtIODevice *virtio_balloon_init(DeviceState *dev); > +VirtIODevice *virtio_9p_init(DeviceState *dev, const char *path); > > void virtio_net_exit(VirtIODevice *vdev); > >