From c2ad1b0c465a9ea8375eaff14bbd85705c673f73 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:40:51 +0100 Subject: [PATCH] block: Allow omitting the file name when using driver-specific options After this patch, using -drive with an empty file name continues to open the file if driver-specific options are used. If no driver-specific options are specified, the semantics stay as it was: It defines a drive without an inserted medium. In order to achieve this, bdrv_open() must be made safe to work with a NULL filename parameter. The assumption that is made is that only block drivers which implement bdrv_parse_filename() support using driver specific options and could therefore work without a filename. These drivers must make sure to cope with NULL in their implementation of .bdrv_open() (this is only NBD for now). For all other drivers, the block layer code will make sure to error out before calling into their code - they can't possibly work without a filename. Now an NBD connection can be opened like this: qemu-system-x86_64 -drive file.driver=nbd,file.port=1234,file.host=::1 Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 49 ++++++++++++++++++++++++++++++++------- blockdev.c | 10 +++++--- include/block/block_int.h | 3 +++ 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/block.c b/block.c index 2c17a348ac..16a92a4e08 100644 --- a/block.c +++ b/block.c @@ -688,7 +688,11 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bdrv_enable_copy_on_read(bs); } - pstrcpy(bs->filename, sizeof(bs->filename), filename); + if (filename != NULL) { + pstrcpy(bs->filename, sizeof(bs->filename), filename); + } else { + bs->filename[0] = '\0'; + } if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) { return -ENOTSUP; @@ -708,6 +712,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bdrv_swap(file, bs); ret = 0; } else { + assert(drv->bdrv_parse_filename || filename != NULL); ret = drv->bdrv_file_open(bs, filename, options, open_flags); } } else { @@ -727,6 +732,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, #ifndef _WIN32 if (bs->is_temporary) { + assert(filename != NULL); unlink(filename); } #endif @@ -753,14 +759,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, { BlockDriverState *bs; BlockDriver *drv; + const char *drvname; int ret; - drv = bdrv_find_protocol(filename); - if (!drv) { - QDECREF(options); - return -ENOENT; - } - /* NULL means an empty set of options */ if (options == NULL) { options = qdict_new(); @@ -770,7 +771,26 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, bs->options = options; options = qdict_clone_shallow(options); - if (drv->bdrv_parse_filename) { + /* Find the right block driver */ + drvname = qdict_get_try_str(options, "driver"); + if (drvname) { + drv = bdrv_find_whitelisted_format(drvname); + qdict_del(options, "driver"); + } else if (filename) { + drv = bdrv_find_protocol(filename); + } else { + qerror_report(ERROR_CLASS_GENERIC_ERROR, + "Must specify either driver or file"); + drv = NULL; + } + + if (!drv) { + ret = -ENOENT; + goto fail; + } + + /* Parse the filename and open it */ + if (drv->bdrv_parse_filename && filename) { Error *local_err = NULL; drv->bdrv_parse_filename(filename, options, &local_err); if (error_is_set(&local_err)) { @@ -779,6 +799,12 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, ret = -EINVAL; goto fail; } + } else if (!drv->bdrv_parse_filename && !filename) { + qerror_report(ERROR_CLASS_GENERIC_ERROR, + "The '%s' block driver requires a file name", + drv->format_name); + ret = -EINVAL; + goto fail; } ret = bdrv_open_common(bs, NULL, filename, options, flags, drv); @@ -899,6 +925,13 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, QEMUOptionParameter *create_options; char backing_filename[PATH_MAX]; + if (qdict_size(options) != 0) { + error_report("Can't use snapshot=on with driver-specific options"); + ret = -EINVAL; + goto fail; + } + assert(filename != NULL); + /* if snapshot, we create a temporary backing file and open it instead of opening 'filename' directly */ diff --git a/blockdev.c b/blockdev.c index 6f2b7592ce..8cdc9ce16a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -658,7 +658,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) abort(); } if (!file || !*file) { - return dinfo; + if (qdict_size(bs_opts)) { + file = NULL; + } else { + return dinfo; + } } if (snapshot) { /* always use cache=unsafe with snapshot */ @@ -697,10 +701,10 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) if (ret < 0) { if (ret == -EMEDIUMTYPE) { error_report("could not open disk image %s: not in %s format", - file, drv->format_name); + file ?: dinfo->id, drv->format_name); } else { error_report("could not open disk image %s: %s", - file, strerror(-ret)); + file ?: dinfo->id, strerror(-ret)); } goto err; } diff --git a/include/block/block_int.h b/include/block/block_int.h index 1b06a11590..0986a2d6ac 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -75,6 +75,9 @@ struct BlockDriver { int instance_size; int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename); int (*bdrv_probe_device)(const char *filename); + + /* Any driver implementing this callback is expected to be able to handle + * NULL file names in its .bdrv_open() implementation */ void (*bdrv_parse_filename)(const char *filename, QDict *options, Error **errp); /* For handling image reopen for split or non-split files */