[PATCH 1/5] tools/kvm/9p: Use the same #define as the kernel

[PATCH 1/5] tools/kvm/9p: Use the same #define as the kernel

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 tools/kvm/virtio/9p.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 1545995..209760b 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -20,7 +20,7 @@
 #include <net/9p/9p.h>
 
 #define NUM_VIRT_QUEUES		1
-#define VIRTIO_P9_QUEUE_SIZE	128
+#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
@@ -114,7 +114,7 @@ static bool virtio_p9_pci_io_in(struct ioport *ioport, struct kvm *kvm,
 		ioport__write32(data, p9dev->vqs[p9dev->queue_selector].pfn);
 		break;
 	case VIRTIO_PCI_QUEUE_NUM:
-		ioport__write16(data, VIRTIO_P9_QUEUE_SIZE);
+		ioport__write16(data, VIRTQUEUE_NUM);
 		break;
 	case VIRTIO_PCI_STATUS:
 		ioport__write8(data, p9dev->status);
@@ -573,7 +573,7 @@ static bool virtio_p9_do_io_request(struct kvm *kvm, struct p9_dev_job *job)
 	p9_handler *handler;
 	struct virt_queue *vq;
 	struct p9_dev *p9dev;
-	struct iovec iov[VIRTIO_P9_QUEUE_SIZE];
+	struct iovec iov[VIRTQUEUE_NUM];
 
 	vq = job->vq;
 	p9dev = job->p9dev;
@@ -629,7 +629,7 @@ static bool virtio_p9_pci_io_out(struct ioport *ioport, struct kvm *kvm,
 		queue->pfn		= ioport__read32(data);
 		p			= guest_pfn_to_host(kvm, queue->pfn);
 
-		vring_init(&queue->vring, VIRTIO_P9_QUEUE_SIZE, p,
+		vring_init(&queue->vring, VIRTQUEUE_NUM, p,
 			   VIRTIO_PCI_VRING_ALIGN);
 
 		*job			= (struct p9_dev_job) {
-- 
1.7.4.1

[PATCH 2/5] tools/kvm/virtio: Add new iov helper

[Aneesh Kumar K.V]

This add the in and out iovec to seperate array

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 tools/kvm/include/kvm/virtio.h |    3 +++
 tools/kvm/virtio/core.c        |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index 66e6fdb..b962705 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -50,6 +50,9 @@ static inline void *guest_pfn_to_host(struct kvm *kvm, u32 pfn)
 struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, u32 head, u32 len);
 
 u16 virt_queue__get_iov(struct virt_queue *queue, struct iovec iov[], u16 *out, u16 *in, struct kvm *kvm);
+u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
+			      struct iovec in_iov[], struct iovec out_iov[],
+			      u16 *in, u16 *out);
 
 void virt_queue__trigger_irq(struct virt_queue *vq, int irq, u8 *isr, struct kvm *kvm);
 
diff --git a/tools/kvm/virtio/core.c b/tools/kvm/virtio/core.c
index 2b3503d..d28cfc6 100644
--- a/tools/kvm/virtio/core.c
+++ b/tools/kvm/virtio/core.c
@@ -58,6 +58,38 @@ u16 virt_queue__get_iov(struct virt_queue *queue, struct iovec iov[], u16 *out,
 	return head;
 }
 
+/* in and out are relative to guest */
+u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
+			      struct iovec in_iov[], struct iovec out_iov[],
+			      u16 *in, u16 *out)
+{
+	u16 head, idx;
+	struct vring_desc *desc;
+
+	idx = head = virt_queue__pop(queue);
+	*out = *in = 0;
+	do {
+		desc = virt_queue__get_desc(queue, idx);
+		if (desc->flags & VRING_DESC_F_WRITE) {
+			in_iov[*in].iov_base = guest_flat_to_host(kvm,
+								  desc->addr);
+			in_iov[*in].iov_len = desc->len;
+			(*in)++;
+		} else {
+			out_iov[*out].iov_base = guest_flat_to_host(kvm,
+								    desc->addr);
+			out_iov[*out].iov_len = desc->len;
+			(*out)++;
+		}
+		if (desc->flags & VRING_DESC_F_NEXT)
+			idx = desc->next;
+		else
+			break;
+	} while (1);
+	return head;
+}
+
+
 void virt_queue__trigger_irq(struct virt_queue *vq, int irq, u8 *isr, struct kvm *kvm)
 {
 	if (vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
-- 
1.7.4.1

[PATCH 3/5] tools/kvm/9p: Don't follow symlink on server

[Aneesh Kumar K.V]

Use lstat instead of stat

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 tools/kvm/virtio/9p.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 209760b..3172c1a 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -227,7 +227,7 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
 	struct p9_fid *new_fid	= &p9dev->fids[topen->fid];
 	struct stat st;
 
-	if (stat(new_fid->abs_path, &st) < 0)
+	if (lstat(new_fid->abs_path, &st) < 0)
 		return false;
 
 	st2qid(&st, &ropen->qid);
@@ -274,7 +274,7 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
 		fid->fd = open(fid->abs_path, omode2uflags(mode) | O_CREAT, 0777);
 	}
 
-	if (stat(fid->abs_path, &st) < 0)
+	if (lstat(fid->abs_path, &st) < 0)
 		return false;
 
 	st2qid(&st, &rcreate->qid);
@@ -308,7 +308,7 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
 			/* Format the new path we're 'walk'ing into */
 			sprintf(tmp, "%s/%.*s", fid->path, str->len, (char *)&str->str);
 
-			if (stat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
+			if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
 				break;
 
 			st2qid(&st, &rwalk->wqids[i]);
@@ -344,7 +344,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
 	for (i = 0; i < VIRTIO_P9_MAX_FID; i++)
 		p9dev->fids[i].fid = P9_NOFID;
 
-	if (stat(p9dev->root_dir, &st) < 0)
+	if (lstat(p9dev->root_dir, &st) < 0)
 		return false;
 
 	st2qid(&st, &rattach->qid);
@@ -421,7 +421,7 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
 		while (cur) {
 			u32 read;
 
-			stat(rel_to_abs(p9dev, cur->d_name, full_path), &st);
+			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;
@@ -453,7 +453,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
 	struct p9_fid *fid = &p9dev->fids[tstat->fid];
 	u32 ret;
 
-	if (stat(fid->abs_path, &st) < 0)
+	if (lstat(fid->abs_path, &st) < 0)
 		return false;
 
 	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
-- 
1.7.4.1

[PATCH 4/5] tools/kvm/9p: Fix the pdu len.

[Aneesh Kumar K.V]

The TSTAT output formwat is wS. So not sure whey we
need sizeof(u16)*2 in pdu length.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 tools/kvm/virtio/9p.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 3172c1a..ef2fbaf 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -458,7 +458,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
 
 	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
 
-	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16) * 2;
+	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16);
 	set_p9msg_hdr(outmsg, *outlen, P9_RSTAT, msg->tag);
 	return true;
 }
-- 
1.7.4.1

[PATCH 5/5] tools/kvm/9p: Simplify the handler

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 tools/kvm/include/net/9p/9p.h |    1 -
 tools/kvm/virtio/9p.c         |  261 +++++++++++++++++++++++------------------
 2 files changed, 147 insertions(+), 115 deletions(-)

diff --git a/tools/kvm/include/net/9p/9p.h b/tools/kvm/include/net/9p/9p.h
index cc11592..61ecff3 100644
--- a/tools/kvm/include/net/9p/9p.h
+++ b/tools/kvm/include/net/9p/9p.h
@@ -635,7 +635,6 @@ struct p9_twrite {
 	u32 fid;
 	u64 offset;
 	u32 count;
-	u8 *data;
 };
 
 struct p9_rwrite {
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index ef2fbaf..f2b9299 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -67,6 +67,15 @@ struct p9_dev {
 	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,
 			      const char *path, char *abs_path)
@@ -184,12 +193,12 @@ static void set_p9msg_hdr(struct p9_msg *msg, u32 size, u8 cmd, u16 tag)
 	};
 }
 
