[PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails

[PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails

[Michael Ellerman]

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/builtin-run.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 95d35a5..569246e 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -997,6 +997,11 @@ 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);
+		pr_err("kvm__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	kvm->single_step = single_step;
 
-- 
1.7.7.3

[PATCH 2/2] kvm tool: Initialise kvm fd's to -1 in kvm__new()

[Michael Ellerman]

In kvm__new() we use calloc() to allocate the kvm structure, which
initialises the memory to 0.

In kvm__init() if we hit an error before sys_fd and vm_fd are opened
we go to cleanup which closes sys_fd and vm_fd. Because they were
initialised to zero this has the effect of closing stdin. Because
we have closed stdin term_cleanup() doesn't work and the users terminal
is fubared.

Set both fd's to -1 in kvm__new() to avoid the problem.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/kvm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 74e37ae..7b72f1e 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -127,6 +127,8 @@ static struct kvm *kvm__new(void)
 	if (!kvm)
 		return ERR_PTR(-ENOMEM);
 
+	kvm->vm_fd = kvm->sys_fd = -1;
+
 	return kvm;
 }
 
-- 
1.7.7.3

Re: [PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails

[Pekka Enberg]

Hi Michael,

On Mon, 6 Feb 2012, Michael Ellerman wrote:
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> tools/kvm/builtin-run.c |    5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 95d35a5..569246e 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -997,6 +997,11 @@ 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);
> +		pr_err("kvm__init() failed with error %d\n", r);
> +		goto fail;
> +	}
>

I just pushed commit 3dfcb6ec85d5430622c8b99ca05451c1afd08bf5 ("kvm tool: 
Don't close not yet opened files and SIGSEV fix") from Cyrillos which was 
which seems to fix both issues. I was flying back from FOSDEM so I 
couldn't merge it earlier, sorry.

 			Pekka

Re: [PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails

[Cyrill Gorcunov]

On Mon, Feb 06, 2012 at 12:22:04PM +0200, Pekka Enberg wrote:
> Hi Michael,
> 
> On Mon, 6 Feb 2012, Michael Ellerman wrote:
> >Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> >---
> >tools/kvm/builtin-run.c |    5 +++++
> >1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> >index 95d35a5..569246e 100644
> >--- a/tools/kvm/builtin-run.c
> >+++ b/tools/kvm/builtin-run.c
> >@@ -997,6 +997,11 @@ 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);
> >+		pr_err("kvm__init() failed with error %d\n", r);
> >+		goto fail;
> >+	}
> >
> 
> I just pushed commit 3dfcb6ec85d5430622c8b99ca05451c1afd08bf5 ("kvm
> tool: Don't close not yet opened files and SIGSEV fix") from
> Cyrillos which was which seems to fix both issues. I was flying back
> from FOSDEM so I couldn't merge it earlier, sorry.
> 

Ouch, I somehow missed these patches from Michael. If I saw them earlier
I would not provide my patch (since Michael's changelog is a way more
descriptive and better than mine). Anyway, thanks!

	Cyrill

Re: [PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Mon, 2012-02-06 at 12:22 +0200, Pekka Enberg wrote:
> On Mon, 6 Feb 2012, Michael Ellerman wrote:
> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> > index 95d35a5..569246e 100644
> > --- a/tools/kvm/builtin-run.c
> > +++ b/tools/kvm/builtin-run.c
> > @@ -997,6 +997,11 @@ 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);
> > +		pr_err("kvm__init() failed with error %d\n", r);
> > +		goto fail;
> > +	}
> >
> 
> I just pushed commit 3dfcb6ec85d5430622c8b99ca05451c1afd08bf5 ("kvm tool: 
> Don't close not yet opened files and SIGSEV fix") from Cyrillos which was 
> which seems to fix both issues. I was flying back from FOSDEM so I 
> couldn't merge it earlier, sorry.

No stress, it's fixed is what matters :)

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]