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

[Sasha Levin <levinsasha928@gmail.com>]

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.