blockdev: Fix drive_add for drives without media

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 <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Markus Armbruster 2011-01-28 11:21:46 +01:00 committed by Kevin Wolf
parent 5645b0f4f2
commit 319ae529b8
5 changed files with 7 additions and 18 deletions

View File

@ -203,7 +203,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 *buf;
const char *file = NULL; const char *file = NULL;
@ -225,8 +225,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
int snapshot = 0; int snapshot = 0;
int ret; int ret;
*fatal_error = 1;
translation = BIOS_ATA_TRANSLATION_AUTO; translation = BIOS_ATA_TRANSLATION_AUTO;
if (default_to_scsi) { if (default_to_scsi) {
@ -499,8 +497,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
abort(); abort();
} }
if (!file || !*file) { if (!file || !*file) {
*fatal_error = 0; return dinfo;
return NULL;
} }
if (snapshot) { if (snapshot) {
/* always use cache=unsafe with snapshot */ /* always use cache=unsafe with snapshot */
@ -529,7 +526,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
if (bdrv_key_required(dinfo->bdrv)) if (bdrv_key_required(dinfo->bdrv))
autostart = 0; autostart = 0;
*fatal_error = 0;
return dinfo; return dinfo;
} }

View File

@ -48,7 +48,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr); QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
const char *optstr); const char *optstr);
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error); DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi);
/* device-hotplug */ /* device-hotplug */

View File

@ -29,7 +29,6 @@
DriveInfo *add_init_drive(const char *optstr) DriveInfo *add_init_drive(const char *optstr)
{ {
int fatal_error;
DriveInfo *dinfo; DriveInfo *dinfo;
QemuOpts *opts; QemuOpts *opts;
@ -37,7 +36,7 @@ DriveInfo *add_init_drive(const char *optstr)
if (!opts) if (!opts)
return NULL; return NULL;
dinfo = drive_init(opts, current_machine->use_scsi, &fatal_error); dinfo = drive_init(opts, current_machine->use_scsi);
if (!dinfo) { if (!dinfo) {
qemu_opts_del(opts); qemu_opts_del(opts);
return NULL; return NULL;

View File

@ -542,7 +542,6 @@ static USBDevice *usb_msd_init(const char *filename)
QemuOpts *opts; QemuOpts *opts;
DriveInfo *dinfo; DriveInfo *dinfo;
USBDevice *dev; USBDevice *dev;
int fatal_error;
const char *p1; const char *p1;
char fmt[32]; char fmt[32];
@ -572,7 +571,7 @@ static USBDevice *usb_msd_init(const char *filename)
qemu_opt_set(opts, "if", "none"); qemu_opt_set(opts, "if", "none");
/* create host drive */ /* create host drive */
dinfo = drive_init(opts, 0, &fatal_error); dinfo = drive_init(opts, 0);
if (!dinfo) { if (!dinfo) {
qemu_opts_del(opts); qemu_opts_del(opts);
return NULL; return NULL;

9
vl.c
View File

@ -631,13 +631,8 @@ static int bt_parse(const char *opt)
static int drive_init_func(QemuOpts *opts, void *opaque) static int drive_init_func(QemuOpts *opts, void *opaque)
{ {
int *use_scsi = opaque; int *use_scsi = opaque;
int fatal_error = 0;
if (drive_init(opts, *use_scsi, &fatal_error) == NULL) { return drive_init(opts, *use_scsi) == NULL;
if (fatal_error)
return 1;
}
return 0;
} }
static int drive_enable_snapshot(QemuOpts *opts, void *opaque) static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
@ -666,7 +661,7 @@ static void default_drive(int enable, int snapshot, int use_scsi,
if (snapshot) { if (snapshot) {
drive_enable_snapshot(opts, NULL); drive_enable_snapshot(opts, NULL);
} }
if (drive_init_func(opts, &use_scsi)) { if (!drive_init(opts, use_scsi)) {
exit(1); exit(1);
} }
} }