From 5c916681ae2383f0425bb8a3680ade9d055f5dfe Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 11 Mar 2013 11:03:28 +0100 Subject: Revert "block: complete all IOs before .bdrv_truncate" brdv_truncate() is also called from readv/writev commands on self- growing file based storage. this will result in requests waiting for theirselves to complete. This reverts commit 9a665b2b8640e464f0a778216fc2dca8d02acf33. Signed-off-by: Kevin Wolf --- block.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/block.c b/block.c index 0a062c9..22647b2 100644 --- a/block.c +++ b/block.c @@ -2487,10 +2487,6 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset) return -EACCES; if (bdrv_in_use(bs)) return -EBUSY; - - /* There better not be any in-flight IOs when we truncate the device. */ - bdrv_drain_all(); - ret = drv->bdrv_truncate(bs, offset); if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); -- cgit v1.1 From 92b7a08d64e5e3129fa885f9d180e5bddcb76b42 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 11 Mar 2013 11:04:24 +0100 Subject: block: complete all IOs before resizing a device this patch ensures that all pending IOs are completed before a device is resized. this is especially important if a device is shrinked as it the bdrv_check_request() result is invalidated. Signed-off-by: Peter Lieven Signed-off-by: Kevin Wolf --- blockdev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/blockdev.c b/blockdev.c index 09f76b7..6f2b759 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1127,6 +1127,9 @@ void qmp_block_resize(const char *device, int64_t size, Error **errp) return; } + /* complete all in-flight operations before resizing the device */ + bdrv_drain_all(); + switch (bdrv_truncate(bs, size)) { case 0: break; -- cgit v1.1 From 787e4a8500020695eb391e2f1cc4767ee071d441 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 6 Mar 2013 11:52:48 +0100 Subject: block: Add options QDict to bdrv_file_open() prototypes The new parameter is unused yet. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 14 +++++++++++--- block/blkdebug.c | 5 +++-- block/blkverify.c | 5 +++-- block/cow.c | 2 +- block/curl.c | 3 ++- block/gluster.c | 2 +- block/iscsi.c | 5 +++-- block/nbd.c | 3 ++- block/qcow.c | 2 +- block/qcow2.c | 2 +- block/qed.c | 2 +- block/raw-posix.c | 15 ++++++++++----- block/sheepdog.c | 7 ++++--- block/vmdk.c | 2 +- block/vvfat.c | 3 ++- include/block/block.h | 3 ++- include/block/block_int.h | 3 ++- qemu-io.c | 2 +- 18 files changed, 51 insertions(+), 29 deletions(-) diff --git a/block.c b/block.c index 22647b2..0fb0165 100644 --- a/block.c +++ b/block.c @@ -708,7 +708,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bdrv_swap(file, bs); ret = 0; } else { - ret = drv->bdrv_file_open(bs, filename, open_flags); + ret = drv->bdrv_file_open(bs, filename, options, open_flags); } } else { assert(file != NULL); @@ -742,13 +742,21 @@ free_and_fail: /* * Opens a file using a protocol (file, host_device, nbd, ...) + * + * options is a QDict of options to pass to the block drivers, or NULL for an + * empty set of options. The reference to the QDict belongs to the block layer + * after the call (even on failure), so if the caller intends to reuse the + * dictionary, it needs to use QINCREF() before calling bdrv_file_open. */ -int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags) +int bdrv_file_open(BlockDriverState **pbs, const char *filename, + QDict *options, int flags) { BlockDriverState *bs; BlockDriver *drv; int ret; + QDECREF(options); + drv = bdrv_find_protocol(filename); if (!drv) { return -ENOENT; @@ -888,7 +896,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, flags |= BDRV_O_ALLOW_RDWR; } - ret = bdrv_file_open(&file, filename, bdrv_open_flags(bs, flags)); + ret = bdrv_file_open(&file, filename, NULL, bdrv_open_flags(bs, flags)); if (ret < 0) { goto fail; } diff --git a/block/blkdebug.c b/block/blkdebug.c index 6f74637..37cfbc7 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -304,7 +304,8 @@ fail: } /* Valid blkdebug filenames look like blkdebug:path/to/config:path/to/image */ -static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) +static int blkdebug_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVBlkdebugState *s = bs->opaque; int ret; @@ -335,7 +336,7 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) s->state = 1; /* Open the backing file */ - ret = bdrv_file_open(&bs->file, filename, flags); + ret = bdrv_file_open(&bs->file, filename, NULL, flags); if (ret < 0) { return ret; } diff --git a/block/blkverify.c b/block/blkverify.c index 2086d97..59e3b05 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -69,7 +69,8 @@ static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb, } /* Valid blkverify filenames look like blkverify:path/to/raw_image:path/to/image */ -static int blkverify_open(BlockDriverState *bs, const char *filename, int flags) +static int blkverify_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVBlkverifyState *s = bs->opaque; int ret; @@ -89,7 +90,7 @@ static int blkverify_open(BlockDriverState *bs, const char *filename, int flags) raw = g_strdup(filename); raw[c - filename] = '\0'; - ret = bdrv_file_open(&bs->file, raw, flags); + ret = bdrv_file_open(&bs->file, raw, NULL, flags); g_free(raw); if (ret < 0) { return ret; diff --git a/block/cow.c b/block/cow.c index d73e08c..9f94599 100644 --- a/block/cow.c +++ b/block/cow.c @@ -279,7 +279,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) return ret; } - ret = bdrv_file_open(&cow_bs, filename, BDRV_O_RDWR); + ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { return ret; } diff --git a/block/curl.c b/block/curl.c index 98947da..186e3b0 100644 --- a/block/curl.c +++ b/block/curl.c @@ -335,7 +335,8 @@ static void curl_clean_state(CURLState *s) s->in_use = 0; } -static int curl_open(BlockDriverState *bs, const char *filename, int flags) +static int curl_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVCURLState *s = bs->opaque; CURLState *state = NULL; diff --git a/block/gluster.c b/block/gluster.c index ccd684d..9ccd4d4 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -283,7 +283,7 @@ static int qemu_gluster_aio_flush_cb(void *opaque) } static int qemu_gluster_open(BlockDriverState *bs, const char *filename, - int bdrv_flags) + QDict *options, int bdrv_flags) { BDRVGlusterState *s = bs->opaque; int open_flags = O_BINARY; diff --git a/block/iscsi.c b/block/iscsi.c index 3d52921..51a2889 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1007,7 +1007,8 @@ out: * We support iscsi url's on the form * iscsi://[%@][:]// */ -static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) +static int iscsi_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { IscsiLun *iscsilun = bs->opaque; struct iscsi_context *iscsi = NULL; @@ -1203,7 +1204,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) bs.opaque = g_malloc0(sizeof(struct IscsiLun)); iscsilun = bs.opaque; - ret = iscsi_open(&bs, filename, 0); + ret = iscsi_open(&bs, filename, NULL, 0); if (ret != 0) { goto out; } diff --git a/block/nbd.c b/block/nbd.c index a581294..0473908 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -376,7 +376,8 @@ static void nbd_teardown_connection(BlockDriverState *bs) closesocket(s->sock); } -static int nbd_open(BlockDriverState *bs, const char* filename, int flags) +static int nbd_open(BlockDriverState *bs, const char* filename, + QDict *options, int flags) { BDRVNBDState *s = bs->opaque; int result; diff --git a/block/qcow.c b/block/qcow.c index f6750a5..13d396b 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -679,7 +679,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) return ret; } - ret = bdrv_file_open(&qcow_bs, filename, BDRV_O_RDWR); + ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { return ret; } diff --git a/block/qcow2.c b/block/qcow2.c index 98bb7f3..8ea696a 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1254,7 +1254,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, return ret; } - ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR); + ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { return ret; } diff --git a/block/qed.c b/block/qed.c index 46e12b3..4651403 100644 --- a/block/qed.c +++ b/block/qed.c @@ -558,7 +558,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, return ret; } - ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR | BDRV_O_CACHE_WB); + ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB); if (ret < 0) { return ret; } diff --git a/block/raw-posix.c b/block/raw-posix.c index 8a3cdbc..99ac869 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -303,7 +303,8 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, return 0; } -static int raw_open(BlockDriverState *bs, const char *filename, int flags) +static int raw_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVRawState *s = bs->opaque; @@ -1292,7 +1293,8 @@ static int check_hdev_writable(BDRVRawState *s) return 0; } -static int hdev_open(BlockDriverState *bs, const char *filename, int flags) +static int hdev_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVRawState *s = bs->opaque; int ret; @@ -1530,7 +1532,8 @@ static BlockDriver bdrv_host_device = { }; #ifdef __linux__ -static int floppy_open(BlockDriverState *bs, const char *filename, int flags) +static int floppy_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVRawState *s = bs->opaque; int ret; @@ -1652,7 +1655,8 @@ static BlockDriver bdrv_host_floppy = { .bdrv_eject = floppy_eject, }; -static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) +static int cdrom_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVRawState *s = bs->opaque; @@ -1760,7 +1764,8 @@ static BlockDriver bdrv_host_cdrom = { #endif /* __linux__ */ #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__) -static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) +static int cdrom_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { BDRVRawState *s = bs->opaque; int ret; diff --git a/block/sheepdog.c b/block/sheepdog.c index 54d3e53..bb67c4c 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1126,7 +1126,8 @@ static int write_object(int fd, char *buf, uint64_t oid, int copies, create, cache_flags); } -static int sd_open(BlockDriverState *bs, const char *filename, int flags) +static int sd_open(BlockDriverState *bs, const char *filename, + QDict *options, int flags) { int ret, fd; uint32_t vid = 0; @@ -1269,7 +1270,7 @@ static int sd_prealloc(const char *filename) void *buf = g_malloc0(SD_DATA_OBJ_SIZE); int ret; - ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR); + ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { goto out; } @@ -1367,7 +1368,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) goto out; } - ret = bdrv_file_open(&bs, backing_file, 0); + ret = bdrv_file_open(&bs, backing_file, NULL, 0); if (ret < 0) { goto out; } diff --git a/block/vmdk.c b/block/vmdk.c index e92104a..7bad757 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -661,7 +661,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, path_combine(extent_path, sizeof(extent_path), desc_file_path, fname); - ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags); + ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags); if (ret) { return ret; } diff --git a/block/vvfat.c b/block/vvfat.c index b8eb38a..ef74c30 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -988,7 +988,8 @@ static void vvfat_rebind(BlockDriverState *bs) s->bs = bs; } -static int vvfat_open(BlockDriverState *bs, const char* dirname, int flags) +static int vvfat_open(BlockDriverState *bs, const char* dirname, + QDict *options, int flags) { BDRVVVFATState *s = bs->opaque; int i, cyls, heads, secs; diff --git a/include/block/block.h b/include/block/block.h index d4f34d6..9dc6aad 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -135,7 +135,8 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); void bdrv_delete(BlockDriverState *bs); int bdrv_parse_cache_flags(const char *mode, int *flags); int bdrv_parse_discard_flags(const char *mode, int *flags); -int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags); +int bdrv_file_open(BlockDriverState **pbs, const char *filename, + QDict *options, int flags); int bdrv_open_backing_file(BlockDriverState *bs); int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, int flags, BlockDriver *drv); diff --git a/include/block/block_int.h b/include/block/block_int.h index ce0aa26..fb2a136 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -83,7 +83,8 @@ struct BlockDriver { void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state); int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags); - int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags); + int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, + QDict *options, int flags); int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num, diff --git a/qemu-io.c b/qemu-io.c index 79be516..475a8bd 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -1766,7 +1766,7 @@ static int openfile(char *name, int flags, int growable) } if (growable) { - if (bdrv_file_open(&bs, name, flags)) { + if (bdrv_file_open(&bs, name, NULL, flags)) { fprintf(stderr, "%s: can't open device %s\n", progname, name); return 1; } -- cgit v1.1 From 707ff8282b66bb9471e253fe5f17b74576d36825 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 6 Mar 2013 12:20:31 +0100 Subject: block: Pass bdrv_file_open() options to block drivers Specify -drive file.option=... on the command line to pass the option to the protocol instead of the format driver. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 0fb0165..ef1ae94 100644 --- a/block.c +++ b/block.c @@ -676,7 +676,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, assert(drv != NULL); assert(bs->file == NULL); - assert(options == NULL || bs->options != options); + assert(options != NULL && bs->options != options); trace_bdrv_open_common(bs, filename, flags, drv->format_name); @@ -755,22 +755,48 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, BlockDriver *drv; int ret; - QDECREF(options); - drv = bdrv_find_protocol(filename); if (!drv) { + QDECREF(options); return -ENOENT; } + /* NULL means an empty set of options */ + if (options == NULL) { + options = qdict_new(); + } + bs = bdrv_new(""); - ret = bdrv_open_common(bs, NULL, filename, NULL, flags, drv); + bs->options = options; + options = qdict_clone_shallow(options); + + ret = bdrv_open_common(bs, NULL, filename, options, flags, drv); if (ret < 0) { - bdrv_delete(bs); - return ret; + goto fail; + } + + /* Check if any unknown options were used */ + if (qdict_size(options) != 0) { + const QDictEntry *entry = qdict_first(options); + qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't " + "support the option '%s'", + drv->format_name, entry->key); + ret = -EINVAL; + goto fail; } + QDECREF(options); + bs->growable = 1; *pbs = bs; return 0; + +fail: + QDECREF(options); + if (!bs->drv) { + QDECREF(bs->options); + } + bdrv_delete(bs); + return ret; } int bdrv_open_backing_file(BlockDriverState *bs) @@ -810,6 +836,25 @@ int bdrv_open_backing_file(BlockDriverState *bs) return 0; } +static void extract_subqdict(QDict *src, QDict **dst, const char *start) +{ + const QDictEntry *entry, *next; + const char *p; + + *dst = qdict_new(); + entry = qdict_first(src); + + while (entry != NULL) { + next = qdict_next(src, entry); + if (strstart(entry->key, start, &p)) { + qobject_incref(entry->value); + qdict_put_obj(*dst, p, entry->value); + qdict_del(src, entry->key); + } + entry = next; + } +} + /* * Opens a disk image (raw, qcow2, vmdk, ...) * @@ -825,6 +870,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ char tmp_filename[PATH_MAX + 1]; BlockDriverState *file = NULL; + QDict *file_options = NULL; /* NULL means an empty set of options */ if (options == NULL) { @@ -896,7 +942,10 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, flags |= BDRV_O_ALLOW_RDWR; } - ret = bdrv_file_open(&file, filename, NULL, bdrv_open_flags(bs, flags)); + extract_subqdict(options, &file_options, "file."); + + ret = bdrv_file_open(&file, filename, file_options, + bdrv_open_flags(bs, flags)); if (ret < 0) { goto fail; } -- cgit v1.1 From e62be8888a83aa0ab7f50eeb954deb2ec4e7201d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 17:14:24 +0100 Subject: qemu-socket: Make socket_optslist public Allow other users to create the QemuOpts needed for inet_connect_opts(). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/qemu/sockets.h | 2 ++ util/qemu-sockets.c | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index ae5c21c..21846f9 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -30,6 +30,8 @@ int inet_aton(const char *cp, struct in_addr *ia); #include "qapi/error.h" #include "qapi/qmp/qerror.h" +extern QemuOptsList socket_optslist; + /* misc helpers */ int qemu_socket(int domain, int type, int protocol); int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen); diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 83e4e08..39717d0 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -33,10 +33,10 @@ static const int on=1, off=0; -/* used temporarely until all users are converted to QemuOpts */ -static QemuOptsList dummy_opts = { - .name = "dummy", - .head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head), +/* used temporarily until all users are converted to QemuOpts */ +QemuOptsList socket_optslist = { + .name = "socket", + .head = QTAILQ_HEAD_INITIALIZER(socket_optslist.head), .desc = { { .name = "path", @@ -583,7 +583,7 @@ int inet_listen(const char *str, char *ostr, int olen, addr = inet_parse(str, errp); if (addr != NULL) { - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); inet_addr_to_opts(opts, addr); qapi_free_InetSocketAddress(addr); sock = inet_listen_opts(opts, port_offset, errp); @@ -656,7 +656,7 @@ int inet_nonblocking_connect(const char *str, addr = inet_parse(str, errp); if (addr != NULL) { - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); inet_addr_to_opts(opts, addr); qapi_free_InetSocketAddress(addr); sock = inet_connect_opts(opts, errp, callback, opaque); @@ -799,7 +799,7 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp) char *path, *optstr; int sock, len; - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); optstr = strchr(str, ','); if (optstr) { @@ -827,7 +827,7 @@ int unix_connect(const char *path, Error **errp) QemuOpts *opts; int sock; - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); qemu_opt_set(opts, "path", path); sock = unix_connect_opts(opts, errp, NULL, NULL); qemu_opts_del(opts); @@ -844,7 +844,7 @@ int unix_nonblocking_connect(const char *path, g_assert(callback != NULL); - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); qemu_opt_set(opts, "path", path); sock = unix_connect_opts(opts, errp, callback, opaque); qemu_opts_del(opts); @@ -895,7 +895,7 @@ int socket_connect(SocketAddress *addr, Error **errp, QemuOpts *opts; int fd; - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); switch (addr->kind) { case SOCKET_ADDRESS_KIND_INET: inet_addr_to_opts(opts, addr->inet); @@ -926,7 +926,7 @@ int socket_listen(SocketAddress *addr, Error **errp) QemuOpts *opts; int fd; - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); switch (addr->kind) { case SOCKET_ADDRESS_KIND_INET: inet_addr_to_opts(opts, addr->inet); @@ -954,7 +954,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp) QemuOpts *opts; int fd; - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); switch (remote->kind) { case SOCKET_ADDRESS_KIND_INET: qemu_opt_set(opts, "host", remote->inet->host); -- cgit v1.1 From f17c90bed11a6e277614b5a5d16434004f24d572 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 11:55:29 +0100 Subject: nbd: Keep hostname and port separate The NBD block supports an URL syntax, for which a URL parser returns separate hostname and port fields. It also supports the traditional qemu syntax encoded in a filename. Until now, after parsing the URL to get each piece of information, a new string is built to be fed to socket functions. Instead of building a string in the URL case that is immediately parsed again, parse the string in both cases and use the QemuOpts interface to qemu-sockets.c. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 49 ++++++++++++++++++++++++++++++++++++++++--------- include/block/nbd.h | 2 ++ include/qemu/sockets.h | 1 + nbd.c | 12 ++++++++++++ util/qemu-sockets.c | 6 +++--- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index 0473908..ecbc892 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -66,7 +66,10 @@ typedef struct BDRVNBDState { struct nbd_reply reply; int is_unix; - char *host_spec; + char *unix_path; + + InetSocketAddress *inet_addr; + char *export_name; /* An NBD server may export several devices */ } BDRVNBDState; @@ -112,7 +115,7 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) ret = -EINVAL; goto out; } - s->host_spec = g_strdup(qp->p[0].value); + s->unix_path = g_strdup(qp->p[0].value); } else { /* nbd[+tcp]://host:port/export */ if (!uri->server) { @@ -122,7 +125,12 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) if (!uri->port) { uri->port = NBD_DEFAULT_PORT; } - s->host_spec = g_strdup_printf("%s:%d", uri->server, uri->port); + + s->inet_addr = g_new0(InetSocketAddress, 1); + *s->inet_addr = (InetSocketAddress) { + .host = g_strdup(uri->server), + .port = g_strdup_printf("%d", uri->port), + }; } out: @@ -140,6 +148,7 @@ static int nbd_config(BDRVNBDState *s, const char *filename) const char *host_spec; const char *unixpath; int err = -EINVAL; + Error *local_err = NULL; if (strstr(filename, "://")) { return nbd_parse_uri(s, filename); @@ -165,10 +174,15 @@ static int nbd_config(BDRVNBDState *s, const char *filename) /* are we a UNIX or TCP socket? */ if (strstart(host_spec, "unix:", &unixpath)) { s->is_unix = true; - s->host_spec = g_strdup(unixpath); + s->unix_path = g_strdup(unixpath); } else { s->is_unix = false; - s->host_spec = g_strdup(host_spec); + s->inet_addr = inet_parse(host_spec, &local_err); + if (local_err != NULL) { + qerror_report_err(local_err); + error_free(local_err); + goto out; + } } err = 0; @@ -177,7 +191,8 @@ out: g_free(file); if (err != 0) { g_free(s->export_name); - g_free(s->host_spec); + g_free(s->unix_path); + qapi_free_InetSocketAddress(s->inet_addr); } return err; } @@ -328,9 +343,24 @@ static int nbd_establish_connection(BlockDriverState *bs) size_t blocksize; if (s->is_unix) { - sock = unix_socket_outgoing(s->host_spec); + sock = unix_socket_outgoing(s->unix_path); } else { - sock = tcp_socket_outgoing_spec(s->host_spec); + QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist); + + qemu_opt_set(opts, "host", s->inet_addr->host); + qemu_opt_set(opts, "port", s->inet_addr->port); + if (s->inet_addr->has_to) { + qemu_opt_set_number(opts, "to", s->inet_addr->to); + } + if (s->inet_addr->has_ipv4) { + qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4); + } + if (s->inet_addr->has_ipv6) { + qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6); + } + + sock = tcp_socket_outgoing_opts(opts); + qemu_opts_del(opts); } /* Failed to establish connection */ @@ -550,7 +580,8 @@ static void nbd_close(BlockDriverState *bs) { BDRVNBDState *s = bs->opaque; g_free(s->export_name); - g_free(s->host_spec); + g_free(s->unix_path); + qapi_free_InetSocketAddress(s->inet_addr); nbd_teardown_connection(bs); } diff --git a/include/block/nbd.h b/include/block/nbd.h index 344f05b..9b52d50 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -22,6 +22,7 @@ #include #include "qemu-common.h" +#include "qemu/option.h" struct nbd_request { uint32_t magic; @@ -64,6 +65,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port); int tcp_socket_incoming(const char *address, uint16_t port); int tcp_socket_outgoing_spec(const char *address_and_port); int tcp_socket_incoming_spec(const char *address_and_port); +int tcp_socket_outgoing_opts(QemuOpts *opts); int unix_socket_outgoing(const char *path); int unix_socket_incoming(const char *path); diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 21846f9..d225f6d 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -47,6 +47,7 @@ int recv_all(int fd, void *buf, int len1, bool single_read); */ typedef void NonBlockingConnectHandler(int fd, void *opaque); +InetSocketAddress *inet_parse(const char *str, Error **errp); int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp); int inet_listen(const char *str, char *ostr, int olen, int socktype, int port_offset, Error **errp); diff --git a/nbd.c b/nbd.c index 0698a02..97879ca 100644 --- a/nbd.c +++ b/nbd.c @@ -218,6 +218,18 @@ int tcp_socket_outgoing_spec(const char *address_and_port) return fd; } +int tcp_socket_outgoing_opts(QemuOpts *opts) +{ + Error *local_err = NULL; + int fd = inet_connect_opts(opts, &local_err, NULL, NULL); + if (local_err != NULL) { + qerror_report_err(local_err); + error_free(local_err); + } + + return fd; +} + int tcp_socket_incoming(const char *address, uint16_t port) { char address_and_port[128]; diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 39717d0..dc7524d 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -485,7 +485,7 @@ err: } /* compatibility wrapper */ -static InetSocketAddress *inet_parse(const char *str, Error **errp) +InetSocketAddress *inet_parse(const char *str, Error **errp) { InetSocketAddress *addr; const char *optstr, *h; @@ -555,7 +555,7 @@ fail: return NULL; } -static void inet_addr_to_opts(QemuOpts *opts, InetSocketAddress *addr) +static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr) { bool ipv4 = addr->ipv4 || !addr->has_ipv4; bool ipv6 = addr->ipv6 || !addr->has_ipv6; @@ -622,7 +622,7 @@ int inet_connect(const char *str, Error **errp) addr = inet_parse(str, errp); if (addr != NULL) { - opts = qemu_opts_create_nofail(&dummy_opts); + opts = qemu_opts_create_nofail(&socket_optslist); inet_addr_to_opts(opts, addr); qapi_free_InetSocketAddress(addr); sock = inet_connect_opts(opts, errp, NULL, NULL); -- cgit v1.1 From 197a4859b914559489f41b63fd71ea4bfda17c3d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 16:46:38 +0100 Subject: nbd: Remove unused functions Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- include/block/nbd.h | 2 -- nbd.c | 19 ------------------- 2 files changed, 21 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 9b52d50..0903d7a 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -61,9 +61,7 @@ enum { #define NBD_BUFFER_SIZE (1024*1024) ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read); -int tcp_socket_outgoing(const char *address, uint16_t port); int tcp_socket_incoming(const char *address, uint16_t port); -int tcp_socket_outgoing_spec(const char *address_and_port); int tcp_socket_incoming_spec(const char *address_and_port); int tcp_socket_outgoing_opts(QemuOpts *opts); int unix_socket_outgoing(const char *path); diff --git a/nbd.c b/nbd.c index 97879ca..d1a67ee 100644 --- a/nbd.c +++ b/nbd.c @@ -199,25 +199,6 @@ static void combine_addr(char *buf, size_t len, const char* address, } } -int tcp_socket_outgoing(const char *address, uint16_t port) -{ - char address_and_port[128]; - combine_addr(address_and_port, 128, address, port); - return tcp_socket_outgoing_spec(address_and_port); -} - -int tcp_socket_outgoing_spec(const char *address_and_port) -{ - Error *local_err = NULL; - int fd = inet_connect(address_and_port, &local_err); - - if (local_err != NULL) { - qerror_report_err(local_err); - error_free(local_err); - } - return fd; -} - int tcp_socket_outgoing_opts(QemuOpts *opts) { Error *local_err = NULL; -- cgit v1.1 From f53a1febcd9d887149ac1429880a3f2fdb2c117f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 7 Mar 2013 16:15:11 +0100 Subject: nbd: Accept -drive options for the network connection The existing parsers for the file name now parse everything into the bdrv_open() options QDict. Instead of using these parsers, you can now directly specify the options on the command line, like this: qemu-system-x86_64 -drive file=nbd:,file.port=1234,file.host=::1 Clearly the file=... part could use further improvement, but it's a start. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 129 ++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 77 insertions(+), 52 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index ecbc892..5ed8502 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -32,6 +32,8 @@ #include "block/block_int.h" #include "qemu/module.h" #include "qemu/sockets.h" +#include "qapi/qmp/qjson.h" +#include "qapi/qmp/qint.h" #include #include @@ -65,20 +67,19 @@ typedef struct BDRVNBDState { Coroutine *recv_coroutine[MAX_NBD_REQUESTS]; struct nbd_reply reply; - int is_unix; - char *unix_path; - - InetSocketAddress *inet_addr; + bool is_unix; + QemuOpts *socket_opts; char *export_name; /* An NBD server may export several devices */ } BDRVNBDState; -static int nbd_parse_uri(BDRVNBDState *s, const char *filename) +static int nbd_parse_uri(const char *filename, QDict *options) { URI *uri; const char *p; QueryParams *qp = NULL; int ret = 0; + bool is_unix; uri = uri_parse(filename); if (!uri) { @@ -87,11 +88,11 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) /* transport */ if (!strcmp(uri->scheme, "nbd")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "nbd+tcp")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "nbd+unix")) { - s->is_unix = true; + is_unix = true; } else { ret = -EINVAL; goto out; @@ -100,24 +101,26 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) p = uri->path ? uri->path : "/"; p += strspn(p, "/"); if (p[0]) { - s->export_name = g_strdup(p); + qdict_put(options, "export", qstring_from_str(p)); } qp = query_params_parse(uri->query); - if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) { + if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) { ret = -EINVAL; goto out; } - if (s->is_unix) { + if (is_unix) { /* nbd+unix:///export?socket=path */ if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) { ret = -EINVAL; goto out; } - s->unix_path = g_strdup(qp->p[0].value); + qdict_put(options, "path", qstring_from_str(qp->p[0].value)); } else { /* nbd[+tcp]://host:port/export */ + char *port_str; + if (!uri->server) { ret = -EINVAL; goto out; @@ -126,11 +129,10 @@ static int nbd_parse_uri(BDRVNBDState *s, const char *filename) uri->port = NBD_DEFAULT_PORT; } - s->inet_addr = g_new0(InetSocketAddress, 1); - *s->inet_addr = (InetSocketAddress) { - .host = g_strdup(uri->server), - .port = g_strdup_printf("%d", uri->port), - }; + port_str = g_strdup_printf("%d", uri->port); + qdict_put(options, "host", qstring_from_str(uri->server)); + qdict_put(options, "port", qstring_from_str(port_str)); + g_free(port_str); } out: @@ -141,17 +143,17 @@ out: return ret; } -static int nbd_config(BDRVNBDState *s, const char *filename) +static int nbd_parse_filename(const char *filename, QDict *options) { char *file; char *export_name; const char *host_spec; const char *unixpath; - int err = -EINVAL; + int ret = -EINVAL; Error *local_err = NULL; if (strstr(filename, "://")) { - return nbd_parse_uri(s, filename); + return nbd_parse_uri(filename, options); } file = g_strdup(filename); @@ -163,7 +165,8 @@ static int nbd_config(BDRVNBDState *s, const char *filename) } export_name[0] = 0; /* truncate 'file' */ export_name += strlen(EN_OPTSTR); - s->export_name = g_strdup(export_name); + + qdict_put(options, "export", qstring_from_str(export_name)); } /* extract the host_spec - fail if it's not nbd:... */ @@ -171,32 +174,65 @@ static int nbd_config(BDRVNBDState *s, const char *filename) goto out; } + if (!*host_spec) { + ret = 1; + goto out; + } + /* are we a UNIX or TCP socket? */ if (strstart(host_spec, "unix:", &unixpath)) { - s->is_unix = true; - s->unix_path = g_strdup(unixpath); + qdict_put(options, "path", qstring_from_str(unixpath)); } else { - s->is_unix = false; - s->inet_addr = inet_parse(host_spec, &local_err); + InetSocketAddress *addr = NULL; + + addr = inet_parse(host_spec, &local_err); if (local_err != NULL) { qerror_report_err(local_err); error_free(local_err); goto out; } - } - err = 0; + qdict_put(options, "host", qstring_from_str(addr->host)); + qdict_put(options, "port", qstring_from_str(addr->port)); + qapi_free_InetSocketAddress(addr); + } + ret = 1; out: g_free(file); - if (err != 0) { - g_free(s->export_name); - g_free(s->unix_path); - qapi_free_InetSocketAddress(s->inet_addr); + return ret; +} + +static int nbd_config(BDRVNBDState *s, QDict *options) +{ + Error *local_err = NULL; + + if (qdict_haskey(options, "path")) { + s->is_unix = true; + } else if (qdict_haskey(options, "host")) { + s->is_unix = false; + } else { + return -EINVAL; } - return err; + + s->socket_opts = qemu_opts_create_nofail(&socket_optslist); + + qemu_opts_absorb_qdict(s->socket_opts, options, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + return -EINVAL; + } + + s->export_name = g_strdup(qdict_get_try_str(options, "export")); + if (s->export_name) { + qdict_del(options, "export"); + } + + return 0; } + static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request) { int i; @@ -343,24 +379,9 @@ static int nbd_establish_connection(BlockDriverState *bs) size_t blocksize; if (s->is_unix) { - sock = unix_socket_outgoing(s->unix_path); + sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path")); } else { - QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist); - - qemu_opt_set(opts, "host", s->inet_addr->host); - qemu_opt_set(opts, "port", s->inet_addr->port); - if (s->inet_addr->has_to) { - qemu_opt_set_number(opts, "to", s->inet_addr->to); - } - if (s->inet_addr->has_ipv4) { - qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4); - } - if (s->inet_addr->has_ipv6) { - qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6); - } - - sock = tcp_socket_outgoing_opts(opts); - qemu_opts_del(opts); + sock = tcp_socket_outgoing_opts(s->socket_opts); } /* Failed to establish connection */ @@ -416,7 +437,12 @@ static int nbd_open(BlockDriverState *bs, const char* filename, qemu_co_mutex_init(&s->free_sema); /* Pop the config into our state object. Exit if invalid. */ - result = nbd_config(s, filename); + result = nbd_parse_filename(filename, options); + if (result < 0) { + return result; + } + + result = nbd_config(s, options); if (result != 0) { return result; } @@ -580,8 +606,7 @@ static void nbd_close(BlockDriverState *bs) { BDRVNBDState *s = bs->opaque; g_free(s->export_name); - g_free(s->unix_path); - qapi_free_InetSocketAddress(s->inet_addr); + qemu_opts_del(s->socket_opts); nbd_teardown_connection(bs); } -- cgit v1.1 From 6963a30d82413bea36c7545137b090b284cc2b18 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 15 Mar 2013 18:47:22 +0100 Subject: block: Introduce .bdrv_parse_filename callback If a driver needs structured data and not just a string, it can provide a .bdrv_parse_filename callback now that parses the command line string into separate options. Keeping this separate from .bdrv_open_filename ensures that the preferred way of directly specifying the options always works as well if parsing the string works. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 11 +++++++++++ block/nbd.c | 29 +++++++++++++---------------- include/block/block_int.h | 1 + 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index ef1ae94..8a95a28 100644 --- a/block.c +++ b/block.c @@ -770,6 +770,17 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, bs->options = options; options = qdict_clone_shallow(options); + if (drv->bdrv_parse_filename) { + Error *local_err = NULL; + drv->bdrv_parse_filename(filename, options, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + ret = -EINVAL; + goto fail; + } + } + ret = bdrv_open_common(bs, NULL, filename, options, flags, drv); if (ret < 0) { goto fail; diff --git a/block/nbd.c b/block/nbd.c index 5ed8502..9858f06 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -143,17 +143,20 @@ out: return ret; } -static int nbd_parse_filename(const char *filename, QDict *options) +static void nbd_parse_filename(const char *filename, QDict *options, + Error **errp) { char *file; char *export_name; const char *host_spec; const char *unixpath; - int ret = -EINVAL; - Error *local_err = NULL; if (strstr(filename, "://")) { - return nbd_parse_uri(filename, options); + int ret = nbd_parse_uri(filename, options); + if (ret < 0) { + error_setg(errp, "No valid URL specified"); + } + return; } file = g_strdup(filename); @@ -171,11 +174,11 @@ static int nbd_parse_filename(const char *filename, QDict *options) /* extract the host_spec - fail if it's not nbd:... */ if (!strstart(file, "nbd:", &host_spec)) { + error_setg(errp, "File name string for NBD must start with 'nbd:'"); goto out; } if (!*host_spec) { - ret = 1; goto out; } @@ -185,10 +188,8 @@ static int nbd_parse_filename(const char *filename, QDict *options) } else { InetSocketAddress *addr = NULL; - addr = inet_parse(host_spec, &local_err); - if (local_err != NULL) { - qerror_report_err(local_err); - error_free(local_err); + addr = inet_parse(host_spec, errp); + if (error_is_set(errp)) { goto out; } @@ -197,10 +198,8 @@ static int nbd_parse_filename(const char *filename, QDict *options) qapi_free_InetSocketAddress(addr); } - ret = 1; out: g_free(file); - return ret; } static int nbd_config(BDRVNBDState *s, QDict *options) @@ -437,11 +436,6 @@ static int nbd_open(BlockDriverState *bs, const char* filename, qemu_co_mutex_init(&s->free_sema); /* Pop the config into our state object. Exit if invalid. */ - result = nbd_parse_filename(filename, options); - if (result < 0) { - return result; - } - result = nbd_config(s, options); if (result != 0) { return result; @@ -622,6 +616,7 @@ static BlockDriver bdrv_nbd = { .format_name = "nbd", .protocol_name = "nbd", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, @@ -635,6 +630,7 @@ static BlockDriver bdrv_nbd_tcp = { .format_name = "nbd", .protocol_name = "nbd+tcp", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, @@ -648,6 +644,7 @@ static BlockDriver bdrv_nbd_unix = { .format_name = "nbd", .protocol_name = "nbd+unix", .instance_size = sizeof(BDRVNBDState), + .bdrv_parse_filename = nbd_parse_filename, .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev = nbd_co_writev, diff --git a/include/block/block_int.h b/include/block/block_int.h index fb2a136..1b06a11 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -75,6 +75,7 @@ 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); + void (*bdrv_parse_filename)(const char *filename, QDict *options, Error **errp); /* For handling image reopen for split or non-split files */ int (*bdrv_reopen_prepare)(BDRVReopenState *reopen_state, -- cgit v1.1 From 08b392e1510b21d8c9bfa8a50525fae31014a2e2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:17:44 +0100 Subject: block: Rename variable to avoid shadowing bdrv_open() uses two different variables called options. Rename one of them to avoid confusion and to allow the outer one to be accessed everywhere. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 8a95a28..2c17a34 100644 --- a/block.c +++ b/block.c @@ -896,7 +896,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, BlockDriverState *bs1; int64_t total_size; BlockDriver *bdrv_qcow2; - QEMUOptionParameter *options; + QEMUOptionParameter *create_options; char backing_filename[PATH_MAX]; /* if snapshot, we create a temporary backing file and open it @@ -928,17 +928,19 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } bdrv_qcow2 = bdrv_find_format("qcow2"); - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); + create_options = parse_option_parameters("", bdrv_qcow2->create_options, + NULL); - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); + set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size); + set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE, + backing_filename); if (drv) { - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, + set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT, drv->format_name); } - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); - free_option_parameters(options); + ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options); + free_option_parameters(create_options); if (ret < 0) { goto fail; } -- cgit v1.1 From f5866fa438bff586f215c137dc71edb4e0770536 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:20:27 +0100 Subject: block: Make find_image_format safe with NULL filename In order to achieve this, the .bdrv_probe callbacks of all drivers must cope with this. The DMG driver is the only one that bases its decision on the filename and it needs to be changed. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/dmg.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/block/dmg.c b/block/dmg.c index c1066df..3141cb5 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -51,9 +51,16 @@ typedef struct BDRVDMGState { static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename) { - int len=strlen(filename); - if(len>4 && !strcmp(filename+len-4,".dmg")) - return 2; + int len; + + if (!filename) { + return 0; + } + + len = strlen(filename); + if (len > 4 && !strcmp(filename + len - 4, ".dmg")) { + return 2; + } return 0; } -- cgit v1.1 From c2ad1b0c465a9ea8375eaff14bbd85705c673f73 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:40:51 +0100 Subject: 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 2c17a34..16a92a4 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 6f2b759..8cdc9ce 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 1b06a11..0986a2d 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 */ -- cgit v1.1 From bebbf7fa9c6235022ecd15f8f934d27e5ccab63a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 16:56:05 +0100 Subject: nbd: Use default port if only host is specified The URL method already takes care to apply the default port when none is specfied. Directly specifying driver-specific options required the port number until now. Allow leaving it out and apply the default. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index 9858f06..67f1df2 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -118,21 +118,18 @@ static int nbd_parse_uri(const char *filename, QDict *options) } qdict_put(options, "path", qstring_from_str(qp->p[0].value)); } else { - /* nbd[+tcp]://host:port/export */ - char *port_str; - + /* nbd[+tcp]://host[:port]/export */ if (!uri->server) { ret = -EINVAL; goto out; } - if (!uri->port) { - uri->port = NBD_DEFAULT_PORT; - } - port_str = g_strdup_printf("%d", uri->port); qdict_put(options, "host", qstring_from_str(uri->server)); - qdict_put(options, "port", qstring_from_str(port_str)); - g_free(port_str); + if (uri->port) { + char* port_str = g_strdup_printf("%d", uri->port); + qdict_put(options, "port", qstring_from_str(port_str)); + g_free(port_str); + } } out: @@ -223,6 +220,10 @@ static int nbd_config(BDRVNBDState *s, QDict *options) return -EINVAL; } + if (!qemu_opt_get(s->socket_opts, "port")) { + qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT); + } + s->export_name = g_strdup(qdict_get_try_str(options, "export")); if (s->export_name) { qdict_del(options, "export"); -- cgit v1.1 From 681e7ad024d80123a1ae8e35f86fb1a7f03b1bc9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 20 Mar 2013 19:23:23 +0100 Subject: nbd: Check against invalid option combinations A file name may only specified if no host or socket path is specified. The latter two may not appear at the same time either. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/nbd.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/block/nbd.c b/block/nbd.c index 67f1df2..3d711b2 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -148,6 +148,15 @@ static void nbd_parse_filename(const char *filename, QDict *options, const char *host_spec; const char *unixpath; + if (qdict_haskey(options, "host") + || qdict_haskey(options, "port") + || qdict_haskey(options, "path")) + { + error_setg(errp, "host/port/path and a file name may not be specified " + "at the same time"); + return; + } + if (strstr(filename, "://")) { int ret = nbd_parse_uri(filename, options); if (ret < 0) { @@ -204,6 +213,11 @@ static int nbd_config(BDRVNBDState *s, QDict *options) Error *local_err = NULL; if (qdict_haskey(options, "path")) { + if (qdict_haskey(options, "host")) { + qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not " + "be used at the same time."); + return -EINVAL; + } s->is_unix = true; } else if (qdict_haskey(options, "host")) { s->is_unix = false; -- cgit v1.1