[PATCH 1/2] kvm tools: convert callback to int and deal with the return value

[PATCH 1/2] kvm tools: convert callback to int and deal with the return value

[Liming Wang]

callback function maybe fail, so we should deal with the return value.

Signed-off-by: Liming Wang <walimisdev@gmail.com>
---
 tools/kvm/builtin-debug.c   |    7 +++----
 tools/kvm/builtin-list.c    |    9 ++++-----
 tools/kvm/builtin-pause.c   |    7 +++----
 tools/kvm/builtin-resume.c  |    7 +++----
 tools/kvm/builtin-stop.c    |    7 +++----
 tools/kvm/include/kvm/kvm.h |    2 +-
 tools/kvm/kvm.c             |    9 ++++++---
 7 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/tools/kvm/builtin-debug.c b/tools/kvm/builtin-debug.c
index 153badd..4cf3b7c 100644
--- a/tools/kvm/builtin-debug.c
+++ b/tools/kvm/builtin-debug.c
@@ -7,9 +7,9 @@
 #include <string.h>
 #include <signal.h>
 
-static void do_debug(const char *name, int pid)
+static int do_debug(const char *name, int pid)
 {
-	kill(pid, SIGQUIT);
+	return kill(pid, SIGQUIT);
 }
 
 int kvm_cmd_debug(int argc, const char **argv, const char *prefix)
@@ -20,8 +20,7 @@ int kvm_cmd_debug(int argc, const char **argv, const char *prefix)
 		die("Usage: kvm debug [instance name]\n");
 
 	if (strcmp(argv[0], "all") == 0) {
-		kvm__enumerate_instances(do_debug);
-		return 0;
+		return kvm__enumerate_instances(do_debug);
 	}
 
 	pid = kvm__get_pid_by_instance(argv[0]);
diff --git a/tools/kvm/builtin-list.c b/tools/kvm/builtin-list.c
index 89a0465..43f37f2 100644
--- a/tools/kvm/builtin-list.c
+++ b/tools/kvm/builtin-list.c
@@ -10,7 +10,7 @@
 
 #define PROCESS_NAME "kvm"
 
-static void print_guest(const char *name, int pid)
+static int print_guest(const char *name, int pid)
 {
 	char proc_name[PATH_MAX];
 	char *comm = NULL;
@@ -31,7 +31,7 @@ static void print_guest(const char *name, int pid)
 
 	fclose(fd);
 
-	return;
+	return 0;
 
 cleanup:
 	if (fd)
@@ -40,11 +40,10 @@ cleanup:
 		free(comm);
 
 	kvm__remove_pidfile(name);
+	return 0;
 }
 
 int kvm_cmd_list(int argc, const char **argv, const char *prefix)
 {
-	kvm__enumerate_instances(print_guest);
-
-	return 0;
+	return kvm__enumerate_instances(print_guest);
 }
diff --git a/tools/kvm/builtin-pause.c b/tools/kvm/builtin-pause.c
index 827b3b4..f5805e6 100644
--- a/tools/kvm/builtin-pause.c
+++ b/tools/kvm/builtin-pause.c
@@ -7,9 +7,9 @@
 #include <kvm/builtin-pause.h>
 #include <kvm/kvm.h>
 
-static void do_pause(const char *name, int pid)
+static int do_pause(const char *name, int pid)
 {
-	kill(pid, SIGUSR2);
+	return kill(pid, SIGUSR2);
 }
 
 int kvm_cmd_pause(int argc, const char **argv, const char *prefix)
@@ -20,8 +20,7 @@ int kvm_cmd_pause(int argc, const char **argv, const char *prefix)
 		die("Usage: kvm pause [instance name]\n");
 
 	if (strcmp(argv[0], "all") == 0) {
-		kvm__enumerate_instances(do_pause);
-		return 0;
+		return kvm__enumerate_instances(do_pause);
 	}
 
 	pid = kvm__get_pid_by_instance(argv[0]);
diff --git a/tools/kvm/builtin-resume.c b/tools/kvm/builtin-resume.c
index 4a85918..a9bf6c5 100644
--- a/tools/kvm/builtin-resume.c
+++ b/tools/kvm/builtin-resume.c
@@ -7,9 +7,9 @@
 #include <kvm/builtin-resume.h>
 #include <kvm/kvm.h>
 
-static void do_resume(const char *name, int pid)
+static int do_resume(const char *name, int pid)
 {
-	kill(pid, SIGKVMRESUME);
+	return kill(pid, SIGKVMRESUME);
 }
 
 int kvm_cmd_resume(int argc, const char **argv, const char *prefix)
