From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=57509 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q0EBY-0000EX-Ib
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:28:21 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mjt@tls.msk.ru>) id 1Q0EBX-0002B7-Ax
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:28:20 -0400
Received: from isrv.corpit.ru ([86.62.121.231]:53115)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mjt@tls.msk.ru>) id 1Q0EBX-0002Au-21
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:28:19 -0400
Message-ID: <4D821A7F.9000204@msgid.tls.msk.ru>
Date: Thu, 17 Mar 2011 17:28:15 +0300
From: Michael Tokarev <mjt@tls.msk.ru>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH +STABLE-0.14] exit if -drive specified is
	invalid instead of ignoring the "wrong" -drive
References: <4D81BF12.4020500@msgid.tls.msk.ru>
	<m3r5a597om.fsf@blackfin.pond.sub.org>
In-Reply-To: <m3r5a597om.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org

17.03.2011 16:51, Markus Armbruster wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
> 
>> Trivial patch.  I've sent it yesterday but somehow it didn't
>> reach the list.
>>
>> This fixes the problem when qemu continues even if -drive specification
>> is somehow invalid, resulting in a mess.  Applicable for both current
>> master and for stable-0.14 (and 0.13 and 0.12 as well).
> 
> Note patch doesn't apply to 0.12 and 0.13.

Yes it doesn't, since to 0.14 the code changed.
What I mean is that the same problem exist in
earlier versions too.  I'll apply a change of
this effect to 0.12.5 as used in Debian now,
something like the one below.

[]
> What about all the other unchecked drive_add() calls in main()?

These are much less worrisome - they fail only of the
internal definitions of options are incorrect, which
should never happen.  For example:

            case QEMU_OPTION_hdd:
                drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
                          HD_OPTS);

There, optarg is just a filename, and HD_OPTS is
defined like this:

#define HD_OPTS "media=disk"

So it should not fail when parsing options, only
when trying to interpret and actually open the
filename, which happens much later in the game.

Thanks,

/mjt

For 0.12 and 0.13:

diff --git a/vl.c b/vl.c
index 77677e8..069a1df 100644
--- a/vl.c
+++ b/vl.c
@@ -5025,7 +5025,8 @@ int main(int argc, char **argv, char **envp)
                 drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
                 break;
             case QEMU_OPTION_drive:
-                drive_add(NULL, "%s", optarg);
+                if (drive_add(NULL, "%s", optarg) == NULL)
+                    exit(1);
                break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)