tools/kvm/9p: Add encode/decode routines for protocol data

Commit Message

Aneesh Kumar K.V June 21, 2011, 10:20 a.m. UTC

The protocol data is in little-endian format.

Signed-off-by: Aneesh Kumar K.V <[e-mail]>
---
 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

Comments

Sasha Levin June 24, 2011, 1:47 p.m. UTC | #1

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 <[e-mail]>
> ---
>  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?

Aneesh Kumar K.V June 24, 2011, 4 p.m. UTC | #2

On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin <[e-mail]> 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 <[e-mail]>
> > ---
> >  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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sasha Levin June 24, 2011, 11:26 p.m. UTC | #3

On Fri, 2011-06-24 at 21:30 +0530, Aneesh Kumar K.V wrote:
> On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin <[e-mail]> wrote:
> > On Tue, 2011-06-21 at 15:50 +0530, Aneesh Kumar K.V wrote:
> > > +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.

I was just thinking about storing the data in a pipe instead of a
scatter-gather list.

This would turn virtio_p9_pdu_read() into a simple wrapper for read()
from a pipe and virtio_p9_pdu_write() into a wrapper for write().

Something like this:

void virtio_9p__init(struct kvm *kvm, const char *root, const char
*tag_name)
{
[...]
	pipe(p9_pipe);
[...]
}

static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t
size)
{
	read(p9_pipe[0], data, size);
}

static void virtio_p9_pdu_write(struct p9_pdu *pdu, const void *data,
size_t size)
{
	write(p9_pipe[1], data, size);
}

Aneesh Kumar K.V June 28, 2011, 6:39 a.m. UTC | #4

On Fri, 24 Jun 2011 19:26:48 -0400, Sasha Levin <[e-mail]> wrote:
> On Fri, 2011-06-24 at 21:30 +0530, Aneesh Kumar K.V wrote:
> > On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin <[e-mail]> wrote:
> > > On Tue, 2011-06-21 at 15:50 +0530, Aneesh Kumar K.V wrote:
> > > > +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.
> 
> I was just thinking about storing the data in a pipe instead of a
> scatter-gather list.

We are building that scatter-gather list from vring. I am not clear
how to push that to pipe ? ie we have iovec in vring and how do i
store that to a pipe ?

> 
> This would turn virtio_p9_pdu_read() into a simple wrapper for read()
> from a pipe and virtio_p9_pdu_write() into a wrapper for write().
> 
> Something like this:
> 
> void virtio_9p__init(struct kvm *kvm, const char *root, const char
> *tag_name)
> {
> [...]
> 	pipe(p9_pipe);
> [...]
> }
> 
> static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t
> size)
> {
> 	read(p9_pipe[0], data, size);
> }
> 
> static void virtio_p9_pdu_write(struct p9_pdu *pdu, const void *data,
> size_t size)
> {
> 	write(p9_pipe[1], data, size);
> }
> 

But why ? what we currently have is a simple memcpy, why replace that
with syscall read/write ?

-aneesh
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;
+};
+
+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];
+};
 
 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;
