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:
parent
5645b0f4f2
commit
319ae529b8
@ -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;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -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 */
|
||||||
|
|
||||||
|
@ -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;
|
||||||
|
@ -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
9
vl.c
@ -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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user