[PATCH 1/7] kvm tools: Fix spelling mistake

[PATCH 1/7] kvm tools: Fix spelling mistake

[Sasha Levin]

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/guest_compat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/guest_compat.c b/tools/kvm/guest_compat.c
index c5bacb8..fac53f5 100644
--- a/tools/kvm/guest_compat.c
+++ b/tools/kvm/guest_compat.c
@@ -91,7 +91,7 @@ int compat__print_all_messages(void)
 
 		msg = list_first_entry(&messages, struct compat_message, list);
 
-		printf("\n\n*** Compatability Warning ***\n\n\t%s\n\n%s\n",
+		printf("\n\n*** Compatibility Warning ***\n\n\t%s\n\n%s\n",
 			msg->title, msg->desc);
 
 		list_del(&msg->list);
-- 
1.7.7

[PATCH 2/7] kvm tools: Use compat message per device instead of per instance

[Sasha Levin]

This prevents multiple messages for the same type of device.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-9p.h |    1 -
 tools/kvm/virtio/9p.c             |    6 ++++--
 tools/kvm/virtio/balloon.c        |    7 ++++---
 tools/kvm/virtio/blk.c            |    7 ++++---
 tools/kvm/virtio/console.c        |    8 +++++---
 tools/kvm/virtio/net.c            |    7 ++++---
 tools/kvm/virtio/rng.c            |    7 ++++---
 7 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
index 07084c3..3096df2 100644
--- a/tools/kvm/include/kvm/virtio-9p.h
+++ b/tools/kvm/include/kvm/virtio-9p.h
@@ -45,7 +45,6 @@ struct p9_dev {
 	struct virtio_pci	vpci;
 
 	struct virtio_9p_config	*config;
-	int			compat_id;
 	u32			features;
 
 	/* virtio queue */
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 22a2732..b9b92b8 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -20,6 +20,7 @@
 #include <net/9p/9p.h>
 
 static LIST_HEAD(devs);
+static int compat_id = -1;
 
 /* Warning: Immediately use value returned from this function */
 static const char *rel_to_abs(struct p9_dev *p9dev,
@@ -1148,7 +1149,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 	struct virt_queue *queue;
 	void *p;
 
-	compat__remove_message(p9dev->compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &p9dev->vqs[vq];
 	queue->pfn		= pfn;
@@ -1247,7 +1248,8 @@ int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name)
 
 	list_add(&p9dev->list, &devs);
 
-	p9dev->compat_id = compat__add_message("virtio-9p device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-9p device was not detected",
 						"While you have requested a virtio-9p device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c
index 0b79132..643392b 100644
--- a/tools/kvm/virtio/balloon.c
+++ b/tools/kvm/virtio/balloon.c
@@ -44,12 +44,12 @@ struct bln_dev {
 	u16			stat_count;
 	int			stat_waitfd;
 
-	int			compat_id;
 	struct virtio_balloon_config config;
 };
 
 static struct bln_dev bdev;
 extern struct kvm *kvm;
+static int compat_id = -1;
 
 static bool virtio_bln_do_io_request(struct kvm *kvm, struct bln_dev *bdev, struct virt_queue *queue)
 {
@@ -226,7 +226,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 	struct virt_queue *queue;
 	void *p;
 
-	compat__remove_message(bdev->compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &bdev->vqs[vq];
 	queue->pfn		= pfn;
@@ -280,7 +280,8 @@ void virtio_bln__init(struct kvm *kvm)
 		.get_size_vq		= get_size_vq,
 	};
 
-	bdev.compat_id = compat__add_message("virtio-balloon device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-balloon device was not detected",
 						"While you have requested a virtio-balloon device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index c508123..272613b 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -44,7 +44,6 @@ struct blk_dev {
 	struct virtio_pci		vpci;
 	struct virtio_blk_config	blk_config;
 	struct disk_image		*disk;
-	int				compat_id;
 	u32				features;
 
 	struct virt_queue		vqs[NUM_VIRT_QUEUES];
@@ -53,6 +52,7 @@ struct blk_dev {
 };
 
 static LIST_HEAD(bdevs);
+static int compat_id;
 
 static void virtio_blk_do_io_request(struct kvm *kvm, void *param)
 {
@@ -154,7 +154,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 	struct virt_queue *queue;
 	void *p;
 
-	compat__remove_message(bdev->compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &bdev->vqs[vq];
 	queue->pfn		= pfn;
@@ -220,7 +220,8 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 
 	list_add_tail(&bdev->list, &bdevs);
 
-	bdev->compat_id = compat__add_message("virtio-blk device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-blk device was not detected",
 						"While you have requested a virtio-blk device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index b880162..ef1ef0d 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -37,7 +37,6 @@ struct con_dev {
 	struct virt_queue		vqs[VIRTIO_CONSOLE_NUM_QUEUES];
 	struct virtio_console_config	config;
 	u32				features;
-	int				compat_id;
 
 	struct thread_pool__job		jobs[VIRTIO_CONSOLE_NUM_QUEUES];
 };
@@ -52,6 +51,8 @@ static struct con_dev cdev = {
 	},
 };
 
+static int compat_id = -1;
+
 /*
  * Interrupts are injected for hvc0 only.
  */
@@ -137,7 +138,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 
 	assert(vq < VIRTIO_CONSOLE_NUM_QUEUES);
 
-	compat__remove_message(cdev.compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &cdev.vqs[vq];
 	queue->pfn		= pfn;
@@ -188,7 +189,8 @@ void virtio_console__init(struct kvm *kvm)
 		.get_size_vq		= get_size_vq,
 	};
 
-	cdev.compat_id = compat__add_message("virtio-console device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-console device was not detected",
 						"While you have requested a virtio-console device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index b55725d..9c39a87 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -48,7 +48,6 @@ struct net_dev {
 	struct virt_queue		vqs[VIRTIO_NET_NUM_QUEUES];
 	struct virtio_net_config	config;
 	u32				features;
-	int				compat_id;
 
 	pthread_t			io_rx_thread;
 	pthread_mutex_t			io_rx_lock;
@@ -69,6 +68,7 @@ struct net_dev {
 };
 
 static LIST_HEAD(ndevs);
+static int compat_id = -1;
 
 static void *virtio_net_rx_thread(void *p)
 {
@@ -327,7 +327,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 	struct virt_queue *queue;
 	void *p;
 
-	compat__remove_message(ndev->compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &ndev->vqs[vq];
 	queue->pfn		= pfn;
@@ -413,7 +413,8 @@ void virtio_net__init(const struct virtio_net_params *params)
 
 	virtio_net__io_thread_init(params->kvm, ndev);
 
-	ndev->compat_id = compat__add_message("virtio-net device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-net device was not detected",
 						"While you have requested a virtio-net device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index 02308c2..17cd492 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -33,7 +33,6 @@ struct rng_dev {
 	struct virtio_pci	vpci;
 
 	int			fd;
-	int			compat_id;
 
 	/* virtio queue */
 	struct virt_queue	vqs[NUM_VIRT_QUEUES];
@@ -41,6 +40,7 @@ struct rng_dev {
 };
 
 static LIST_HEAD(rdevs);
+static int compat_id = -1;
 
 static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
 {
@@ -97,7 +97,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 pfn)
 	struct rng_dev_job *job;
 	void *p;
 
-	compat__remove_message(rdev->compat_id);
+	compat__remove_message(compat_id);
 
 	queue			= &rdev->vqs[vq];
 	queue->pfn		= pfn;
@@ -164,7 +164,8 @@ void virtio_rng__init(struct kvm *kvm)
 
 	list_add_tail(&rdev->list, &rdevs);
 
-	rdev->compat_id = compat__add_message("virtio-rng device was not detected",
+	if (compat_id != -1)
+		compat_id = compat__add_message("virtio-rng device was not detected",
 						"While you have requested a virtio-rng device, "
 						"the guest kernel didn't seem to detect it.\n"
 						"Please make sure that the kernel was compiled "
-- 
1.7.7

[PATCH 3/7] kvm tools: Improve compat message

[Sasha Levin]

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/9p.c      |    7 ++++---
 tools/kvm/virtio/balloon.c |    7 ++++---
 tools/kvm/virtio/blk.c     |    7 ++++---
 tools/kvm/virtio/console.c |    7 ++++---
 tools/kvm/virtio/net.c     |    7 ++++---
 tools/kvm/virtio/rng.c     |    7 ++++---
 6 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index b9b92b8..8dbd016 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -1251,9 +1251,10 @@ int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-9p device was not detected",
 						"While you have requested a virtio-9p device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_NET_9P_VIRTIO.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_NET_9P_VIRTIO=y enabled "
+						"in its .config");
 
 	return err;
 
diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c
index 643392b..1691b79 100644
--- a/tools/kvm/virtio/balloon.c
+++ b/tools/kvm/virtio/balloon.c
@@ -283,7 +283,8 @@ void virtio_bln__init(struct kvm *kvm)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-balloon device was not detected",
 						"While you have requested a virtio-balloon device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_VIRTIO_BALLOON.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_VIRTIO_BALLOON=y enabled "
+						"in its .config");
 }
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index 272613b..8605951 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -223,9 +223,10 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-blk device was not detected",
 						"While you have requested a virtio-blk device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_VIRTIO_BLK.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_VIRTIO_BLK=y enabled "
+						"in its .config");
 }
 
 void virtio_blk__init_all(struct kvm *kvm)
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index ef1ef0d..36997bd 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -192,7 +192,8 @@ void virtio_console__init(struct kvm *kvm)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-console device was not detected",
 						"While you have requested a virtio-console device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_VIRTIO_CONSOLE.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_VIRTIO_CONSOLE=y enabled "
+						"in its .config");
 }
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 9c39a87..bfa2c7b 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -416,7 +416,8 @@ void virtio_net__init(const struct virtio_net_params *params)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-net device was not detected",
 						"While you have requested a virtio-net device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_VIRTIO_NET.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_VIRTIO_NET=y enabled "
+						"in its .config");
 }
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index 17cd492..c7e7f2d 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -167,9 +167,10 @@ void virtio_rng__init(struct kvm *kvm)
 	if (compat_id != -1)
 		compat_id = compat__add_message("virtio-rng device was not detected",
 						"While you have requested a virtio-rng device, "
-						"the guest kernel didn't seem to detect it.\n"
-						"Please make sure that the kernel was compiled "
-						"with CONFIG_HW_RANDOM_VIRTIO.");
+						"the guest kernel did not initialize it.\n"
+						"Please make sure that the guest kernel was "
+						"compiled with CONFIG_HW_RANDOM_VIRTIO=y enabled "
+						"in its .config");
 }
 
 void virtio_rng__delete_all(struct kvm *kvm)
-- 
1.7.7

[PATCH 4/7] kvm tools: Add basic help when starting 'kvm' without args

[Sasha Levin]

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-help.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/builtin-help.c b/tools/kvm/builtin-help.c
index e70044a..344ff39 100644
--- a/tools/kvm/builtin-help.c
+++ b/tools/kvm/builtin-help.c
@@ -34,7 +34,8 @@ static void list_common_cmds_help(void)
 
 static void kvm_help(void)
 {
-	printf("\n usage: %s\n\n", kvm_usage_string);
+	printf("\n To start a simple non-privliged shell run 'kvm run'\n\n"
+		"usage: %s\n\n", kvm_usage_string);
 	list_common_cmds_help();
 	printf("\n %s\n\n", kvm_more_info_string);
 }
-- 
1.7.7

[PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Sasha Levin]

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-setup.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/builtin-setup.c b/tools/kvm/builtin-setup.c
index 3e569e7..a2e893c 100644
--- a/tools/kvm/builtin-setup.c
+++ b/tools/kvm/builtin-setup.c
@@ -47,6 +47,8 @@ static void parse_setup_options(int argc, const char **argv)
 
 void kvm_setup_help(void)
 {
+	printf("\nkvm setup creates a new rootfs and stores it under ~/.kvm-tools/ .\n"
+		"This can be used later by the '-d' parameter of 'kvm run'.\n");
 	usage_with_options(setup_usage, setup_options);
 }
 
@@ -205,10 +207,18 @@ int kvm_setup_create_new(const char *guestfs_name)
 
 int kvm_cmd_setup(int argc, const char **argv, const char *prefix)
 {
+	int r;
+
 	parse_setup_options(argc, argv);
 
 	if (instance_name == NULL)
 		kvm_setup_help();
 
-	return do_setup(instance_name);
+	r = do_setup(instance_name);
+	if (r == 0)
+		pr_info("Your new rootfs named %s has been created.\n"
+			"You can now start it by running 'kvm run -d %s'\n",
+			instance_name, instance_name);
+
+	return r;
 }
-- 
1.7.7

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> +	r = do_setup(instance_name);
> +	if (r == 0)
> +		pr_info("Your new rootfs named %s has been created.\n"
> +			"You can now start it by running 'kvm run -d %s'\n",
> +			instance_name, instance_name);

Btw., will 'kvm run' select the last-created rootfs by default?

The patches look good otherwise.

Thanks,

	Ingo

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Sasha Levin]

On Thu, 2011-10-13 at 12:00 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
> 
> > +	r = do_setup(instance_name);
> > +	if (r == 0)
> > +		pr_info("Your new rootfs named %s has been created.\n"
> > +			"You can now start it by running 'kvm run -d %s'\n",
> > +			instance_name, instance_name);
> 
> Btw., will 'kvm run' select the last-created rootfs by default?
> 
> The patches look good otherwise.

A simple 'kvm run' (creates if needed) and starts a rootfs named
'default'.

-- 

Sasha.

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Pekka Enberg]