+}
+
+static void virtio_p9_wstat_free(struct p9_wstat *stbuf)
+{
+	free(stbuf->name);
+	free(stbuf->uid);
+	free(stbuf->gid);
+	free(stbuf->muid);
+}
+
+static int virtio_p9_decode(struct p9_pdu *pdu, const char *fmt, va_list ap)
+{
+	int retval = 0;
+	const char *ptr;
+
+	for (ptr = fmt; *ptr; ptr++) {
+		switch (*ptr) {
+		case 'b':
+		{
+			int8_t *val = va_arg(ap, int8_t *);
+			virtio_p9_pdu_read(pdu, val, sizeof(*val));
+		}
+		break;
+		case 'w':
+		{
+			int16_t le_val;
+			int16_t *val = va_arg(ap, int16_t *);
+			virtio_p9_pdu_read(pdu, &le_val, sizeof(le_val));
+			*val = le16toh(le_val);
+		}
+		break;
+		case 'd':
+		{
+			int32_t le_val;
+			int32_t *val = va_arg(ap, int32_t *);
+			virtio_p9_pdu_read(pdu, &le_val, sizeof(le_val));
+			*val = le32toh(le_val);
+		}
+		break;
+		case 'q':
+		{
+			int64_t le_val;
+			int64_t *val = va_arg(ap, int64_t *);
+			virtio_p9_pdu_read(pdu, &le_val, sizeof(le_val));
+			*val = le64toh(le_val);
+		}
+		break;
+		case 's':
+		{
+			int16_t len;
+			char **str = va_arg(ap, char **);
+
+			virtio_p9_pdu_readf(pdu, "w", &len);
+			*str = malloc(len + 1);
+			if (*str == NULL) {
+				retval = ENOMEM;
+				break;
+			}
+			virtio_p9_pdu_read(pdu, *str, len);
+			(*str)[len] = 0;
+		}
+		break;
+		case 'Q':
+		{
+			struct p9_qid *qid = va_arg(ap, struct p9_qid *);
+			retval = virtio_p9_pdu_readf(pdu, "bdq",
+						     &qid->type, &qid->version,
+						     &qid->path);
+		}
+		break;
+		case 'S':
+		{
+			struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
+			memset(stbuf, 0, sizeof(struct p9_wstat));
+			stbuf->n_uid = stbuf->n_gid = stbuf->n_muid = -1;
+			retval = virtio_p9_pdu_readf(pdu, "wwdQdddqssss",
+						&stbuf->size, &stbuf->type,
+						&stbuf->dev, &stbuf->qid,
+						&stbuf->mode, &stbuf->atime,
+						&stbuf->mtime, &stbuf->length,
+						&stbuf->name, &stbuf->uid,
+						&stbuf->gid, &stbuf->muid);
+			if (retval)
+				virtio_p9_wstat_free(stbuf);
+		}
+		break;
+		default:
+			retval = EINVAL;
+			break;
+		}
+	}
+	return retval;
+}
+
+static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap)
+{
+	int retval = 0;
+	const char *ptr;
+
+	for (ptr = fmt; *ptr; ptr++) {
+		switch (*ptr) {
+		case 'b':
+		{
+			int8_t val = va_arg(ap, int);
+			virtio_p9_pdu_write(pdu, &val, sizeof(val));
+		}
+		break;
+		case 'w':
+		{
+			int16_t val = htole16(va_arg(ap, int));
+			virtio_p9_pdu_write(pdu, &val, sizeof(val));
+		}
+		break;
+		case 'd':
+		{
+			int32_t val = htole32(va_arg(ap, int32_t));
+			virtio_p9_pdu_write(pdu, &val, sizeof(val));
+		}
+		break;
+		case 'q':
+		{
+			int64_t val = htole64(va_arg(ap, int64_t));
+			virtio_p9_pdu_write(pdu, &val, sizeof(val));
+		}
+		break;
+		case 's':
+		{
+			uint16_t len = 0;
+			const char *s = va_arg(ap, char *);
+			if (s)
+				len = MIN(strlen(s), USHRT_MAX);
+			virtio_p9_pdu_writef(pdu, "w", len);
+			virtio_p9_pdu_write(pdu, s, len);
+		}
+		break;
+		case 'Q':
+		{
+			struct p9_qid *qid = va_arg(ap, struct p9_qid *);
+			retval = virtio_p9_pdu_writef(pdu, "bdq",
+						      qid->type, qid->version,
+						      qid->path);
+		}
+		break;
+		case 'S':
+		{
+			struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
+			retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss",
+						stbuf->size, stbuf->type,
+						stbuf->dev, &stbuf->qid,
+						stbuf->mode, stbuf->atime,
+						stbuf->mtime, stbuf->length,
+						stbuf->name, stbuf->uid,
+						stbuf->gid, stbuf->muid);
+		}
+		break;
+		default:
+			retval = EINVAL;
+			break;
+		}
+	}
+	return retval;
+}
+
+int virtio_p9_pdu_readf(struct p9_pdu *pdu, const char *fmt, ...)
+{
+	int ret;
+	va_list ap;
+
+	va_start(ap, fmt);
+	ret = virtio_p9_decode(pdu, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+int virtio_p9_pdu_writef(struct p9_pdu *pdu, const char *fmt, ...)
+{
+	int ret;
+	va_list ap;
+
+	va_start(ap, fmt);
+	ret = virtio_p9_pdu_encode(pdu, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index d2d738d..7bc6c95 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -1,81 +1,4 @@ 
 #include "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;
-};
-
-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;
-	int offset;
-	u16 out_iov_cnt;
-	u16 in_iov_cnt;
-	struct iovec in_iov[VIRTQUEUE_NUM];
-	struct iovec out_iov[VIRTQUEUE_NUM];
-};
 
 /* Warning: Immediately use value returned from this function */
 static const char *rel_to_abs(struct p9_dev *p9dev,
@@ -185,13 +108,16 @@  static void close_fid(struct p9_dev *p9dev, u32 fid)
 	}
 }
 
-static void set_p9msg_hdr(struct p9_msg *msg, u32 size, u8 cmd, u16 tag)
+static void virtio_p9_set_reply_header(struct p9_pdu *pdu, u32 size)
 {
-	*msg = (struct p9_msg) {
-		.size	= size,
-		.tag	= tag,
-		.cmd	= cmd,
-	};
+	u8 cmd;
+	u16 tag;
+
+	pdu->read_offset = sizeof(u32);
+	virtio_p9_pdu_readf(pdu, "bw", &cmd, &tag);
+	pdu->write_offset = 0;
+	/* cmd + 1 is the reply message */
+	virtio_p9_pdu_writef(pdu, "dbw", size, cmd + 1, tag);
 }
 
 static u16 virtio_p9_update_iov_cnt(struct iovec iov[], u32 count, int iov_cnt)
@@ -213,67 +139,61 @@  static u16 virtio_p9_update_iov_cnt(struct iovec iov[], u32 count, int iov_cnt)
 static void virtio_p9_error_reply(struct p9_dev *p9dev,
 				  struct p9_pdu *pdu, int err, u32 *outlen)
 {
+	u16 tag;
 	char *err_str;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_rerror *rerror  = (struct p9_rerror *)inmsg->msg;
 
 	err_str = strerror(err);
-	rerror->error.len = strlen(err_str);
-	memcpy(&rerror->error.str, err_str, rerror->error.len);
+	pdu->write_offset = VIRTIO_P9_HDR_LEN;
+	virtio_p9_pdu_writef(pdu, "s", err_str);
+	*outlen = pdu->write_offset;
+
+	pdu->read_offset = sizeof(u32) + sizeof(u8);
+	virtio_p9_pdu_readf(pdu, "w", &tag);
 
-	*outlen = VIRTIO_P9_HDR_LEN + rerror->error.len + sizeof(u16);
-	set_p9msg_hdr(inmsg, *outlen, P9_RERROR, outmsg->tag);
+	pdu->write_offset = 0;
+	virtio_p9_pdu_writef(pdu, "dbw", *outlen, P9_RERROR, tag);
 }
 
 static void virtio_p9_version(struct p9_dev *p9dev,
 			      struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_rversion *rversion = (struct p9_rversion *)inmsg->msg;
-
-	rversion->msize		= 4096;
-	rversion->version.len	= strlen(VIRTIO_P9_VERSION);
-	memcpy(&rversion->version.str, VIRTIO_P9_VERSION, rversion->version.len);
-
-	*outlen = VIRTIO_P9_HDR_LEN +
-		rversion->version.len + sizeof(u16) + sizeof(u32);
-	set_p9msg_hdr(inmsg, *outlen, P9_RVERSION, outmsg->tag);
+	virtio_p9_pdu_writef(pdu, "ds", 4096, VIRTIO_P9_VERSION);
 
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 }
 
 static void virtio_p9_clunk(struct p9_dev *p9dev,
 			    struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_tclunk *tclunk = (struct p9_tclunk *)outmsg->msg;
+	u32 fid;
 
-	close_fid(p9dev, tclunk->fid);
-
-	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(inmsg, *outlen, P9_RCLUNK, outmsg->tag);
+	virtio_p9_pdu_readf(pdu, "d", &fid);
+	close_fid(p9dev, fid);
 
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 }
 
 static void virtio_p9_open(struct p9_dev *p9dev,
 			   struct p9_pdu *pdu, u32 *outlen)
 {
+	u8 mode;
+	u32 fid;
 	struct stat st;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_topen *topen	= (struct p9_topen *)outmsg->msg;
-	struct p9_ropen *ropen	= (struct p9_ropen *)inmsg->msg;
-	struct p9_fid *new_fid	= &p9dev->fids[topen->fid];
+	struct p9_qid qid;
+	struct p9_fid *new_fid;
+
+
+	virtio_p9_pdu_readf(pdu, "db", &fid, &mode);
+	new_fid = &p9dev->fids[fid];
 
 	if (lstat(new_fid->abs_path, &st) < 0)
 		goto err_out;
 
-	st2qid(&st, &ropen->qid);
-	ropen->iounit = 0;
+	st2qid(&st, &qid);
 
 	if (new_fid->is_dir) {
 		new_fid->dir = opendir(new_fid->abs_path);
@@ -281,12 +201,14 @@  static void virtio_p9_open(struct p9_dev *p9dev,
 			goto err_out;
 	} else {
 		new_fid->fd  = open(new_fid->abs_path,
-				   omode2uflags(topen->mode) | O_NOFOLLOW);
+				    omode2uflags(mode) | O_NOFOLLOW);
 		if (new_fid->fd < 0)
 			goto err_out;
 	}
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*ropen);
-	set_p9msg_hdr(inmsg, *outlen, P9_ROPEN, outmsg->tag);
+	virtio_p9_pdu_writef(pdu, "Qd", &qid, 0);
+
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 err_out:
 	virtio_p9_error_reply(p9dev, pdu, errno, outlen);
@@ -298,39 +220,33 @@  static void virtio_p9_create(struct p9_dev *p9dev,
 {
 	u8 mode;
 	u32 perm;
+	char *name;
+	u32 fid_val;
 	struct stat st;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_tcreate *tcreate	= (struct p9_tcreate *)outmsg->msg;
-	struct p9_rcreate *rcreate	= (struct p9_rcreate *)inmsg->msg;
-	struct p9_fid *fid		= &p9dev->fids[tcreate->fid];
-
-
-	rcreate->iounit = 0;
-
-	/* Get last byte of the variable length struct */
-	mode = *((u8 *)outmsg + outmsg->size - 1);
-	perm = *(u32 *)((u8 *)outmsg + outmsg->size - 5);
+	struct p9_qid qid;
+	struct p9_fid *fid;
 
-	sprintf(fid->path, "%s/%.*s", fid->path, tcreate->name.len, (char *)&tcreate->name.str);
+	virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode);
+	fid = &p9dev->fids[fid_val];
 
-	close_fid(p9dev, tcreate->fid);
+	sprintf(fid->path, "%s/%.*s", fid->path, strlen(name), name);
+	close_fid(p9dev, fid_val);
 
 	if (perm & P9_DMDIR) {
 		mkdir(fid->abs_path, perm & 0xFFFF);
 		fid->dir = opendir(fid->abs_path);
 		fid->is_dir = 1;
 	} else {
-		fid->fd = open(fid->abs_path, omode2uflags(mode) | O_CREAT, 0777);
+		fid->fd = open(fid->abs_path,
+			       omode2uflags(mode) | O_CREAT, 0777);
 	}
-
 	if (lstat(fid->abs_path, &st) < 0)
 		goto err_out;
 
-	st2qid(&st, &rcreate->qid);
-
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rcreate);
-	set_p9msg_hdr(inmsg, *outlen, P9_RCREATE, outmsg->tag);
+	st2qid(&st, &qid);
+	virtio_p9_pdu_writef(pdu, "Qd", &qid, 0);
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 err_out:
 	virtio_p9_error_reply(p9dev, pdu, errno, outlen);
@@ -341,45 +257,57 @@  static void virtio_p9_walk(struct p9_dev *p9dev,
 			   struct p9_pdu *pdu, u32 *outlen)
 {
 	u8 i;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_twalk *twalk	= (struct p9_twalk *)outmsg->msg;
-	struct p9_rwalk *rwalk	= (struct p9_rwalk *)inmsg->msg;
-	struct p9_str *str	= twalk->wnames;
-	struct p9_fid *new_fid	= &p9dev->fids[twalk->newfid];
+	u16 nwqid;
+	char *str;
+	u16 nwname;
+	u32 fid_val;
+	u32 newfid_val;
+	struct p9_qid wqid;
+	struct p9_fid *new_fid;
+
 
+	virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
+	new_fid	= &p9dev->fids[newfid_val];
 
-	rwalk->nwqid = 0;
-	if (twalk->nwname) {
-		struct p9_fid *fid = &p9dev->fids[twalk->fid];
+	nwqid = 0;
+	if (nwname) {
+		struct p9_fid *fid = &p9dev->fids[fid_val];
 
-		for (i = 0; i < twalk->nwname; i++) {
+		/* skip the space for count */
+		pdu->write_offset += sizeof(u16);
+		for (i = 0; i < nwname; i++) {
+			struct stat st;
 			char tmp[PATH_MAX] = {0};
 			char full_path[PATH_MAX];
-			struct stat st;
+
+			virtio_p9_pdu_readf(pdu, "s", &str);
 
 			/* Format the new path we're 'walk'ing into */
-			sprintf(tmp, "%s/%.*s", fid->path,
-				str->len, (char *)&str->str);
+			sprintf(tmp, "%s/%.*s", fid->path, strlen(str), str);
 
 			if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
 				goto err_out;
 
-			st2qid(&st, &rwalk->wqids[i]);
+			st2qid(&st, &wqid);
 			new_fid->is_dir = S_ISDIR(st.st_mode);
 			strcpy(new_fid->path, tmp);
-			new_fid->fid = twalk->newfid;
-			rwalk->nwqid++;
+			new_fid->fid = newfid_val;
+			nwqid++;
+			virtio_p9_pdu_writef(pdu, "Q", &wqid);
 		}
 	} else {
-		new_fid->is_dir = p9dev->fids[twalk->fid].is_dir;
-		strcpy(new_fid->path, p9dev->fids[twalk->fid].path);
-		new_fid->fid	= twalk->newfid;
+		/*
+		 * update write_offset so our outlen get correct value
+		 */
+		pdu->write_offset += sizeof(u16);
+		new_fid->is_dir = p9dev->fids[fid_val].is_dir;
+		strcpy(new_fid->path, p9dev->fids[fid_val].path);
+		new_fid->fid	= newfid_val;
 	}
-
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u16) +
-		sizeof(struct p9_qid)*rwalk->nwqid;
-	set_p9msg_hdr(inmsg, *outlen, P9_RWALK, outmsg->tag);
+	*outlen = pdu->write_offset;
+	pdu->write_offset = VIRTIO_P9_HDR_LEN;
+	virtio_p9_pdu_writef(pdu, "d", nwqid);
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 err_out:
 	virtio_p9_error_reply(p9dev, pdu, errno, outlen);
@@ -390,12 +318,15 @@  static void virtio_p9_attach(struct p9_dev *p9dev,
 			     struct p9_pdu *pdu, u32 *outlen)
 {
 	u32 i;
+	u32 fid_val;
+	u32 afid;
+	char *uname;
+	char *aname;
 	struct stat st;
+	struct p9_qid qid;
 	struct p9_fid *fid;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_rattach *rattach = (struct p9_rattach *)inmsg->msg;
-	struct p9_tattach *tattach = (struct p9_tattach *)outmsg->msg;
+
+	virtio_p9_pdu_readf(pdu, "ddss", &fid_val, &afid, &uname, &aname);
 
 	/* Reset everything */
 	for (i = 0; i < VIRTIO_P9_MAX_FID; i++)
@@ -404,103 +335,106 @@  static void virtio_p9_attach(struct p9_dev *p9dev,
 	if (lstat(p9dev->root_dir, &st) < 0)
 		goto err_out;
 
-	st2qid(&st, &rattach->qid);
+	st2qid(&st, &qid);
 
-	fid = &p9dev->fids[tattach->fid];
-	fid->fid = tattach->fid;
+	fid = &p9dev->fids[fid_val];
+	fid->fid = fid_val;
 	fid->is_dir = 1;
 	strcpy(fid->path, "/");
 
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rattach);
-	set_p9msg_hdr(inmsg, *outlen, P9_RATTACH, outmsg->tag);
+	virtio_p9_pdu_writef(pdu, "Q", &qid);
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 err_out:
 	virtio_p9_error_reply(p9dev, pdu, errno, outlen);
 	return;
 }
 
-static u32 virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
-			       struct stat *st, struct p9_rstat *rstat)
+static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
+				struct stat *st, struct p9_wstat *wstat)
 {
-	struct p9_str *str;
-
-	rstat->stat.type = 0;
-	rstat->stat.dev = 0;
-	st2qid(st, &rstat->stat.qid);
-	rstat->stat.mode = st->st_mode;
-	rstat->stat.length = st->st_size;
+	wstat->type = 0;
+	wstat->dev = 0;
+	st2qid(st, &wstat->qid);
+	wstat->mode = st->st_mode;
+	wstat->length = st->st_size;
 	if (S_ISDIR(st->st_mode)) {
-		rstat->stat.length = 0;
-		rstat->stat.mode |= P9_DMDIR;
+		wstat->length = 0;
+		wstat->mode |= P9_DMDIR;
 	}
 
-	rstat->stat.atime = st->st_atime;
-	rstat->stat.mtime = st->st_mtime;
-
-	str = (struct p9_str *)&rstat->stat.name;
-	str->len = strlen(name);
-	memcpy(&str->str, name, str->len);
-	str = (void *)str + str->len + sizeof(u16);
-
-	/* TODO: Pass usernames to the client */
-	str->len = 0;
-	str = (void *)str + sizeof(u16);
-	str->len = 0;
-	str = (void *)str + sizeof(u16);
-	str->len = 0;
-	str = (void *)str + sizeof(u16);
-
-	/*
-	 * We subtract a u16 here because rstat->size
-	 * doesn't include rstat->size itself
-	 */
-	rstat->stat.size = (void *)str - (void *)&rstat->stat - sizeof(u16);
-
-	return rstat->stat.size + sizeof(u16);
+	wstat->atime = st->st_atime;
+	wstat->mtime = st->st_mtime;
+
+	wstat->name = strdup(name);
+	wstat->uid = NULL;
+	wstat->gid = NULL;
+	wstat->muid = NULL;
+
+	/* NOTE: size shouldn't include its own length */
+	/* size[2] type[2] dev[4] qid[13] */
+	/* mode[4] atime[4] mtime[4] length[8]*/
+	/* name[s] uid[s] gid[s] muid[s] */
+	wstat->size = 2+4+13+4+4+4+8+2+2+2+2;
+	if (wstat->name)
+		wstat->size += strlen(wstat->name);
 }
 
 static void virtio_p9_read(struct p9_dev *p9dev,
 			   struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_tread *tread	= (struct p9_tread *)outmsg->msg;
-	struct p9_rread *rread	= (struct p9_rread *)inmsg->msg;
-	struct p9_rstat *rstat	= (struct p9_rstat *)pdu->in_iov[1].iov_base;
-	struct p9_fid *fid	= &p9dev->fids[tread->fid];
+	u64 offset;
+	u32 fid_val;
+	u32 count, rcount;
 	struct stat st;
+	struct p9_fid *fid;
+	struct p9_wstat wstat;
 
-	rread->count = 0;
-
+	rcount = 0;
+	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
+	fid = &p9dev->fids[fid_val];
 	if (fid->is_dir) {
 		/* If reading a dir, fill the buffer with p9_stat entries */
-		struct dirent *cur = readdir(fid->dir);
 		char full_path[PATH_MAX];
+		struct dirent *cur = readdir(fid->dir);
 
+		/* Skip the space for writing count */
+		pdu->write_offset += sizeof(u32);
 		while (cur) {
 			u32 read;
 
 			lstat(rel_to_abs(p9dev, cur->d_name, full_path), &st);
-			read = virtio_p9_fill_stat(p9dev, cur->d_name,
-						   &st, rstat);
-			rread->count += read;
-			rstat = (void *)rstat + read;
+			virtio_p9_fill_stat(p9dev, cur->d_name, &st, &wstat);
+
+			read = pdu->write_offset;
+			virtio_p9_pdu_writef(pdu, "S", &wstat);
+			rcount += pdu->write_offset - read;
+
 			cur = readdir(fid->dir);
 		}
 	} else {
 		pdu->in_iov[0].iov_base += VIRTIO_P9_HDR_LEN + sizeof(u32);
 		pdu->in_iov[0].iov_len -= VIRTIO_P9_HDR_LEN + sizeof(u32);
 		pdu->in_iov_cnt = virtio_p9_update_iov_cnt(pdu->in_iov,
-							    tread->count,
-							    pdu->in_iov_cnt);
-		rread->count = preadv(fid->fd, pdu->in_iov,
-				      pdu->in_iov_cnt, tread->offset);
-		if (rread->count > tread->count)
-			rread->count = tread->count;
-	}
+							   count,
+							   pdu->in_iov_cnt);
+		rcount = preadv(fid->fd, pdu->in_iov,
+				pdu->in_iov_cnt, offset);
+		if (rcount > count)
+			rcount = count;
+		/*
+		 * Update the iov_base back, so that rest of
+		 * pdu_writef works correctly.
+		 */
+		pdu->in_iov[0].iov_base -= VIRTIO_P9_HDR_LEN + sizeof(u32);
+		pdu->in_iov[0].iov_len += VIRTIO_P9_HDR_LEN + sizeof(u32);
 
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32) + rread->count;
-	set_p9msg_hdr(inmsg, *outlen, P9_RREAD, outmsg->tag);
+	}
+	pdu->write_offset = VIRTIO_P9_HDR_LEN;
+	virtio_p9_pdu_writef(pdu, "d", rcount);
+	*outlen = pdu->write_offset + rcount;
+	virtio_p9_set_reply_header(pdu, *outlen);
 
 	return;
 }
