* [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open()
@ 2014-02-18 17:33 Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
bdrv_file_open() is now nearly a subset of bdrv_open(), except for the
fact that bdrv_file_open() is for protocols and bdrv_open() for block
drivers. It is possible to use bdrv_file_open() with a block driver, but
in that case that block driver must be explicitly specified.
Due to these great similarities, bdrv_file_open() can be integrated and
made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is
specified, bdrv_open() will now do what bdrv_file_open() used to do:
Auto-detecting a protocol instead of a block driver.
This series implements this and changes all calls to bdrv_file_open() to
bdrv_open() calls with BDRV_O_PROTOCOL specified.
Note that this flag cannot be discerned automatically since it is
impossible for bdrv_open() to know by itself whether a given file should
be opened with or without the format layer involved: Both are valid
alternatives. Therefore, it still has to be specified by the user.
v4 (rebased on Kevin's block branch):
- patch 2: use Benoît's bdrv_lookup_bs() instead of bdrv_find()
- patch 4: resolved rebase conflict (the v3 hunk removed a bdrv_find(),
whereas now it is a bdrv_lookup_bs())
v3:
- patch 1:
- Kevin: add assert(pbs) and assert(*pbs == NULL) in
bdrv_open_image()
- Benoît: remove spurious new line removal
- patch 2:
- Benoît: document reference parameter of bdrv_open()
- patch 3:
- add the same assertions before every bdrv_file_open() translated to
bdrv_open() and try not to initialize BDS pointers to NULL but
rather set them right in front of bdrv_open() as in v2 for patch 1
- remove addition of *pbs = NULL in bdrv_open_image() in front of the
bdrv_file_open() being translated to bdrv_open()
- patch 7:
- Kevin: remove wrongly added
"else if (drv->bdrv_needs_filename && !filename)" block
- patch 8:
- since patch 3 now does not add the *pbs = NULL anymore, this patch
does not need to remove it either (I added the Reviewed-bys anyway,
since this is just about temporary code introduced by this series)
v2:
- patch 1:
- added appropriate assertions before every bdrv_open() and
bdrv_open_image() call where the BDS should be NULL (i.e., where a
new BDS should be created) and it is not obvious
- don't initialize BDS pointers to NULL if that is necessary for a
bdrv_open(); rather, set them to NULL directly before calling
bdrv_open() in order to make the expected behavior of bdrv_open()
(create a new BDS) more obvious
- make bdrv_open_image() behave the same way as bdrv_open() in regard
to BDS creation/reuse (completed in patch 8)
- keep "options == NULL" check in bdrv_open() together with the block
for setting "bs->options"
- patch 2:
- due to options now possibly being NULL in the reference handling
block of bdrv_open(), the condition for "options_non_empty" has to
be adjusted
- contextual changes due to patch 1
- patch 3:
- adjust comment for BDRV_O_PROTOCOL
- contextual changes due to the previous patches
- patch 4 (previously 5):
- contextual changes due to the previous patches
- patch 5 (previously 6):
- bdrv_close() is unnecessary if the BDS could not be opened
- patch 6:
- melded together from previous patches 4, 7 and 8
- patch 7 (previously 9):
- make the "options" parameter of bdrv_file_open() an indirect
pointer; if bdrv_file_open() takes the reference, it will be set to
NULL; otherwise, the caller is responsible for freeing the QDict
- patch 8 (previously 10):
- make bdrv_open_image() behave the same way as bdrv_open() in regard
to BDS creation/reuse
- contextual changes due to the previous patches
Max Reitz (8):
block: Change BDS parameter of bdrv_open() to **
block: Add reference parameter to bdrv_open()
block: Make bdrv_file_open() static
block: Reuse reference handling from bdrv_open()
block: Remove bdrv_new() from bdrv_file_open()
block: Handle bs->options in bdrv_open() only
block: Reuse success path from bdrv_open()
block: Remove bdrv_open_image()'s force_raw option
block.c | 226 +++++++++++++++++++++++++-------------------------
block/blkdebug.c | 3 +-
block/blkverify.c | 6 +-
block/cow.c | 5 +-
block/qcow.c | 5 +-
block/qcow2.c | 18 ++--
block/qed.c | 8 +-
block/sheepdog.c | 7 +-
block/vhdx.c | 4 +-
block/vmdk.c | 19 +++--
block/vvfat.c | 6 +-
blockdev.c | 22 +++--
hw/block/xen_disk.c | 4 +-
include/block/block.h | 13 +--
qemu-img.c | 12 ++-
qemu-io.c | 8 +-
qemu-nbd.c | 2 +-
17 files changed, 197 insertions(+), 171 deletions(-)
--
1.8.5.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 1/8] block: Change BDS parameter of bdrv_open() to **
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 2/8] block: Add reference parameter to bdrv_open() Max Reitz
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Make bdrv_open() take a pointer to a BDS pointer, similarly to
bdrv_file_open(). If a pointer to a NULL pointer is given, bdrv_open()
will create a new BDS with an empty name; if the BDS pointer is not
NULL, that existing BDS will be reused (in the same way as bdrv_open()
already did).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 64 ++++++++++++++++++++++++++++++++-------------------
block/blkdebug.c | 1 +
block/blkverify.c | 2 ++
block/qcow2.c | 14 +++++++----
block/vmdk.c | 5 ++--
block/vvfat.c | 6 ++---
blockdev.c | 20 ++++++++--------
hw/block/xen_disk.c | 2 +-
include/block/block.h | 2 +-
qemu-img.c | 10 ++++----
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
12 files changed, 73 insertions(+), 57 deletions(-)
diff --git a/block.c b/block.c
index 0da2f72..a736b03 100644
--- a/block.c
+++ b/block.c
@@ -1046,7 +1046,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
}
if (!drv->bdrv_file_open) {
- ret = bdrv_open(bs, filename, options, flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, options, flags, drv, &local_err);
options = NULL;
} else {
ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
@@ -1115,8 +1115,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
sizeof(backing_filename));
}
- bs->backing_hd = bdrv_new("");
-
if (bs->backing_format[0] != '\0') {
back_drv = bdrv_find_format(bs->backing_format);
}
@@ -1125,11 +1123,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
BDRV_O_COPY_ON_READ);
- ret = bdrv_open(bs->backing_hd,
+ assert(bs->backing_hd == NULL);
+ ret = bdrv_open(&bs->backing_hd,
*backing_filename ? backing_filename : NULL, options,
back_flags, back_drv, &local_err);
if (ret < 0) {
- bdrv_unref(bs->backing_hd);
bs->backing_hd = NULL;
bs->open_flags |= BDRV_O_NO_BACKING;
error_setg(errp, "Could not open backing file: %s",
@@ -1166,6 +1164,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
* BlockdevRef.
*
* The BlockdevRef will be removed from the options QDict.
+ *
+ * To conform with the behavior of bdrv_open(), *pbs has to be NULL.
*/
int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
@@ -1176,6 +1176,9 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
char *bdref_key_dot;
const char *reference;
+ assert(pbs);
+ assert(*pbs == NULL);
+
bdref_key_dot = g_strdup_printf("%s.", bdref_key);
qdict_extract_subqdict(options, &image_options, bdref_key_dot);
g_free(bdref_key_dot);
@@ -1196,8 +1199,6 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
/* If a filename is given and the block driver should be detected
automatically (instead of using none), use bdrv_open() in order to do
that auto-detection. */
- BlockDriverState *bs;
-
if (reference) {
error_setg(errp, "Cannot reference an existing block device while "
"giving a filename");
@@ -1205,13 +1206,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
goto done;
}
- bs = bdrv_new("");
- ret = bdrv_open(bs, filename, image_options, flags, NULL, errp);
- if (ret < 0) {
- bdrv_unref(bs);
- } else {
- *pbs = bs;
- }
+ ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
} else {
ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
errp);
@@ -1229,17 +1224,28 @@ done:
* 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_open.
+ *
+ * If *pbs is NULL, a new BDS will be created with a pointer to it stored there.
+ * If it is not NULL, the referenced BDS will be reused.
*/
-int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
+int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
int flags, BlockDriver *drv, Error **errp)
{
int ret;
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
char tmp_filename[PATH_MAX + 1];
- BlockDriverState *file = NULL;
+ BlockDriverState *file = NULL, *bs;
const char *drvname;
Error *local_err = NULL;
+ assert(pbs);
+
+ if (*pbs) {
+ bs = *pbs;
+ } else {
+ bs = bdrv_new("");
+ }
+
/* NULL means an empty set of options */
if (options == NULL) {
options = qdict_new();
@@ -1260,12 +1266,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
instead of opening 'filename' directly */
/* Get the required size from the image */
- bs1 = bdrv_new("");
QINCREF(options);
- ret = bdrv_open(bs1, filename, options, BDRV_O_NO_BACKING,
+ bs1 = NULL;
+ ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING,
drv, &local_err);
if (ret < 0) {
- bdrv_unref(bs1);
goto fail;
}
total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
@@ -1322,6 +1327,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
flags |= BDRV_O_ALLOW_RDWR;
}
+ assert(file == NULL);
ret = bdrv_open_image(&file, filename, options, "file",
bdrv_open_flags(bs, flags | BDRV_O_UNMAP), true, true,
&local_err);
@@ -1393,6 +1399,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
bdrv_dev_change_media_cb(bs, true);
}
+ *pbs = bs;
return 0;
unlink_and_fail:
@@ -1406,13 +1413,24 @@ fail:
QDECREF(bs->options);
QDECREF(options);
bs->options = NULL;
+ if (!*pbs) {
+ /* If *pbs is NULL, a new BDS has been created in this function and
+ needs to be freed now. Otherwise, it does not need to be closed,
+ since it has not really been opened yet. */
+ bdrv_unref(bs);
+ }
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
}
return ret;
close_and_fail:
- bdrv_close(bs);
+ /* See fail path, but now the BDS has to be always closed */
+ if (*pbs) {
+ bdrv_close(bs);
+ } else {
+ bdrv_unref(bs);
+ }
QDECREF(options);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
@@ -5290,9 +5308,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
back_flags =
flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
- bs = bdrv_new("");
-
- ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
+ bs = NULL;
+ ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags,
backing_drv, &local_err);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s': %s",
@@ -5300,7 +5317,6 @@ void bdrv_img_create(const char *filename, const char *fmt,
error_get_pretty(local_err));
error_free(local_err);
local_err = NULL;
- bdrv_unref(bs);
goto out;
}
bdrv_get_geometry(bs, &size);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 8eb0db0..053fa4c 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -410,6 +410,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
s->state = 1;
/* Open the backing file */
+ assert(bs->file == NULL);
ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image",
flags, true, false, &local_err);
if (ret < 0) {
diff --git a/block/blkverify.c b/block/blkverify.c
index cfcbcf4..86585e7 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -135,6 +135,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Open the raw file */
+ assert(bs->file == NULL);
ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options,
"raw", flags, true, false, &local_err);
if (ret < 0) {
@@ -143,6 +144,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Open the test file */
+ assert(s->test_file == NULL);
ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options,
"test", flags, false, false, &local_err);
if (ret < 0) {
diff --git a/block/qcow2.c b/block/qcow2.c
index 0b4335c..c8e8ba7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1543,7 +1543,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
goto out;
}
- bdrv_close(bs);
+ bdrv_unref(bs);
+ bs = NULL;
/*
* And now open the image and make it consistent first (i.e. increase the
@@ -1552,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
*/
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
- ret = bdrv_open(bs, filename, NULL,
+ ret = bdrv_open(&bs, filename, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1599,10 +1600,11 @@ static int qcow2_create2(const char *filename, int64_t total_size,
}
}
- bdrv_close(bs);
+ bdrv_unref(bs);
+ bs = NULL;
/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
- ret = bdrv_open(bs, filename, NULL,
+ ret = bdrv_open(&bs, filename, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
drv, &local_err);
if (error_is_set(&local_err)) {
@@ -1612,7 +1614,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
ret = 0;
out:
- bdrv_unref(bs);
+ if (bs) {
+ bdrv_unref(bs);
+ }
return ret;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index ff6f5ee..0622db5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1755,10 +1755,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
goto exit;
}
if (backing_file) {
- BlockDriverState *bs = bdrv_new("");
- ret = bdrv_open(bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
+ BlockDriverState *bs = NULL;
+ ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
if (ret != 0) {
- bdrv_unref(bs);
goto exit;
}
if (strcmp(bs->drv->format_name, "vmdk")) {
diff --git a/block/vvfat.c b/block/vvfat.c
index 664941c..ae7bc6f 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2936,15 +2936,13 @@ static int enable_write_target(BDRVVVFATState *s)
goto err;
}
- s->qcow = bdrv_new("");
-
- ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
+ s->qcow = NULL;
+ ret = bdrv_open(&s->qcow, s->qcow_filename, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
&local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
- bdrv_unref(s->qcow);
goto err;
}
diff --git a/blockdev.c b/blockdev.c
index dfb5ec7..9d54f33 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -504,7 +504,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
QINCREF(bs_opts);
- ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
+ ret = bdrv_open(&dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
if (ret < 0) {
error_setg(errp, "could not open disk image %s: %s",
@@ -1326,12 +1326,12 @@ static void external_snapshot_prepare(BlkTransactionState *common,
qstring_from_str(snapshot_node_name));
}
- /* We will manually add the backing_hd field to the bs later */
- state->new_bs = bdrv_new("");
/* TODO Inherit bs->options or only take explicit options with an
* extended QMP command? */
- ret = bdrv_open(state->new_bs, new_image_file, options,
+ assert(state->new_bs == NULL);
+ ret = bdrv_open(&state->new_bs, new_image_file, options,
flags | BDRV_O_NO_BACKING, drv, &local_err);
+ /* We will manually add the backing_hd field to the bs later */
if (ret != 0) {
error_propagate(errp, local_err);
}
@@ -1578,7 +1578,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
Error *local_err = NULL;
int ret;
- ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, bdrv_flags, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
@@ -2014,10 +2014,9 @@ void qmp_drive_backup(const char *device, const char *target,
return;
}
- target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
+ target_bs = NULL;
+ ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
if (ret < 0) {
- bdrv_unref(target_bs);
error_propagate(errp, local_err);
return;
}
@@ -2158,11 +2157,10 @@ void qmp_drive_mirror(const char *device, const char *target,
/* Mirroring takes care of copy-on-write using the source's backing
* file.
*/
- target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+ target_bs = NULL;
+ ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
&local_err);
if (ret < 0) {
- bdrv_unref(target_bs);
error_propagate(errp, local_err);
return;
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 098f6c6..14e6d0b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -813,7 +813,7 @@ static int blk_connect(struct XenDevice *xendev)
Error *local_err = NULL;
BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
readonly);
- if (bdrv_open(blkdev->bs,
+ if (bdrv_open(&blkdev->bs,
blkdev->filename, NULL, qflags, drv, &local_err) != 0)
{
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
diff --git a/include/block/block.h b/include/block/block.h
index 963a61f..980869d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -190,7 +190,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
bool force_raw, bool allow_none, Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
-int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
+int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
int flags, BlockDriver *drv, Error **errp);
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
BlockDriverState *bs, int flags);
diff --git a/qemu-img.c b/qemu-img.c
index c989850..897aa56 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -289,7 +289,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
drv = NULL;
}
- ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, flags, drv, &local_err);
if (ret < 0) {
error_report("Could not open '%s': %s", filename,
error_get_pretty(local_err));
@@ -310,9 +310,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
}
return bs;
fail:
- if (bs) {
- bdrv_unref(bs);
- }
+ bdrv_unref(bs);
return NULL;
}
@@ -2314,7 +2312,7 @@ static int img_rebase(int argc, char **argv)
bs_old_backing = bdrv_new("old_backing");
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
- ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
+ ret = bdrv_open(&bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
old_backing_drv, &local_err);
if (ret) {
error_report("Could not open old backing file '%s': %s",
@@ -2324,7 +2322,7 @@ static int img_rebase(int argc, char **argv)
}
if (out_baseimg[0]) {
bs_new_backing = bdrv_new("new_backing");
- ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
+ ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
new_backing_drv, &local_err);
if (ret) {
error_report("Could not open new backing file '%s': %s",
diff --git a/qemu-io.c b/qemu-io.c
index 7f459d8..8da8f6e 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -68,7 +68,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
} else {
qemuio_bs = bdrv_new("hda");
- if (bdrv_open(qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
+ if (bdrv_open(&qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
error_get_pretty(local_err));
error_free(local_err);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 136e8c9..0cf123c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -597,7 +597,7 @@ int main(int argc, char **argv)
bs = bdrv_new("hda");
srcpath = argv[optind];
- ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
+ ret = bdrv_open(&bs, srcpath, NULL, flags, drv, &local_err);
if (ret < 0) {
errno = -ret;
err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 2/8] block: Add reference parameter to bdrv_open()
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 3/8] block: Make bdrv_file_open() static Max Reitz
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Allow bdrv_open() to handle references to existing block devices just as
bdrv_file_open() is already capable of.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 44 +++++++++++++++++++++++++++++++++++++-------
block/qcow2.c | 4 ++--
block/vmdk.c | 3 ++-
block/vvfat.c | 2 +-
blockdev.c | 12 ++++++------
hw/block/xen_disk.c | 4 ++--
include/block/block.h | 5 +++--
qemu-img.c | 8 ++++----
qemu-io.c | 4 +++-
qemu-nbd.c | 2 +-
10 files changed, 61 insertions(+), 27 deletions(-)
diff --git a/block.c b/block.c
index a736b03..114f057 100644
--- a/block.c
+++ b/block.c
@@ -1046,7 +1046,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
}
if (!drv->bdrv_file_open) {
- ret = bdrv_open(&bs, filename, options, flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, options, flags, drv, &local_err);
options = NULL;
} else {
ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
@@ -1125,7 +1125,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
assert(bs->backing_hd == NULL);
ret = bdrv_open(&bs->backing_hd,
- *backing_filename ? backing_filename : NULL, options,
+ *backing_filename ? backing_filename : NULL, NULL, options,
back_flags, back_drv, &local_err);
if (ret < 0) {
bs->backing_hd = NULL;
@@ -1206,7 +1206,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
goto done;
}
- ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
+ ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
} else {
ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
errp);
@@ -1227,9 +1227,14 @@ done:
*
* If *pbs is NULL, a new BDS will be created with a pointer to it stored there.
* If it is not NULL, the referenced BDS will be reused.
+ *
+ * The reference parameter may be used to specify an existing block device which
+ * should be opened. If specified, neither options nor a filename may be given,
+ * nor can an existing BDS be reused (that is, *pbs has to be NULL).
*/
-int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
- int flags, BlockDriver *drv, Error **errp)
+int bdrv_open(BlockDriverState **pbs, const char *filename,
+ const char *reference, QDict *options, int flags,
+ BlockDriver *drv, Error **errp)
{
int ret;
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -1240,6 +1245,31 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
assert(pbs);
+ if (reference) {
+ bool options_non_empty = options ? qdict_size(options) : false;
+ QDECREF(options);
+
+ if (*pbs) {
+ error_setg(errp, "Cannot reuse an existing BDS when referencing "
+ "another block device");
+ return -EINVAL;
+ }
+
+ if (filename || options_non_empty) {
+ error_setg(errp, "Cannot reference an existing block device with "
+ "additional options or a new filename");
+ return -EINVAL;
+ }
+
+ bs = bdrv_lookup_bs(reference, reference, errp);
+ if (!bs) {
+ return -ENODEV;
+ }
+ bdrv_ref(bs);
+ *pbs = bs;
+ return 0;
+ }
+
if (*pbs) {
bs = *pbs;
} else {
@@ -1268,7 +1298,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
/* Get the required size from the image */
QINCREF(options);
bs1 = NULL;
- ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING,
+ ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
drv, &local_err);
if (ret < 0) {
goto fail;
@@ -5309,7 +5339,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
bs = NULL;
- ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags,
+ ret = bdrv_open(&bs, backing_file->value.s, NULL, NULL, back_flags,
backing_drv, &local_err);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s': %s",
diff --git a/block/qcow2.c b/block/qcow2.c
index c8e8ba7..6996276 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1553,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
*/
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
- ret = bdrv_open(&bs, filename, NULL,
+ ret = bdrv_open(&bs, filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1604,7 +1604,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
bs = NULL;
/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
- ret = bdrv_open(&bs, filename, NULL,
+ ret = bdrv_open(&bs, filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
drv, &local_err);
if (error_is_set(&local_err)) {
diff --git a/block/vmdk.c b/block/vmdk.c
index 0622db5..5df486f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1756,7 +1756,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
}
if (backing_file) {
BlockDriverState *bs = NULL;
- ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
+ ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BACKING, NULL,
+ errp);
if (ret != 0) {
goto exit;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index ae7bc6f..d7a830e 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2937,7 +2937,7 @@ static int enable_write_target(BDRVVVFATState *s)
}
s->qcow = NULL;
- ret = bdrv_open(&s->qcow, s->qcow_filename, NULL,
+ ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
&local_err);
if (ret < 0) {
diff --git a/blockdev.c b/blockdev.c
index 9d54f33..f083612 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -504,7 +504,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
QINCREF(bs_opts);
- ret = bdrv_open(&dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
+ ret = bdrv_open(&dinfo->bdrv, file, NULL, bs_opts, bdrv_flags, drv, &error);
if (ret < 0) {
error_setg(errp, "could not open disk image %s: %s",
@@ -1329,7 +1329,7 @@ static void external_snapshot_prepare(BlkTransactionState *common,
/* TODO Inherit bs->options or only take explicit options with an
* extended QMP command? */
assert(state->new_bs == NULL);
- ret = bdrv_open(&state->new_bs, new_image_file, options,
+ ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
flags | BDRV_O_NO_BACKING, drv, &local_err);
/* We will manually add the backing_hd field to the bs later */
if (ret != 0) {
@@ -1578,7 +1578,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
Error *local_err = NULL;
int ret;
- ret = bdrv_open(&bs, filename, NULL, bdrv_flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
@@ -2015,7 +2015,7 @@ void qmp_drive_backup(const char *device, const char *target,
}
target_bs = NULL;
- ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
+ ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
@@ -2158,8 +2158,8 @@ void qmp_drive_mirror(const char *device, const char *target,
* file.
*/
target_bs = NULL;
- ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
- &local_err);
+ ret = bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_NO_BACKING,
+ drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 14e6d0b..61e6ff3 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -813,8 +813,8 @@ static int blk_connect(struct XenDevice *xendev)
Error *local_err = NULL;
BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
readonly);
- if (bdrv_open(&blkdev->bs,
- blkdev->filename, NULL, qflags, drv, &local_err) != 0)
+ if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, qflags,
+ drv, &local_err) != 0)
{
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
error_get_pretty(local_err));
diff --git a/include/block/block.h b/include/block/block.h
index 980869d..a421041 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -190,8 +190,9 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
bool force_raw, bool allow_none, Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
-int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
- int flags, BlockDriver *drv, Error **errp);
+int bdrv_open(BlockDriverState **pbs, const char *filename,
+ const char *reference, QDict *options, int flags,
+ BlockDriver *drv, Error **errp);
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
BlockDriverState *bs, int flags);
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index 897aa56..b834d52 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -289,7 +289,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
drv = NULL;
}
- ret = bdrv_open(&bs, filename, NULL, flags, drv, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, NULL, flags, drv, &local_err);
if (ret < 0) {
error_report("Could not open '%s': %s", filename,
error_get_pretty(local_err));
@@ -2312,7 +2312,7 @@ static int img_rebase(int argc, char **argv)
bs_old_backing = bdrv_new("old_backing");
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
- ret = bdrv_open(&bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
+ ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, BDRV_O_FLAGS,
old_backing_drv, &local_err);
if (ret) {
error_report("Could not open old backing file '%s': %s",
@@ -2322,8 +2322,8 @@ static int img_rebase(int argc, char **argv)
}
if (out_baseimg[0]) {
bs_new_backing = bdrv_new("new_backing");
- ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
- new_backing_drv, &local_err);
+ ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL,
+ BDRV_O_FLAGS, new_backing_drv, &local_err);
if (ret) {
error_report("Could not open new backing file '%s': %s",
out_baseimg, error_get_pretty(local_err));
diff --git a/qemu-io.c b/qemu-io.c
index 8da8f6e..61d54c0 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -68,7 +68,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
} else {
qemuio_bs = bdrv_new("hda");
- if (bdrv_open(&qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
+ if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &local_err)
+ < 0)
+ {
fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
error_get_pretty(local_err));
error_free(local_err);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 0cf123c..711162c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -597,7 +597,7 @@ int main(int argc, char **argv)
bs = bdrv_new("hda");
srcpath = argv[optind];
- ret = bdrv_open(&bs, srcpath, NULL, flags, drv, &local_err);
+ ret = bdrv_open(&bs, srcpath, NULL, NULL, flags, drv, &local_err);
if (ret < 0) {
errno = -ret;
err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 3/8] block: Make bdrv_file_open() static
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 2/8] block: Add reference parameter to bdrv_open() Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the
call to bdrv_file_open(). Additionally, make bdrv_file_open() static and
therefore bdrv_open() the only way to call it.
Consequently, all existing calls to bdrv_file_open() have to be adjusted
to use bdrv_open() with the BDRV_O_PROTOCOL flag instead.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
block.c | 16 +++++++++++-----
block/cow.c | 5 +++--
block/qcow.c | 5 +++--
block/qcow2.c | 4 +++-
block/qed.c | 8 +++++---
block/sheepdog.c | 7 +++++--
block/vhdx.c | 4 +++-
block/vmdk.c | 13 +++++++++----
include/block/block.h | 6 +++---
qemu-io.c | 4 +++-
10 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/block.c b/block.c
index 114f057..6f85d37 100644
--- a/block.c
+++ b/block.c
@@ -960,9 +960,9 @@ free_and_fail:
* 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,
- const char *reference, QDict *options, int flags,
- Error **errp)
+static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+ const char *reference, QDict *options, int flags,
+ Error **errp)
{
BlockDriverState *bs = NULL;
BlockDriver *drv;
@@ -1208,8 +1208,8 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
} else {
- ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
- errp);
+ ret = bdrv_open(pbs, filename, reference, image_options,
+ flags | BDRV_O_PROTOCOL, NULL, errp);
}
done:
@@ -1245,6 +1245,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
assert(pbs);
+ if (flags & BDRV_O_PROTOCOL) {
+ assert(!drv);
+ return bdrv_file_open(pbs, filename, reference, options,
+ flags & ~BDRV_O_PROTOCOL, errp);
+ }
+
if (reference) {
bool options_non_empty = options ? qdict_size(options) : false;
QDECREF(options);
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..d0385be 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -351,8 +351,9 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
return ret;
}
- ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
- &local_err);
+ cow_bs = NULL;
+ ret = bdrv_open(&cow_bs, filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
diff --git a/block/qcow.c b/block/qcow.c
index 948b0c5..8d00853 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -691,8 +691,9 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
return ret;
}
- ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR,
- &local_err);
+ qcow_bs = NULL;
+ ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 6996276..395194b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1493,7 +1493,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+ bs = NULL;
+ ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+ NULL, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return ret;
diff --git a/block/qed.c b/block/qed.c
index b9ca7ac..9bc181f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -562,7 +562,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
size_t l1_size = header.cluster_size * header.table_size;
Error *local_err = NULL;
int ret = 0;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
ret = bdrv_create_file(filename, NULL, &local_err);
if (ret < 0) {
@@ -571,8 +571,10 @@ static int qed_create(const char *filename, uint32_t cluster_size,
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB, &local_err);
+ bs = NULL;
+ ret = bdrv_open(&bs, filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL,
+ &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 672b9c9..c767aad 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1534,7 +1534,8 @@ static int sd_prealloc(const char *filename)
Error *local_err = NULL;
int ret;
- ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+ ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+ NULL, &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
@@ -1695,7 +1696,9 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
goto out;
}
- ret = bdrv_file_open(&bs, backing_file, NULL, NULL, 0, &local_err);
+ bs = NULL;
+ ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
+ &local_err);
if (ret < 0) {
qerror_report_err(local_err);
error_free(local_err);
diff --git a/block/vhdx.c b/block/vhdx.c
index 55689cf..366ff2e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1797,7 +1797,9 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
goto exit;
}
- ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+ bs = NULL;
+ ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+ NULL, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto exit;
diff --git a/block/vmdk.c b/block/vmdk.c
index 5df486f..54a1c59 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -776,8 +776,9 @@ 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, NULL, NULL,
- bs->open_flags, errp);
+ extent_file = NULL;
+ ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
+ bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
if (ret) {
return ret;
}
@@ -1493,7 +1494,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
goto exit;
}
- ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+ assert(bs == NULL);
+ ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+ NULL, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto exit;
@@ -1831,7 +1834,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
goto exit;
}
}
- ret = bdrv_file_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+ assert(new_bs == NULL);
+ ret = bdrv_open(&new_bs, filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not write description");
goto exit;
diff --git a/include/block/block.h b/include/block/block.h
index a421041..bf78db5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -102,6 +102,9 @@ typedef enum {
#define BDRV_O_CHECK 0x1000 /* open solely for consistency check */
#define BDRV_O_ALLOW_RDWR 0x2000 /* allow reopen to change from r/o to r/w */
#define BDRV_O_UNMAP 0x4000 /* execute guest UNMAP/TRIM operations */
+#define BDRV_O_PROTOCOL 0x8000 /* if no block driver is explicitly given:
+ select an appropriate protocol driver,
+ ignoring the format layer */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
@@ -183,9 +186,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
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,
- const char *reference, QDict *options, int flags,
- Error **errp);
int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
bool force_raw, bool allow_none, Error **errp);
diff --git a/qemu-io.c b/qemu-io.c
index 61d54c0..71d7806 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -59,7 +59,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
}
if (growable) {
- if (bdrv_file_open(&qemuio_bs, name, NULL, opts, flags, &local_err)) {
+ if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL,
+ NULL, &local_err))
+ {
fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
error_get_pretty(local_err));
error_free(local_err);
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 4/8] block: Reuse reference handling from bdrv_open()
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (2 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 3/8] block: Make bdrv_file_open() static Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Remove the reference parameter and the related handling code from
bdrv_file_open(), since it exists in bdrv_open() now as well.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/block.c b/block.c
index 6f85d37..f38776b 100644
--- a/block.c
+++ b/block.c
@@ -961,8 +961,7 @@ free_and_fail:
* dictionary, it needs to use QINCREF() before calling bdrv_file_open.
*/
static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- const char *reference, QDict *options, int flags,
- Error **errp)
+ QDict *options, int flags, Error **errp)
{
BlockDriverState *bs = NULL;
BlockDriver *drv;
@@ -976,23 +975,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
options = qdict_new();
}
- if (reference) {
- if (filename || qdict_size(options)) {
- error_setg(errp, "Cannot reference an existing block device with "
- "additional options or a new filename");
- return -EINVAL;
- }
- QDECREF(options);
-
- bs = bdrv_lookup_bs(reference, reference, errp);
- if (!bs) {
- return -ENODEV;
- }
- bdrv_ref(bs);
- *pbs = bs;
- return 0;
- }
-
bs = bdrv_new("");
bs->options = options;
options = qdict_clone_shallow(options);
@@ -1245,12 +1227,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
assert(pbs);
- if (flags & BDRV_O_PROTOCOL) {
- assert(!drv);
- return bdrv_file_open(pbs, filename, reference, options,
- flags & ~BDRV_O_PROTOCOL, errp);
- }
-
if (reference) {
bool options_non_empty = options ? qdict_size(options) : false;
QDECREF(options);
@@ -1276,6 +1252,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
return 0;
}
+ if (flags & BDRV_O_PROTOCOL) {
+ assert(!drv);
+ return bdrv_file_open(pbs, filename, options, flags & ~BDRV_O_PROTOCOL,
+ errp);
+ }
+
if (*pbs) {
bs = *pbs;
} else {
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 5/8] block: Remove bdrv_new() from bdrv_file_open()
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (3 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Change bdrv_file_open() to take a simple pointer to an already existing
BDS instead of an indirect one. The BDS will be created in bdrv_open()
if necessary.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index f38776b..d452f2f 100644
--- a/block.c
+++ b/block.c
@@ -960,10 +960,9 @@ free_and_fail:
* 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.
*/
-static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+static int bdrv_file_open(BlockDriverState *bs, const char *filename,
QDict *options, int flags, Error **errp)
{
- BlockDriverState *bs = NULL;
BlockDriver *drv;
const char *drvname;
bool allow_protocol_prefix = false;
@@ -975,7 +974,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
options = qdict_new();
}
- bs = bdrv_new("");
bs->options = options;
options = qdict_clone_shallow(options);
@@ -1049,7 +1047,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
QDECREF(options);
bs->growable = 1;
- *pbs = bs;
return 0;
fail:
@@ -1057,7 +1054,6 @@ fail:
if (!bs->drv) {
QDECREF(bs->options);
}
- bdrv_unref(bs);
return ret;
}
@@ -1252,18 +1248,24 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
return 0;
}
- if (flags & BDRV_O_PROTOCOL) {
- assert(!drv);
- return bdrv_file_open(pbs, filename, options, flags & ~BDRV_O_PROTOCOL,
- errp);
- }
-
if (*pbs) {
bs = *pbs;
} else {
bs = bdrv_new("");
}
+ if (flags & BDRV_O_PROTOCOL) {
+ assert(!drv);
+ ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+ errp);
+ if (ret && !*pbs) {
+ bdrv_unref(bs);
+ } else if (!ret) {
+ *pbs = bs;
+ }
+ return ret;
+ }
+
/* NULL means an empty set of options */
if (options == NULL) {
options = qdict_new();
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 6/8] block: Handle bs->options in bdrv_open() only
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (4 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 7/8] block: Reuse success path from bdrv_open() Max Reitz
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
The fail paths of bdrv_file_open() and bdrv_open() naturally exhibit
similarities, thus it is possible to reuse the one from bdrv_open() and
shorten the one in bdrv_file_open() accordingly.
Also, setting bs->options in bdrv_file_open() is not necessary if it is
already done in bdrv_open().
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/block.c b/block.c
index d452f2f..7e50e7a 100644
--- a/block.c
+++ b/block.c
@@ -969,14 +969,6 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
Error *local_err = NULL;
int ret;
- /* NULL means an empty set of options */
- if (options == NULL) {
- options = qdict_new();
- }
-
- bs->options = options;
- options = qdict_clone_shallow(options);
-
/* Fetch the file name from the options QDict if necessary */
if (!filename) {
filename = qdict_get_try_str(options, "filename");
@@ -1051,9 +1043,6 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
fail:
QDECREF(options);
- if (!bs->drv) {
- QDECREF(bs->options);
- }
return ret;
}
@@ -1254,18 +1243,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
bs = bdrv_new("");
}
- if (flags & BDRV_O_PROTOCOL) {
- assert(!drv);
- ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
- errp);
- if (ret && !*pbs) {
- bdrv_unref(bs);
- } else if (!ret) {
- *pbs = bs;
- }
- return ret;
- }
-
/* NULL means an empty set of options */
if (options == NULL) {
options = qdict_new();
@@ -1274,6 +1251,21 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
bs->options = options;
options = qdict_clone_shallow(options);
+ if (flags & BDRV_O_PROTOCOL) {
+ assert(!drv);
+ ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+ &local_err);
+ options = NULL;
+ if (!ret) {
+ *pbs = bs;
+ return 0;
+ } else if (bs->drv) {
+ goto close_and_fail;
+ } else {
+ goto fail;
+ }
+ }
+
/* For snapshot=on, create a temporary qcow2 overlay */
if (flags & BDRV_O_SNAPSHOT) {
BlockDriverState *bs1;
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 7/8] block: Reuse success path from bdrv_open()
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (5 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
2014-02-19 9:47 ` [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
The fail and success paths of bdrv_file_open() may be further shortened
by reusing code already existent in bdrv_open(). This includes
bdrv_file_open() not taking the reference to options which allows the
removal of QDECREF(options) in that function.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
block.c | 63 +++++++++++++++++++++++++++++----------------------------------
1 file changed, 29 insertions(+), 34 deletions(-)
diff --git a/block.c b/block.c
index 7e50e7a..71ed977 100644
--- a/block.c
+++ b/block.c
@@ -955,13 +955,15 @@ 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.
+ * options is an indirect pointer to a QDict of options to pass to the block
+ * drivers, or pointer to NULL for an empty set of options. If this function
+ * takes ownership of the QDict reference, it will set *options to NULL;
+ * otherwise, it will contain unused/unrecognized options after this function
+ * returns. Then, the caller is responsible for freeing it. If it intends to
+ * reuse the QDict, QINCREF() should be called beforehand.
*/
static int bdrv_file_open(BlockDriverState *bs, const char *filename,
- QDict *options, int flags, Error **errp)
+ QDict **options, int flags, Error **errp)
{
BlockDriver *drv;
const char *drvname;
@@ -971,9 +973,9 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
/* Fetch the file name from the options QDict if necessary */
if (!filename) {
- filename = qdict_get_try_str(options, "filename");
- } else if (filename && !qdict_haskey(options, "filename")) {
- qdict_put(options, "filename", qstring_from_str(filename));
+ filename = qdict_get_try_str(*options, "filename");
+ } else if (filename && !qdict_haskey(*options, "filename")) {
+ qdict_put(*options, "filename", qstring_from_str(filename));
allow_protocol_prefix = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at the "
@@ -983,13 +985,13 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
}
/* Find the right block driver */
- drvname = qdict_get_try_str(options, "driver");
+ drvname = qdict_get_try_str(*options, "driver");
if (drvname) {
drv = bdrv_find_format(drvname);
if (!drv) {
error_setg(errp, "Unknown driver '%s'", drvname);
}
- qdict_del(options, "driver");
+ qdict_del(*options, "driver");
} else if (filename) {
drv = bdrv_find_protocol(filename, allow_protocol_prefix);
if (!drv) {
@@ -1008,41 +1010,30 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
/* Parse the filename and open it */
if (drv->bdrv_parse_filename && filename) {
- drv->bdrv_parse_filename(filename, options, &local_err);
+ drv->bdrv_parse_filename(filename, *options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
- qdict_del(options, "filename");
+ qdict_del(*options, "filename");
}
if (!drv->bdrv_file_open) {
- ret = bdrv_open(&bs, filename, NULL, options, flags, drv, &local_err);
- options = NULL;
+ ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err);
+ *options = NULL;
} else {
- ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
+ ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err);
}
if (ret < 0) {
error_propagate(errp, local_err);
goto fail;
}
- /* Check if any unknown options were used */
- if (options && (qdict_size(options) != 0)) {
- const QDictEntry *entry = qdict_first(options);
- error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
- drv->format_name, entry->key);
- ret = -EINVAL;
- goto fail;
- }
- QDECREF(options);
-
bs->growable = 1;
return 0;
fail:
- QDECREF(options);
return ret;
}
@@ -1253,12 +1244,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
if (flags & BDRV_O_PROTOCOL) {
assert(!drv);
- ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+ ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL,
&local_err);
- options = NULL;
if (!ret) {
- *pbs = bs;
- return 0;
+ goto done;
} else if (bs->drv) {
goto close_and_fail;
} else {
@@ -1395,12 +1384,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
}
}
+done:
/* Check if any unknown options were used */
- if (qdict_size(options) != 0) {
+ if (options && (qdict_size(options) != 0)) {
const QDictEntry *entry = qdict_first(options);
- error_setg(errp, "Block format '%s' used by device '%s' doesn't "
- "support the option '%s'", drv->format_name, bs->device_name,
- entry->key);
+ if (flags & BDRV_O_PROTOCOL) {
+ error_setg(errp, "Block protocol '%s' doesn't support the option "
+ "'%s'", drv->format_name, entry->key);
+ } else {
+ error_setg(errp, "Block format '%s' used by device '%s' doesn't "
+ "support the option '%s'", drv->format_name,
+ bs->device_name, entry->key);
+ }
ret = -EINVAL;
goto close_and_fail;
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v4 8/8] block: Remove bdrv_open_image()'s force_raw option
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (6 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 7/8] block: Reuse success path from bdrv_open() Max Reitz
@ 2014-02-18 17:33 ` Max Reitz
2014-02-19 9:47 ` [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
8 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-02-18 17:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
This option is now unnecessary since specifying BDRV_O_PROTOCOL as flag
will do exactly the same.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 27 ++++-----------------------
block/blkdebug.c | 2 +-
block/blkverify.c | 4 ++--
include/block/block.h | 2 +-
4 files changed, 8 insertions(+), 27 deletions(-)
diff --git a/block.c b/block.c
index 71ed977..80310fe 100644
--- a/block.c
+++ b/block.c
@@ -1109,10 +1109,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
* Opens a disk image whose options are given as BlockdevRef in another block
* device's options.
*
- * If force_raw is true, bdrv_file_open() will be used, thereby preventing any
- * image format auto-detection. If it is false and a filename is given,
- * bdrv_open() will be used for auto-detection.
- *
* If allow_none is true, no image will be opened if filename is false and no
* BlockdevRef is given. *pbs will remain unchanged and 0 will be returned.
*
@@ -1127,7 +1123,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
*/
int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
- bool force_raw, bool allow_none, Error **errp)
+ bool allow_none, Error **errp)
{
QDict *image_options;
int ret;
@@ -1153,22 +1149,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
goto done;
}
- if (filename && !force_raw) {
- /* If a filename is given and the block driver should be detected
- automatically (instead of using none), use bdrv_open() in order to do
- that auto-detection. */
- if (reference) {
- error_setg(errp, "Cannot reference an existing block device while "
- "giving a filename");
- ret = -EINVAL;
- goto done;
- }
-
- ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
- } else {
- ret = bdrv_open(pbs, filename, reference, image_options,
- flags | BDRV_O_PROTOCOL, NULL, errp);
- }
+ ret = bdrv_open(pbs, filename, reference, image_options, flags, NULL, errp);
done:
qdict_del(options, bdref_key);
@@ -1330,8 +1311,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
assert(file == NULL);
ret = bdrv_open_image(&file, filename, options, "file",
- bdrv_open_flags(bs, flags | BDRV_O_UNMAP), true, true,
- &local_err);
+ bdrv_open_flags(bs, flags | BDRV_O_UNMAP) |
+ BDRV_O_PROTOCOL, true, &local_err);
if (ret < 0) {
goto fail;
}
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 053fa4c..6707f49 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -412,7 +412,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
/* Open the backing file */
assert(bs->file == NULL);
ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image",
- flags, true, false, &local_err);
+ flags | BDRV_O_PROTOCOL, false, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto out;
diff --git a/block/blkverify.c b/block/blkverify.c
index 86585e7..b57da59 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -137,7 +137,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
/* Open the raw file */
assert(bs->file == NULL);
ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options,
- "raw", flags, true, false, &local_err);
+ "raw", flags | BDRV_O_PROTOCOL, false, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto fail;
@@ -146,7 +146,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
/* Open the test file */
assert(s->test_file == NULL);
ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options,
- "test", flags, false, false, &local_err);
+ "test", flags, false, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
s->test_file = NULL;
diff --git a/include/block/block.h b/include/block/block.h
index bf78db5..780f48b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -188,7 +188,7 @@ int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_discard_flags(const char *mode, int *flags);
int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
- bool force_raw, bool allow_none, Error **errp);
+ bool allow_none, Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
int bdrv_open(BlockDriverState **pbs, const char *filename,
const char *reference, QDict *options, int flags,
--
1.8.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open()
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
` (7 preceding siblings ...)
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
@ 2014-02-19 9:47 ` Kevin Wolf
8 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2014-02-19 9:47 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi, Benoît Canet
Am 18.02.2014 um 18:33 hat Max Reitz geschrieben:
> bdrv_file_open() is now nearly a subset of bdrv_open(), except for the
> fact that bdrv_file_open() is for protocols and bdrv_open() for block
> drivers. It is possible to use bdrv_file_open() with a block driver, but
> in that case that block driver must be explicitly specified.
>
> Due to these great similarities, bdrv_file_open() can be integrated and
> made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is
> specified, bdrv_open() will now do what bdrv_file_open() used to do:
> Auto-detecting a protocol instead of a block driver.
>
> This series implements this and changes all calls to bdrv_file_open() to
> bdrv_open() calls with BDRV_O_PROTOCOL specified.
>
> Note that this flag cannot be discerned automatically since it is
> impossible for bdrv_open() to know by itself whether a given file should
> be opened with or without the format layer involved: Both are valid
> alternatives. Therefore, it still has to be specified by the user.
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-02-19 9:49 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-18 17:33 [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 2/8] block: Add reference parameter to bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 3/8] block: Make bdrv_file_open() static Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 7/8] block: Reuse success path from bdrv_open() Max Reitz
2014-02-18 17:33 ` [Qemu-devel] [PATCH v4 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
2014-02-19 9:47 ` [Qemu-devel] [PATCH v4 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.