On Thu, 13 Oct 2011, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
>
>> +	r = do_setup(instance_name);
>> +	if (r == 0)
>> +		pr_info("Your new rootfs named %s has been created.\n"
>> +			"You can now start it by running 'kvm run -d %s'\n",
>> +			instance_name, instance_name);
>
> Btw., will 'kvm run' select the last-created rootfs by default?

No, it runs rootfs that's named 'default'. We can change that but I'm not 
completely convinced running that last-created rootfs is the right thing 
to do here. Hmm.

> The patches look good otherwise.

Thanks for the review!

 			Pekka

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Ingo Molnar]

* Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> On Thu, 13 Oct 2011, Ingo Molnar wrote:
> >* Sasha Levin <levinsasha928@gmail.com> wrote:
> >
> >>+	r = do_setup(instance_name);
> >>+	if (r == 0)
> >>+		pr_info("Your new rootfs named %s has been created.\n"
> >>+			"You can now start it by running 'kvm run -d %s'\n",
> >>+			instance_name, instance_name);
> >
> >Btw., will 'kvm run' select the last-created rootfs by default?
> 
> No, it runs rootfs that's named 'default'. We can change that but 
> I'm not completely convinced running that last-created rootfs is 
> the right thing to do here. Hmm.

I'm not convinced either - just wanted to raise the issue.

