All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm tools: dynamically allocate p9 fids
@ 2012-07-22 16:29 Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: support injecting arbitrary sysrqs Sasha Levin
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Sasha Levin @ 2012-07-22 16:29 UTC (permalink / raw)
  To: penberg, mingo, gorcunov; +Cc: kvm, Sasha Levin

This removes the limit for p9 fids and the huge fid array that came along with
it. Instead, it dynamically allocates fids and stores them in a rb-tree.

This is useful when the guest needs a lot of fids, such as when stress
testing guests.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-9p.h |    5 +-
 tools/kvm/virtio/9p.c             |  149 ++++++++++++++++++++++++++----------
 2 files changed, 110 insertions(+), 44 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
index 5902701..186fe05 100644
--- a/tools/kvm/include/kvm/virtio-9p.h
+++ b/tools/kvm/include/kvm/virtio-9p.h
@@ -7,12 +7,12 @@
 #include <sys/types.h>
 #include <dirent.h>
 #include <linux/list.h>
+#include <linux/rbtree.h>
 
 #define NUM_VIRT_QUEUES		1
 #define VIRTQUEUE_NUM		128
 #define	VIRTIO_9P_DEFAULT_TAG	"kvm_9p"
 #define VIRTIO_9P_HDR_LEN	(sizeof(u32)+sizeof(u8)+sizeof(u16))
-#define VIRTIO_9P_MAX_FID	16384
 #define VIRTIO_9P_VERSION_DOTL	"9P2000.L"
 #define MAX_TAG_LEN		32
 