-static bool virtio_p9_version(struct p9_dev *p9dev, struct p9_msg *msg,
-			      u32 len, struct iovec *iov,
-			      int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_version(struct p9_dev *p9dev,
+			      struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_rversion *rversion = (struct p9_rversion *)outmsg->msg;
+	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);
@@ -197,35 +206,35 @@ static bool virtio_p9_version(struct p9_dev *p9dev, struct p9_msg *msg,
 
 	*outlen = VIRTIO_P9_HDR_LEN +
 		rversion->version.len + sizeof(u16) + sizeof(u32);
-	set_p9msg_hdr(outmsg, *outlen, P9_RVERSION, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RVERSION, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_clunk(struct p9_dev *p9dev, struct p9_msg *msg,
-			    u32 len, struct iovec *iov,
-			    int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_clunk(struct p9_dev *p9dev,
+			    struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_tclunk *tclunk = (struct p9_tclunk *)msg->msg;
+	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;
 
 	close_fid(p9dev, tclunk->fid);
 
 	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(outmsg, *outlen, P9_RCLUNK, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RCLUNK, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
-			   u32 len, struct iovec *iov,
-			   int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_open(struct p9_dev *p9dev,
+			   struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg	= iov[0].iov_base;
-	struct p9_topen *topen	= (struct p9_topen *)msg->msg;
-	struct p9_ropen *ropen	= (struct p9_ropen *)outmsg->msg;
-	struct p9_fid *new_fid	= &p9dev->fids[topen->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];
 
 	if (lstat(new_fid->abs_path, &st) < 0)
 		return false;
@@ -239,28 +248,29 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
 		new_fid->fd	= open(new_fid->abs_path, omode2uflags(topen->mode));
 
 	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*ropen);
-	set_p9msg_hdr(outmsg, *outlen, P9_ROPEN, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_ROPEN, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
-			     u32 len, struct iovec *iov,
-			     int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_create(struct p9_dev *p9dev,
+			     struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg		= iov[0].iov_base;
-	struct p9_tcreate *tcreate	= (struct p9_tcreate *)msg->msg;
-	struct p9_rcreate *rcreate	= (struct p9_rcreate *)outmsg->msg;
-	struct p9_fid *fid		= &p9dev->fids[tcreate->fid];
-	struct stat st;
 	u8 mode;
 	u32 perm;
+	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 *)msg + msg->size - 1);
-	perm = *(u32 *)((u8 *)msg + msg->size - 5);
+	mode = *((u8 *)outmsg + outmsg->size - 1);
+	perm = *(u32 *)((u8 *)outmsg + outmsg->size - 5);
 
 	sprintf(fid->path, "%s/%.*s", fid->path, tcreate->name.len, (char *)&tcreate->name.str);
 
@@ -280,21 +290,22 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
 	st2qid(&st, &rcreate->qid);
 
 	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rcreate);
-	set_p9msg_hdr(outmsg, *outlen, P9_RCREATE, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RCREATE, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
-			   u32 len, struct iovec *iov,
-			   int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_walk(struct p9_dev *p9dev,
+			   struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg	= iov[0].iov_base;
-	struct p9_twalk *twalk	= (struct p9_twalk *)msg->msg;
-	struct p9_rwalk *rwalk	= (struct p9_rwalk *)outmsg->msg;
+	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];
-	u8 i;
+
 
 	rwalk->nwqid = 0;
 	if (twalk->nwname) {
@@ -306,7 +317,8 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
 			struct stat st;
 
 			/* 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,
+				str->len, (char *)&str->str);
 
 			if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
 				break;
@@ -323,22 +335,23 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
 		new_fid->fid	= twalk->newfid;
 	}
 
-	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u16) + sizeof(struct p9_qid) * rwalk->nwqid;
-	set_p9msg_hdr(outmsg, *outlen, P9_RWALK, msg->tag);
+	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u16) +
+		sizeof(struct p9_qid)*rwalk->nwqid;
+	set_p9msg_hdr(inmsg, *outlen, P9_RWALK, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
-			     u32 len, struct iovec *iov,
-			     int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_attach(struct p9_dev *p9dev,
+			     struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_rattach *rattach = (struct p9_rattach *)outmsg->msg;
-	struct p9_tattach *tattach = (struct p9_tattach *)msg->msg;
+	u32 i;
 	struct stat st;
 	struct p9_fid *fid;
-	u32 i;
+	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;
 
 	/* Reset everything */
 	for (i = 0; i < VIRTIO_P9_MAX_FID; i++)
@@ -355,7 +368,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
 	strcpy(fid->path, "/");
 
 	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rattach);
-	set_p9msg_hdr(outmsg, *outlen, P9_RATTACH, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RATTACH, outmsg->tag);
 
 	return true;
 }
@@ -400,14 +413,14 @@ static u32 virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
 	return rstat->stat.size + sizeof(u16);
 }
 
-static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
-			   u32 len, struct iovec *iov,
-			   int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_read(struct p9_dev *p9dev,
+			   struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg	= iov[0].iov_base;
-	struct p9_tread *tread	= (struct p9_tread *)msg->msg;
-	struct p9_rread *rread	= (struct p9_rread *)outmsg->msg;
-	struct p9_rstat *rstat	= (struct p9_rstat *)iov[1].iov_base;
+	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];
 	struct stat st;
 
@@ -429,29 +442,30 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
 			cur = readdir(fid->dir);
 		}
 	} else {
-		iov[0].iov_base += VIRTIO_P9_HDR_LEN + sizeof(u32);
-		iov[0].iov_len -= VIRTIO_P9_HDR_LEN + sizeof(u32);
-		rread->count = preadv(fid->fd, iov, iniovcnt, tread->offset);
+		pdu->in_iov[0].iov_base += VIRTIO_P9_HDR_LEN + sizeof(u32);
+		pdu->in_iov[0].iov_len -= VIRTIO_P9_HDR_LEN + sizeof(u32);
+		rread->count = preadv(fid->fd, pdu->in_iov,
+				      pdu->in_iov_cnt, tread->offset);
 		if (rread->count > tread->count)
 			rread->count = tread->count;
 	}
 
 	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32) + rread->count;
-	set_p9msg_hdr(outmsg, *outlen, P9_RREAD, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RREAD, outmsg->tag);
 
 	return true;
 }
 