Also, i raised this in the other thread, why not use .kvmtool of the 
current directory? That way it's cwd local like Git and perf. A 'kvm 
run' (union?) mount the cwd or so - so this would be a natural 
equivalent to chroot. The $HOME/.kvmtool is a Qemu-ish global 
workflow.

Thanks,

	Ingo

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Pekka Enberg]

On Thu, Oct 13, 2011 at 7:52 PM, Ingo Molnar <mingo@elte.hu> wrote:
>> >Btw., will 'kvm run' select the last-created rootfs by default?
>>
>> No, it runs rootfs that's named 'default'. We can change that but
>> I'm not completely convinced running that last-created rootfs is
>> the right thing to do here. Hmm.
>
> I'm not convinced either - just wanted to raise the issue.

Right. So 'kvm run' is supposed to setup and launch a 'default' rootfs
if no rootfs is specified.

> Also, i raised this in the other thread, why not use .kvmtool of the
> current directory? That way it's cwd local like Git and perf. A 'kvm
> run' (union?) mount the cwd or so - so this would be a natural
> equivalent to chroot. The $HOME/.kvmtool is a Qemu-ish global
> workflow.

Yeah, that definitely makes sense. 'kvm setup rootfs' wouldn't create
rootfs under $HOME/.kvmtool/rootfs but under $(PWD)/rootfs.