@@ -20,8 +20,7 @@ int kvm_cmd_resume(int argc, const char **argv, const char *prefix)
 		die("Usage: kvm resume [instance name]\n");
 
 	if (strcmp(argv[0], "all") == 0) {
-		kvm__enumerate_instances(do_resume);
-		return 0;
+		return kvm__enumerate_instances(do_resume);
 	}
 
 	pid = kvm__get_pid_by_instance(argv[0]);
diff --git a/tools/kvm/builtin-stop.c b/tools/kvm/builtin-stop.c
index 7dd2015..46be393 100644
--- a/tools/kvm/builtin-stop.c
+++ b/tools/kvm/builtin-stop.c
@@ -7,9 +7,9 @@
 #include <string.h>
 #include <signal.h>
 
-static void do_stop(const char *name, int pid)
+static int do_stop(const char *name, int pid)
 {
-	kill(pid, SIGKVMSTOP);
+	return kill(pid, SIGKVMSTOP);
 }
 
 int kvm_cmd_stop(int argc, const char **argv, const char *prefix)
@@ -20,8 +20,7 @@ int kvm_cmd_stop(int argc, const char **argv, const char *prefix)
 		die("Usage: kvm stop [instance name]\n");
 
 	if (strcmp(argv[0], "all") == 0) {
-		kvm__enumerate_instances(do_stop);
-		return 0;
+		return kvm__enumerate_instances(do_stop);
 	}
 
 	pid = kvm__get_pid_by_instance(argv[0]);
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 4f12d11..1cbe3c7 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -72,7 +72,7 @@ void kvm__pause(void);
 void kvm__continue(void);
 void kvm__notify_paused(void);
 int kvm__get_pid_by_instance(const char *name);