-static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
-			   u32 len, struct iovec *iov,
-			   int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_stat(struct p9_dev *p9dev,
+			   struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_tstat *tstat = (struct p9_tstat *)msg->msg;
-	struct p9_rstat *rstat = (struct p9_rstat *)(outmsg->msg + sizeof(u16));
+	u32 ret;
 	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];
-	u32 ret;
 
 	if (lstat(fid->abs_path, &st) < 0)
 		return false;
@@ -459,19 +473,20 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
 	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
 
 	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16);
-	set_p9msg_hdr(outmsg, *outlen, P9_RSTAT, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RSTAT, outmsg->tag);
 	return true;
 }
 
-static bool virtio_p9_wstat(struct p9_dev *p9dev, struct p9_msg *msg,
-			    u32 len, struct iovec *iov,
-			    int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_wstat(struct p9_dev *p9dev,
+			    struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_twstat *twstat = (struct p9_twstat *)msg->msg;
+	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];
-	int res = 0;
+
 
 	if (twstat->stat.length != -1UL)
 		res = ftruncate(fid->fd, twstat->stat.length);
@@ -497,17 +512,17 @@ static bool virtio_p9_wstat(struct p9_dev *p9dev, struct p9_msg *msg,
 	}
 
 	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(outmsg, *outlen, P9_RWSTAT, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RWSTAT, outmsg->tag);
 
 	return res == 0;
 }
 
-static bool virtio_p9_remove(struct p9_dev *p9dev, struct p9_msg *msg,
-			     u32 len, struct iovec *iov,
-			     int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_remove(struct p9_dev *p9dev,
+			     struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg = iov[0].iov_base;
-	struct p9_tremove *tremove = (struct p9_tremove *)msg->msg;
+	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];
 
 	close_fid(p9dev, tremove->fid);
@@ -517,39 +532,32 @@ static bool virtio_p9_remove(struct p9_dev *p9dev, struct p9_msg *msg,
 		unlink(fid->abs_path);
 
 	*outlen = VIRTIO_P9_HDR_LEN;
-	set_p9msg_hdr(outmsg, *outlen, P9_RREMOVE, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RREMOVE, outmsg->tag);
 	return true;
 }
 
-static bool virtio_p9_write(struct p9_dev *p9dev, struct p9_msg *msg,
-			    u32 len, struct iovec *iov,
-			    int outiovcnt, int iniovcnt, u32 *outlen)
+static bool virtio_p9_write(struct p9_dev *p9dev,
+			    struct p9_pdu *pdu, u32 *outlen)
 {
-	struct p9_msg *outmsg;
-	struct p9_rwrite *rwrite;
-	struct p9_twrite *twrite = (struct p9_twrite *)msg->msg;
+	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];
 
-	if (outiovcnt == 1) {
-		outmsg = iov[0].iov_base;
-		rwrite = (struct p9_rwrite *)outmsg->msg;
-		rwrite->count = pwrite(fid->fd, &twrite->data,
-				       twrite->count, twrite->offset);
-	} else {
-		outmsg = iov[2].iov_base;
-		rwrite = (struct p9_rwrite *)outmsg->msg;
-		rwrite->count = pwrite(fid->fd, iov[1].iov_base,
-				       twrite->count, twrite->offset);
-	}
+
+	pdu->out_iov[0].iov_base += (sizeof(*outmsg) + sizeof(*twrite));
+	pdu->out_iov[0].iov_len -= (sizeof(*outmsg) + sizeof(*twrite));
+	rwrite->count = pwritev(fid->fd, pdu->out_iov,
+				pdu->out_iov_cnt, twrite->offset);
 	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32);
-	set_p9msg_hdr(outmsg, *outlen, P9_RWRITE, msg->tag);
+	set_p9msg_hdr(inmsg, *outlen, P9_RWRITE, outmsg->tag);
 
 	return true;
 }
 
