Re: [PATCH] tools/kvm/9p: Add encode/decode routines for protocol data

Sasha Levin
2011-06-24T13:47:23Z

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@linux.vnet.ibm.com>
> ---
> 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

> 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.

> +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?

--

Sasha.