* [PATCH] kvm tool: rewrite kvm__init
@ 2012-02-09 5:40 Yang Bai
2012-02-09 13:01 ` Pekka Enberg
0 siblings, 1 reply; 8+ messages in thread
From: Yang Bai @ 2012-02-09 5:40 UTC (permalink / raw)
To: penberg; +Cc: kvm, Yang Bai
Since the different issues have been handled in the
internal of kvm__init, it can only return NULL if error
happened.
Signed-off-by: Yang Bai <hamo.by@gmail.com>
---
tools/kvm/builtin-run.c | 4 ++--
tools/kvm/kvm.c | 20 +++++++-------------
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 466169e..b71816f 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -997,8 +997,8 @@ static int kvm_cmd_run_init(int argc, const char **argv)
}
kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
- if (IS_ERR(kvm)) {
- r = PTR_ERR(kvm);
+ if (!kvm) {
+ r = -EFAULT;
goto fail;
}
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 9a0bd67..e3280a8 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -334,17 +334,17 @@ int kvm__max_cpus(struct kvm *kvm)
struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
{
- struct kvm *kvm;
+ struct kvm *kvm = NULL;
int ret;
if (!kvm__arch_cpu_supports_vm()) {
pr_err("Your CPU does not support hardware virtualization");
- return ERR_PTR(-ENOSYS);
+ return NULL;
}
kvm = kvm__new();
if (IS_ERR(kvm))
- return kvm;
+ return NULL;
kvm->sys_fd = open(kvm_dev, O_RDWR);
if (kvm->sys_fd < 0) {
@@ -358,32 +358,26 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s
else
pr_err("Could not open %s: ", kvm_dev);
- ret = -errno;
goto err_free;
}
ret = ioctl(kvm->sys_fd, KVM_GET_API_VERSION, 0);
if (ret != KVM_API_VERSION) {
pr_err("KVM_API_VERSION ioctl");
- ret = -errno;
goto err_sys_fd;
}
kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
- if (kvm->vm_fd < 0) {
- ret = kvm->vm_fd;
+ if (kvm->vm_fd < 0)
goto err_sys_fd;
- }
kvm->name = strdup(name);
- if (!kvm->name) {
- ret = -ENOMEM;
+ if (!kvm->name)
goto err;
- }
if (kvm__check_extensions(kvm)) {
pr_err("A required KVM extention is not supported by OS");
- ret = -ENOSYS;
+ goto err;
}
kvm__arch_init(kvm, hugetlbfs_path, ram_size);
@@ -400,7 +394,7 @@ err_sys_fd:
err_free:
free(kvm);
- return ERR_PTR(ret);
+ return NULL;
}
/* RFC 1952 */
--
1.7.8.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-09 5:40 [PATCH] kvm tool: rewrite kvm__init Yang Bai
@ 2012-02-09 13:01 ` Pekka Enberg
2012-02-09 13:07 ` Cyrill Gorcunov
0 siblings, 1 reply; 8+ messages in thread
From: Pekka Enberg @ 2012-02-09 13:01 UTC (permalink / raw)
To: Yang Bai; +Cc: kvm, Sasha Levin, Asias He, Cyrill Gorcunov, Ingo Molnar
On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
> Since the different issues have been handled in the
> internal of kvm__init, it can only return NULL if error
> happened.
>
> Signed-off-by: Yang Bai <hamo.by@gmail.com>
Sorry, I don't understand what this patch is attempting to fix? Why do
you think it's better to drop the explicit error codes and always
return NULL upon error?
> ---
> tools/kvm/builtin-run.c | 4 ++--
> tools/kvm/kvm.c | 20 +++++++-------------
> 2 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 466169e..b71816f 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -997,8 +997,8 @@ static int kvm_cmd_run_init(int argc, const char **argv)
> }
>
> kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> - if (IS_ERR(kvm)) {
> - r = PTR_ERR(kvm);
> + if (!kvm) {
> + r = -EFAULT;
> goto fail;
> }
>
> diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> index 9a0bd67..e3280a8 100644
> --- a/tools/kvm/kvm.c
> +++ b/tools/kvm/kvm.c
> @@ -334,17 +334,17 @@ int kvm__max_cpus(struct kvm *kvm)
>
> struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
> {
> - struct kvm *kvm;
> + struct kvm *kvm = NULL;
> int ret;
>
> if (!kvm__arch_cpu_supports_vm()) {
> pr_err("Your CPU does not support hardware virtualization");
> - return ERR_PTR(-ENOSYS);
> + return NULL;
> }
>
> kvm = kvm__new();
> if (IS_ERR(kvm))
> - return kvm;
> + return NULL;
>
> kvm->sys_fd = open(kvm_dev, O_RDWR);
> if (kvm->sys_fd < 0) {
> @@ -358,32 +358,26 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s
> else
> pr_err("Could not open %s: ", kvm_dev);
>
> - ret = -errno;
> goto err_free;
> }
>
> ret = ioctl(kvm->sys_fd, KVM_GET_API_VERSION, 0);
> if (ret != KVM_API_VERSION) {
> pr_err("KVM_API_VERSION ioctl");
> - ret = -errno;
> goto err_sys_fd;
> }
>
> kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
> - if (kvm->vm_fd < 0) {
> - ret = kvm->vm_fd;
> + if (kvm->vm_fd < 0)
> goto err_sys_fd;
> - }
>
> kvm->name = strdup(name);
> - if (!kvm->name) {
> - ret = -ENOMEM;
> + if (!kvm->name)
> goto err;
> - }
>
> if (kvm__check_extensions(kvm)) {
> pr_err("A required KVM extention is not supported by OS");
> - ret = -ENOSYS;
> + goto err;
> }
>
> kvm__arch_init(kvm, hugetlbfs_path, ram_size);
> @@ -400,7 +394,7 @@ err_sys_fd:
> err_free:
> free(kvm);
>
> - return ERR_PTR(ret);
> + return NULL;
> }
>
> /* RFC 1952 */
> --
> 1.7.8.3
>
> --
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-09 13:01 ` Pekka Enberg
@ 2012-02-09 13:07 ` Cyrill Gorcunov
2012-02-10 2:34 ` Yang Bai
0 siblings, 1 reply; 8+ messages in thread
From: Cyrill Gorcunov @ 2012-02-09 13:07 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Yang Bai, kvm, Sasha Levin, Asias He, Ingo Molnar
On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
> > Since the different issues have been handled in the
> > internal of kvm__init, it can only return NULL if error
> > happened.
> >
> > Signed-off-by: Yang Bai <hamo.by@gmail.com>
>
> Sorry, I don't understand what this patch is attempting to fix? Why do
> you think it's better to drop the explicit error codes and always
> return NULL upon error?
>
Yeah, I somehow don't get it as well.
Cyrill
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-09 13:07 ` Cyrill Gorcunov
@ 2012-02-10 2:34 ` Yang Bai
2012-02-10 4:53 ` Sasha Levin
0 siblings, 1 reply; 8+ messages in thread
From: Yang Bai @ 2012-02-10 2:34 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Pekka Enberg, kvm, Sasha Levin, Asias He, Ingo Molnar
On Thu, Feb 9, 2012 at 9:07 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
>> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
>> > Since the different issues have been handled in the
>> > internal of kvm__init, it can only return NULL if error
>> > happened.
>> >
>> > Signed-off-by: Yang Bai <hamo.by@gmail.com>
>>
>> Sorry, I don't understand what this patch is attempting to fix? Why do
>> you think it's better to drop the explicit error codes and always
>> return NULL upon error?
>>
Ok. Since the different issues have been handled in the internal of
this function and the caller does not care about the real error
reasons. So just return NULL if error will simplify the error handle
of the caller.
>
> Yeah, I somehow don't get it as well.
>
> Cyrill
--
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-10 2:34 ` Yang Bai
@ 2012-02-10 4:53 ` Sasha Levin
2012-02-10 3:03 ` Yang Bai
0 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2012-02-10 4:53 UTC (permalink / raw)
To: Yang Bai; +Cc: Cyrill Gorcunov, Pekka Enberg, kvm, Asias He, Ingo Molnar
On Fri, 2012-02-10 at 10:34 +0800, Yang Bai wrote:
> On Thu, Feb 9, 2012 at 9:07 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
> >> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
> >> > Since the different issues have been handled in the
> >> > internal of kvm__init, it can only return NULL if error
> >> > happened.
> >> >
> >> > Signed-off-by: Yang Bai <hamo.by@gmail.com>
> >>
> >> Sorry, I don't understand what this patch is attempting to fix? Why do
> >> you think it's better to drop the explicit error codes and always
> >> return NULL upon error?
> >>
>
> Ok. Since the different issues have been handled in the internal of
> this function and the caller does not care about the real error
> reasons. So just return NULL if error will simplify the error handle
> of the caller.
Um... why doesn't the caller care about the real error? It's whats being
sent back to userspace and can help the caller determine whats going on.
--
Sasha.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-10 4:53 ` Sasha Levin
@ 2012-02-10 3:03 ` Yang Bai
2012-02-10 4:17 ` Sasha Levin
0 siblings, 1 reply; 8+ messages in thread
From: Yang Bai @ 2012-02-10 3:03 UTC (permalink / raw)
To: Sasha Levin; +Cc: Cyrill Gorcunov, Pekka Enberg, kvm, Asias He, Ingo Molnar
On Fri, Feb 10, 2012 at 12:53 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Fri, 2012-02-10 at 10:34 +0800, Yang Bai wrote:
>> On Thu, Feb 9, 2012 at 9:07 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>> > On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
>> >> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
>> >> > Since the different issues have been handled in the
>> >> > internal of kvm__init, it can only return NULL if error
>> >> > happened.
>> >> >
>> >> > Signed-off-by: Yang Bai <hamo.by@gmail.com>
>> >>
>> >> Sorry, I don't understand what this patch is attempting to fix? Why do
>> >> you think it's better to drop the explicit error codes and always
>> >> return NULL upon error?
>> >>
>>
>> Ok. Since the different issues have been handled in the internal of
>> this function and the caller does not care about the real error
>> reasons. So just return NULL if error will simplify the error handle
>> of the caller.
>
> Um... why doesn't the caller care about the real error? It's whats being
> sent back to userspace and can help the caller determine whats going on.
>
Reading the source code, I found that the caller handle it as following:
static int kvm_cmd_run_init(int argc, const char **argv)
{
[snip]
kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
if (IS_ERR(kvm)) {
r = PTR_ERR(kvm);
goto fail;
}
[snip]
fail:
return r;
}
int kvm_cmd_run(int argc, const char **argv, const char *prefix)
{
[snip]
r = kvm_cmd_run_init(argc, argv);
if (r < 0)
return r;
[snip]
}
So the real reason is ignored.
> --
>
> Sasha.
>
--
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-10 3:03 ` Yang Bai
@ 2012-02-10 4:17 ` Sasha Levin
2012-02-10 5:04 ` Yang Bai
0 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2012-02-10 4:17 UTC (permalink / raw)
To: Yang Bai; +Cc: Cyrill Gorcunov, Pekka Enberg, kvm, Asias He, Ingo Molnar
On Fri, 2012-02-10 at 11:03 +0800, Yang Bai wrote:
> On Fri, Feb 10, 2012 at 12:53 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > On Fri, 2012-02-10 at 10:34 +0800, Yang Bai wrote:
> >> On Thu, Feb 9, 2012 at 9:07 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> >> > On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
> >> >> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
> >> >> > Since the different issues have been handled in the
> >> >> > internal of kvm__init, it can only return NULL if error
> >> >> > happened.
> >> >> >
> >> >> > Signed-off-by: Yang Bai <hamo.by@gmail.com>
> >> >>
> >> >> Sorry, I don't understand what this patch is attempting to fix? Why do
> >> >> you think it's better to drop the explicit error codes and always
> >> >> return NULL upon error?
> >> >>
> >>
> >> Ok. Since the different issues have been handled in the internal of
> >> this function and the caller does not care about the real error
> >> reasons. So just return NULL if error will simplify the error handle
> >> of the caller.
> >
> > Um... why doesn't the caller care about the real error? It's whats being
> > sent back to userspace and can help the caller determine whats going on.
> >
> Reading the source code, I found that the caller handle it as following:
>
> static int kvm_cmd_run_init(int argc, const char **argv)
> {
> [snip]
> kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> if (IS_ERR(kvm)) {
> r = PTR_ERR(kvm);
> goto fail;
> }
> [snip]
> fail:
> return r;
> }
>
> int kvm_cmd_run(int argc, const char **argv, const char *prefix)
> {
> [snip]
> r = kvm_cmd_run_init(argc, argv);
> if (r < 0)
> return r;
> [snip]
> }
>From what I gather from the code snippet you pasted is that we return
the error to the user, which can help him determine whats going on...
--
Sasha.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] kvm tool: rewrite kvm__init
2012-02-10 4:17 ` Sasha Levin
@ 2012-02-10 5:04 ` Yang Bai
0 siblings, 0 replies; 8+ messages in thread
From: Yang Bai @ 2012-02-10 5:04 UTC (permalink / raw)
To: Sasha Levin; +Cc: Cyrill Gorcunov, Pekka Enberg, kvm, Asias He, Ingo Molnar
On Fri, Feb 10, 2012 at 12:17 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Fri, 2012-02-10 at 11:03 +0800, Yang Bai wrote:
>> On Fri, Feb 10, 2012 at 12:53 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> > On Fri, 2012-02-10 at 10:34 +0800, Yang Bai wrote:
>> >> On Thu, Feb 9, 2012 at 9:07 PM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>> >> > On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote:
>> >> >> On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai <hamo.by@gmail.com> wrote:
>
> From what I gather from the code snippet you pasted is that we return
> the error to the user, which can help him determine whats going on...
Fine. Thanks for you pointing it out. Just drop this patch.
Thanks.
>
> --
>
> Sasha.
>
--
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-10 5:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-09 5:40 [PATCH] kvm tool: rewrite kvm__init Yang Bai
2012-02-09 13:01 ` Pekka Enberg
2012-02-09 13:07 ` Cyrill Gorcunov
2012-02-10 2:34 ` Yang Bai
2012-02-10 4:53 ` Sasha Levin
2012-02-10 3:03 ` Yang Bai
2012-02-10 4:17 ` Sasha Levin
2012-02-10 5:04 ` Yang Bai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).