-typedef bool p9_handler(struct p9_dev *p9dev, struct p9_msg *msg,
-			u32 len, struct iovec *iov,
-			int outiovcnt, int iniovcnt, u32 *outlen);
+typedef bool p9_handler(struct p9_dev *p9dev,
+			struct p9_pdu *pdu, u32 *outlen);
 
 static p9_handler *virtio_9p_handler [] = {
 	[P9_TVERSION] = virtio_p9_version,
@@ -565,30 +573,55 @@ static p9_handler *virtio_9p_handler [] = {
 	[P9_TWRITE]   =	virtio_p9_write,
 };
 
+static struct p9_pdu *virtio_p9_pdu_init(struct kvm *kvm, struct virt_queue *vq)
+{
+	struct p9_pdu *pdu = calloc(1, sizeof(*pdu));
+	if (!pdu)
+		return NULL;
+
+	pdu->queue_head  = virt_queue__get_inout_iov(kvm, vq, pdu->in_iov,
+						     pdu->out_iov,
+						     &pdu->in_iov_cnt,
+						     &pdu->out_iov_cnt);
+	return pdu;
+}
+
+static u8 virtio_p9_get_cmd(struct p9_pdu *pdu)
+{
+	struct p9_msg *msg;
+	/*
+	 * we can peek directly into pdu for a u8
+	 * value. The host endianess won't be an issue
+	 */
+	msg = pdu->out_iov[0].iov_base;
+	return msg->cmd;
+}
+
 static bool virtio_p9_do_io_request(struct kvm *kvm, struct p9_dev_job *job)
 {
+	u8 cmd;
 	u32 len = 0;
-	u16 out, in, head;
-	struct p9_msg *msg;
 	p9_handler *handler;
-	struct virt_queue *vq;
 	struct p9_dev *p9dev;
-	struct iovec iov[VIRTQUEUE_NUM];
+	struct virt_queue *vq;
+	struct p9_pdu *p9pdu;
 
 	vq = job->vq;
 	p9dev = job->p9dev;
-	head  = virt_queue__get_iov(vq, iov, &out, &in, kvm);
-	msg   = iov[0].iov_base;
 
-	if (msg->cmd >= ARRAY_SIZE(virtio_9p_handler) ||
-	    !virtio_9p_handler[msg->cmd]) {
-		printf("Unsupported P9 message type: %u\n", msg->cmd);
+	p9pdu = virtio_p9_pdu_init(kvm, vq);
+	cmd = virtio_p9_get_cmd(p9pdu);
+
+	if (cmd >= ARRAY_SIZE(virtio_9p_handler) ||
+	    !virtio_9p_handler[cmd]) {
+		printf("Unsupported P9 message type: %u\n", cmd);
 
 	} else {
-		handler = virtio_9p_handler[msg->cmd];
-		handler(p9dev, msg, iov[0].iov_len, iov+1, out, in, &len);
+		handler = virtio_9p_handler[cmd];
+		handler(p9dev, p9pdu, &len);
 	}
-	virt_queue__set_used_elem(vq, head, len);
+	virt_queue__set_used_elem(vq, p9pdu->queue_head, len);
+	free(p9pdu);
 	return true;
 }
 
-- 
1.7.4.1

Re: [PATCH 1/5] tools/kvm/9p: Use the same #define as the kernel

[Sasha Levin]

On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  tools/kvm/virtio/9p.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index 1545995..209760b 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -20,7 +20,7 @@
>  #include <net/9p/9p.h>
>  
>  #define NUM_VIRT_QUEUES		1
> -#define VIRTIO_P9_QUEUE_SIZE	128
> +#define VIRTQUEUE_NUM	128

The change is ok, but it should be vertically aligned with the rest of
the defines there.

>  #define	VIRTIO_P9_DEFAULT_TAG	"kvm_9p"
>  #define VIRTIO_P9_HDR_LEN	(sizeof(u32)+sizeof(u8)+sizeof(u16))
>  #define VIRTIO_P9_MAX_FID	128
> @@ -114,7 +114,7 @@ static bool virtio_p9_pci_io_in(struct ioport *ioport, struct kvm *kvm,
>  		ioport__write32(data, p9dev->vqs[p9dev->queue_selector].pfn);
>  		break;
>  	case VIRTIO_PCI_QUEUE_NUM:
> -		ioport__write16(data, VIRTIO_P9_QUEUE_SIZE);
> +		ioport__write16(data, VIRTQUEUE_NUM);
>  		break;
>  	case VIRTIO_PCI_STATUS:
>  		ioport__write8(data, p9dev->status);
> @@ -573,7 +573,7 @@ static bool virtio_p9_do_io_request(struct kvm *kvm, struct p9_dev_job *job)
>  	p9_handler *handler;
>  	struct virt_queue *vq;
>  	struct p9_dev *p9dev;
> -	struct iovec iov[VIRTIO_P9_QUEUE_SIZE];
> +	struct iovec iov[VIRTQUEUE_NUM];
>  
>  	vq = job->vq;
>  	p9dev = job->p9dev;
> @@ -629,7 +629,7 @@ static bool virtio_p9_pci_io_out(struct ioport *ioport, struct kvm *kvm,
>  		queue->pfn		= ioport__read32(data);
>  		p			= guest_pfn_to_host(kvm, queue->pfn);
>  
> -		vring_init(&queue->vring, VIRTIO_P9_QUEUE_SIZE, p,
> +		vring_init(&queue->vring, VIRTQUEUE_NUM, p,
>  			   VIRTIO_PCI_VRING_ALIGN);
>  
>  		*job			= (struct p9_dev_job) {

-- 

Sasha.

Re: [PATCH 3/5] tools/kvm/9p: Don't follow symlink on server

[Sasha Levin]

On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> Use lstat instead of stat

This patch should probably come after 9p2000.u support is added, since
now we transparently work 'through' symlinks (because legacy 9p2000
doesn't support them).
Once 9p2000.u support is added we could represent symlinks and then a
switch to lstat might be due.

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  tools/kvm/virtio/9p.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index 209760b..3172c1a 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -227,7 +227,7 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
>  	struct p9_fid *new_fid	= &p9dev->fids[topen->fid];
>  	struct stat st;
>  
> -	if (stat(new_fid->abs_path, &st) < 0)
> +	if (lstat(new_fid->abs_path, &st) < 0)
>  		return false;
>  
>  	st2qid(&st, &ropen->qid);
> @@ -274,7 +274,7 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
>  		fid->fd = open(fid->abs_path, omode2uflags(mode) | O_CREAT, 0777);
>  	}
>  
> -	if (stat(fid->abs_path, &st) < 0)
> +	if (lstat(fid->abs_path, &st) < 0)
>  		return false;
>  
>  	st2qid(&st, &rcreate->qid);
> @@ -308,7 +308,7 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
>  			/* Format the new path we're 'walk'ing into */
>  			sprintf(tmp, "%s/%.*s", fid->path, str->len, (char *)&str->str);
>  
> -			if (stat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
> +			if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
>  				break;
>  
>  			st2qid(&st, &rwalk->wqids[i]);
> @@ -344,7 +344,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
>  	for (i = 0; i < VIRTIO_P9_MAX_FID; i++)
>  		p9dev->fids[i].fid = P9_NOFID;
>  
> -	if (stat(p9dev->root_dir, &st) < 0)
> +	if (lstat(p9dev->root_dir, &st) < 0)
>  		return false;
>  
>  	st2qid(&st, &rattach->qid);
> @@ -421,7 +421,7 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
>  		while (cur) {
>  			u32 read;
>  
> -			stat(rel_to_abs(p9dev, cur->d_name, full_path), &st);
> +			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;
> @@ -453,7 +453,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
>  	struct p9_fid *fid = &p9dev->fids[tstat->fid];
>  	u32 ret;
>  
> -	if (stat(fid->abs_path, &st) < 0)
> +	if (lstat(fid->abs_path, &st) < 0)
>  		return false;
>  
>  	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);

-- 

Sasha.

Re: [PATCH 4/5] tools/kvm/9p: Fix the pdu len.

[Sasha Levin]

On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> The TSTAT output formwat is wS. So not sure whey we
> need sizeof(u16)*2 in pdu length.

See http://ericvh.github.com/9p-rfc/rfc9p2000.html#anchor32

"BUGS

    To make the contents of a directory, such as returned by read(5),
easy to parse, each directory entry begins with a size field. For
consistency, the entries in Twstat and Rstat messages also contain their
size, which means the size appears twice. For example, the Rstat message
is formatted as ``(4+1+2+2+n)[4] Rstat tag[2] n[2] (n-2)[2] type[2]
dev[4]...,'' where n is the value returned by convD2M."



> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  tools/kvm/virtio/9p.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index 3172c1a..ef2fbaf 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -458,7 +458,7 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
>  
>  	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
>  
> -	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16) * 2;
> +	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16);
>  	set_p9msg_hdr(outmsg, *outlen, P9_RSTAT, msg->tag);
>  	return true;
>  }

-- 

Sasha.

Re: [PATCH 5/5] tools/kvm/9p: Simplify the handler

[Sasha Levin]

On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  tools/kvm/include/net/9p/9p.h |    1 -
>  tools/kvm/virtio/9p.c         |  261 +++++++++++++++++++++++------------------
>  2 files changed, 147 insertions(+), 115 deletions(-)

After applying the patches (within a 9p mounted dir):

 # mkdir new_dir
 # cp 10MB new_dir/
cp: cannot create regular file `new_dir/10MB': No such file or directory

> diff --git a/tools/kvm/include/net/9p/9p.h b/tools/kvm/include/net/9p/9p.h
> index cc11592..61ecff3 100644
> --- a/tools/kvm/include/net/9p/9p.h
> +++ b/tools/kvm/include/net/9p/9p.h
> @@ -635,7 +635,6 @@ struct p9_twrite {
>  	u32 fid;
>  	u64 offset;
>  	u32 count;
> -	u8 *data;
>  };
>  
>  struct p9_rwrite {
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index ef2fbaf..f2b9299 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -67,6 +67,15 @@ struct p9_dev {
>  	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,
>  			      const char *path, char *abs_path)
> @@ -184,12 +193,12 @@ static void set_p9msg_hdr(struct p9_msg *msg, u32 size, u8 cmd, u16 tag)
>  	};
>  }
>  
> -static bool virtio_p9_version(struct p9_dev *p9dev, struct p9_msg *msg,
> -			      u32 len, struct iovec *iov,
> -			      int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_version(struct p9_dev *p9dev,
> +			      struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_rversion *rversion = (struct p9_rversion *)outmsg->msg;
> +	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);
> @@ -197,35 +206,35 @@ static bool virtio_p9_version(struct p9_dev *p9dev, struct p9_msg *msg,
>  
>  	*outlen = VIRTIO_P9_HDR_LEN +
>  		rversion->version.len + sizeof(u16) + sizeof(u32);
> -	set_p9msg_hdr(outmsg, *outlen, P9_RVERSION, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RVERSION, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_clunk(struct p9_dev *p9dev, struct p9_msg *msg,
> -			    u32 len, struct iovec *iov,
> -			    int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_clunk(struct p9_dev *p9dev,
> +			    struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_tclunk *tclunk = (struct p9_tclunk *)msg->msg;
> +	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;
>  
>  	close_fid(p9dev, tclunk->fid);
>  
>  	*outlen = VIRTIO_P9_HDR_LEN;
> -	set_p9msg_hdr(outmsg, *outlen, P9_RCLUNK, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RCLUNK, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
> -			   u32 len, struct iovec *iov,
> -			   int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_open(struct p9_dev *p9dev,
> +			   struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg	= iov[0].iov_base;
> -	struct p9_topen *topen	= (struct p9_topen *)msg->msg;
> -	struct p9_ropen *ropen	= (struct p9_ropen *)outmsg->msg;
> -	struct p9_fid *new_fid	= &p9dev->fids[topen->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];
>  
>  	if (lstat(new_fid->abs_path, &st) < 0)
>  		return false;
> @@ -239,28 +248,29 @@ static bool virtio_p9_open(struct p9_dev *p9dev, struct p9_msg *msg,
>  		new_fid->fd	= open(new_fid->abs_path, omode2uflags(topen->mode));
>  
>  	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*ropen);
> -	set_p9msg_hdr(outmsg, *outlen, P9_ROPEN, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_ROPEN, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
> -			     u32 len, struct iovec *iov,
> -			     int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_create(struct p9_dev *p9dev,
> +			     struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg		= iov[0].iov_base;
> -	struct p9_tcreate *tcreate	= (struct p9_tcreate *)msg->msg;
> -	struct p9_rcreate *rcreate	= (struct p9_rcreate *)outmsg->msg;
> -	struct p9_fid *fid		= &p9dev->fids[tcreate->fid];
> -	struct stat st;
>  	u8 mode;
>  	u32 perm;
> +	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 *)msg + msg->size - 1);
> -	perm = *(u32 *)((u8 *)msg + msg->size - 5);
> +	mode = *((u8 *)outmsg + outmsg->size - 1);
> +	perm = *(u32 *)((u8 *)outmsg + outmsg->size - 5);
>  
>  	sprintf(fid->path, "%s/%.*s", fid->path, tcreate->name.len, (char *)&tcreate->name.str);
>  
> @@ -280,21 +290,22 @@ static bool virtio_p9_create(struct p9_dev *p9dev, struct p9_msg *msg,
>  	st2qid(&st, &rcreate->qid);
>  
>  	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rcreate);
> -	set_p9msg_hdr(outmsg, *outlen, P9_RCREATE, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RCREATE, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
> -			   u32 len, struct iovec *iov,
> -			   int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_walk(struct p9_dev *p9dev,
> +			   struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg	= iov[0].iov_base;
> -	struct p9_twalk *twalk	= (struct p9_twalk *)msg->msg;
> -	struct p9_rwalk *rwalk	= (struct p9_rwalk *)outmsg->msg;
> +	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];
> -	u8 i;
> +
>  
>  	rwalk->nwqid = 0;
>  	if (twalk->nwname) {
> @@ -306,7 +317,8 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
>  			struct stat st;
>  
>  			/* 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,
> +				str->len, (char *)&str->str);
>  
>  			if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
>  				break;
> @@ -323,22 +335,23 @@ static bool virtio_p9_walk(struct p9_dev *p9dev, struct p9_msg *msg,
>  		new_fid->fid	= twalk->newfid;
>  	}
>  
> -	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u16) + sizeof(struct p9_qid) * rwalk->nwqid;
> -	set_p9msg_hdr(outmsg, *outlen, P9_RWALK, msg->tag);
> +	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u16) +
> +		sizeof(struct p9_qid)*rwalk->nwqid;
> +	set_p9msg_hdr(inmsg, *outlen, P9_RWALK, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
> -			     u32 len, struct iovec *iov,
> -			     int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_attach(struct p9_dev *p9dev,
> +			     struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_rattach *rattach = (struct p9_rattach *)outmsg->msg;
> -	struct p9_tattach *tattach = (struct p9_tattach *)msg->msg;
> +	u32 i;
>  	struct stat st;
>  	struct p9_fid *fid;
> -	u32 i;
> +	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;
>  
>  	/* Reset everything */
>  	for (i = 0; i < VIRTIO_P9_MAX_FID; i++)
> @@ -355,7 +368,7 @@ static bool virtio_p9_attach(struct p9_dev *p9dev, struct p9_msg *msg,
>  	strcpy(fid->path, "/");
>  
>  	*outlen = VIRTIO_P9_HDR_LEN + sizeof(*rattach);
> -	set_p9msg_hdr(outmsg, *outlen, P9_RATTACH, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RATTACH, outmsg->tag);
>  
>  	return true;
>  }
> @@ -400,14 +413,14 @@ static u32 virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
>  	return rstat->stat.size + sizeof(u16);
>  }
>  
> -static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
> -			   u32 len, struct iovec *iov,
> -			   int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_read(struct p9_dev *p9dev,
> +			   struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg	= iov[0].iov_base;
> -	struct p9_tread *tread	= (struct p9_tread *)msg->msg;
> -	struct p9_rread *rread	= (struct p9_rread *)outmsg->msg;
> -	struct p9_rstat *rstat	= (struct p9_rstat *)iov[1].iov_base;
> +	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];
>  	struct stat st;
>  
> @@ -429,29 +442,30 @@ static bool virtio_p9_read(struct p9_dev *p9dev, struct p9_msg *msg,
>  			cur = readdir(fid->dir);
>  		}
>  	} else {
> -		iov[0].iov_base += VIRTIO_P9_HDR_LEN + sizeof(u32);
> -		iov[0].iov_len -= VIRTIO_P9_HDR_LEN + sizeof(u32);
> -		rread->count = preadv(fid->fd, iov, iniovcnt, tread->offset);
> +		pdu->in_iov[0].iov_base += VIRTIO_P9_HDR_LEN + sizeof(u32);
> +		pdu->in_iov[0].iov_len -= VIRTIO_P9_HDR_LEN + sizeof(u32);
> +		rread->count = preadv(fid->fd, pdu->in_iov,
> +				      pdu->in_iov_cnt, tread->offset);
>  		if (rread->count > tread->count)
>  			rread->count = tread->count;
>  	}
>  
>  	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32) + rread->count;
> -	set_p9msg_hdr(outmsg, *outlen, P9_RREAD, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RREAD, outmsg->tag);
>  
>  	return true;
>  }
>  
> -static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
> -			   u32 len, struct iovec *iov,
> -			   int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_stat(struct p9_dev *p9dev,
> +			   struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_tstat *tstat = (struct p9_tstat *)msg->msg;
> -	struct p9_rstat *rstat = (struct p9_rstat *)(outmsg->msg + sizeof(u16));
> +	u32 ret;
>  	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];
> -	u32 ret;
>  
>  	if (lstat(fid->abs_path, &st) < 0)
>  		return false;
> @@ -459,19 +473,20 @@ static bool virtio_p9_stat(struct p9_dev *p9dev, struct p9_msg *msg,
>  	ret = virtio_p9_fill_stat(p9dev, fid->path, &st, rstat);
>  
>  	*outlen = VIRTIO_P9_HDR_LEN + ret + sizeof(u16);
> -	set_p9msg_hdr(outmsg, *outlen, P9_RSTAT, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RSTAT, outmsg->tag);
>  	return true;
>  }
>  
> -static bool virtio_p9_wstat(struct p9_dev *p9dev, struct p9_msg *msg,
> -			    u32 len, struct iovec *iov,
> -			    int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_wstat(struct p9_dev *p9dev,
> +			    struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_twstat *twstat = (struct p9_twstat *)msg->msg;
> +	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];
> -	int res = 0;
> +
>  
>  	if (twstat->stat.length != -1UL)
>  		res = ftruncate(fid->fd, twstat->stat.length);
> @@ -497,17 +512,17 @@ static bool virtio_p9_wstat(struct p9_dev *p9dev, struct p9_msg *msg,
>  	}
>  
>  	*outlen = VIRTIO_P9_HDR_LEN;
> -	set_p9msg_hdr(outmsg, *outlen, P9_RWSTAT, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RWSTAT, outmsg->tag);
>  
>  	return res == 0;
>  }
>  
> -static bool virtio_p9_remove(struct p9_dev *p9dev, struct p9_msg *msg,
> -			     u32 len, struct iovec *iov,
> -			     int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_remove(struct p9_dev *p9dev,
> +			     struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg = iov[0].iov_base;
> -	struct p9_tremove *tremove = (struct p9_tremove *)msg->msg;
> +	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];
>  
>  	close_fid(p9dev, tremove->fid);
> @@ -517,39 +532,32 @@ static bool virtio_p9_remove(struct p9_dev *p9dev, struct p9_msg *msg,
>  		unlink(fid->abs_path);
>  
>  	*outlen = VIRTIO_P9_HDR_LEN;
> -	set_p9msg_hdr(outmsg, *outlen, P9_RREMOVE, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RREMOVE, outmsg->tag);
>  	return true;
>  }
>  
> -static bool virtio_p9_write(struct p9_dev *p9dev, struct p9_msg *msg,
> -			    u32 len, struct iovec *iov,
> -			    int outiovcnt, int iniovcnt, u32 *outlen)
> +static bool virtio_p9_write(struct p9_dev *p9dev,
> +			    struct p9_pdu *pdu, u32 *outlen)
>  {
> -	struct p9_msg *outmsg;
> -	struct p9_rwrite *rwrite;
> -	struct p9_twrite *twrite = (struct p9_twrite *)msg->msg;
> +	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];
>  
> -	if (outiovcnt == 1) {
> -		outmsg = iov[0].iov_base;
> -		rwrite = (struct p9_rwrite *)outmsg->msg;
> -		rwrite->count = pwrite(fid->fd, &twrite->data,
> -				       twrite->count, twrite->offset);
> -	} else {
> -		outmsg = iov[2].iov_base;
> -		rwrite = (struct p9_rwrite *)outmsg->msg;
> -		rwrite->count = pwrite(fid->fd, iov[1].iov_base,
> -				       twrite->count, twrite->offset);
> -	}
> +
> +	pdu->out_iov[0].iov_base += (sizeof(*outmsg) + sizeof(*twrite));
> +	pdu->out_iov[0].iov_len -= (sizeof(*outmsg) + sizeof(*twrite));
> +	rwrite->count = pwritev(fid->fd, pdu->out_iov,
> +				pdu->out_iov_cnt, twrite->offset);
>  	*outlen = VIRTIO_P9_HDR_LEN + sizeof(u32);
> -	set_p9msg_hdr(outmsg, *outlen, P9_RWRITE, msg->tag);
> +	set_p9msg_hdr(inmsg, *outlen, P9_RWRITE, outmsg->tag);
>  
>  	return true;
>  }
>  
> -typedef bool p9_handler(struct p9_dev *p9dev, struct p9_msg *msg,
> -			u32 len, struct iovec *iov,
> -			int outiovcnt, int iniovcnt, u32 *outlen);
> +typedef bool p9_handler(struct p9_dev *p9dev,
> +			struct p9_pdu *pdu, u32 *outlen);
>  
>  static p9_handler *virtio_9p_handler [] = {
>  	[P9_TVERSION] = virtio_p9_version,
> @@ -565,30 +573,55 @@ static p9_handler *virtio_9p_handler [] = {
>  	[P9_TWRITE]   =	virtio_p9_write,
>  };
>  
> +static struct p9_pdu *virtio_p9_pdu_init(struct kvm *kvm, struct virt_queue *vq)
> +{
> +	struct p9_pdu *pdu = calloc(1, sizeof(*pdu));
> +	if (!pdu)
> +		return NULL;
> +
> +	pdu->queue_head  = virt_queue__get_inout_iov(kvm, vq, pdu->in_iov,
> +						     pdu->out_iov,
> +						     &pdu->in_iov_cnt,
> +						     &pdu->out_iov_cnt);
> +	return pdu;
> +}
> +
> +static u8 virtio_p9_get_cmd(struct p9_pdu *pdu)
> +{
> +	struct p9_msg *msg;
> +	/*
> +	 * we can peek directly into pdu for a u8
> +	 * value. The host endianess won't be an issue
> +	 */
> +	msg = pdu->out_iov[0].iov_base;
> +	return msg->cmd;
> +}
> +
>  static bool virtio_p9_do_io_request(struct kvm *kvm, struct p9_dev_job *job)
>  {
> +	u8 cmd;
>  	u32 len = 0;
> -	u16 out, in, head;
> -	struct p9_msg *msg;
>  	p9_handler *handler;
> -	struct virt_queue *vq;
>  	struct p9_dev *p9dev;
> -	struct iovec iov[VIRTQUEUE_NUM];
> +	struct virt_queue *vq;
> +	struct p9_pdu *p9pdu;
>  
>  	vq = job->vq;
>  	p9dev = job->p9dev;
> -	head  = virt_queue__get_iov(vq, iov, &out, &in, kvm);
> -	msg   = iov[0].iov_base;
>  
> -	if (msg->cmd >= ARRAY_SIZE(virtio_9p_handler) ||
> -	    !virtio_9p_handler[msg->cmd]) {
> -		printf("Unsupported P9 message type: %u\n", msg->cmd);
> +	p9pdu = virtio_p9_pdu_init(kvm, vq);
> +	cmd = virtio_p9_get_cmd(p9pdu);
> +
> +	if (cmd >= ARRAY_SIZE(virtio_9p_handler) ||
> +	    !virtio_9p_handler[cmd]) {
> +		printf("Unsupported P9 message type: %u\n", cmd);
>  
>  	} else {
> -		handler = virtio_9p_handler[msg->cmd];
> -		handler(p9dev, msg, iov[0].iov_len, iov+1, out, in, &len);
> +		handler = virtio_9p_handler[cmd];
> +		handler(p9dev, p9pdu, &len);
>  	}
> -	virt_queue__set_used_elem(vq, head, len);
> +	virt_queue__set_used_elem(vq, p9pdu->queue_head, len);
> +	free(p9pdu);
>  	return true;
>  }
>  

-- 

Sasha.

Re: [PATCH 4/5] tools/kvm/9p: Fix the pdu len.

[Aneesh Kumar K.V]

On Sat, 18 Jun 2011 14:51:19 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > The TSTAT output formwat is wS. So not sure whey we
> > need sizeof(u16)*2 in pdu length.
> 
> See http://ericvh.github.com/9p-rfc/rfc9p2000.html#anchor32
> 
> "BUGS
> 
>     To make the contents of a directory, such as returned by read(5),
> easy to parse, each directory entry begins with a size field. For
> consistency, the entries in Twstat and Rstat messages also contain their
> size, which means the size appears twice. For example, the Rstat message
> is formatted as ``(4+1+2+2+n)[4] Rstat tag[2] n[2] (n-2)[2] type[2]
> dev[4]...,'' where n is the value returned by convD2M."
> 

The value returned by virtio_p9_fill_stat will include the accounting
for extra size field added to rstat reply. So we just need an additional
sizeof(u16). Not sizeof(u16) * 2 right ?

-aneesh

Re: [PATCH 5/5] tools/kvm/9p: Simplify the handler

[Aneesh Kumar K.V]

On Sat, 18 Jun 2011 14:51:34 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >  tools/kvm/include/net/9p/9p.h |    1 -
> >  tools/kvm/virtio/9p.c         |  261 +++++++++++++++++++++++------------------
> >  2 files changed, 147 insertions(+), 115 deletions(-)
> 
> After applying the patches (within a 9p mounted dir):
> 
>  # mkdir new_dir
>  # cp 10MB new_dir/
> cp: cannot create regular file `new_dir/10MB': No such file or directory
> 

works fine here

sh-2.05b# ls -al 10M
-rwxr-xr-x    1 4294967294 4294967294 10485760 Jun 19  2011 10M
sh-2.05b# mkdir new_dir
sh-2.05b# cp 10M new_dir/
sh-2.05b# 

Can you check the before and after behaviour with the patch ?

-aneesh

Re: [PATCH 1/5] tools/kvm/9p: Use the same #define as the kernel

[Aneesh Kumar K.V]

On Sat, 18 Jun 2011 14:50:25 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >  tools/kvm/virtio/9p.c |    8 ++++----
> >  1 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> > index 1545995..209760b 100644
> > --- a/tools/kvm/virtio/9p.c
> > +++ b/tools/kvm/virtio/9p.c
> > @@ -20,7 +20,7 @@
> >  #include <net/9p/9p.h>
> >  
> >  #define NUM_VIRT_QUEUES		1
> > -#define VIRTIO_P9_QUEUE_SIZE	128
> > +#define VIRTQUEUE_NUM	128
> 
> The change is ok, but it should be vertically aligned with the rest of
> the defines there.
> 

Will update

-aneesh

Re: [PATCH 3/5] tools/kvm/9p: Don't follow symlink on server

[Aneesh Kumar K.V]

On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > Use lstat instead of stat
> 
> This patch should probably come after 9p2000.u support is added, since
> now we transparently work 'through' symlinks (because legacy 9p2000
> doesn't support them).
> Once 9p2000.u support is added we could represent symlinks and then a
> switch to lstat might be due.
> 

Shouldn't we aim to support 9p2000.L directly rather than supporting all
the three versions of protocol ? Linux guest have good support for
9p2000.L

Do you think the patch breaks any of the expectation of 9p2000 ?. It
would be nice to get the correct file attributes when fetching
attributes from the server, irrespective of whether client support symlink
or not.

-aneesh

Re: [PATCH 4/5] tools/kvm/9p: Fix the pdu len.

[Sasha Levin]

On Sun, 2011-06-19 at 10:41 +0530, Aneesh Kumar K.V wrote:
> On Sat, 18 Jun 2011 14:51:19 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > > The TSTAT output formwat is wS. So not sure whey we
> > > need sizeof(u16)*2 in pdu length.
> > 
> > See http://ericvh.github.com/9p-rfc/rfc9p2000.html#anchor32
> > 
> > "BUGS
> > 
> >     To make the contents of a directory, such as returned by read(5),
> > easy to parse, each directory entry begins with a size field. For
> > consistency, the entries in Twstat and Rstat messages also contain their
> > size, which means the size appears twice. For example, the Rstat message
> > is formatted as ``(4+1+2+2+n)[4] Rstat tag[2] n[2] (n-2)[2] type[2]
> > dev[4]...,'' where n is the value returned by convD2M."
> > 
> 
> The value returned by virtio_p9_fill_stat will include the accounting
> for extra size field added to rstat reply. So we just need an additional
> sizeof(u16). Not sizeof(u16) * 2 right ?

virtio_p9_fill_stat() fills a stat structure as defined in section 13.9
of the RFC. rstat specifically requires an additional u16 before that
stat structure, so virtio_p9_fill_stat() doesn't take it into account.

Since that additional u16 is specific only to rstat - and is not needed
in other places which expect the stat structure (such as when reading a
directory), I wouldn't want virtio_p9_fill_stat() to take it into
account.

-- 

Sasha.

Re: [PATCH 3/5] tools/kvm/9p: Don't follow symlink on server

[Sasha Levin]

On Sun, 2011-06-19 at 10:47 +0530, Aneesh Kumar K.V wrote:
> On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > > Use lstat instead of stat
> > 
> > This patch should probably come after 9p2000.u support is added, since
> > now we transparently work 'through' symlinks (because legacy 9p2000
> > doesn't support them).
> > Once 9p2000.u support is added we could represent symlinks and then a
> > switch to lstat might be due.
> > 
> 
> Shouldn't we aim to support 9p2000.L directly rather than supporting all
> the three versions of protocol ? Linux guest have good support for
> 9p2000.L

I'm perfectly fine with adding support directly for 9p2000.L, I just
figured that we'd need to go through the 9p2000.u milestone before
getting there instead of getting to 9p2000.L directly.

> Do you think the patch breaks any of the expectation of 9p2000 ?. It
> would be nice to get the correct file attributes when fetching
> attributes from the server, irrespective of whether client support symlink
> or not.

The idea behind following symlinks was that we could make them
transparently work even without protocol support (yes, not exactly what
the protocol defines - but it made it much more useful).

Removing symlink following also changes more than just the lstat thing,
you'd also need O_NOFOLLOW when opening files for example.

-- 

Sasha.

Re: [PATCH 5/5] tools/kvm/9p: Simplify the handler

[Sasha Levin]

On Sun, 2011-06-19 at 10:43 +0530, Aneesh Kumar K.V wrote:
> On Sat, 18 Jun 2011 14:51:34 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > > ---
> > >  tools/kvm/include/net/9p/9p.h |    1 -
> > >  tools/kvm/virtio/9p.c         |  261 +++++++++++++++++++++++------------------
> > >  2 files changed, 147 insertions(+), 115 deletions(-)
> > 
> > After applying the patches (within a 9p mounted dir):
> > 
> >  # mkdir new_dir
> >  # cp 10MB new_dir/
> > cp: cannot create regular file `new_dir/10MB': No such file or directory
> > 
> 
> works fine here
> 
> sh-2.05b# ls -al 10M
> -rwxr-xr-x    1 4294967294 4294967294 10485760 Jun 19  2011 10M
> sh-2.05b# mkdir new_dir
> sh-2.05b# cp 10M new_dir/
> sh-2.05b# 
> 
> Can you check the before and after behaviour with the patch ?

It works ok before the patch here.

I've added debug to the mount command, looks like it was caused due to
patch 4/5 (the removal of one u16) - the host didn't quite like it :)

-- 

Sasha.

Re: [PATCH 3/5] tools/kvm/9p: Don't follow symlink on server

[Aneesh Kumar K.V]

On Sun, 19 Jun 2011 10:47:33 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Sat, 18 Jun 2011 14:51:13 -0400, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Sat, 2011-06-18 at 23:19 +0530, Aneesh Kumar K.V wrote:
> > > Use lstat instead of stat
> > 
> > This patch should probably come after 9p2000.u support is added, since
> > now we transparently work 'through' symlinks (because legacy 9p2000
> > doesn't support them).
> > Once 9p2000.u support is added we could represent symlinks and then a
> > switch to lstat might be due.
> > 
> 
> Shouldn't we aim to support 9p2000.L directly rather than supporting all
> the three versions of protocol ? Linux guest have good support for
> 9p2000.L
> 
> Do you think the patch breaks any of the expectation of 9p2000 ?. It
> would be nice to get the correct file attributes when fetching
> attributes from the server, irrespective of whether client support symlink
> or not.

I sent Message-Id: <1308474599-9832-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
which add error handling to tools/kvm/9p. I guess we don't want server
to follow symlink outside export path, hence open with O_NOFOLLOW.

-aneesh