I guess we'll lose the ability to 'kvm list' all available rootfs
directories, though?

                        Pekka

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Asias He]

On 10/14/2011 02:22 AM, Pekka Enberg wrote:
> On Thu, Oct 13, 2011 at 7:52 PM, Ingo Molnar <mingo@elte.hu> wrote:
>>>> Btw., will 'kvm run' select the last-created rootfs by default?
>>>
>>> No, it runs rootfs that's named 'default'. We can change that but
>>> I'm not completely convinced running that last-created rootfs is
>>> the right thing to do here. Hmm.
>>
>> I'm not convinced either - just wanted to raise the issue.
> 
> Right. So 'kvm run' is supposed to setup and launch a 'default' rootfs
> if no rootfs is specified.
> 
>> Also, i raised this in the other thread, why not use .kvmtool of the
>> current directory? That way it's cwd local like Git and perf. A 'kvm
>> run' (union?) mount the cwd or so - so this would be a natural
>> equivalent to chroot. The $HOME/.kvmtool is a Qemu-ish global
>> workflow.
> 
> Yeah, that definitely makes sense. 'kvm setup rootfs' wouldn't create
> rootfs under $HOME/.kvmtool/rootfs but under $(PWD)/rootfs.

I also think $(PWD)/rootfs is much better than $HOME/.kvmtool/rootfs.
I think 'kvm setup $absolute_rootfs_path' make sense as well.

