From mboxrd@z Thu Jan  1 00:00:00 1970
From: Cyrill Gorcunov <gorcunov@openvz.org>
Subject: Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
Date: Sat, 4 Feb 2012 21:48:44 +0400
Message-ID: <20120204174844.GC14818@moon>
References: <20120204163204.GB15888@moon>
 <alpine.LFD.2.02.1202041937570.1809@tux.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Sasha Levin <levinsasha928@gmail.com>,
	Asias He <asias.hejun@gmail.com>, Ingo Molnar <mingo@elte.hu>,
	KVM-ML <kvm@vger.kernel.org>
To: Pekka Enberg <penberg@kernel.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-bk0-f46.google.com ([209.85.214.46]:58969 "EHLO
	mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752663Ab2BDRst (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 4 Feb 2012 12:48:49 -0500
Received: by bkcjm19 with SMTP id jm19so3816418bkc.19
        for <kvm@vger.kernel.org>; Sat, 04 Feb 2012 09:48:48 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <alpine.LFD.2.02.1202041937570.1809@tux.localdomain>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, Feb 04, 2012 at 07:38:41PM +0200, Pekka Enberg wrote:
> On Sat, 4 Feb 2012, Cyrill Gorcunov wrote:
> >In case if there error happened in kvm__init and we have
> >no files opened -- we should not try to close them.
> >
> >Also once kvm failed to init the caller should not try
> >to dereference a pointer obtained, otherwise we might get
> >SIGSEV
> >
> >| [cyrill@moon kvm]$ ./lkvm run ...
> >|  Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded.
> >| Segmentation fault (core dumped)
> >| [cyrill@moon kvm]$
> >
> >Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> >---
> >tools/kvm/builtin-run.c |    4 ++++
> >tools/kvm/kvm.c         |   11 ++++++++---
> >2 files changed, 12 insertions(+), 3 deletions(-)
> >
> >Index: linux-2.6.git/tools/kvm/builtin-run.c
> >===================================================================
> >--- linux-2.6.git.orig/tools/kvm/builtin-run.c
> >+++ linux-2.6.git/tools/kvm/builtin-run.c
> >@@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co
> >	}
> >
> >	kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> >+	if (IS_ERR_OR_NULL(kvm)) {
> 
> PTR_ERR is still not going to work if kvm is NULL.
> 

It should be IS_ERR rather (kvm__init never returns with NULL),
but it's a candidate for another patch where both kvm__init
and kvm__new should test for IS_ERR only.

Pekka, could you please s/IS_ERR_OR_NULL/IS_ERR/ in this patch,
and that's all. Hm?

	Cyrill