-int kvm__enumerate_instances(void (*callback)(const char *name, int pid));
+int kvm__enumerate_instances(int (*callback)(const char *name, int pid));
 void kvm__remove_pidfile(const char *name);
 
 /*
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 107a922..740851d 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -164,12 +164,13 @@ int kvm__get_pid_by_instance(const char *name)
 	return pid;
 }
 
-int kvm__enumerate_instances(void (*callback)(const char *name, int pid))
+int kvm__enumerate_instances(int (*callback)(const char *name, int pid))
 {
 	char full_name[PATH_MAX];
 	int pid;
 	DIR *dir;
 	struct dirent entry, *result;
+	int ret = 0;
 
 	sprintf(full_name, "%s/%s", HOME_DIR, KVM_PID_FILE_PATH);
 	dir = opendir(full_name);
@@ -181,13 +182,15 @@ int kvm__enumerate_instances(void (*callback)(const char *name, int pid))
 		if (entry.d_type == DT_REG) {
 			entry.d_name[strlen(entry.d_name)-4] = 0;
 			pid = kvm__get_pid_by_instance(entry.d_name);
-			callback(entry.d_name, pid);
+			ret = callback(entry.d_name, pid);
+			if (ret < 0)
+				break;
 		}
 	}
 
 	closedir(dir);
 
-	return 0;
+	return ret;
 }
 
 void kvm__delete(struct kvm *kvm)
-- 
1.7.0.4

[PATCH 2/2] kvm tools: handle failure of command

[Liming Wang]

handle failure of calling command function, especially, only handle
EPERM error now.

Signed-off-by: Liming Wang <walimisdev@gmail.com>
---
 tools/kvm/kvm-cmd.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c
index 63a2dbf..6cd4270 100644
--- a/tools/kvm/kvm-cmd.c
+++ b/tools/kvm/kvm-cmd.c
@@ -15,6 +15,7 @@
 #include "kvm/builtin-help.h"
 #include "kvm/kvm-cmd.h"
 #include "kvm/builtin-run.h"
+#include "kvm/util.h"
 
 struct cmd_struct kvm_commands[] = {
 	{ "pause",	kvm_cmd_pause,		NULL,         0 },
@@ -59,6 +60,7 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
 {
 	struct cmd_struct *p;
 	const char *prefix = NULL;
+	int ret = 0;
 
 	if (!argv || !*argv) {
 		p = kvm_get_command(command, "help");
@@ -74,5 +76,12 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
 		return EINVAL;
 	}
 
-	return p->fn(argc - 1, &argv[1], prefix);
+	ret = p->fn(argc - 1, &argv[1], prefix);
+	if (ret < 0)
+	{
+		if (errno == EPERM)
+			die("Permission error - are you root?");
+	}
+
+	return ret;
 }
-- 
1.7.0.4

Re: [PATCH 2/2] kvm tools: handle failure of command

[Pekka Enberg]

On Wed, Aug 10, 2011 at 5:44 AM, Liming Wang <walimisdev@gmail.com> wrote:
> handle failure of calling command function, especially, only handle
> EPERM error now.
>
> Signed-off-by: Liming Wang <walimisdev@gmail.com>

I applied the patch because I absolutely loved the error handling
cleanups from the previous patch. However, out of curiosity, are you
seeing EPERM with some command in particular?

> ---
>  tools/kvm/kvm-cmd.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c
> index 63a2dbf..6cd4270 100644
> --- a/tools/kvm/kvm-cmd.c
> +++ b/tools/kvm/kvm-cmd.c
> @@ -15,6 +15,7 @@
>  #include "kvm/builtin-help.h"
>  #include "kvm/kvm-cmd.h"
>  #include "kvm/builtin-run.h"
> +#include "kvm/util.h"
>
>  struct cmd_struct kvm_commands[] = {
>        { "pause",      kvm_cmd_pause,          NULL,         0 },
> @@ -59,6 +60,7 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
>  {
>        struct cmd_struct *p;
>        const char *prefix = NULL;
> +       int ret = 0;
>
>        if (!argv || !*argv) {
>                p = kvm_get_command(command, "help");
> @@ -74,5 +76,12 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
>                return EINVAL;
>        }
>
> -       return p->fn(argc - 1, &argv[1], prefix);
> +       ret = p->fn(argc - 1, &argv[1], prefix);
> +       if (ret < 0)
> +       {
> +               if (errno == EPERM)
> +                       die("Permission error - are you root?");
> +       }
> +
> +       return ret;
>  }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 2/2] kvm tools: handle failure of command

[walimis]

On Wed, Aug 10, 2011 at 08:02:01AM +0300, Pekka Enberg wrote:
>On Wed, Aug 10, 2011 at 5:44 AM, Liming Wang <walimisdev@gmail.com> wrote:
>> handle failure of calling command function, especially, only handle
>> EPERM error now.
>>
>> Signed-off-by: Liming Wang <walimisdev@gmail.com>
>
>I applied the patch because I absolutely loved the error handling
>cleanups from the previous patch. However, out of curiosity, are you
>seeing EPERM with some command in particular?
Yes. Because I'm using ubuntu and if I don't use sudo to execute
"kvm pause", pause command can't work and doesn't prompt anything.
BTW, do you always login as "root" user?

walimis
>
>> ---
>> 爐ools/kvm/kvm-cmd.c | ? 11 ++++++++++-
>> ?1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/tools/kvm/kvm-cmd.c b/tools/kvm/kvm-cmd.c
>> index 63a2dbf..6cd4270 100644
>> --- a/tools/kvm/kvm-cmd.c
>> +++ b/tools/kvm/kvm-cmd.c
>> @@ -15,6 +15,7 @@
>> ?#include "kvm/builtin-help.h"
>> ?#include "kvm/kvm-cmd.h"
>> ?#include "kvm/builtin-run.h"
>> +#include "kvm/util.h"
>>
>> 爏truct cmd_struct kvm_commands[] = {
>> ? ? ? 爗 "pause", ? ? 爇vm_cmd_pause, ? ? ? ? 燦ULL, ? ? ? ? 0 },
>> @@ -59,6 +60,7 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
>> 爗
>> ? ? ? 爏truct cmd_struct *p;
>> ? ? ? 燾onst char *prefix = NULL;
>> + ? ? ? int ret = 0;
>>
>> ? ? ? 爄f (!argv || !*argv) {
>> ? ? ? ? ? ? ? 爌 = kvm_get_command(command, "help");
>> @@ -74,5 +76,12 @@ int handle_command(struct cmd_struct *command, int argc, const char **argv)
>> ? ? ? ? ? ? ? 爎eturn EINVAL;
>> ? ? ? 爙
>>
>> - ? ? ? return p->fn(argc - 1, &argv[1], prefix);
>> + ? ? ? ret = p->fn(argc - 1, &argv[1], prefix);
>> + ? ? ? if (ret < 0)
>> + ? ? ? {
>> + ? ? ? ? ? ? ? if (errno == EPERM)
>> + ? ? ? ? ? ? ? ? ? ? ? die("Permission error - are you root?");
>> + ? ? ? }
>> +
>> + ? ? ? return ret;
>> 爙
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at 爃ttp://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 2/2] kvm tools: handle failure of command

[Pekka Enberg]

On 8/10/11 8:03 AM, walimis wrote:
> On Wed, Aug 10, 2011 at 08:02:01AM +0300, Pekka Enberg wrote:
>> On Wed, Aug 10, 2011 at 5:44 AM, Liming Wang<walimisdev@gmail.com>  wrote:
>>> handle failure of calling command function, especially, only handle
>>> EPERM error now.
>>>
>>> Signed-off-by: Liming Wang<walimisdev@gmail.com>
>> I applied the patch because I absolutely loved the error handling
>> cleanups from the previous patch. However, out of curiosity, are you
>> seeing EPERM with some command in particular?
> Yes. Because I'm using ubuntu and if I don't use sudo to execute
> "kvm pause", pause command can't work and doesn't prompt anything.

That's odd. I don't use sudo with the tool either. Sasha?
> BTW, do you always login as "root" user?

Depends on which image you're using.

                             Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[walimis]

On Wed, Aug 10, 2011 at 08:30:19AM +0300, Pekka Enberg wrote:
>On 8/10/11 8:03 AM, walimis wrote:
>>On Wed, Aug 10, 2011 at 08:02:01AM +0300, Pekka Enberg wrote:
>>>On Wed, Aug 10, 2011 at 5:44 AM, Liming Wang<walimisdev@gmail.com>  wrote:
>>>>handle failure of calling command function, especially, only handle
>>>>EPERM error now.
>>>>
>>>>Signed-off-by: Liming Wang<walimisdev@gmail.com>
>>>I applied the patch because I absolutely loved the error handling
>>>cleanups from the previous patch. However, out of curiosity, are you
>>>seeing EPERM with some command in particular?
>>Yes. Because I'm using ubuntu and if I don't use sudo to execute
>>"kvm pause", pause command can't work and doesn't prompt anything.
>
>That's odd. I don't use sudo with the tool either. Sasha?
If I don't use sudo to run "kvm run", kvm prompt me:
  Fatal, could not open /dev/kvm: Permission denied

So I use sudo to execute "kvm run".
>>BTW, do you always login as "root" user?
>
>Depends on which image you're using.
I don't refer to the guest image. I mean that in your host PC, which user
you login and operate as.

walimis
>
>                            Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[Pekka Enberg]

On Wed, Aug 10, 2011 at 8:33 AM, walimis <walimisdev@gmail.com> wrote:
>>>Yes. Because I'm using ubuntu and if I don't use sudo to execute
>>>"kvm pause", pause command can't work and doesn't prompt anything.
>>
>>That's odd. I don't use sudo with the tool either. Sasha?
> If I don't use sudo to run "kvm run", kvm prompt me:
>  Fatal, could not open /dev/kvm: Permission denied

You should probably just add yourself to the 'kvm' group:

https://help.ubuntu.com/community/KVM/Installation

> So I use sudo to execute "kvm run".
>>>BTW, do you always login as "root" user?
>>
>>Depends on which image you're using.
> I don't refer to the guest image. I mean that in your host PC, which user
> you login and operate as.

Heh, I never login as 'root' to the host machine. :-)

                        Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[walimis]

On Wed, Aug 10, 2011 at 08:53:53AM +0300, Pekka Enberg wrote:
>On Wed, Aug 10, 2011 at 8:33 AM, walimis <walimisdev@gmail.com> wrote:
>>>>Yes. Because I'm using ubuntu and if I don't use sudo to execute
>>>>"kvm pause", pause command can't work and doesn't prompt anything.
>>>
>>>That's odd. I don't use sudo with the tool either. Sasha?
>> If I don't use sudo to run "kvm run", kvm prompt me:
>> 燜atal, could not open /dev/kvm: Permission denied
>
>You should probably just add yourself to the 'kvm' group:
>
>https://help.ubuntu.com/community/KVM/Installation
OK, that works for me to access /dev/kvm without sudo. Thanks.
But another sudo requirement is to use tap interface:
  Warning: Config tap device error. Are you root?

So how do you use tap interface in your environment?

walimis
>
>> So I use sudo to execute "kvm run".
>>>>BTW, do you always login as "root" user?
>>>
>>>Depends on which image you're using.
>> I don't refer to the guest image. I mean that in your host PC, which user
>> you login and operate as.
>
>Heh, I never login as 'root' to the host machine. :-)
>
>                        Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[walimis]

On Wed, Aug 10, 2011 at 09:35:12AM +0300, Pekka Enberg wrote:
>On Wed, Aug 10, 2011 at 9:12 AM, walimis <walimisdev@gmail.com> wrote:
>> On Wed, Aug 10, 2011 at 08:53:53AM +0300, Pekka Enberg wrote:
>>>On Wed, Aug 10, 2011 at 8:33 AM, walimis <walimisdev@gmail.com> wrote:
>>>>>>Yes. Because I'm using ubuntu and if I don't use sudo to execute
>>>>>>"kvm pause", pause command can't work and doesn't prompt anything.
>>>>>
>>>>>That's odd. I don't use sudo with the tool either. Sasha?
>>>> If I don't use sudo to run "kvm run", kvm prompt me:
>>>> 燜atal, could not open /dev/kvm: Permission denied
>>>
>>>You should probably just add yourself to the 'kvm' group:
>>>
>>>https://help.ubuntu.com/community/KVM/Installation
>> OK, that works for me to access /dev/kvm without sudo. Thanks.
>> But another sudo requirement is to use tap interface:
>>  Warning: Config tap device error. Are you root?
>>
>> So how do you use tap interface in your environment?
>
>I don't use it. We enable userspace networking by default so all you
>need to do in the guest is to make sure dhcp is enabled and you should
>have a working network setup. Does that not work for you?
I used to use tap to enable network for guest os, so that I can mount 
nfs rootfs. meanwhile, I can access the guest os from host os through the
network. For kvm tools, I don't know how to do that through userspace 
networking. Maybe we need a brief explanation in README.

walimis
>
>                        Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[Pekka Enberg]

On Wed, Aug 10, 2011 at 9:12 AM, walimis <walimisdev@gmail.com> wrote:
> On Wed, Aug 10, 2011 at 08:53:53AM +0300, Pekka Enberg wrote:
>>On Wed, Aug 10, 2011 at 8:33 AM, walimis <walimisdev@gmail.com> wrote:
>>>>>Yes. Because I'm using ubuntu and if I don't use sudo to execute
>>>>>"kvm pause", pause command can't work and doesn't prompt anything.
>>>>
>>>>That's odd. I don't use sudo with the tool either. Sasha?
>>> If I don't use sudo to run "kvm run", kvm prompt me:
>>> 燜atal, could not open /dev/kvm: Permission denied
>>
>>You should probably just add yourself to the 'kvm' group:
>>
>>https://help.ubuntu.com/community/KVM/Installation
> OK, that works for me to access /dev/kvm without sudo. Thanks.
> But another sudo requirement is to use tap interface:
>  Warning: Config tap device error. Are you root?
>
> So how do you use tap interface in your environment?

I don't use it. We enable userspace networking by default so all you
need to do in the guest is to make sure dhcp is enabled and you should
have a working network setup. Does that not work for you?

                        Pekka

Re: [PATCH 2/2] kvm tools: handle failure of command

[Pekka Enberg]

On Wed, Aug 10, 2011 at 9:32 AM, walimis <walimisdev@gmail.com> wrote:
> I used to use tap to enable network for guest os, so that I can mount
> nfs rootfs. meanwhile, I can access the guest os from host os through the
> network. For kvm tools, I don't know how to do that through userspace
> networking. Maybe we need a brief explanation in README.

Right. I've never used that. Asias is that supported?

Re: [PATCH 2/2] kvm tools: handle failure of command

[Asias He]

On 08/10/2011 02:49 PM, Pekka Enberg wrote:
> On Wed, Aug 10, 2011 at 9:32 AM, walimis <walimisdev@gmail.com> wrote:
>> I used to use tap to enable network for guest os, so that I can mount
>> nfs rootfs. meanwhile, I can access the guest os from host os through the
>> network. For kvm tools, I don't know how to do that through userspace
>> networking. Maybe we need a brief explanation in README.
> 
> Right. I've never used that. Asias is that supported?
> 

I have not tried nfs with our user mode network. But I think walimis can
do it with

	sudo ./kvm --network tap

We need sudo because we need CAP_NET_ADMIN to open /dev/net/tun.

By default, the ip address in host side is:

192.168.33.1/24

In guest side, you need to config a ip address for our guest:

	$ sudo ifconfig eth0 192.168.33.15
	$ sudo route add default gw 192.168.33.1
	$ ping 192.168.33.1

That's it.

If you want to access the real world in guest, try NAT or Bridge the tap
device.



-- 
Best Regards,
Asias He