@@ -31,6 +31,7 @@ struct p9_fid {
 	char			*path;
 	DIR			*dir;
 	int			fd;
+	struct rb_node		node;
 };
 
 struct p9_dev_job {
@@ -42,6 +43,7 @@ struct p9_dev_job {
 struct p9_dev {
 	struct list_head	list;
 	struct virtio_device	vdev;
+	struct rb_root		fids;
 
 	struct virtio_9p_config	*config;
 	u32			features;
@@ -49,7 +51,6 @@ struct p9_dev {
 	/* virtio queue */
 	struct virt_queue	vqs[NUM_VIRT_QUEUES];
 	struct p9_dev_job	jobs[NUM_VIRT_QUEUES];
-	struct p9_fid		fids[VIRTIO_9P_MAX_FID];
 	char			root_dir[PATH_MAX];
 };
 
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 830fc50..201ea95 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -22,12 +22,65 @@
 static LIST_HEAD(devs);
 static int compat_id = -1;
 
+static int insert_new_fid(struct p9_dev *dev, struct p9_fid *fid);
+static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid)
+{
+	struct rb_node *node = dev->fids.rb_node;
+	struct p9_fid *pfid = NULL;
+
+	while (node) {
+		struct p9_fid *cur = rb_entry(node, struct p9_fid, node);
+
+		if (fid < cur->fid) {
+			node = node->rb_left;
+		} else if (fid > cur->fid) {
+			node = node->rb_right;
+		} else {
+			return cur;
+		}
+	}
+
+	pfid = calloc(sizeof(*pfid), 1);
+	if (!pfid)
+		return NULL;
+
+	pfid->fid = fid;
+	strcpy(pfid->abs_path, dev->root_dir);
+	pfid->path = pfid->abs_path + strlen(dev->root_dir);
+
+	insert_new_fid(dev, pfid);
+
+	return pfid;
+}
+
+static int insert_new_fid(struct p9_dev *dev, struct p9_fid *fid)
+{
+	struct rb_node **node = &(dev->fids.rb_node), *parent = NULL;
+
+	while (*node) {
+		int result = fid->fid - rb_entry(*node, struct p9_fid, node)->fid;
+
+		parent = *node;
+		if (result < 0)
+			node    = &((*node)->rb_left);
+		else if (result > 0)
+			node    = &((*node)->rb_right);
+		else
+			return -EEXIST;
+	}
+
+	rb_link_node(&fid->node, parent, node);
+	rb_insert_color(&fid->node, &dev->fids);
+	return 0;
+}
+
 static struct p9_fid *get_fid(struct p9_dev *p9dev, int fid)
 {
-	if (fid >= VIRTIO_9P_MAX_FID)
-		die("virtio-9p max FID (%u) reached!", VIRTIO_9P_MAX_FID);
+	struct p9_fid *new;
+
+	new = find_or_create_fid(p9dev, fid);
 
-	return &p9dev->fids[fid];
+	return new;
 }
 
 /* Warning: Immediately use value returned from this function */
@@ -52,15 +105,36 @@ static void stat2qid(struct stat *st, struct p9_qid *qid)
 
 static void close_fid(struct p9_dev *p9dev, u32 fid)
 {
-	if (p9dev->fids[fid].fd > 0) {
-		close(p9dev->fids[fid].fd);
-		p9dev->fids[fid].fd = -1;
-	}
-	if (p9dev->fids[fid].dir) {
-		closedir(p9dev->fids[fid].dir);
-		p9dev->fids[fid].dir = NULL;
+	struct p9_fid *pfid = get_fid(p9dev, fid);
+
+	if (pfid->fd > 0)
+		close(pfid->fd);
+
+	if (pfid->dir)
+		closedir(pfid->dir);
+
+	rb_erase(&pfid->node, &p9dev->fids);
+	free(pfid);
+}
+
+static void clear_all_fids(struct p9_dev *p9dev)
+{
+	struct rb_node *node = rb_first(&p9dev->fids);
+
+	while (node) {
+		struct p9_fid *fid = rb_entry(node, struct p9_fid, node);
+
+		if (fid->fd > 0)
+			close(fid->fd);
+
+		if (fid->dir)
+			closedir(fid->dir);
+
+		rb_erase(&fid->node, &p9dev->fids);
+		free(fid);
+
+		node = rb_first(&p9dev->fids);
 	}
-	p9dev->fids[fid].fid = P9_NOFID;
 }
 
 static void virtio_p9_set_reply_header(struct p9_pdu *pdu, u32 size)
@@ -213,7 +287,6 @@ static void virtio_p9_create(struct p9_dev *p9dev,
 	fd = open(full_path, flags | O_CREAT, mode);
 	if (fd < 0)
 		goto err_out;
-	close_fid(p9dev, dfid_val);
 	dfid->fd = fd;
 
 	if (lstat(full_path, &st) < 0)
@@ -290,7 +363,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 	u16 nwqid;
 	u16 nwname;
 	struct p9_qid wqid;
-	struct p9_fid *new_fid;
+	struct p9_fid *new_fid, *old_fid;
 	u32 fid_val, newfid_val;
 
 
@@ -323,7 +396,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 			stat2qid(&st, &wqid);
 			new_fid->is_dir = S_ISDIR(st.st_mode);
 			strcpy(new_fid->path, tmp);
-			new_fid->fid = newfid_val;
 			new_fid->uid = fid->uid;
 			nwqid++;
 			virtio_p9_pdu_writef(pdu, "Q", &wqid);
@@ -333,10 +405,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 		 * 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;
-		new_fid->uid    = p9dev->fids[fid_val].uid;
+		old_fid = get_fid(p9dev, fid_val);
+		new_fid->is_dir = old_fid->is_dir;
+		strcpy(new_fid->path, old_fid->path);
+		new_fid->uid    = old_fid->uid;
 	}
 	*outlen = pdu->write_offset;
 	pdu->write_offset = VIRTIO_9P_HDR_LEN;
@@ -351,7 +423,6 @@ err_out:
 static void virtio_p9_attach(struct p9_dev *p9dev,
 			     struct p9_pdu *pdu, u32 *outlen)
 {
-	int i;
 	char *uname;
 	char *aname;
 	struct stat st;
@@ -365,9 +436,7 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
 	free(uname);
 	free(aname);
 
-	/* Reset everything */
-	for (i = 0; i < VIRTIO_9P_MAX_FID; i++)
-		p9dev->fids[i].fid = P9_NOFID;
+	clear_all_fids(p9dev);
 
 	if (lstat(p9dev->root_dir, &st) < 0)
 		goto err_out;
@@ -375,7 +444,6 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
 	stat2qid(&st, &qid);
 
 	fid = get_fid(p9dev, fid_val);
-	fid->fid = fid_val;
 	fid->uid = uid;
 	fid->is_dir = 1;
 	strcpy(fid->path, "/");
@@ -729,7 +797,6 @@ static void virtio_p9_rename(struct p9_dev *p9dev,
 	ret = rename(fid->abs_path, full_path);
 	if (ret < 0)
 		goto err_out;
-	close_fid(p9dev, fid_val);
 	*outlen = pdu->write_offset;
 	virtio_p9_set_reply_header(pdu, *outlen);
 	return;
@@ -1001,10 +1068,24 @@ static void virtio_p9_fix_path(char *fid_path, char *old_name, char *new_name)
 	return;
 }
 
+static void rename_fids(struct p9_dev *p9dev, char *old_name, char *new_name)
+{
+	struct rb_node *node = rb_first(&p9dev->fids);
+
+	while (node) {
+		struct p9_fid *fid = rb_entry(node, struct p9_fid, node);
+
+		if (fid->fid != P9_NOFID && virtio_p9_ancestor(fid->path, old_name)) {
+				virtio_p9_fix_path(fid->path, old_name, new_name);
+		}
+		node = rb_next(node);
+	}
+}
+
 static void virtio_p9_renameat(struct p9_dev *p9dev,
 			       struct p9_pdu *pdu, u32 *outlen)
 {
-	int i, ret;
+	int ret;
 	char *old_name, *new_name;
 	u32 old_dfid_val, new_dfid_val;
 	struct p9_fid *old_dfid, *new_dfid;
@@ -1026,13 +1107,7 @@ static void virtio_p9_renameat(struct p9_dev *p9dev,
 	 * Now fix path in other fids, if the renamed path is part of
 	 * that.
 	 */
-	for (i = 0; i < VIRTIO_9P_MAX_FID; i++) {
-		if (get_fid(p9dev, i)->fid != P9_NOFID &&
-		    virtio_p9_ancestor(get_fid(p9dev, i)->path, old_name)) {
-			virtio_p9_fix_path(get_fid(p9dev, i)->path, old_name,
-					   new_name);
-		}
-	}
+	rename_fids(p9dev, old_name, new_name);
 	free(old_name);
 	free(new_name);
 	*outlen = pdu->write_offset;
@@ -1289,7 +1364,6 @@ int virtio_9p__init(struct kvm *kvm)
 int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name)
 {
 	struct p9_dev *p9dev;
-	u32 i, root_len;
 	int err = 0;
 
 	p9dev = calloc(1, sizeof(*p9dev));
@@ -1306,15 +1380,6 @@ int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name)
 	}
 
 	strcpy(p9dev->root_dir, root);
-	root_len = strlen(root);
-	/*
-	 * We prefix the full path in all fids, This allows us to get the
-	 * absolute path of an fid without playing with strings.
-	 */
-	for (i = 0; i < VIRTIO_9P_MAX_FID; i++) {
-		strcpy(get_fid(p9dev, i)->abs_path, root);
-		get_fid(p9dev, i)->path = get_fid(p9dev, i)->abs_path + root_len;
-	}
 	p9dev->config->tag_len = strlen(tag_name);
 	if (p9dev->config->tag_len > MAX_TAG_LEN) {
 		err = -EINVAL;
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] kvm tools: support injecting arbitrary sysrqs
  2012-07-22 16:29 [PATCH] kvm tools: dynamically allocate p9 fids Sasha Levin
@ 2012-07-22 16:29 ` Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: remove unused field from virtio-blk Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2012-07-22 16:29 UTC (permalink / raw)
  To: penberg, mingo, gorcunov; +Cc: kvm, Sasha Levin

Add support to 'lkvm debug' to inject arbitrary sysrqs using a new
'-s <sysrq>' argument.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-debug.c             |    7 +++++++
 tools/kvm/builtin-run.c               |    5 ++++-
 tools/kvm/hw/serial.c                 |    7 +++----
 tools/kvm/include/kvm/8250-serial.h   |    2 +-
 tools/kvm/include/kvm/builtin-debug.h |    2 ++
 5 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/builtin-debug.c b/tools/kvm/builtin-debug.c
index 20e27ff..4ae51d2 100644
--- a/tools/kvm/builtin-debug.c
+++ b/tools/kvm/builtin-debug.c
@@ -16,6 +16,7 @@ static bool all;
 static int nmi = -1;
 static bool dump;
 static const char *instance_name;
+static const char *sysrq;
 
 static const char * const debug_usage[] = {
 	"lkvm debug [--all] [-n name] [-d] [-m vcpu]",
@@ -26,6 +27,7 @@ static const struct option debug_options[] = {
 	OPT_GROUP("General options:"),
 	OPT_BOOLEAN('d', "dump", &dump, "Generate a debug dump from guest"),
 	OPT_INTEGER('m', "nmi", &nmi, "Generate NMI on VCPU"),
+	OPT_STRING('s', "sysrq", &sysrq, "sysrq", "Inject a sysrq"),
 	OPT_GROUP("Instance options:"),
 	OPT_BOOLEAN('a', "all", &all, "Debug all instances"),
 	OPT_STRING('n', "name", &instance_name, "name", "Instance name"),
@@ -61,6 +63,11 @@ static int do_debug(const char *name, int sock)
 		cmd.cpu = nmi;
 	}
 
+	if (sysrq) {
+		cmd.dbg_type |= KVM_DEBUG_CMD_TYPE_SYSRQ;
+		cmd.sysrq = sysrq[0];
+	}
+
 	r = kvm_ipc__send_msg(sock, KVM_IPC_DEBUG, sizeof(cmd), (u8 *)&cmd);
 	if (r < 0)
 		return r;
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index a120fe2..750d30c 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -557,6 +557,9 @@ static void handle_debug(int fd, u32 type, u32 len, u8 *msg)
 	dbg_type = params->dbg_type;
 	vcpu = params->cpu;
 
+	if (dbg_type & KVM_DEBUG_CMD_TYPE_SYSRQ)
+		serial8250__inject_sysrq(kvm, params->sysrq);
+
 	if (dbg_type & KVM_DEBUG_CMD_TYPE_NMI) {
 		if ((int)vcpu >= kvm->nrcpus)
 			return;
@@ -589,7 +592,7 @@ static void handle_debug(int fd, u32 type, u32 len, u8 *msg)
 
 	close(fd);
 
-	serial8250__inject_sysrq(kvm);
+	serial8250__inject_sysrq(kvm, 'p');
 }
 
 static void handle_sigalrm(int sig)
diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
index 852aea4..956307c 100644
--- a/tools/kvm/hw/serial.c
+++ b/tools/kvm/hw/serial.c
@@ -153,14 +153,13 @@ static void serial8250_update_irq(struct kvm *kvm, struct serial8250_device *dev
 }
 
 #define SYSRQ_PENDING_NONE		0
-#define SYSRQ_PENDING_BREAK		1
 
 static int sysrq_pending;
 
 static void serial8250__sysrq(struct kvm *kvm, struct serial8250_device *dev)
 {
 	dev->lsr |= UART_LSR_DR | UART_LSR_BI;
-	dev->rxbuf[dev->rxcnt++] = 'p';
+	dev->rxbuf[dev->rxcnt++] = sysrq_pending;
 	sysrq_pending	= SYSRQ_PENDING_NONE;
 }
 
@@ -219,9 +218,9 @@ void serial8250__update_consoles(struct kvm *kvm)
 	}
 }
 
-void serial8250__inject_sysrq(struct kvm *kvm)
+void serial8250__inject_sysrq(struct kvm *kvm, char sysrq)
 {
-	sysrq_pending	= SYSRQ_PENDING_BREAK;
+	sysrq_pending = sysrq;
 }
 
 static struct serial8250_device *find_device(u16 port)
diff --git a/tools/kvm/include/kvm/8250-serial.h b/tools/kvm/include/kvm/8250-serial.h
index df8ef53..e954551 100644
--- a/tools/kvm/include/kvm/8250-serial.h
+++ b/tools/kvm/include/kvm/8250-serial.h
@@ -6,6 +6,6 @@ struct kvm;
 int serial8250__init(struct kvm *kvm);
 int serial8250__exit(struct kvm *kvm);
 void serial8250__update_consoles(struct kvm *kvm);
-void serial8250__inject_sysrq(struct kvm *kvm);
+void serial8250__inject_sysrq(struct kvm *kvm, char sysrq);
 
 #endif /* KVM__8250_SERIAL_H */
diff --git a/tools/kvm/include/kvm/builtin-debug.h b/tools/kvm/include/kvm/builtin-debug.h
index 6105a8c..efa0268 100644
--- a/tools/kvm/include/kvm/builtin-debug.h
+++ b/tools/kvm/include/kvm/builtin-debug.h
@@ -6,10 +6,12 @@
 
 #define KVM_DEBUG_CMD_TYPE_DUMP	(1 << 0)
 #define KVM_DEBUG_CMD_TYPE_NMI	(1 << 1)
+#define KVM_DEBUG_CMD_TYPE_SYSRQ (1 << 2)
 
 struct debug_cmd_params {
 	u32 dbg_type;
 	u32 cpu;
+	char sysrq;
 };
 
 int kvm_cmd_debug(int argc, const char **argv, const char *prefix);
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] kvm tools: remove unused field from virtio-blk
  2012-07-22 16:29 [PATCH] kvm tools: dynamically allocate p9 fids Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: support injecting arbitrary sysrqs Sasha Levin
@ 2012-07-22 16:29 ` Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: use correct error value for virtio-9p RLERROR Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: don't bother tracking is_dir Sasha Levin
  3 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2012-07-22 16:29 UTC (permalink / raw)
  To: penberg, mingo, gorcunov; +Cc: kvm, Sasha Levin

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/blk.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index beebd24..1fb969f 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -40,7 +40,6 @@ struct blk_dev {
 	pthread_mutex_t			mutex;
 
 	struct list_head		list;
-	struct list_head		req_list;
 
 	struct virtio_device		vdev;
 	struct virtio_blk_config	blk_config;
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] kvm tools: use correct error value for virtio-9p RLERROR
  2012-07-22 16:29 [PATCH] kvm tools: dynamically allocate p9 fids Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: support injecting arbitrary sysrqs Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: remove unused field from virtio-blk Sasha Levin
@ 2012-07-22 16:29 ` Sasha Levin
  2012-07-22 16:29 ` [PATCH] kvm tools: don't bother tracking is_dir Sasha Levin
  3 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2012-07-22 16:29 UTC (permalink / raw)
  To: penberg, mingo, gorcunov; +Cc: kvm, Sasha Levin

RLERROR expects positive error values, passing negative values causes guest kernel
panic.

Signed-off-by: Sasha Levin <levinsasha928@gmail.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 201ea95..c663dab 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -548,7 +548,7 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
 	fid = get_fid(p9dev, fid_val);
 
 	if (!fid->is_dir) {
-		errno = -EINVAL;
+		errno = EINVAL;
 		goto err_out;
 	}
 
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] kvm tools: don't bother tracking is_dir
  2012-07-22 16:29 [PATCH] kvm tools: dynamically allocate p9 fids Sasha Levin
                   ` (2 preceding siblings ...)
  2012-07-22 16:29 ` [PATCH] kvm tools: use correct error value for virtio-9p RLERROR Sasha Levin
@ 2012-07-22 16:29 ` Sasha Levin
  2012-07-31  6:24   ` Pekka Enberg
  3 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2012-07-22 16:29 UTC (permalink / raw)
  To: penberg, mingo, gorcunov; +Cc: kvm, Sasha Levin

This is something we can calculate on the fly, and doesn't justify the overhead
of tracking it all over fid transitions.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-9p.h |    1 -
 tools/kvm/virtio/9p.c             |   16 +++++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
index 186fe05..cb590d1 100644
--- a/tools/kvm/include/kvm/virtio-9p.h
+++ b/tools/kvm/include/kvm/virtio-9p.h
@@ -26,7 +26,6 @@ struct p9_msg {
 struct p9_fid {
 	u32			fid;
 	u32			uid;
-	u8			is_dir;
 	char			abs_path[PATH_MAX];
 	char			*path;
 	DIR			*dir;
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index c663dab..2cf99a0 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -228,6 +228,15 @@ static int virtio_p9_openflags(int flags)
 	return flags;
 }
 
+static bool is_dir(struct p9_fid *fid)
+{
+	struct stat st;
+
+	stat(fid->abs_path, &st);
+
+	return S_ISDIR(st.st_mode);
+}
+
 static void virtio_p9_open(struct p9_dev *p9dev,
 			   struct p9_pdu *pdu, u32 *outlen)
 {
@@ -245,7 +254,7 @@ static void virtio_p9_open(struct p9_dev *p9dev,
 
 	stat2qid(&st, &qid);
 
-	if (new_fid->is_dir) {
+	if (is_dir(new_fid)) {
 		new_fid->dir = opendir(new_fid->abs_path);
 		if (!new_fid->dir)
 			goto err_out;
@@ -394,7 +403,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 				goto err_out;
 
 			stat2qid(&st, &wqid);
-			new_fid->is_dir = S_ISDIR(st.st_mode);
 			strcpy(new_fid->path, tmp);
 			new_fid->uid = fid->uid;
 			nwqid++;
@@ -406,7 +414,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 		 */
 		pdu->write_offset += sizeof(u16);
 		old_fid = get_fid(p9dev, fid_val);
-		new_fid->is_dir = old_fid->is_dir;
 		strcpy(new_fid->path, old_fid->path);
 		new_fid->uid    = old_fid->uid;
 	}
@@ -445,7 +452,6 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
 
 	fid = get_fid(p9dev, fid_val);
 	fid->uid = uid;
-	fid->is_dir = 1;
 	strcpy(fid->path, "/");
 
 	virtio_p9_pdu_writef(pdu, "Q", &qid);
@@ -547,7 +553,7 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
 	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
 	fid = get_fid(p9dev, fid_val);
 
-	if (!fid->is_dir) {
+	if (!is_dir(fid)) {
 		errno = EINVAL;
 		goto err_out;
 	}
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kvm tools: don't bother tracking is_dir
  2012-07-22 16:29 ` [PATCH] kvm tools: don't bother tracking is_dir Sasha Levin
@ 2012-07-31  6:24   ` Pekka Enberg
  2012-08-01 18:10     ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Pekka Enberg @ 2012-07-31  6:24 UTC (permalink / raw)
  To: Sasha Levin; +Cc: mingo, gorcunov, kvm

On Sun, 22 Jul 2012, Sasha Levin wrote:
> This is something we can calculate on the fly, and doesn't justify the overhead
> of tracking it all over fid transitions.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

What kind of overhead are you talking about?

> ---
>  tools/kvm/include/kvm/virtio-9p.h |    1 -
>  tools/kvm/virtio/9p.c             |   16 +++++++++++-----
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
> index 186fe05..cb590d1 100644
> --- a/tools/kvm/include/kvm/virtio-9p.h
> +++ b/tools/kvm/include/kvm/virtio-9p.h
> @@ -26,7 +26,6 @@ struct p9_msg {
>  struct p9_fid {
>  	u32			fid;
>  	u32			uid;
> -	u8			is_dir;
>  	char			abs_path[PATH_MAX];
>  	char			*path;
>  	DIR			*dir;
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index c663dab..2cf99a0 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -228,6 +228,15 @@ static int virtio_p9_openflags(int flags)
>  	return flags;
>  }
>  
> +static bool is_dir(struct p9_fid *fid)
> +{
> +	struct stat st;
> +
> +	stat(fid->abs_path, &st);
> +
> +	return S_ISDIR(st.st_mode);
> +}
> +
>  static void virtio_p9_open(struct p9_dev *p9dev,
>  			   struct p9_pdu *pdu, u32 *outlen)
>  {
> @@ -245,7 +254,7 @@ static void virtio_p9_open(struct p9_dev *p9dev,
>  
>  	stat2qid(&st, &qid);
>  
> -	if (new_fid->is_dir) {
> +	if (is_dir(new_fid)) {
>  		new_fid->dir = opendir(new_fid->abs_path);
>  		if (!new_fid->dir)
>  			goto err_out;
> @@ -394,7 +403,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
>  				goto err_out;
>  
>  			stat2qid(&st, &wqid);
> -			new_fid->is_dir = S_ISDIR(st.st_mode);
>  			strcpy(new_fid->path, tmp);
>  			new_fid->uid = fid->uid;
>  			nwqid++;
> @@ -406,7 +414,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
>  		 */
>  		pdu->write_offset += sizeof(u16);
>  		old_fid = get_fid(p9dev, fid_val);
> -		new_fid->is_dir = old_fid->is_dir;
>  		strcpy(new_fid->path, old_fid->path);
>  		new_fid->uid    = old_fid->uid;
>  	}
> @@ -445,7 +452,6 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
>  
>  	fid = get_fid(p9dev, fid_val);
>  	fid->uid = uid;
> -	fid->is_dir = 1;
>  	strcpy(fid->path, "/");
>  
>  	virtio_p9_pdu_writef(pdu, "Q", &qid);
> @@ -547,7 +553,7 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
>  	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
>  	fid = get_fid(p9dev, fid_val);
>  
> -	if (!fid->is_dir) {
> +	if (!is_dir(fid)) {
>  		errno = EINVAL;
>  		goto err_out;
>  	}
> -- 
> 1.7.8.6
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kvm tools: don't bother tracking is_dir
  2012-07-31  6:24   ` Pekka Enberg
@ 2012-08-01 18:10     ` Sasha Levin
  0 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2012-08-01 18:10 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: mingo, gorcunov, kvm

On 07/31/2012 08:24 AM, Pekka Enberg wrote:
> On Sun, 22 Jul 2012, Sasha Levin wrote:
>> This is something we can calculate on the fly, and doesn't justify the overhead
>> of tracking it all over fid transitions.
>>
>> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> 
> What kind of overhead are you talking about?

Having to carry it between fids when they are copied/created.

This flag is required in only about 2-3 spots, and I've stumbled on a bunch of issues when writing the dynamic fid allocation due to failure to copy this flag over only when required.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-08-01 18:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-22 16:29 [PATCH] kvm tools: dynamically allocate p9 fids Sasha Levin
2012-07-22 16:29 ` [PATCH] kvm tools: support injecting arbitrary sysrqs Sasha Levin
2012-07-22 16:29 ` [PATCH] kvm tools: remove unused field from virtio-blk Sasha Levin
2012-07-22 16:29 ` [PATCH] kvm tools: use correct error value for virtio-9p RLERROR Sasha Levin
2012-07-22 16:29 ` [PATCH] kvm tools: don't bother tracking is_dir Sasha Levin
2012-07-31  6:24   ` Pekka Enberg
2012-08-01 18:10     ` Sasha Levin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.