diff mbox

[-V2] tools/kvm/9p: Add encode/decode routines for protocol data

Message ID 1309154715-7555-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aneesh Kumar K.V June 27, 2011, 6:05 a.m. UTC
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             |  479 ++++++++++++++++---------------------
 4 files changed, 527 insertions(+), 271 deletions(-)
 create mode 100644 tools/kvm/virtio/9p-pdu.c

Comments

Pekka Enberg June 27, 2011, 7:35 a.m. UTC | #1
This breaks 'make check':

./kvm run -d tests/boot/boot_test.iso -p "init=init"
   Error: unknown switch `d'

  usage: kvm run [<options>] [<kernel image>]

Basic options:
     -0, make: *** [check] Segmentation fault


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aneesh Kumar K.V June 27, 2011, 9:35 a.m. UTC | #2
On Mon, 27 Jun 2011 10:35:49 +0300 (EEST), Pekka Enberg <penberg@kernel.org> wrote:
> This breaks 'make check':
> 
> ./kvm run -d tests/boot/boot_test.iso -p "init=init"
>    Error: unknown switch `d'
> 
>   usage: kvm run [<options>] [<kernel image>]
> 
> Basic options:
>      -0, make: *** [check] Segmentation fault
> 
> 

I am able to reproduce this on debian x86_64. A simple test as below
kvm run -sda will result in segfault. I am find the option array
corrupt. Not yet sure why the patch would make a difference. Here
is the gdb dump of the structure

====Not working dump====
(gdb) p *(opts)
$5 = {type = OPTION_GROUP, short_name = 0, long_name = 0x0, value = 0x0, argh = 0x0, help = 0x40f2b0 "Basic options:", flags = 0,
  callback = 0, defval = 38654705664}
(gdb) p *(opts+1)
$6 = {type = 99, short_name = 4256447, long_name = 0x61551800000000 <Address 0x61551800000000 out of bounds>, value = 0x0, 
  argh = 0x40f2c400000000 <Address 0x40f2c400000000 out of bounds>, help = 0x0, flags = 0, callback = 0, defval = 468151435276}
(gdb) 


==== Working dump ====
(gdb) p *(opts)
$4 = {type = OPTION_GROUP, short_name = 0, long_name = 0x0, value = 0x0, argh = 0x0, help = 0x40e9b0 "Basic options:", flags = 0,
  callback = 0, defval = 0}
(gdb) p *(opts+1)
$5 = {type = OPTION_INTEGER, short_name = 99, long_name = 0x40e9bf "cpus", value = 0x6151c8, argh = 0x0, help = 0x40e9c4 "Number of CPUs", 
  flags = 0, callback = 0, defval = 0}
(gdb) 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aneesh Kumar K.V June 27, 2011, 10:28 a.m. UTC | #3
On Mon, 27 Jun 2011 15:05:13 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Mon, 27 Jun 2011 10:35:49 +0300 (EEST), Pekka Enberg <penberg@kernel.org> wrote:
> > This breaks 'make check':
> > 
> > ./kvm run -d tests/boot/boot_test.iso -p "init=init"
> >    Error: unknown switch `d'
> > 
> >   usage: kvm run [<options>] [<kernel image>]
> > 
> > Basic options:
> >      -0, make: *** [check] Segmentation fault
> > 
> > 
> 
> I am able to reproduce this on debian x86_64. A simple test as below
> kvm run -sda will result in segfault. I am find the option array
> corrupt. Not yet sure why the patch would make a difference. Here
> is the gdb dump of the structure
> 
> ====Not working dump====
> (gdb) p *(opts)
> $5 = {type = OPTION_GROUP, short_name = 0, long_name = 0x0, value = 0x0, argh = 0x0, help = 0x40f2b0 "Basic options:", flags = 0,
>   callback = 0, defval = 38654705664}
> (gdb) p *(opts+1)
> $6 = {type = 99, short_name = 4256447, long_name = 0x61551800000000 <Address 0x61551800000000 out of bounds>, value = 0x0, 
>   argh = 0x40f2c400000000 <Address 0x40f2c400000000 out of bounds>, help = 0x0, flags = 0, callback = 0, defval = 468151435276}
> (gdb) 
> 
> 
> ==== Working dump ====
> (gdb) p *(opts)
> $4 = {type = OPTION_GROUP, short_name = 0, long_name = 0x0, value = 0x0, argh = 0x0, help = 0x40e9b0 "Basic options:", flags = 0,
>   callback = 0, defval = 0}
> (gdb) p *(opts+1)
> $5 = {type = OPTION_INTEGER, short_name = 99, long_name = 0x40e9bf "cpus", value = 0x6151c8, argh = 0x0, help = 0x40e9c4 "Number of CPUs", 
>   flags = 0, callback = 0, defval = 0}
> (gdb) 


net/9p/9p.h had 

#pragma pack(1)

Moved the include into .c files. that fixed the issue

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

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..ddcb146 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..29efb23 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, (int)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;
 
-			/* Format the new path we're 'walk'ing into */
-			sprintf(tmp, "%s/%.*s", fid->path,
-				str->len, (char *)&str->str);
+			virtio_p9_pdu_readf(pdu, "s", &str);
 
+			/* Format the new path we're 'walk'ing into */
+			sprintf(tmp, "%s/%.*s",
+				fid->path, (int)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,