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 <arm...@redhat.com>
---
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