@@ -508,21 +442,21 @@  static void virtio_p9_read(struct p9_dev *p9dev,
 static void virtio_p9_stat(struct p9_dev *p9dev,
 			   struct p9_pdu *pdu, u32 *outlen)
 {
-	u32 ret;
+	u32 fid_val;
 	struct stat st;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_tstat *tstat = (struct p9_tstat *)outmsg->msg;
-	struct p9_rstat *rstat = (struct p9_rstat *)(inmsg->msg + sizeof(u16));
-	struct p9_fid *fid = &p9dev->fids[tstat->fid];
+	struct p9_fid *fid;
+	struct p9_wstat wstat;
 
+	virtio_p9_pdu_readf(pdu, "d", &fid_val);
+	fid = &p9dev->fids[fid_val];
 	if (lstat(fid->abs_path, &st) < 0)
 		goto err_out;
 
-	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
+	virtio_p9_fill_stat(p9dev, fid->path, &st, &wstat);
 
-	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16);
-	set_p9msg_hdr(inmsg, *outlen, P9_RSTAT, outmsg->tag);
+	virtio_p9_pdu_writef(pdu, "wS", 0, &wstat);
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 err_out:
 	virtio_p9_error_reply(p9dev, pdu, errno, outlen);
@@ -533,21 +467,21 @@  static void virtio_p9_wstat(struct p9_dev *p9dev,
 			    struct p9_pdu *pdu, u32 *outlen)
 {
 	int res = 0;
-	struct p9_str *str;
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_twstat *twstat = (struct p9_twstat *)outmsg->msg;
-	struct p9_fid *fid = &p9dev->fids[twstat->fid];
+	u32 fid_val;
+	u16 unused;
+	struct p9_fid *fid;
+	struct p9_wstat wstat;
 
+	virtio_p9_pdu_readf(pdu, "dwS", &fid_val, &unused, &wstat);
+	fid = &p9dev->fids[fid_val];
 
-	if (twstat->stat.length != -1UL)
-		res = ftruncate(fid->fd, twstat->stat.length);
+	if (wstat.length != -1UL)
+		res = ftruncate(fid->fd, wstat.length);
 
-	if (twstat->stat.mode != -1U)
-		chmod(fid->abs_path, twstat->stat.mode & 0xFFFF);
+	if (wstat.mode != -1U)
+		chmod(fid->abs_path, wstat.mode & 0xFFFF);
 
-	str = (void *)&twstat->stat.name + sizeof(u16);
-	if (str->len > 0) {
+	if (strlen(wstat.name) > 0) {
 		char new_name[PATH_MAX] = {0};
 		char full_path[PATH_MAX];
 		char *last_dir = strrchr(fid->path, '/');
@@ -556,57 +490,59 @@  static void virtio_p9_wstat(struct p9_dev *p9dev,
 		if (last_dir)
 			strncpy(new_name, fid->path, last_dir - fid->path + 1);
 
-		memcpy(new_name + strlen(new_name), &str->str, str->len);
+		memcpy(new_name + strlen(new_name),
+		       wstat.name, strlen(wstat.name));
 
 		/* fid is reused for the new file */
 		rename(fid->abs_path, rel_to_abs(p9dev, new_name, full_path));
 		sprintf(fid->path, "%s", new_name);
 	}
-
 	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(inmsg, *outlen, P9_RWSTAT, outmsg->tag);
-
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 }
 
 static void virtio_p9_remove(struct p9_dev *p9dev,
 			     struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_tremove *tremove = (struct p9_tremove *)outmsg->msg;
-	struct p9_fid *fid = &p9dev->fids[tremove->fid];
+	u32 fid_val;
+	struct p9_fid *fid;
 
-	close_fid(p9dev, tremove->fid);
+	virtio_p9_pdu_readf(pdu, "d", &fid_val);
+	fid = &p9dev->fids[fid_val];
+	close_fid(p9dev, fid_val);
 	if (fid->is_dir)
 		rmdir(fid->abs_path);
 	else
 		unlink(fid->abs_path);
 
 	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(inmsg, *outlen, P9_RREMOVE, outmsg->tag);
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 }
 
 static void virtio_p9_write(struct p9_dev *p9dev,
 			    struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *inmsg  = pdu->in_iov[0].iov_base;
-	struct p9_msg *outmsg = pdu->out_iov[0].iov_base;
-	struct p9_twrite *twrite = (struct p9_twrite *)outmsg->msg;
-	struct p9_rwrite *rwrite = (struct p9_rwrite *)inmsg->msg;
-	struct p9_fid *fid = &p9dev->fids[twrite->fid];
+	u64 offset;
+	u32 fid_val;
+	u32 count, rcount;
+	struct p9_fid *fid;
 
+	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
+	fid = &p9dev->fids[fid_val];
 
-	pdu->out_iov[0].iov_base += (sizeof(*outmsg) + sizeof(*twrite));
-	pdu->out_iov[0].iov_len -= (sizeof(*outmsg) + sizeof(*twrite));
-	pdu->out_iov_cnt = virtio_p9_update_iov_cnt(pdu->out_iov, twrite->count,
+	/* Adjust the iovec to skip the header and meta data */
+	pdu->out_iov[0].iov_base += (sizeof(struct p9_msg) +
+				     sizeof(struct p9_twrite));
+	pdu->out_iov[0].iov_len -=  (sizeof(struct p9_msg) +
+				     sizeof(struct p9_twrite));
+	pdu->out_iov_cnt = virtio_p9_update_iov_cnt(pdu->out_iov, count,
 						    pdu->out_iov_cnt);
-	rwrite->count = pwritev(fid->fd, pdu->out_iov,
-				pdu->out_iov_cnt, twrite->offset);
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32);
-	set_p9msg_hdr(inmsg, *outlen, P9_RWRITE, outmsg->tag);
-
+	rcount = pwritev(fid->fd, pdu->out_iov, pdu->out_iov_cnt, offset);
+	virtio_p9_pdu_writef(pdu, "d", rcount);
+	*outlen = pdu->write_offset;
+	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
 }
 
@@ -633,6 +569,9 @@  static struct p9_pdu *virtio_p9_pdu_init(struct kvm *kvm, struct virt_queue *vq)
 	if (!pdu)
 		return NULL;
 
+	/* skip the pdu header p9_msg */
+	pdu->read_offset  = VIRTIO_P9_HDR_LEN;
+	pdu->write_offset = VIRTIO_P9_HDR_LEN;
 	pdu->queue_head  = virt_queue__get_inout_iov(kvm, vq, pdu->in_iov,
 						     pdu->out_iov,
 						     &pdu->in_iov_cnt,