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:
		
				
					committed by
					
						 Kevin Wolf
						Kevin Wolf
					
				
			
			
				
	
			
			
			
						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 *file = NULL; | ||||
| @@ -225,8 +225,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) { | ||||
| @@ -499,8 +497,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 */ | ||||
| @@ -529,7 +526,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; | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -48,7 +48,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); | ||||
| QemuOpts *drive_def(const char *optstr); | ||||
| QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, | ||||
|                     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 */ | ||||
|  | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -542,7 +542,6 @@ static USBDevice *usb_msd_init(const char *filename) | ||||
|     QemuOpts *opts; | ||||
|     DriveInfo *dinfo; | ||||
|     USBDevice *dev; | ||||
|     int fatal_error; | ||||
|     const char *p1; | ||||
|     char fmt[32]; | ||||
|  | ||||
| @@ -572,7 +571,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; | ||||
|   | ||||
							
								
								
									
										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) | ||||
| { | ||||
|     int *use_scsi = opaque; | ||||
|     int fatal_error = 0; | ||||
|  | ||||
|     if (drive_init(opts, *use_scsi, &fatal_error) == NULL) { | ||||
|         if (fatal_error) | ||||
|             return 1; | ||||
|     } | ||||
|     return 0; | ||||
|     return drive_init(opts, *use_scsi) == NULL; | ||||
| } | ||||
|  | ||||
| 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) { | ||||
|         drive_enable_snapshot(opts, NULL); | ||||
|     } | ||||
|     if (drive_init_func(opts, &use_scsi)) { | ||||
|     if (!drive_init(opts, use_scsi)) { | ||||
|         exit(1); | ||||
|     } | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user