From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53971 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Petrg-0002cD-8i for qemu-devel@nongnu.org; Mon, 17 Jan 2011 13:31:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Petrb-00022w-IY for qemu-devel@nongnu.org; Mon, 17 Jan 2011 13:31:40 -0500 Received: from oxygen.pond.sub.org ([213.239.205.148]:46221) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Petrb-00022V-5k for qemu-devel@nongnu.org; Mon, 17 Jan 2011 13:31:35 -0500 Received: from blackfin.pond.sub.org (p5B32B9B0.dip.t-dialin.net [91.50.185.176]) by oxygen.pond.sub.org (Postfix) with ESMTPA id BF30C2DA973 for ; Mon, 17 Jan 2011 19:31:31 +0100 (CET) From: Markus Armbruster Date: Mon, 17 Jan 2011 19:31:30 +0100 Message-Id: <1295289090-18236-6-git-send-email-armbru@redhat.com> In-Reply-To: <1295289090-18236-1-git-send-email-armbru@redhat.com> References: <1295289090-18236-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH 5/5] blockdev: Fix drive_add for drives without media List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com Watch this: (qemu) drive_add 0 if=none (qemu) info block none0: type=hd removable=0 [not inserted] (qemu) drive_del none0 Segmentation fault (core dumped) add_init_drive() is confused about drive_init()'s failure modes, and cleans up when it shouldn't. This leaves the DriveInfo with member opts dangling. drive_del attempts to free it, and dies. drive_init() behaves as follows: * If it created a drive with media, it returns its DriveInfo. * If it created a drive without media, it clears *fatal_error and returns NULL. * If it couldn't create a drive, it sets *fatal_error and returns NULL. Of its three callers: * drive_init_func() is correct. * usb_msd_init() assumes drive_init() failed when it returns NULL. This is correct only because it always passes option "file", and "drive without media" can't happen then. * add_init_drive() assumes drive_init() failed when it returns NULL. This is incorrect. Clean up drive_init() to return NULL on failure and only on failure. Drop its parameter fatal_error. Signed-off-by: Markus Armbruster --- blockdev.c | 8 ++------ blockdev.h | 2 +- hw/device-hotplug.c | 3 +-- hw/usb-msd.c | 3 +-- vl.c | 6 ++---- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/blockdev.c b/blockdev.c index 51a2da3..8a74bf3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -135,7 +135,7 @@ static int parse_block_error_action(const char *buf, int is_read) } } -DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error) +DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) { const char *buf; const char *file = NULL; @@ -157,8 +157,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error) int snapshot = 0; int ret; - *fatal_error = 1; - translation = BIOS_ATA_TRANSLATION_AUTO; if (default_to_scsi) { @@ -463,8 +461,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error) abort(); } if (!file || !*file) { - *fatal_error = 0; - return NULL; + return dinfo; } if (snapshot) { /* always use cache=unsafe with snapshot */ @@ -493,7 +490,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error) if (bdrv_key_required(dinfo->bdrv)) autostart = 0; - *fatal_error = 0; return dinfo; } diff --git a/blockdev.h b/blockdev.h index 4536b5c..38ee5d2 100644 --- a/blockdev.h +++ b/blockdev.h @@ -40,7 +40,7 @@ void drive_uninit(DriveInfo *dinfo); DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3); -DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error); +DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi); /* device-hotplug */ diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c index 9704e2f..c310cb8 100644 --- a/hw/device-hotplug.c +++ b/hw/device-hotplug.c @@ -29,7 +29,6 @@ DriveInfo *add_init_drive(const char *optstr) { - int fatal_error; DriveInfo *dinfo; QemuOpts *opts; @@ -37,7 +36,7 @@ DriveInfo *add_init_drive(const char *optstr) if (!opts) return NULL; - dinfo = drive_init(opts, current_machine->use_scsi, &fatal_error); + dinfo = drive_init(opts, current_machine->use_scsi); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 0a95d8d..00f64b0 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -570,7 +570,6 @@ static USBDevice *usb_msd_init(const char *filename) QemuOpts *opts; DriveInfo *dinfo; USBDevice *dev; - int fatal_error; const char *p1; char fmt[32]; @@ -600,7 +599,7 @@ static USBDevice *usb_msd_init(const char *filename) qemu_opt_set(opts, "if", "none"); /* create host drive */ - dinfo = drive_init(opts, 0, &fatal_error); + dinfo = drive_init(opts, 0); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/vl.c b/vl.c index 0292184..5ffa12b 100644 --- a/vl.c +++ b/vl.c @@ -631,11 +631,9 @@ static int bt_parse(const char *opt) static int drive_init_func(QemuOpts *opts, void *opaque) { int *use_scsi = opaque; - int fatal_error = 0; - if (drive_init(opts, *use_scsi, &fatal_error) == NULL) { - if (fatal_error) - return 1; + if (drive_init(opts, *use_scsi) == NULL) { + return 1; } return 0; } -- 1.7.2.3