> I guess we'll lose the ability to 'kvm list' all available rootfs
> directories, though?

User can create and put the rootfs directories where he/she likes. We do
not need to track where the rootfs directories is, thus we do not need
to 'kvm list' them.

-- 
Asias He

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Ingo Molnar]

* Asias He <asias.hejun@gmail.com> wrote:

> On 10/14/2011 02:22 AM, Pekka Enberg wrote:
> > On Thu, Oct 13, 2011 at 7:52 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >>>> Btw., will 'kvm run' select the last-created rootfs by default?
> >>>
> >>> No, it runs rootfs that's named 'default'. We can change that but
> >>> I'm not completely convinced running that last-created rootfs is
> >>> the right thing to do here. Hmm.
> >>
> >> I'm not convinced either - just wanted to raise the issue.
> > 
> > Right. So 'kvm run' is supposed to setup and launch a 'default' rootfs
> > if no rootfs is specified.
> > 
> >> Also, i raised this in the other thread, why not use .kvmtool of the
> >> current directory? That way it's cwd local like Git and perf. A 'kvm
> >> run' (union?) mount the cwd or so - so this would be a natural
> >> equivalent to chroot. The $HOME/.kvmtool is a Qemu-ish global
> >> workflow.
> > 
> > Yeah, that definitely makes sense. 'kvm setup rootfs' wouldn't create
> > rootfs under $HOME/.kvmtool/rootfs but under $(PWD)/rootfs.
> 
> I also think $(PWD)/rootfs is much better than 
> $HOME/.kvmtool/rootfs. I think 'kvm setup $absolute_rootfs_path' 
> make sense as well.

The nice thing about $CWD and chroot is that it takes the current 
directly as the 'root directory' of the chroot environment.

The canonical extension of that would be to create .kvmtool/ in $CWD 
and to union-mount the files in the current directory on top of the 
.kvmtool 'binary environment' - or such.

Or, an approximation of that would be to simply mount $CWD as /home 
of the guest environment.

Thus $CWD/.kvmtool/default/ would be the default root fs (multiple 
instances can be created), and $CWD would be the default /home or 
/host filesystem.

