On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin <levinsasha928@xxxxxxxxx> wrote:
> On Tue, 2011-06-21 at 15:50 +0530, Aneesh Kumar K.V wrote:
> > The protocol data is in little-endian format.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
> > ---
> > tools/kvm/Makefile | 1 +
> > tools/kvm/include/kvm/virtio-9p.h | 81 ++++++-
> > tools/kvm/virtio/9p-pdu.c | 237 ++++++++++++++++++
> > tools/kvm/virtio/9p.c | 477 ++++++++++++++++---------------------
> > 4 files changed, 526 insertions(+), 270 deletions(-)
> > create mode 100644 tools/kvm/virtio/9p-pdu.c
>
> I'm seeing this when trying to build after this patch:
>
> cc1: warnings being treated as errors
> virtio/9p.c: In function 'virtio_p9_create':
> virtio/9p.c:232:2: error: field precision should have type 'int', but
> argument 4 has type 'size_t'
> virtio/9p.c: In function 'virtio_p9_walk':
> virtio/9p.c:286:4: error: field precision should have type 'int', but
> argument 4 has type 'size_t'
> make: *** [virtio/9p.o] Error 1
Updated the patch to fix the error
- sprintf(fid->path, "%s/%.*s", fid->path, strlen(name), name);
+ sprintf(fid->path, "%s/%.*s", fid->path, (int)strlen(name), name);
>
> > diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> > index d368c22..559fefc 100644
> > --- a/tools/kvm/Makefile
> > +++ b/tools/kvm/Makefile
> > @@ -57,6 +57,7 @@ OBJS += util/parse-options.o
> > OBJS += util/rbtree-interval.o
> > OBJS += util/strbuf.o
> > OBJS += virtio/9p.o
> > +OBJS += virtio/9p-pdu.o
> > OBJS += hw/vesa.o
> > OBJS += hw/i8042.o
> >
> > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
> > index d99bf96..55f963b 100644
> > --- a/tools/kvm/include/kvm/virtio-9p.h
> > +++ b/tools/kvm/include/kvm/virtio-9p.h
> > @@ -1,8 +1,87 @@
> > #ifndef KVM__VIRTIO_9P_H
> > #define KVM__VIRTIO_9P_H
> > +#include "kvm/virtio-pci-dev.h"
> > +#include "kvm/virtio.h"
> > +#include "kvm/ioport.h"
> > +#include "kvm/mutex.h"
> > +#include "kvm/util.h"
> > +#include "kvm/kvm.h"
> > +#include "kvm/pci.h"
> > +#include "kvm/threadpool.h"
> > +#include "kvm/irq.h"
> > +#include "kvm/ioeventfd.h"
> > +
> > +#include <fcntl.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <pthread.h>
> > +#include <dirent.h>
> > +
> > +#include <linux/virtio_ring.h>
> > +#include <linux/virtio_9p.h>
> > +#include <net/9p/9p.h>
> > +
> > +#define NUM_VIRT_QUEUES 1
> > +#define VIRTQUEUE_NUM 128
> > +#define VIRTIO_P9_DEFAULT_TAG "kvm_9p"
> > +#define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16))
> > +#define VIRTIO_P9_MAX_FID 128
> > +#define VIRTIO_P9_VERSION "9P2000"
> > +#define MAX_TAG_LEN 32
> > +
> > +
> > +struct p9_msg {
> > + u32 size;
> > + u8 cmd;
> > + u16 tag;
> > + u8 msg[0];
> > +} __attribute__((packed));
> > +
> > +struct p9_fid {
> > + u32 fid;
> > + u8 is_dir;
> > + char abs_path[PATH_MAX];
> > + char *path;
> > + DIR *dir;
> > + int fd;
> > +};
> > +
> > +struct p9_dev_job {
> > + struct virt_queue *vq;
> > + struct p9_dev *p9dev;
> > + void *job_id;
> > +};
> > +
>
> This struct isn't vertically aligned with the rest of the file.
Update the patch to align them. But Do we really need to add those many
whitespace ?. Other part of kernel doesn't follow that coding style.
>
> > +struct p9_dev {
> > + u8 status;
> > + u8 isr;
> > + u16 config_vector;
> > + u32 features;
> > + struct virtio_9p_config *config;
> > + u16 base_addr;
> > +
> > + /* virtio queue */
> > + u16 queue_selector;
> > + struct virt_queue vqs[NUM_VIRT_QUEUES];
> > + struct p9_dev_job jobs[NUM_VIRT_QUEUES];
> > + struct p9_fid fids[VIRTIO_P9_MAX_FID];
> > + char root_dir[PATH_MAX];
> > + struct pci_device_header pci_hdr;
> > +};
> > +
> > +struct p9_pdu {
> > + u32 queue_head;
> > + size_t read_offset;
> > + size_t write_offset;
> > + u16 out_iov_cnt;
> > + u16 in_iov_cnt;
> > + struct iovec in_iov[VIRTQUEUE_NUM];
> > + struct iovec out_iov[VIRTQUEUE_NUM];
> > +};
>
> This struct is just isn't vertically aligned.
>
> > struct kvm;
> >
> > void virtio_9p__init(struct kvm *kvm, const char *root, const char *tag_name);
> > -
> > +int virtio_p9_pdu_readf(struct p9_pdu *pdu, const char *fmt, ...);
> > +int virtio_p9_pdu_writef(struct p9_pdu *pdu, const char *fmt, ...);
> > #endif
> > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c
> > new file mode 100644
> > index 0000000..da9f263
> > --- /dev/null
> > +++ b/tools/kvm/virtio/9p-pdu.c
> > @@ -0,0 +1,237 @@
> > +#include "kvm/virtio-9p.h"
> > +
> > +#include <endian.h>
> > +
> > +static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t size)
> > +{
> > + size_t len;
> > + int i, copied = 0;
> > + u16 iov_cnt = pdu->out_iov_cnt;
> > + size_t offset = pdu->read_offset;
> > + struct iovec *iov = pdu->out_iov;
> > +
> > + for (i = 0; i < iov_cnt && size; i++) {
> > + if (offset >= iov[i].iov_len) {
> > + offset -= iov[i].iov_len;
> > + continue;
> > + } else {
> > + len = MIN(iov[i].iov_len - offset, size);
> > + memcpy(data, iov[i].iov_base + offset, len);
> > + size -= len;
> > + data += len;
> > + offset = 0;
> > + copied += len;
> > + }
> > + }
> > + pdu->read_offset += copied;
> > +}
> > +
> > +static void virtio_p9_pdu_write(struct p9_pdu *pdu,
> > + const void *data, size_t size)
> > +{
> > + size_t len;
> > + int i, copied = 0;
> > + u16 iov_cnt = pdu->in_iov_cnt;
> > + size_t offset = pdu->write_offset;
> > + struct iovec *iov = pdu->in_iov;
> > +
> > + for (i = 0; i < iov_cnt && size; i++) {
> > + if (offset >= iov[i].iov_len) {
> > + offset -= iov[i].iov_len;
> > + continue;
> > + } else {
> > + len = MIN(iov[i].iov_len - offset, size);
> > + memcpy(iov[i].iov_base + offset, data, len);
> > + size -= len;
> > + data += len;
> > + offset = 0;
> > + copied += len;
> > + }
> > + }
> > + pdu->write_offset += copied;
> > +}
> > +
>
> Just wondering here, can we use pipes (read/write/vmsplice) instead of a
> scatter-gather list?
Can you elaborate on this. The function is to encode/decode data out of
protocol buffer.
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