> > I guess we'll lose the ability to 'kvm list' all available rootfs 
> > directories, though?
> 
> User can create and put the rootfs directories where he/she likes. 
> We do not need to track where the rootfs directories is, thus we do 
> not need to 'kvm list' them.

listing the current ones in $CWD would still be handy.

*If* users prefer some kind of global workflow then $HOME/.kvmtool 
can still be special and we can do a 'kvm list --all' kind of thing 
which would also list $HOME/.kvmtool instances.

Or 'kvm list' could list both $CWD and $HOME instances and would be 
able to run them. Naming clashes could be handled via some 
straightforward way, for example by naming the default rootfs in 
$HOME 'global' - it would be in $HOME/.kvmtool/global. The default 
one in $CWD would be named 'local' and be in $CWD/.kvmtool/local.

Or so.

Thanks,

	Ingo

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Pekka Enberg]

On Fri, Oct 14, 2011 at 9:08 AM, Ingo Molnar <mingo@elte.hu> wrote:
> The canonical extension of that would be to create .kvmtool/ in $CWD
> and to union-mount the files in the current directory on top of the
> .kvmtool 'binary environment' - or such.

Union mount is off the table for now because neither overlayfs nor
union mounts are in Linus' tree.

> Or, an approximation of that would be to simply mount $CWD as /home
> of the guest environment.
>
> Thus $CWD/.kvmtool/default/ would be the default root fs (multiple
> instances can be created), and $CWD would be the default /home or
> /host filesystem.

I like it!

Should we also introduce KVMTOOLDIR environment variable (much like
GIT_DIR) that can override the default $CWD?

                        Pekka

Re: [PATCH 5/7] kvm tools: Add help and info messages to 'kvm setup'

[Ingo Molnar]

* Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> On Fri, Oct 14, 2011 at 9:08 AM, Ingo Molnar <mingo@elte.hu> wrote:
> > The canonical extension of that would be to create .kvmtool/ in $CWD
> > and to union-mount the files in the current directory on top of the
> > .kvmtool 'binary environment' - or such.
> 
> Union mount is off the table for now because neither overlayfs nor
> union mounts are in Linus' tree.
> 
> > Or, an approximation of that would be to simply mount $CWD as /home
> > of the guest environment.
> >
> > Thus $CWD/.kvmtool/default/ would be the default root fs (multiple
> > instances can be created), and $CWD would be the default /home or
> > /host filesystem.
> 
> I like it!
> 
> Should we also introduce KVMTOOLDIR environment variable (much like 
> GIT_DIR) that can override the default $CWD?

Per KISS i'd suggest to only implement it on request - but allow an 
option to point to whatever explicit combination of rootfs and 
hostfs.

Thanks,

	Ingo

[PATCH 6/7] kvm tools: Modify 'kvm setup' parameters

[Sasha Levin]

This patch makes the name as the default parameter, this makes usability
easier since this is the only parameter available at the moment anyway.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-setup.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/builtin-setup.c b/tools/kvm/builtin-setup.c
index a2e893c..4410653 100644
--- a/tools/kvm/builtin-setup.c
+++ b/tools/kvm/builtin-setup.c
@@ -25,13 +25,11 @@
 static const char *instance_name;
 
 static const char * const setup_usage[] = {
-	"kvm setup [-n name]",
+	"kvm setup [name]",
 	NULL
 };
 
 static const struct option setup_options[] = {
-	OPT_GROUP("General options:"),
-	OPT_STRING('n', "name", &instance_name, "name", "Instance name"),
 	OPT_END()
 };
 
@@ -40,8 +38,12 @@ static void parse_setup_options(int argc, const char **argv)
 	while (argc != 0) {
 		argc = parse_options(argc, argv, setup_options, setup_usage,
 				PARSE_OPT_STOP_AT_NON_OPTION);
-		if (argc != 0)
+		if (argc != 0 && instance_name)
 			kvm_setup_help();
+		else
+			instance_name = argv[0];
+		argv++;
+		argc--;
 	}
 }
 
@@ -219,6 +221,8 @@ int kvm_cmd_setup(int argc, const char **argv, const char *prefix)
 		pr_info("Your new rootfs named %s has been created.\n"
 			"You can now start it by running 'kvm run -d %s'\n",
 			instance_name, instance_name);
+	else
+		perror("Error creating rootfs");
 
 	return r;
 }
-- 
1.7.7

[PATCH 7/7] kvm tools: Add option to list rootfs in 'kvm list'

[Sasha Levin]

Previously we would only list running instances, now that we create
rootfs instances it's appropriate to add a method to list them as
well.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-list.c |   68 ++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/builtin-list.c b/tools/kvm/builtin-list.c
index fcf9bb0..43a0aa8 100644
--- a/tools/kvm/builtin-list.c
+++ b/tools/kvm/builtin-list.c
@@ -4,6 +4,7 @@
 #include <kvm/kvm.h>
 #include <kvm/parse-options.h>
 
+#include <dirent.h>
 #include <stdio.h>
 #include <string.h>
 #include <signal.h>
@@ -11,12 +12,18 @@
 
 #define PROCESS_NAME "kvm"
 
+static bool run;
+static bool rootfs;
+
 static const char * const list_usage[] = {
 	"kvm list",
 	NULL
 };
 
 static const struct option list_options[] = {
+	OPT_GROUP("General options:"),
+	OPT_BOOLEAN('i', "run", &run, "List running instances"),
+	OPT_BOOLEAN('r', "rootfs", &rootfs, "List rootfs instances"),
 	OPT_END()
 };
 
@@ -58,9 +65,66 @@ cleanup:
 	return 0;
 }
 
-int kvm_cmd_list(int argc, const char **argv, const char *prefix)
+static int kvm_list_running_instances(void)
 {
 	printf("  PID GUEST\n");
 
-	return kvm__enumerate_instances(print_guest);
+	return kvm__enumerate_instances(print_guest);
+}
+
+static int kvm_list_rootfs(void)
+{
+	char name[PATH_MAX];
+	DIR *dir;
+	struct dirent *dirent;
+
+	snprintf(name, PATH_MAX, "%s%s", HOME_DIR, KVM_PID_FILE_PATH);
+	dir = opendir(name);
+	if (dir == NULL)
+		return -1;
+
+	printf("  ROOTFS\n");
+
+	while ((dirent = readdir(dir))) {
+		if (dirent->d_type == DT_DIR &&
+			strcmp(dirent->d_name, ".") &&
+			strcmp(dirent->d_name, ".."))
+			printf("%s\n", dirent->d_name);
+	}
+
+	return 0;
+}
+
+static void parse_setup_options(int argc, const char **argv)
+{
+	while (argc != 0) {
+		argc = parse_options(argc, argv, list_options, list_usage,
+				PARSE_OPT_STOP_AT_NON_OPTION);
+		if (argc != 0)
+			kvm_list_help();
+	}
+}
+
+int kvm_cmd_list(int argc, const char **argv, const char *prefix)
+{
+	int r;
+
+	parse_setup_options(argc, argv);
+
+	if (!run && !rootfs)
+		kvm_list_help();
+
+	if (run) {
+		r = kvm_list_running_instances();
+		if (r < 0)
+			perror("Error listing instances");
+	}
+
+	if (rootfs) {
+		r = kvm_list_rootfs();
+		if (r < 0)
+			perror("Error listing rootfs");
+	}
+
+	return 0;
 }
-- 
1.7.7

Re: [PATCH 7/7] kvm tools: Add option to list rootfs in 'kvm list'

[Pekka Enberg]

On Thu, 13 Oct 2011, Sasha Levin wrote:
> Previously we would only list running instances, now that we create
> rootfs instances it's appropriate to add a method to list them as
> well.
>
> Suggested-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

I applied the patch but we really ought to print out both of them by 
default. Something like this:

  PID            GUEST
1234            debian
<not running>   default

 			Pekka