* [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open()
@ 2015-01-26 15:00 Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend Max Reitz
` (15 more replies)
0 siblings, 16 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
This series removes the "growable" field from the BlockDriverState
object. Its use was to clamp guest requests against the limits of the
BDS; however, this can now be done more easily by moving those checks
into the BlockBackend functions.
In a future series, "growable" may be reintroduced (maybe with a
different name); it will then signify whether a BDS is able to grow (in
contrast to the current "growable", which signifies whether it is
allowed to). Maybe I will add it to the BlockDriver instead of the BDS,
though.
To be able to remove that field, qemu-io needs to be converted to
BlockBackend, which is done by this series as well. While working on
that I decided to convert blk_new_with_bs()+bdrv_open() to
blk_new_open(). I was skeptical about that decision at first, but it
seems good now that I was able to replace nearly every blk_new_with_bs()
call by blk_new_open(). In a future series I may try to convert some
remaining bdrv_open() calls to blk_new_open() as well. (And, in fact, in
a future series I *will* replace the last remaining blk_new_with_bs()
outside of blk_new_open() by blk_new_open().)
Finally, the question needs to be asked: If, after this series, every
BDS is allowed to grow, are there any users which do not use BB, but
should still be disallowed from reading/writing beyond a BDS's limits?
The only users I could see were the block jobs. Some of them should
indeed be converted to BB; but none of them takes a user-supplied offset
or size, all work on the full BDS (or only on parts which have been
modified, etc.). Therefore, it is by design impossible for them to
exceed the BDS's limits, which makes making all BDS's growable safe.
v3:
- Rebased (onto Stefan's branch rebased onto master)
- Fixed patch 4 [Stefano]
v2:
- Rebased [Kevin]
- Patch 2: Added a TODO comment about removing @filename and @flags from
blk_new_open() when possible [Kevin]
git-backport-diff against v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/14:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
002/14:[----] [--] 'block: Add blk_new_open()'
003/14:[----] [--] 'blockdev: Use blk_new_open() in blockdev_init()'
004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
010/14:[----] [--] 'qemu-io: Remove "growable" option'
011/14:[----] [--] 'qemu-io: Use BlockBackend'
012/14:[----] [--] 'block: Clamp BlockBackend requests'
013/14:[----] [--] 'block: Remove "growable" from BDS'
014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
git-backport-diff against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/14:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
002/14:[0006] [FC] 'block: Add blk_new_open()'
003/14:[----] [-C] 'blockdev: Use blk_new_open() in blockdev_init()'
004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
010/14:[0002] [FC] 'qemu-io: Remove "growable" option'
011/14:[----] [--] 'qemu-io: Use BlockBackend'
012/14:[----] [--] 'block: Clamp BlockBackend requests'
013/14:[----] [--] 'block: Remove "growable" from BDS'
014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
Max Reitz (14):
block: Lift some BDS functions to the BlockBackend
block: Add blk_new_open()
blockdev: Use blk_new_open() in blockdev_init()
block/xen: Use blk_new_open() in blk_connect()
qemu-img: Use blk_new_open() in img_open()
qemu-img: Use blk_new_open() in img_rebase()
qemu-img: Use BlockBackend as far as possible
qemu-nbd: Use blk_new_open() in main()
qemu-io: Use blk_new_open() in openfile()
qemu-io: Remove "growable" option
qemu-io: Use BlockBackend
block: Clamp BlockBackend requests
block: Remove "growable" from BDS
block: Keep bdrv_check*_request()'s return value
block.c | 59 +++++-----
block/block-backend.c | 219 +++++++++++++++++++++++++++++++++++++
block/qcow2.c | 6 --
block/raw-posix.c | 2 +-
block/raw-win32.c | 2 +-
block/sheepdog.c | 2 +-
blockdev.c | 92 ++++++++--------
hmp.c | 9 +-
hw/block/xen_disk.c | 28 +++--
include/block/block_int.h | 3 -
include/qemu-io.h | 4 +-
include/sysemu/block-backend.h | 12 +++
qemu-img.c | 171 ++++++++++++++---------------
qemu-io-cmds.c | 238 ++++++++++++++++++++++-------------------
qemu-io.c | 58 ++++------
qemu-nbd.c | 25 ++---
tests/qemu-iotests/016 | 73 -------------
tests/qemu-iotests/016.out | 23 ----
tests/qemu-iotests/051.out | 60 +++++------
tests/qemu-iotests/087.out | 8 +-
tests/qemu-iotests/group | 1 -
21 files changed, 592 insertions(+), 503 deletions(-)
delete mode 100755 tests/qemu-iotests/016
delete mode 100644 tests/qemu-iotests/016.out
--
2.1.0
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-26 21:48 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open() Max Reitz
` (14 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Create the blk_* counterparts for the following bdrv_* functions (which
make sense to call on the BlockBackend level):
- bdrv_co_write_zeroes()
- bdrv_write_compressed()
- bdrv_truncate()
- bdrv_discard()
- bdrv_load_vmstate()
- bdrv_save_vmstate()
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 33 +++++++++++++++++++++++++++++++++
include/sysemu/block-backend.h | 9 +++++++++
2 files changed, 42 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index d00c129..b359545 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -663,3 +663,36 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
{
return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
}
+
+int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t sector_num,
+ int nb_sectors, BdrvRequestFlags flags)
+{
+ return bdrv_co_write_zeroes(blk->bs, sector_num, nb_sectors, flags);
+}
+
+int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
+ const uint8_t *buf, int nb_sectors)
+{
+ return bdrv_write_compressed(blk->bs, sector_num, buf, nb_sectors);
+}
+
+int blk_truncate(BlockBackend *blk, int64_t offset)
+{
+ return bdrv_truncate(blk->bs, offset);
+}
+
+int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+{
+ return bdrv_discard(blk->bs, sector_num, nb_sectors);
+}
+
+int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
+ int64_t pos, int size)
+{
+ return bdrv_save_vmstate(blk->bs, buf, pos, size);
+}
+
+int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
+{
+ return bdrv_load_vmstate(blk->bs, buf, pos, size);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8871a02..38afa79 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -150,5 +150,14 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk);
void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
+int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t sector_num,
+ int nb_sectors, BdrvRequestFlags flags);
+int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
+ const uint8_t *buf, int nb_sectors);
+int blk_truncate(BlockBackend *blk, int64_t offset);
+int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
+ int64_t pos, int size);
+int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-26 21:56 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init() Max Reitz
` (13 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
blk_new_with_bs() creates a BlockBackend with an empty BlockDriverState
attached to it. Empty BDSs are not nice, therefore add an alternative
function which combines blk_new_with_bs() with bdrv_open().
Note: In contrast to bdrv_open() which takes a BlockDriver parameter,
blk_new_open() does not take such a parameter. This is because
bdrv_open() opens a BlockDriverState, therefore it is naturally to be
able to set the BlockDriver for that BDS. The fact that bdrv_open() can
open more than a single BDS is merely some form of a byproduct.
blk_new_open() on the other hand is intended to be used to create a
whole tree of BlockDriverStates. Therefore, setting a single BlockDriver
does not make much sense. Instead, the drivers to be used for each of
the nodes must be configured through the "options" QDict; including the
driver of the root BDS.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 34 ++++++++++++++++++++++++++++++++++
include/sysemu/block-backend.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index b359545..543edaa 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -91,6 +91,40 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
return blk;
}
+/*
+ * Calls blk_new_with_bs() and then calls bdrv_open() on the BlockDriverState.
+ *
+ * Just as with bdrv_open(), after having called this function the reference to
+ * @options belongs to the block layer (even on failure).
+ *
+ * TODO: Remove @filename and @flags; it should be possible to specify a whole
+ * BDS tree just by specifying the @options QDict (or @reference,
+ * alternatively). At the time of adding this function, this is not possible,
+ * though, so callers of this function have to be able to specify @filename and
+ * @flags.
+ */
+BlockBackend *blk_new_open(const char *name, const char *filename,
+ const char *reference, QDict *options, int flags,
+ Error **errp)
+{
+ BlockBackend *blk;
+ int ret;
+
+ blk = blk_new_with_bs(name, errp);
+ if (!blk) {
+ QDECREF(options);
+ return NULL;
+ }
+
+ ret = bdrv_open(&blk->bs, filename, reference, options, flags, NULL, errp);
+ if (ret < 0) {
+ blk_unref(blk);
+ return NULL;
+ }
+
+ return blk;
+}
+
static void blk_delete(BlockBackend *blk)
{
assert(!blk->refcnt);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 38afa79..f39bb5c 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -62,6 +62,9 @@ typedef struct BlockDevOps {
BlockBackend *blk_new(const char *name, Error **errp);
BlockBackend *blk_new_with_bs(const char *name, Error **errp);
+BlockBackend *blk_new_open(const char *name, const char *filename,
+ const char *reference, QDict *options, int flags,
+ Error **errp);
void blk_ref(BlockBackend *blk);
void blk_unref(BlockBackend *blk);
const char *blk_name(BlockBackend *blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-26 22:37 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect() Max Reitz
` (12 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Due to different error propagation, this breaks tests 051 and 087; fix
their output.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 92 +++++++++++++++++++++-------------------------
tests/qemu-iotests/051.out | 60 +++++++++++++++---------------
tests/qemu-iotests/087.out | 8 ++--
3 files changed, 76 insertions(+), 84 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index d59efd3..7573746 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -354,13 +354,11 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
ThrottleConfig cfg;
int snapshot = 0;
bool copy_on_read;
- int ret;
Error *error = NULL;
QemuOpts *opts;
const char *id;
bool has_driver_specific_opts;
BlockdevDetectZeroesOptions detect_zeroes;
- BlockDriver *drv = NULL;
/* Check common options by copying from bs_opts to opts, all other options
* stay in bs_opts for processing by bdrv_open(). */
@@ -426,11 +424,11 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
goto early_err;
}
- drv = bdrv_find_format(buf);
- if (!drv) {
- error_setg(errp, "'%s' invalid format", buf);
+ if (qdict_haskey(bs_opts, "driver")) {
+ error_setg(errp, "Cannot specify both 'driver' and 'format'");
goto early_err;
}
+ qdict_put_obj(bs_opts, "driver", QOBJECT(qstring_from_str(buf)));
}
/* disk I/O throttling */
@@ -505,70 +503,64 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
}
/* init */
- blk = blk_new_with_bs(qemu_opts_id(opts), errp);
- if (!blk) {
- goto early_err;
- }
- bs = blk_bs(blk);
- bs->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
- bs->read_only = ro;
- bs->detect_zeroes = detect_zeroes;
-
- bdrv_set_on_error(bs, on_read_error, on_write_error);
+ if ((!file || !*file) && !has_driver_specific_opts) {
+ blk = blk_new_with_bs(qemu_opts_id(opts), errp);
+ if (!blk) {
+ goto early_err;
+ }
- /* disk I/O throttling */
- if (throttle_enabled(&cfg)) {
- bdrv_io_limits_enable(bs);
- bdrv_set_io_limits(bs, &cfg);
- }
+ bs = blk_bs(blk);
+ bs->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
+ bs->read_only = ro;
- if (!file || !*file) {
- if (has_driver_specific_opts) {
+ QDECREF(bs_opts);
+ } else {
+ if (file && !*file) {
file = NULL;
- } else {
- QDECREF(bs_opts);
- qemu_opts_del(opts);
- return blk;
}
- }
- if (snapshot) {
- /* always use cache=unsafe with snapshot */
- bdrv_flags &= ~BDRV_O_CACHE_MASK;
- bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH);
- }
- if (copy_on_read) {
- bdrv_flags |= BDRV_O_COPY_ON_READ;
- }
+ if (snapshot) {
+ /* always use cache=unsafe with snapshot */
+ bdrv_flags &= ~BDRV_O_CACHE_MASK;
+ bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH);
+ }
+
+ if (copy_on_read) {
+ bdrv_flags |= BDRV_O_COPY_ON_READ;
+ }
+
+ if (runstate_check(RUN_STATE_INMIGRATE)) {
+ bdrv_flags |= BDRV_O_INCOMING;
+ }
- if (runstate_check(RUN_STATE_INMIGRATE)) {
- bdrv_flags |= BDRV_O_INCOMING;
+ bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
+
+ blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
+ errp);
+ if (!blk) {
+ goto err_no_bs_opts;
+ }
+ bs = blk_bs(blk);
}
- bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
+ bs->detect_zeroes = detect_zeroes;
- QINCREF(bs_opts);
- ret = bdrv_open(&bs, file, NULL, bs_opts, bdrv_flags, drv, &error);
- assert(bs == blk_bs(blk));
+ bdrv_set_on_error(bs, on_read_error, on_write_error);
- if (ret < 0) {
- error_setg(errp, "could not open disk image %s: %s",
- file ?: blk_name(blk), error_get_pretty(error));
- error_free(error);
- goto err;
+ /* disk I/O throttling */
+ if (throttle_enabled(&cfg)) {
+ bdrv_io_limits_enable(bs);
+ bdrv_set_io_limits(bs, &cfg);
}
if (bdrv_key_required(bs)) {
autostart = 0;
}
- QDECREF(bs_opts);
+err_no_bs_opts:
qemu_opts_del(opts);
-
return blk;
-err:
- blk_unref(blk);
early_err:
qemu_opts_del(opts);
err_no_opts:
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index f497c57..ecf0a31 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -5,43 +5,43 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR
=== Unknown option ===
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
=== Unknown protocol option ===
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=: Block protocol 'file' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=on: Block protocol 'file' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=1234
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=1234: Block protocol 'file' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't support the option 'unknown_opt'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=foo: Block protocol 'file' doesn't support the option 'unknown_opt'
=== Invalid format ===
Testing: -drive file=TEST_DIR/t.qcow2,format=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=foo: 'foo' invalid format
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=foo: Unknown driver 'foo'
Testing: -drive file=TEST_DIR/t.qcow2,driver=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk image TEST_DIR/t.qcow2: Unknown driver 'foo'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: Unknown driver 'foo'
Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: Cannot specify both 'driver' and 'format'
=== Overriding backing file ===
@@ -55,13 +55,13 @@ ide0-hd0: TEST_DIR/t.qcow2 (qcow2)
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig: Driver doesn't support backing files
Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig: Driver doesn't support backing files
Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig: Driver doesn't support backing files
=== Enable and disable lazy refcounting on the command line, plus some invalid values ===
@@ -75,20 +75,20 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
=== With version 2 images enabling lazy refcounts must fail ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off
QEMU X.Y.Z monitor - type 'help' for more information
@@ -248,31 +248,31 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device '' doesn't support the option 'filename'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Block format 'qcow2' used by device '' doesn't support the option 'filename'
=== Leaving out required options ===
Testing: -drive driver=file
-QEMU_PROG: -drive driver=file: could not open disk image ide0-hd0: The 'file' block driver requires a file name
+QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: could not open disk image ide0-hd0: one of path and host must be specified.
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
Testing: -drive driver=raw
-QEMU_PROG: -drive driver=raw: could not open disk image ide0-hd0: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level
Testing: -drive file.driver=file
-QEMU_PROG: -drive file.driver=file: could not open disk image ide0-hd0: The 'file' block driver requires a file name
+QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name
Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: could not open disk image ide0-hd0: one of path and host must be specified.
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
Testing: -drive file.driver=raw
-QEMU_PROG: -drive file.driver=raw: could not open disk image ide0-hd0: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level
Testing: -drive foo=bar
-QEMU_PROG: -drive foo=bar: could not open disk image ide0-hd0: Must specify either driver or file
+QEMU_PROG: -drive foo=bar: Must specify either driver or file
=== Specifying both an option and its legacy alias ===
@@ -323,13 +323,13 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' a
=== Parsing protocol from file name ===
Testing: -hda foo:bar
-QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
+QEMU_PROG: -hda foo:bar: Unknown protocol
Testing: -drive file=foo:bar
-QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol
+QEMU_PROG: -drive file=foo:bar: Unknown protocol
Testing: -drive file.filename=foo:bar
-QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
+QEMU_PROG: -drive file.filename=foo:bar: Could not open 'foo:bar': No such file or directory
Testing: -hda file:TEST_DIR/t.qcow2
QEMU X.Y.Z monitor - type 'help' for more information
@@ -340,7 +340,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file.filename=file:TEST_DIR/t.qcow2
-QEMU_PROG: -drive file.filename=file:TEST_DIR/t.qcow2: could not open disk image ide0-hd0: Could not open 'file:TEST_DIR/t.qcow2': No such file or directory
+QEMU_PROG: -drive file.filename=file:TEST_DIR/t.qcow2: Could not open 'file:TEST_DIR/t.qcow2': No such file or directory
=== Snapshot mode ===
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 91f4ea1..0ba2e43 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -21,9 +21,9 @@ QMP_VERSION
{"return": {}}
{"error": {"class": "GenericError", "desc": "Device with id 'disk' already exists"}}
{"error": {"class": "GenericError", "desc": "Device name 'test-node' conflicts with an existing node name"}}
-{"error": {"class": "GenericError", "desc": "could not open disk image disk2: node-name=disk is conflicting with a device id"}}
-{"error": {"class": "GenericError", "desc": "could not open disk image disk2: Duplicate node name"}}
-{"error": {"class": "GenericError", "desc": "could not open disk image disk3: node-name=disk3 is conflicting with a device id"}}
+{"error": {"class": "GenericError", "desc": "node-name=disk is conflicting with a device id"}}
+{"error": {"class": "GenericError", "desc": "Duplicate node name"}}
+{"error": {"class": "GenericError", "desc": "node-name=disk3 is conflicting with a device id"}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
@@ -57,7 +57,7 @@ QMP_VERSION
Testing:
QMP_VERSION
{"return": {}}
-{"error": {"class": "GenericError", "desc": "could not open disk image disk: Guest must be stopped for opening of encrypted image"}}
+{"error": {"class": "GenericError", "desc": "Guest must be stopped for opening of encrypted image"}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (2 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-26 22:46 ` Eric Blake
2015-02-02 18:27 ` Kevin Wolf
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open() Max Reitz
` (11 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/block/xen_disk.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 21842a0..1b0257c 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -40,6 +40,8 @@
#include "xen_blkif.h"
#include "sysemu/blockdev.h"
#include "sysemu/block-backend.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
/* ------------------------------------------------------------- */
@@ -897,30 +899,24 @@ static int blk_connect(struct XenDevice *xendev)
blkdev->dinfo = drive_get(IF_XEN, 0, index);
if (!blkdev->dinfo) {
Error *local_err = NULL;
- BlockBackend *blk;
- BlockDriver *drv;
- BlockDriverState *bs;
+ QDict *options = NULL;
- /* setup via xenbus -> create new block driver instance */
- xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
- blk = blk_new_with_bs(blkdev->dev, NULL);
- if (!blk) {
- return -1;
+ if (strcmp(blkdev->fileproto, "<unset>")) {
+ options = qdict_new();
+ qdict_put_obj(options, "driver",
+ QOBJECT(qstring_from_str(blkdev->fileproto)));
}
- blkdev->blk = blk;
- bs = blk_bs(blk);
- drv = bdrv_find_whitelisted_format(blkdev->fileproto, readonly);
- if (bdrv_open(&bs, blkdev->filename, NULL, NULL, qflags,
- drv, &local_err) != 0) {
+ /* setup via xenbus -> create new block driver instance */
+ xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
+ blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
+ qflags, &local_err);
+ if (!blkdev->blk) {
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
error_get_pretty(local_err));
error_free(local_err);
- blk_unref(blk);
- blkdev->blk = NULL;
return -1;
}
- assert(bs == blk_bs(blk));
} else {
/* setup via qemu cmdline -> already setup for us */
xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (3 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-26 22:47 ` Eric Blake
2015-02-02 18:35 ` Kevin Wolf
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase() Max Reitz
` (10 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 4e9a7f5..be1953d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -291,32 +291,24 @@ static BlockBackend *img_open(const char *id, const char *filename,
{
BlockBackend *blk;
BlockDriverState *bs;
- BlockDriver *drv;
char password[256];
Error *local_err = NULL;
- int ret;
-
- blk = blk_new_with_bs(id, &error_abort);
- bs = blk_bs(blk);
+ QDict *options = NULL;
if (fmt) {
- drv = bdrv_find_format(fmt);
- if (!drv) {
- error_report("Unknown file format '%s'", fmt);
- goto fail;
- }
- } else {
- drv = NULL;
+ options = qdict_new();
+ qdict_put_obj(options, "driver", QOBJECT(qstring_from_str(fmt)));
}
- ret = bdrv_open(&bs, filename, NULL, NULL, flags, drv, &local_err);
- if (ret < 0) {
+ blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
+ if (!blk) {
error_report("Could not open '%s': %s", filename,
error_get_pretty(local_err));
error_free(local_err);
goto fail;
}
+ bs = blk_bs(blk);
if (bdrv_is_encrypted(bs) && require_io) {
qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
if (read_password(password, sizeof(password)) < 0) {
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (4 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 3:05 ` Eric Blake
2015-02-02 19:00 ` Kevin Wolf
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible Max Reitz
` (9 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 57 ++++++++++++++++++++++++---------------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index be1953d..0b23c87 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2430,7 +2430,6 @@ static int img_rebase(int argc, char **argv)
{
BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL;
- BlockDriver *old_backing_drv, *new_backing_drv;
char *filename;
const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
int c, flags, src_flags, ret;
@@ -2524,54 +2523,46 @@ static int img_rebase(int argc, char **argv)
}
bs = blk_bs(blk);
- /* Find the right drivers for the backing files */
- old_backing_drv = NULL;
- new_backing_drv = NULL;
-
- if (!unsafe && bs->backing_format[0] != '\0') {
- old_backing_drv = bdrv_find_format(bs->backing_format);
- if (old_backing_drv == NULL) {
- error_report("Invalid format name: '%s'", bs->backing_format);
- ret = -1;
- goto out;
- }
- }
-
- if (out_basefmt != NULL) {
- new_backing_drv = bdrv_find_format(out_basefmt);
- if (new_backing_drv == NULL) {
- error_report("Invalid format name: '%s'", out_basefmt);
- ret = -1;
- goto out;
- }
- }
-
/* For safe rebasing we need to compare old and new backing file */
if (!unsafe) {
char backing_name[PATH_MAX];
+ QDict *options = NULL;
+
+ if (!unsafe && bs->backing_format[0] != '\0') {
+ options = qdict_new();
+ qdict_put_obj(options, "driver",
+ QOBJECT(qstring_from_str(bs->backing_format)));
+ }
- blk_old_backing = blk_new_with_bs("old_backing", &error_abort);
- bs_old_backing = blk_bs(blk_old_backing);
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
- ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags,
- old_backing_drv, &local_err);
- if (ret) {
+ blk_old_backing = blk_new_open("old_backing", backing_name, NULL,
+ options, src_flags, &local_err);
+ if (!blk_old_backing) {
error_report("Could not open old backing file '%s': %s",
backing_name, error_get_pretty(local_err));
error_free(local_err);
goto out;
}
+ bs_old_backing = blk_bs(blk_old_backing);
+
if (out_baseimg[0]) {
- blk_new_backing = blk_new_with_bs("new_backing", &error_abort);
- bs_new_backing = blk_bs(blk_new_backing);
- ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags,
- new_backing_drv, &local_err);
- if (ret) {
+ if (out_basefmt) {
+ options = qdict_new();
+ qdict_put_obj(options, "driver",
+ QOBJECT(qstring_from_str(out_basefmt)));
+ } else {
+ options = NULL;
+ }
+
+ blk_new_backing = blk_new_open("new_backing", out_baseimg, NULL,
+ options, src_flags, &local_err);
+ if (!blk_new_backing) {
error_report("Could not open new backing file '%s': %s",
out_baseimg, error_get_pretty(local_err));
error_free(local_err);
goto out;
}
+ bs_new_backing = blk_bs(blk_new_backing);
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (5 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 3:38 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main() Max Reitz
` (8 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Although qemu-img already creates BlockBackends, it does not do accesses
to the images through them. This patch converts all of the bdrv_* calls
for which this is currently possible to blk_* calls. Most of the
remaining calls will probably stay bdrv_* calls because they really do
operate on the BDS level instead of the BB level.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 98 ++++++++++++++++++++++++++++++++++----------------------------
1 file changed, 54 insertions(+), 44 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 0b23c87..8b4139e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1008,19 +1008,19 @@ static int64_t sectors_to_process(int64_t total, int64_t from)
* Returns 0 in case sectors are filled with 0, 1 if sectors contain non-zero
* data and negative value on error.
*
- * @param bs: Driver used for accessing file
+ * @param blk: BlockBackend for the image
* @param sect_num: Number of first sector to check
* @param sect_count: Number of sectors to check
* @param filename: Name of disk file we are checking (logging purpose)
* @param buffer: Allocated buffer for storing read data
* @param quiet: Flag for quiet mode
*/
-static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
+static int check_empty_sectors(BlockBackend *blk, int64_t sect_num,
int sect_count, const char *filename,
uint8_t *buffer, bool quiet)
{
int pnum, ret = 0;
- ret = bdrv_read(bs, sect_num, buffer, sect_count);
+ ret = blk_read(blk, sect_num, buffer, sect_count);
if (ret < 0) {
error_report("Error while reading offset %" PRId64 " of %s: %s",
sectors_to_bytes(sect_num), filename, strerror(-ret));
@@ -1130,22 +1130,26 @@ static int img_compare(int argc, char **argv)
}
bs2 = blk_bs(blk2);
- buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
- buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
- total_sectors1 = bdrv_nb_sectors(bs1);
+ buf1 = blk_blockalign(blk1, IO_BUF_SIZE);
+ buf2 = blk_blockalign(blk2, IO_BUF_SIZE);
+ total_sectors1 = blk_getlength(blk1);
if (total_sectors1 < 0) {
error_report("Can't get size of %s: %s",
filename1, strerror(-total_sectors1));
ret = 4;
goto out;
}
- total_sectors2 = bdrv_nb_sectors(bs2);
+ total_sectors2 = blk_getlength(blk2);
if (total_sectors2 < 0) {
error_report("Can't get size of %s: %s",
filename2, strerror(-total_sectors2));
ret = 4;
goto out;
}
+
+ total_sectors1 /= BDRV_SECTOR_SIZE;
+ total_sectors2 /= BDRV_SECTOR_SIZE;
+
total_sectors = MIN(total_sectors1, total_sectors2);
progress_base = MAX(total_sectors1, total_sectors2);
@@ -1181,7 +1185,7 @@ static int img_compare(int argc, char **argv)
if (allocated1 == allocated2) {
if (allocated1) {
- ret = bdrv_read(bs1, sector_num, buf1, nb_sectors);
+ ret = blk_read(blk1, sector_num, buf1, nb_sectors);
if (ret < 0) {
error_report("Error while reading offset %" PRId64 " of %s:"
" %s", sectors_to_bytes(sector_num), filename1,
@@ -1189,7 +1193,7 @@ static int img_compare(int argc, char **argv)
ret = 4;
goto out;
}
- ret = bdrv_read(bs2, sector_num, buf2, nb_sectors);
+ ret = blk_read(blk2, sector_num, buf2, nb_sectors);
if (ret < 0) {
error_report("Error while reading offset %" PRId64
" of %s: %s", sectors_to_bytes(sector_num),
@@ -1216,10 +1220,10 @@ static int img_compare(int argc, char **argv)
}
if (allocated1) {
- ret = check_empty_sectors(bs1, sector_num, nb_sectors,
+ ret = check_empty_sectors(blk1, sector_num, nb_sectors,
filename1, buf1, quiet);
} else {
- ret = check_empty_sectors(bs2, sector_num, nb_sectors,
+ ret = check_empty_sectors(blk2, sector_num, nb_sectors,
filename2, buf1, quiet);
}
if (ret) {
@@ -1236,18 +1240,18 @@ static int img_compare(int argc, char **argv)
}
if (total_sectors1 != total_sectors2) {
- BlockDriverState *bs_over;
+ BlockBackend *blk_over;
int64_t total_sectors_over;
const char *filename_over;
qprintf(quiet, "Warning: Image size mismatch!\n");
if (total_sectors1 > total_sectors2) {
total_sectors_over = total_sectors1;
- bs_over = bs1;
+ blk_over = blk1;
filename_over = filename1;
} else {
total_sectors_over = total_sectors2;
- bs_over = bs2;
+ blk_over = blk2;
filename_over = filename2;
}
@@ -1256,7 +1260,7 @@ static int img_compare(int argc, char **argv)
if (nb_sectors <= 0) {
break;
}
- ret = bdrv_is_allocated_above(bs_over, NULL, sector_num,
+ ret = bdrv_is_allocated_above(blk_bs(blk_over), NULL, sector_num,
nb_sectors, &pnum);
if (ret < 0) {
ret = 3;
@@ -1267,7 +1271,7 @@ static int img_compare(int argc, char **argv)
}
nb_sectors = pnum;
if (ret) {
- ret = check_empty_sectors(bs_over, sector_num, nb_sectors,
+ ret = check_empty_sectors(blk_over, sector_num, nb_sectors,
filename_over, buf1, quiet);
if (ret) {
if (ret < 0) {
@@ -1476,13 +1480,14 @@ static int img_convert(int argc, char **argv)
goto out;
}
bs[bs_i] = blk_bs(blk[bs_i]);
- bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]);
+ bs_sectors[bs_i] = blk_getlength(blk[bs_i]);
if (bs_sectors[bs_i] < 0) {
error_report("Could not get size of %s: %s",
argv[optind + bs_i], strerror(-bs_sectors[bs_i]));
ret = -1;
goto out;
}
+ bs_sectors[bs_i] /= BDRV_SECTOR_SIZE;
total_sectors += bs_sectors[bs_i];
}
@@ -1625,16 +1630,19 @@ static int img_convert(int argc, char **argv)
out_bs->bl.discard_alignment))
);
- buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
+ buf = blk_blockalign(out_blk, bufsectors * BDRV_SECTOR_SIZE);
if (skip_create) {
- int64_t output_sectors = bdrv_nb_sectors(out_bs);
+ int64_t output_sectors = blk_getlength(out_blk);
if (output_sectors < 0) {
error_report("unable to get output image length: %s\n",
strerror(-output_sectors));
ret = -1;
goto out;
- } else if (output_sectors < total_sectors) {
+ }
+
+ output_sectors /= BDRV_SECTOR_SIZE;
+ if (output_sectors < total_sectors) {
error_report("output file is smaller than input file");
ret = -1;
goto out;
@@ -1696,7 +1704,7 @@ static int img_convert(int argc, char **argv)
nlow = remainder > bs_sectors[bs_i] - bs_num
? bs_sectors[bs_i] - bs_num : remainder;
- ret = bdrv_read(bs[bs_i], bs_num, buf2, nlow);
+ ret = blk_read(blk[bs_i], bs_num, buf2, nlow);
if (ret < 0) {
error_report("error while reading sector %" PRId64 ": %s",
bs_num, strerror(-ret));
@@ -1711,7 +1719,7 @@ static int img_convert(int argc, char **argv)
assert (remainder == 0);
if (!buffer_is_zero(buf, n * BDRV_SECTOR_SIZE)) {
- ret = bdrv_write_compressed(out_bs, sector_num, buf, n);
+ ret = blk_write_compressed(out_blk, sector_num, buf, n);
if (ret != 0) {
error_report("error while compressing sector %" PRId64
": %s", sector_num, strerror(-ret));
@@ -1722,7 +1730,7 @@ static int img_convert(int argc, char **argv)
qemu_progress_print(100.0 * sector_num / total_sectors, 0);
}
/* signal EOF to align */
- bdrv_write_compressed(out_bs, 0, NULL, 0);
+ blk_write_compressed(out_blk, 0, NULL, 0);
} else {
int64_t sectors_to_read, sectors_read, sector_num_next_status;
bool count_allocated_sectors;
@@ -1823,7 +1831,7 @@ restart:
}
n1 = n;
- ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
+ ret = blk_read(blk[bs_i], sector_num - bs_offset, buf, n);
if (ret < 0) {
error_report("error while reading sector %" PRId64 ": %s",
sector_num - bs_offset, strerror(-ret));
@@ -1836,7 +1844,7 @@ restart:
while (n > 0) {
if (!has_zero_init ||
is_allocated_sectors_min(buf1, n, &n1, min_sparse)) {
- ret = bdrv_write(out_bs, sector_num, buf1, n1);
+ ret = blk_write(out_blk, sector_num, buf1, n1);
if (ret < 0) {
error_report("error while writing sector %" PRId64
": %s", sector_num, strerror(-ret));
@@ -2260,7 +2268,7 @@ static int img_map(int argc, char **argv)
printf("%-16s%-16s%-16s%s\n", "Offset", "Length", "Mapped to", "File");
}
- length = bdrv_getlength(bs);
+ length = blk_getlength(blk);
while (curr.start + curr.length < length) {
int64_t nsectors_left;
int64_t sector_num;
@@ -2429,7 +2437,7 @@ static int img_snapshot(int argc, char **argv)
static int img_rebase(int argc, char **argv)
{
BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
- BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL;
+ BlockDriverState *bs = NULL;
char *filename;
const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
int c, flags, src_flags, ret;
@@ -2543,7 +2551,6 @@ static int img_rebase(int argc, char **argv)
error_free(local_err);
goto out;
}
- bs_old_backing = blk_bs(blk_old_backing);
if (out_baseimg[0]) {
if (out_basefmt) {
@@ -2562,7 +2569,6 @@ static int img_rebase(int argc, char **argv)
error_free(local_err);
goto out;
}
- bs_new_backing = blk_bs(blk_new_backing);
}
}
@@ -2585,17 +2591,17 @@ static int img_rebase(int argc, char **argv)
uint8_t * buf_new;
float local_progress = 0;
- buf_old = qemu_blockalign(bs, IO_BUF_SIZE);
- buf_new = qemu_blockalign(bs, IO_BUF_SIZE);
+ buf_old = blk_blockalign(blk, IO_BUF_SIZE);
+ buf_new = blk_blockalign(blk, IO_BUF_SIZE);
- num_sectors = bdrv_nb_sectors(bs);
+ num_sectors = blk_getlength(blk);
if (num_sectors < 0) {
error_report("Could not get size of '%s': %s",
filename, strerror(-num_sectors));
ret = -1;
goto out;
}
- old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing);
+ old_backing_num_sectors = blk_getlength(blk_old_backing);
if (old_backing_num_sectors < 0) {
char backing_name[PATH_MAX];
@@ -2605,14 +2611,20 @@ static int img_rebase(int argc, char **argv)
ret = -1;
goto out;
}
- if (bs_new_backing) {
- new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing);
+
+ num_sectors /= BDRV_SECTOR_SIZE;
+ old_backing_num_sectors /= BDRV_SECTOR_SIZE;
+
+ if (blk_new_backing) {
+ new_backing_num_sectors = blk_getlength(blk_new_backing);
if (new_backing_num_sectors < 0) {
error_report("Could not get size of '%s': %s",
out_baseimg, strerror(-new_backing_num_sectors));
ret = -1;
goto out;
}
+
+ new_backing_num_sectors /= BDRV_SECTOR_SIZE;
}
if (num_sectors != 0) {
@@ -2651,21 +2663,21 @@ static int img_rebase(int argc, char **argv)
n = old_backing_num_sectors - sector;
}
- ret = bdrv_read(bs_old_backing, sector, buf_old, n);
+ ret = blk_read(blk_old_backing, sector, buf_old, n);
if (ret < 0) {
error_report("error while reading from old backing file");
goto out;
}
}
- if (sector >= new_backing_num_sectors || !bs_new_backing) {
+ if (sector >= new_backing_num_sectors || !blk_new_backing) {
memset(buf_new, 0, n * BDRV_SECTOR_SIZE);
} else {
if (sector + n > new_backing_num_sectors) {
n = new_backing_num_sectors - sector;
}
- ret = bdrv_read(bs_new_backing, sector, buf_new, n);
+ ret = blk_read(blk_new_backing, sector, buf_new, n);
if (ret < 0) {
error_report("error while reading from new backing file");
goto out;
@@ -2681,8 +2693,8 @@ static int img_rebase(int argc, char **argv)
if (compare_sectors(buf_old + written * 512,
buf_new + written * 512, n - written, &pnum))
{
- ret = bdrv_write(bs, sector + written,
- buf_old + written * 512, pnum);
+ ret = blk_write(blk, sector + written,
+ buf_old + written * 512, pnum);
if (ret < 0) {
error_report("Error while writing to COW image: %s",
strerror(-ret));
@@ -2747,7 +2759,6 @@ static int img_resize(int argc, char **argv)
int64_t n, total_size;
bool quiet = false;
BlockBackend *blk = NULL;
- BlockDriverState *bs = NULL;
QemuOpts *param;
static QemuOptsList resize_options = {
.name = "resize_options",
@@ -2829,10 +2840,9 @@ static int img_resize(int argc, char **argv)
ret = -1;
goto out;
}
- bs = blk_bs(blk);
if (relative) {
- total_size = bdrv_getlength(bs) + n * relative;
+ total_size = blk_getlength(blk) + n * relative;
} else {
total_size = n;
}
@@ -2842,7 +2852,7 @@ static int img_resize(int argc, char **argv)
goto out;
}
- ret = bdrv_truncate(bs, total_size);
+ ret = blk_truncate(blk, total_size);
switch (ret) {
case 0:
qprintf(quiet, "Image resized.\n");
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (6 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 4:59 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile() Max Reitz
` (7 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-nbd.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d222512..fdc9590 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -386,7 +386,6 @@ int main(int argc, char **argv)
{
BlockBackend *blk;
BlockDriverState *bs;
- BlockDriver *drv;
off_t dev_offset = 0;
uint32_t nbdflags = 0;
bool disconnect = false;
@@ -429,7 +428,7 @@ int main(int argc, char **argv)
char *end;
int flags = BDRV_O_RDWR;
int partition = -1;
- int ret;
+ int ret = 0;
int fd;
bool seen_cache = false;
bool seen_discard = false;
@@ -440,6 +439,7 @@ int main(int argc, char **argv)
const char *fmt = NULL;
Error *local_err = NULL;
BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
+ QDict *options = NULL;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -684,24 +684,17 @@ int main(int argc, char **argv)
atexit(bdrv_close_all);
if (fmt) {
- drv = bdrv_find_format(fmt);
- if (!drv) {
- errx(EXIT_FAILURE, "Unknown file format '%s'", fmt);
- }
- } else {
- drv = NULL;
+ options = qdict_new();
+ qdict_put_obj(options, "driver", QOBJECT(qstring_from_str(fmt)));
}
- blk = blk_new_with_bs("hda", &error_abort);
- bs = blk_bs(blk);
-
srcpath = argv[optind];
- 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],
- error_get_pretty(local_err));
+ blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
+ if (!blk) {
+ errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
+ error_get_pretty(local_err));
}
+ bs = blk_bs(blk);
if (sn_opts) {
ret = bdrv_snapshot_load_tmp(bs,
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (7 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 16:23 ` Eric Blake
2015-02-02 19:34 ` Kevin Wolf
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option Max Reitz
` (6 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 91a445a..81f8f64 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -39,7 +39,6 @@ static ReadLineState *readline_state;
static int close_f(BlockDriverState *bs, int argc, char **argv)
{
blk_unref(qemuio_blk);
- qemuio_bs = NULL;
qemuio_blk = NULL;
return 0;
}
@@ -51,34 +50,29 @@ static const cmdinfo_t close_cmd = {
.oneline = "close the current open file",
};
-static int openfile(char *name, BlockDriver *drv, int flags, int growable,
- QDict *opts)
+static int openfile(char *name, int flags, int growable, QDict *opts)
{
Error *local_err = NULL;
- if (qemuio_bs) {
+ if (qemuio_blk) {
fprintf(stderr, "file open already, try 'help close'\n");
QDECREF(opts);
return 1;
}
- qemuio_blk = blk_new_with_bs("hda", &error_abort);
- qemuio_bs = blk_bs(qemuio_blk);
-
if (growable) {
flags |= BDRV_O_PROTOCOL;
}
- if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, drv, &local_err) < 0) {
+ qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err);
+ if (!qemuio_blk) {
fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
name ? " device " : "", name ?: "",
error_get_pretty(local_err));
error_free(local_err);
- blk_unref(qemuio_blk);
- qemuio_bs = NULL;
- qemuio_blk = NULL;
return 1;
}
+ qemuio_bs = blk_bs(qemuio_blk);
return 0;
}
@@ -170,9 +164,9 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- return openfile(argv[optind], NULL, flags, growable, opts);
+ return openfile(argv[optind], flags, growable, opts);
} else if (optind == argc) {
- return openfile(NULL, NULL, flags, growable, opts);
+ return openfile(NULL, flags, growable, opts);
} else {
QDECREF(opts);
return qemuio_command_usage(&open_cmd);
@@ -387,8 +381,8 @@ int main(int argc, char **argv)
int c;
int opt_index = 0;
int flags = BDRV_O_UNMAP;
- BlockDriver *drv = NULL;
Error *local_error = NULL;
+ QDict *opts = NULL;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -414,11 +408,10 @@ int main(int argc, char **argv)
}
break;
case 'f':
- drv = bdrv_find_format(optarg);
- if (!drv) {
- error_report("Invalid format '%s'", optarg);
- exit(EXIT_FAILURE);
+ if (!opts) {
+ opts = qdict_new();
}
+ qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str(optarg)));
break;
case 'c':
add_user_command(optarg);
@@ -489,7 +482,7 @@ int main(int argc, char **argv)
}
if ((argc - optind) == 1) {
- openfile(argv[optind], drv, flags, growable, NULL);
+ openfile(argv[optind], flags, growable, opts);
}
command_loop();
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (8 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile() Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 16:59 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend Max Reitz
` (5 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Remove "growable" option from the "open" command and from the qemu-io
command line. qemu-io is about to be converted to BlockBackend which
will make sure that no request exceeds the image size, so the only way
to keep "growable" would be to use BlockBackend if it is not given and
to directly access the BDS if it is.
qemu-io is a debugging tool, therefore removing a rarely used option
will have only a very small impact, if any. There was only one
qemu-iotest which used the option; since it is not critical, this patch
just removes it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io.c | 23 +++------------
tests/qemu-iotests/016 | 73 ----------------------------------------------
tests/qemu-iotests/016.out | 23 ---------------
tests/qemu-iotests/group | 1 -
4 files changed, 4 insertions(+), 116 deletions(-)
delete mode 100755 tests/qemu-iotests/016
delete mode 100644 tests/qemu-iotests/016.out
diff --git a/qemu-io.c b/qemu-io.c
index 81f8f64..0237ecb 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -50,7 +50,7 @@ static const cmdinfo_t close_cmd = {
.oneline = "close the current open file",
};
-static int openfile(char *name, int flags, int growable, QDict *opts)
+static int openfile(char *name, int flags, QDict *opts)
{
Error *local_err = NULL;
@@ -60,10 +60,6 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
return 1;
}
- if (growable) {
- flags |= BDRV_O_PROTOCOL;
- }
-
qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err);
if (!qemuio_blk) {
fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
@@ -90,7 +86,6 @@ static void open_help(void)
" -r, -- open file read-only\n"
" -s, -- use snapshot file\n"
" -n, -- disable host cache\n"
-" -g, -- allow file to grow (only applies to protocols)\n"
" -o, -- options to be given to the block driver"
"\n");
}
@@ -123,7 +118,6 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
{
int flags = 0;
int readonly = 0;
- int growable = 0;
int c;
QemuOpts *qopts;
QDict *opts;
@@ -139,9 +133,6 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
case 'r':
readonly = 1;
break;
- case 'g':
- growable = 1;
- break;
case 'o':
if (!qemu_opts_parse(&empty_opts, optarg, 0)) {
printf("could not parse option list -- %s\n", optarg);
@@ -164,9 +155,9 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- return openfile(argv[optind], flags, growable, opts);
+ return openfile(argv[optind], flags, opts);
} else if (optind == argc) {
- return openfile(NULL, flags, growable, opts);
+ return openfile(NULL, flags, opts);
} else {
QDECREF(opts);
return qemuio_command_usage(&open_cmd);
@@ -200,7 +191,6 @@ static void usage(const char *name)
" -r, --read-only export read-only\n"
" -s, --snapshot use snapshot file\n"
" -n, --nocache disable host cache\n"
-" -g, --growable allow file to grow (only applies to protocols)\n"
" -m, --misalign misalign allocations for O_DIRECT\n"
" -k, --native-aio use kernel AIO implementation (on Linux only)\n"
" -t, --cache=MODE use the given cache mode for the image\n"
@@ -359,7 +349,6 @@ static void reenable_tty_echo(void)
int main(int argc, char **argv)
{
int readonly = 0;
- int growable = 0;
const char *sopt = "hVc:d:f:rsnmgkt:T:";
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
@@ -371,7 +360,6 @@ int main(int argc, char **argv)
{ "snapshot", 0, NULL, 's' },
{ "nocache", 0, NULL, 'n' },
{ "misalign", 0, NULL, 'm' },
- { "growable", 0, NULL, 'g' },
{ "native-aio", 0, NULL, 'k' },
{ "discard", 1, NULL, 'd' },
{ "cache", 1, NULL, 't' },
@@ -422,9 +410,6 @@ int main(int argc, char **argv)
case 'm':
qemuio_misalign = true;
break;
- case 'g':
- growable = 1;
- break;
case 'k':
flags |= BDRV_O_NATIVE_AIO;
break;
@@ -482,7 +467,7 @@ int main(int argc, char **argv)
}
if ((argc - optind) == 1) {
- openfile(argv[optind], flags, growable, opts);
+ openfile(argv[optind], flags, opts);
}
command_loop();
diff --git a/tests/qemu-iotests/016 b/tests/qemu-iotests/016
deleted file mode 100755
index 52397aa..0000000
--- a/tests/qemu-iotests/016
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/bin/bash
-#
-# Test I/O after EOF for growable images.
-#
-# Copyright (C) 2009 Red Hat, Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program. If not, see <http://www.gnu.org/licenses/>.
-#
-
-# creator
-owner=hch@lst.de
-
-seq=`basename $0`
-echo "QA output created by $seq"
-
-here=`pwd`
-tmp=/tmp/$$
-status=1 # failure is the default!
-
-_cleanup()
-{
- _cleanup_test_img
-}
-trap "_cleanup; exit \$status" 0 1 2 3 15
-
-# get standard environment, filters and checks
-. ./common.rc
-. ./common.filter
-
-_supported_fmt raw
-_supported_proto file sheepdog nfs
-_supported_os Linux
-
-
-# No -f, use probing for the protocol driver
-QEMU_IO_PROTO="$QEMU_IO_PROG -g --cache $CACHEMODE"
-
-size=128M
-_make_test_img $size
-
-echo
-echo "== reading at EOF =="
-$QEMU_IO_PROTO -c "read -P 0 $size 512" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== reading far past EOF =="
-$QEMU_IO_PROTO -c "read -P 0 256M 512" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== writing at EOF =="
-$QEMU_IO_PROTO -c "write -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== writing far past EOF =="
-$QEMU_IO_PROTO -c "write -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io
-
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
diff --git a/tests/qemu-iotests/016.out b/tests/qemu-iotests/016.out
deleted file mode 100644
index acbd60b..0000000
--- a/tests/qemu-iotests/016.out
+++ /dev/null
@@ -1,23 +0,0 @@
-QA output created by 016
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-
-== reading at EOF ==
-read 512/512 bytes at offset 134217728
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== reading far past EOF ==
-read 512/512 bytes at offset 268435456
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== writing at EOF ==
-wrote 512/512 bytes at offset 134217728
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 512/512 bytes at offset 134217728
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== writing far past EOF ==
-wrote 512/512 bytes at offset 268435456
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 512/512 bytes at offset 268435456
-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 4b2b93b..ddfedb5 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -22,7 +22,6 @@
013 rw auto
014 rw auto
015 rw snapshot auto
-016 rw auto quick
017 rw backing auto quick
018 rw backing auto quick
019 rw backing auto quick
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (9 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 17:08 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests Max Reitz
` (4 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
qemu-io should behave like a guest, therefore it should use BlockBackend
to access the block layer.
There are a couple of places where that is infeasible: First, the
bdrv_debug_* functions could theoretically be mirrored in the
BlockBackend, but since these are functions internal to the block layer,
they should not be visible externally (qemu-io as a test tool is excempt
from this).
Second, bdrv_get_info() and bdrv_get_specific_info() work on a single
BDS alone, therefore they should stay BDS-specific.
Third, bdrv_is_allocated() mainly works on a single BDS as well. Some
data may be passed through from the BDS's file (if sectors which are
apparently allocated in the file are not really allocated there but just
zero).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hmp.c | 9 ++-
include/qemu-io.h | 4 +-
qemu-io-cmds.c | 238 ++++++++++++++++++++++++++++--------------------------
qemu-io.c | 12 +--
4 files changed, 138 insertions(+), 125 deletions(-)
diff --git a/hmp.c b/hmp.c
index 481be80..f806387 100644
--- a/hmp.c
+++ b/hmp.c
@@ -16,6 +16,7 @@
#include "hmp.h"
#include "net/net.h"
#include "sysemu/char.h"
+#include "sysemu/block-backend.h"
#include "qemu/option.h"
#include "qemu/timer.h"
#include "qmp-commands.h"
@@ -1714,14 +1715,14 @@ void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
void hmp_qemu_io(Monitor *mon, const QDict *qdict)
{
- BlockDriverState *bs;
+ BlockBackend *blk;
const char* device = qdict_get_str(qdict, "device");
const char* command = qdict_get_str(qdict, "command");
Error *err = NULL;
- bs = bdrv_find(device);
- if (bs) {
- qemuio_command(bs, command);
+ blk = blk_by_name(device);
+ if (blk) {
+ qemuio_command(blk, command);
} else {
error_set(&err, QERR_DEVICE_NOT_FOUND, device);
}
diff --git a/include/qemu-io.h b/include/qemu-io.h
index 5d6006f..4d402b9 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -22,7 +22,7 @@
#define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
-typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv);
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
typedef void (*helpfunc_t)(void);
typedef struct cmdinfo {
@@ -40,7 +40,7 @@ typedef struct cmdinfo {
extern bool qemuio_misalign;
-bool qemuio_command(BlockDriverState *bs, const char *cmd);
+bool qemuio_command(BlockBackend *blk, const char *cmd);
void qemuio_add_command(const cmdinfo_t *ci);
int qemuio_command_usage(const cmdinfo_t *ci);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e708552..fa21fc8 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -9,7 +9,9 @@
*/
#include "qemu-io.h"
-#include "block/block_int.h"
+#include "sysemu/block-backend.h"
+#include "block/block.h"
+#include "block/block_int.h" /* for info_f() */
#include "block/qapi.h"
#include "qemu/main-loop.h"
#include "qemu/timer.h"
@@ -40,24 +42,24 @@ int qemuio_command_usage(const cmdinfo_t *ci)
return 0;
}
-static int init_check_command(BlockDriverState *bs, const cmdinfo_t *ct)
+static int init_check_command(BlockBackend *blk, const cmdinfo_t *ct)
{
if (ct->flags & CMD_FLAG_GLOBAL) {
return 1;
}
- if (!(ct->flags & CMD_NOFILE_OK) && !bs) {
+ if (!(ct->flags & CMD_NOFILE_OK) && !blk) {
fprintf(stderr, "no file open, try 'help open'\n");
return 0;
}
return 1;
}
-static int command(BlockDriverState *bs, const cmdinfo_t *ct, int argc,
+static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
char **argv)
{
char *cmd = argv[0];
- if (!init_check_command(bs, ct)) {
+ if (!init_check_command(blk, ct)) {
return 0;
}
@@ -78,7 +80,7 @@ static int command(BlockDriverState *bs, const cmdinfo_t *ct, int argc,
return 0;
}
optind = 0;
- return ct->cfunc(bs, argc, argv);
+ return ct->cfunc(blk, argc, argv);
}
static const cmdinfo_t *find_command(const char *cmd)
@@ -267,14 +269,14 @@ static int parse_pattern(const char *arg)
*/
#define MISALIGN_OFFSET 16
-static void *qemu_io_alloc(BlockDriverState *bs, size_t len, int pattern)
+static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern)
{
void *buf;
if (qemuio_misalign) {
len += MISALIGN_OFFSET;
}
- buf = qemu_blockalign(bs, len);
+ buf = blk_blockalign(blk, len);
memset(buf, pattern, len);
if (qemuio_misalign) {
buf += MISALIGN_OFFSET;
@@ -340,7 +342,7 @@ static void print_report(const char *op, struct timeval *t, int64_t offset,
* vector matching it.
*/
static void *
-create_iovec(BlockDriverState *bs, QEMUIOVector *qiov, char **argv, int nr_iov,
+create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov,
int pattern)
{
size_t *sizes = g_new0(size_t, nr_iov);
@@ -377,7 +379,7 @@ create_iovec(BlockDriverState *bs, QEMUIOVector *qiov, char **argv, int nr_iov,
qemu_iovec_init(qiov, nr_iov);
- buf = p = qemu_io_alloc(bs, count, pattern);
+ buf = p = qemu_io_alloc(blk, count, pattern);
for (i = 0; i < nr_iov; i++) {
qemu_iovec_add(qiov, p, sizes[i]);
@@ -389,12 +391,12 @@ fail:
return buf;
}
-static int do_read(BlockDriverState *bs, char *buf, int64_t offset, int count,
+static int do_read(BlockBackend *blk, char *buf, int64_t offset, int count,
int *total)
{
int ret;
- ret = bdrv_read(bs, offset >> 9, (uint8_t *)buf, count >> 9);
+ ret = blk_read(blk, offset >> 9, (uint8_t *)buf, count >> 9);
if (ret < 0) {
return ret;
}
@@ -402,12 +404,12 @@ static int do_read(BlockDriverState *bs, char *buf, int64_t offset, int count,
return 1;
}
-static int do_write(BlockDriverState *bs, char *buf, int64_t offset, int count,
+static int do_write(BlockBackend *blk, char *buf, int64_t offset, int count,
int *total)
{
int ret;
- ret = bdrv_write(bs, offset >> 9, (uint8_t *)buf, count >> 9);
+ ret = blk_write(blk, offset >> 9, (uint8_t *)buf, count >> 9);
if (ret < 0) {
return ret;
}
@@ -415,20 +417,20 @@ static int do_write(BlockDriverState *bs, char *buf, int64_t offset, int count,
return 1;
}
-static int do_pread(BlockDriverState *bs, char *buf, int64_t offset, int count,
+static int do_pread(BlockBackend *blk, char *buf, int64_t offset, int count,
int *total)
{
- *total = bdrv_pread(bs, offset, (uint8_t *)buf, count);
+ *total = blk_pread(blk, offset, (uint8_t *)buf, count);
if (*total < 0) {
return *total;
}
return 1;
}
-static int do_pwrite(BlockDriverState *bs, char *buf, int64_t offset, int count,
+static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, int count,
int *total)
{
- *total = bdrv_pwrite(bs, offset, (uint8_t *)buf, count);
+ *total = blk_pwrite(blk, offset, (uint8_t *)buf, count);
if (*total < 0) {
return *total;
}
@@ -436,7 +438,7 @@ static int do_pwrite(BlockDriverState *bs, char *buf, int64_t offset, int count,
}
typedef struct {
- BlockDriverState *bs;
+ BlockBackend *blk;
int64_t offset;
int count;
int *total;
@@ -448,8 +450,8 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
{
CoWriteZeroes *data = opaque;
- data->ret = bdrv_co_write_zeroes(data->bs, data->offset / BDRV_SECTOR_SIZE,
- data->count / BDRV_SECTOR_SIZE, 0);
+ data->ret = blk_co_write_zeroes(data->blk, data->offset / BDRV_SECTOR_SIZE,
+ data->count / BDRV_SECTOR_SIZE, 0);
data->done = true;
if (data->ret < 0) {
*data->total = data->ret;
@@ -459,12 +461,12 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
*data->total = data->count;
}
-static int do_co_write_zeroes(BlockDriverState *bs, int64_t offset, int count,
+static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count,
int *total)
{
Coroutine *co;
CoWriteZeroes data = {
- .bs = bs,
+ .blk = blk,
.offset = offset,
.count = count,
.total = total,
@@ -474,7 +476,7 @@ static int do_co_write_zeroes(BlockDriverState *bs, int64_t offset, int count,
co = qemu_coroutine_create(co_write_zeroes_entry);
qemu_coroutine_enter(co, &data);
while (!data.done) {
- aio_poll(bdrv_get_aio_context(bs), true);
+ aio_poll(blk_get_aio_context(blk), true);
}
if (data.ret < 0) {
return data.ret;
@@ -483,12 +485,12 @@ static int do_co_write_zeroes(BlockDriverState *bs, int64_t offset, int count,
}
}
-static int do_write_compressed(BlockDriverState *bs, char *buf, int64_t offset,
+static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset,
int count, int *total)
{
int ret;
- ret = bdrv_write_compressed(bs, offset >> 9, (uint8_t *)buf, count >> 9);
+ ret = blk_write_compressed(blk, offset >> 9, (uint8_t *)buf, count >> 9);
if (ret < 0) {
return ret;
}
@@ -496,20 +498,20 @@ static int do_write_compressed(BlockDriverState *bs, char *buf, int64_t offset,
return 1;
}
-static int do_load_vmstate(BlockDriverState *bs, char *buf, int64_t offset,
+static int do_load_vmstate(BlockBackend *blk, char *buf, int64_t offset,
int count, int *total)
{
- *total = bdrv_load_vmstate(bs, (uint8_t *)buf, offset, count);
+ *total = blk_load_vmstate(blk, (uint8_t *)buf, offset, count);
if (*total < 0) {
return *total;
}
return 1;
}
-static int do_save_vmstate(BlockDriverState *bs, char *buf, int64_t offset,
+static int do_save_vmstate(BlockBackend *blk, char *buf, int64_t offset,
int count, int *total)
{
- *total = bdrv_save_vmstate(bs, (uint8_t *)buf, offset, count);
+ *total = blk_save_vmstate(blk, (uint8_t *)buf, offset, count);
if (*total < 0) {
return *total;
}
@@ -522,13 +524,13 @@ static void aio_rw_done(void *opaque, int ret)
*(int *)opaque = ret;
}
-static int do_aio_readv(BlockDriverState *bs, QEMUIOVector *qiov,
+static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov,
int64_t offset, int *total)
{
int async_ret = NOT_DONE;
- bdrv_aio_readv(bs, offset >> 9, qiov, qiov->size >> 9,
- aio_rw_done, &async_ret);
+ blk_aio_readv(blk, offset >> 9, qiov, qiov->size >> 9,
+ aio_rw_done, &async_ret);
while (async_ret == NOT_DONE) {
main_loop_wait(false);
}
@@ -537,13 +539,13 @@ static int do_aio_readv(BlockDriverState *bs, QEMUIOVector *qiov,
return async_ret < 0 ? async_ret : 1;
}
-static int do_aio_writev(BlockDriverState *bs, QEMUIOVector *qiov,
+static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov,
int64_t offset, int *total)
{
int async_ret = NOT_DONE;
- bdrv_aio_writev(bs, offset >> 9, qiov, qiov->size >> 9,
- aio_rw_done, &async_ret);
+ blk_aio_writev(blk, offset >> 9, qiov, qiov->size >> 9,
+ aio_rw_done, &async_ret);
while (async_ret == NOT_DONE) {
main_loop_wait(false);
}
@@ -567,7 +569,7 @@ static void multiwrite_cb(void *opaque, int ret)
}
}
-static int do_aio_multiwrite(BlockDriverState *bs, BlockRequest* reqs,
+static int do_aio_multiwrite(BlockBackend *blk, BlockRequest* reqs,
int num_reqs, int *total)
{
int i, ret;
@@ -583,7 +585,7 @@ static int do_aio_multiwrite(BlockDriverState *bs, BlockRequest* reqs,
*total += reqs[i].qiov->size;
}
- ret = bdrv_aio_multiwrite(bs, reqs, num_reqs);
+ ret = blk_aio_multiwrite(blk, reqs, num_reqs);
if (ret < 0) {
return ret;
}
@@ -609,7 +611,7 @@ static void read_help(void)
" -b, -- read from the VM state rather than the virtual disk\n"
" -C, -- report statistics in a machine parsable format\n"
" -l, -- length for pattern verification (only with -P)\n"
-" -p, -- use bdrv_pread to read the file\n"
+" -p, -- use blk_pread to read the file\n"
" -P, -- use a pattern to verify read data\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -s, -- start offset for pattern verification (only with -P)\n"
@@ -617,7 +619,7 @@ static void read_help(void)
"\n");
}
-static int read_f(BlockDriverState *bs, int argc, char **argv);
+static int read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t read_cmd = {
.name = "read",
@@ -630,7 +632,7 @@ static const cmdinfo_t read_cmd = {
.help = read_help,
};
-static int read_f(BlockDriverState *bs, int argc, char **argv)
+static int read_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, pflag = 0, qflag = 0, vflag = 0;
@@ -736,15 +738,15 @@ static int read_f(BlockDriverState *bs, int argc, char **argv)
}
}
- buf = qemu_io_alloc(bs, count, 0xab);
+ buf = qemu_io_alloc(blk, count, 0xab);
gettimeofday(&t1, NULL);
if (pflag) {
- cnt = do_pread(bs, buf, offset, count, &total);
+ cnt = do_pread(blk, buf, offset, count, &total);
} else if (bflag) {
- cnt = do_load_vmstate(bs, buf, offset, count, &total);
+ cnt = do_load_vmstate(blk, buf, offset, count, &total);
} else {
- cnt = do_read(bs, buf, offset, count, &total);
+ cnt = do_read(blk, buf, offset, count, &total);
}
gettimeofday(&t2, NULL);
@@ -801,7 +803,7 @@ static void readv_help(void)
"\n");
}
-static int readv_f(BlockDriverState *bs, int argc, char **argv);
+static int readv_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t readv_cmd = {
.name = "readv",
@@ -813,7 +815,7 @@ static const cmdinfo_t readv_cmd = {
.help = readv_help,
};
-static int readv_f(BlockDriverState *bs, int argc, char **argv)
+static int readv_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, qflag = 0, vflag = 0;
@@ -869,13 +871,13 @@ static int readv_f(BlockDriverState *bs, int argc, char **argv)
}
nr_iov = argc - optind;
- buf = create_iovec(bs, &qiov, &argv[optind], nr_iov, 0xab);
+ buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab);
if (buf == NULL) {
return 0;
}
gettimeofday(&t1, NULL);
- cnt = do_aio_readv(bs, &qiov, offset, &total);
+ cnt = do_aio_readv(blk, &qiov, offset, &total);
gettimeofday(&t2, NULL);
if (cnt < 0) {
@@ -923,16 +925,16 @@ static void write_help(void)
" Writes into a segment of the currently open file, using a buffer\n"
" filled with a set pattern (0xcdcdcdcd).\n"
" -b, -- write to the VM state rather than the virtual disk\n"
-" -c, -- write compressed data with bdrv_write_compressed\n"
-" -p, -- use bdrv_pwrite to write the file\n"
+" -c, -- write compressed data with blk_write_compressed\n"
+" -p, -- use blk_pwrite to write the file\n"
" -P, -- use different pattern to fill file\n"
" -C, -- report statistics in a machine parsable format\n"
" -q, -- quiet mode, do not show I/O statistics\n"
-" -z, -- write zeroes using bdrv_co_write_zeroes\n"
+" -z, -- write zeroes using blk_co_write_zeroes\n"
"\n");
}
-static int write_f(BlockDriverState *bs, int argc, char **argv);
+static int write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t write_cmd = {
.name = "write",
@@ -945,7 +947,7 @@ static const cmdinfo_t write_cmd = {
.help = write_help,
};
-static int write_f(BlockDriverState *bs, int argc, char **argv)
+static int write_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, pflag = 0, qflag = 0, bflag = 0, Pflag = 0, zflag = 0;
@@ -1032,20 +1034,20 @@ static int write_f(BlockDriverState *bs, int argc, char **argv)
}
if (!zflag) {
- buf = qemu_io_alloc(bs, count, pattern);
+ buf = qemu_io_alloc(blk, count, pattern);
}
gettimeofday(&t1, NULL);
if (pflag) {
- cnt = do_pwrite(bs, buf, offset, count, &total);
+ cnt = do_pwrite(blk, buf, offset, count, &total);
} else if (bflag) {
- cnt = do_save_vmstate(bs, buf, offset, count, &total);
+ cnt = do_save_vmstate(blk, buf, offset, count, &total);
} else if (zflag) {
- cnt = do_co_write_zeroes(bs, offset, count, &total);
+ cnt = do_co_write_zeroes(blk, offset, count, &total);
} else if (cflag) {
- cnt = do_write_compressed(bs, buf, offset, count, &total);
+ cnt = do_write_compressed(blk, buf, offset, count, &total);
} else {
- cnt = do_write(bs, buf, offset, count, &total);
+ cnt = do_write(blk, buf, offset, count, &total);
}
gettimeofday(&t2, NULL);
@@ -1088,7 +1090,7 @@ writev_help(void)
"\n");
}
-static int writev_f(BlockDriverState *bs, int argc, char **argv);
+static int writev_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t writev_cmd = {
.name = "writev",
@@ -1100,7 +1102,7 @@ static const cmdinfo_t writev_cmd = {
.help = writev_help,
};
-static int writev_f(BlockDriverState *bs, int argc, char **argv)
+static int writev_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, qflag = 0;
@@ -1150,13 +1152,13 @@ static int writev_f(BlockDriverState *bs, int argc, char **argv)
}
nr_iov = argc - optind;
- buf = create_iovec(bs, &qiov, &argv[optind], nr_iov, pattern);
+ buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern);
if (buf == NULL) {
return 0;
}
gettimeofday(&t1, NULL);
- cnt = do_aio_writev(bs, &qiov, offset, &total);
+ cnt = do_aio_writev(blk, &qiov, offset, &total);
gettimeofday(&t2, NULL);
if (cnt < 0) {
@@ -1197,7 +1199,7 @@ static void multiwrite_help(void)
"\n");
}
-static int multiwrite_f(BlockDriverState *bs, int argc, char **argv);
+static int multiwrite_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t multiwrite_cmd = {
.name = "multiwrite",
@@ -1209,7 +1211,7 @@ static const cmdinfo_t multiwrite_cmd = {
.help = multiwrite_help,
};
-static int multiwrite_f(BlockDriverState *bs, int argc, char **argv)
+static int multiwrite_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, qflag = 0;
@@ -1290,7 +1292,7 @@ static int multiwrite_f(BlockDriverState *bs, int argc, char **argv)
nr_iov = j - optind;
/* Build request */
- buf[i] = create_iovec(bs, &qiovs[i], &argv[optind], nr_iov, pattern);
+ buf[i] = create_iovec(blk, &qiovs[i], &argv[optind], nr_iov, pattern);
if (buf[i] == NULL) {
goto out;
}
@@ -1308,7 +1310,7 @@ static int multiwrite_f(BlockDriverState *bs, int argc, char **argv)
nr_reqs = i;
gettimeofday(&t1, NULL);
- cnt = do_aio_multiwrite(bs, reqs, nr_reqs, &total);
+ cnt = do_aio_multiwrite(blk, reqs, nr_reqs, &total);
gettimeofday(&t2, NULL);
if (cnt < 0) {
@@ -1436,7 +1438,7 @@ static void aio_read_help(void)
"\n");
}
-static int aio_read_f(BlockDriverState *bs, int argc, char **argv);
+static int aio_read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_read_cmd = {
.name = "aio_read",
@@ -1448,7 +1450,7 @@ static const cmdinfo_t aio_read_cmd = {
.help = aio_read_help,
};
-static int aio_read_f(BlockDriverState *bs, int argc, char **argv)
+static int aio_read_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
@@ -1499,15 +1501,15 @@ static int aio_read_f(BlockDriverState *bs, int argc, char **argv)
}
nr_iov = argc - optind;
- ctx->buf = create_iovec(bs, &ctx->qiov, &argv[optind], nr_iov, 0xab);
+ ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, 0xab);
if (ctx->buf == NULL) {
g_free(ctx);
return 0;
}
gettimeofday(&ctx->t1, NULL);
- bdrv_aio_readv(bs, ctx->offset >> 9, &ctx->qiov,
- ctx->qiov.size >> 9, aio_read_done, ctx);
+ blk_aio_readv(blk, ctx->offset >> 9, &ctx->qiov,
+ ctx->qiov.size >> 9, aio_read_done, ctx);
return 0;
}
@@ -1531,7 +1533,7 @@ static void aio_write_help(void)
"\n");
}
-static int aio_write_f(BlockDriverState *bs, int argc, char **argv);
+static int aio_write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_write_cmd = {
.name = "aio_write",
@@ -1543,7 +1545,7 @@ static const cmdinfo_t aio_write_cmd = {
.help = aio_write_help,
};
-static int aio_write_f(BlockDriverState *bs, int argc, char **argv)
+static int aio_write_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
int pattern = 0xcd;
@@ -1591,21 +1593,21 @@ static int aio_write_f(BlockDriverState *bs, int argc, char **argv)
}
nr_iov = argc - optind;
- ctx->buf = create_iovec(bs, &ctx->qiov, &argv[optind], nr_iov, pattern);
+ ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, pattern);
if (ctx->buf == NULL) {
g_free(ctx);
return 0;
}
gettimeofday(&ctx->t1, NULL);
- bdrv_aio_writev(bs, ctx->offset >> 9, &ctx->qiov,
- ctx->qiov.size >> 9, aio_write_done, ctx);
+ blk_aio_writev(blk, ctx->offset >> 9, &ctx->qiov,
+ ctx->qiov.size >> 9, aio_write_done, ctx);
return 0;
}
-static int aio_flush_f(BlockDriverState *bs, int argc, char **argv)
+static int aio_flush_f(BlockBackend *blk, int argc, char **argv)
{
- bdrv_drain_all();
+ blk_drain_all();
return 0;
}
@@ -1615,9 +1617,9 @@ static const cmdinfo_t aio_flush_cmd = {
.oneline = "completes all outstanding aio requests"
};
-static int flush_f(BlockDriverState *bs, int argc, char **argv)
+static int flush_f(BlockBackend *blk, int argc, char **argv)
{
- bdrv_flush(bs);
+ blk_flush(blk);
return 0;
}
@@ -1628,7 +1630,7 @@ static const cmdinfo_t flush_cmd = {
.oneline = "flush all in-core file state to disk",
};
-static int truncate_f(BlockDriverState *bs, int argc, char **argv)
+static int truncate_f(BlockBackend *blk, int argc, char **argv)
{
int64_t offset;
int ret;
@@ -1639,7 +1641,7 @@ static int truncate_f(BlockDriverState *bs, int argc, char **argv)
return 0;
}
- ret = bdrv_truncate(bs, offset);
+ ret = blk_truncate(blk, offset);
if (ret < 0) {
printf("truncate: %s\n", strerror(-ret));
return 0;
@@ -1658,12 +1660,12 @@ static const cmdinfo_t truncate_cmd = {
.oneline = "truncates the current file at the given offset",
};
-static int length_f(BlockDriverState *bs, int argc, char **argv)
+static int length_f(BlockBackend *blk, int argc, char **argv)
{
int64_t size;
char s1[64];
- size = bdrv_getlength(bs);
+ size = blk_getlength(blk);
if (size < 0) {
printf("getlength: %s\n", strerror(-size));
return 0;
@@ -1683,8 +1685,9 @@ static const cmdinfo_t length_cmd = {
};
-static int info_f(BlockDriverState *bs, int argc, char **argv)
+static int info_f(BlockBackend *blk, int argc, char **argv)
{
+ BlockDriverState *bs = blk_bs(blk);
BlockDriverInfo bdi;
ImageInfoSpecific *spec_info;
char s1[64], s2[64];
@@ -1742,7 +1745,7 @@ static void discard_help(void)
"\n");
}
-static int discard_f(BlockDriverState *bs, int argc, char **argv);
+static int discard_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t discard_cmd = {
.name = "discard",
@@ -1755,7 +1758,7 @@ static const cmdinfo_t discard_cmd = {
.help = discard_help,
};
-static int discard_f(BlockDriverState *bs, int argc, char **argv)
+static int discard_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
int Cflag = 0, qflag = 0;
@@ -1794,8 +1797,8 @@ static int discard_f(BlockDriverState *bs, int argc, char **argv)
}
gettimeofday(&t1, NULL);
- ret = bdrv_discard(bs, offset >> BDRV_SECTOR_BITS,
- count >> BDRV_SECTOR_BITS);
+ ret = blk_discard(blk, offset >> BDRV_SECTOR_BITS,
+ count >> BDRV_SECTOR_BITS);
gettimeofday(&t2, NULL);
if (ret < 0) {
@@ -1813,8 +1816,9 @@ out:
return 0;
}
-static int alloc_f(BlockDriverState *bs, int argc, char **argv)
+static int alloc_f(BlockBackend *blk, int argc, char **argv)
{
+ BlockDriverState *bs = blk_bs(blk);
int64_t offset, sector_num;
int nb_sectors, remaining;
char s1[64];
@@ -1910,20 +1914,28 @@ static int map_is_allocated(BlockDriverState *bs, int64_t sector_num,
return firstret;
}
-static int map_f(BlockDriverState *bs, int argc, char **argv)
+static int map_f(BlockBackend *blk, int argc, char **argv)
{
int64_t offset;
- int64_t nb_sectors;
+ int64_t nb_sectors, total_sectors;
char s1[64];
int64_t num;
int ret;
const char *retstr;
offset = 0;
- nb_sectors = bs->total_sectors;
+ total_sectors = blk_getlength(blk);
+ if (total_sectors < 0) {
+ error_report("Failed to query image length: %s",
+ strerror(-total_sectors));
+ return 0;
+ }
+
+ total_sectors /= BDRV_SECTOR_SIZE;
+ nb_sectors = total_sectors;
do {
- ret = map_is_allocated(bs, offset, nb_sectors, &num);
+ ret = map_is_allocated(blk_bs(blk), offset, nb_sectors, &num);
if (ret < 0) {
error_report("Failed to get allocation status: %s", strerror(-ret));
return 0;
@@ -1940,7 +1952,7 @@ static int map_f(BlockDriverState *bs, int argc, char **argv)
offset += num;
nb_sectors -= num;
- } while (offset < bs->total_sectors);
+ } while (offset < total_sectors);
return 0;
}
@@ -1954,11 +1966,11 @@ static const cmdinfo_t map_cmd = {
.oneline = "prints the allocated areas of a file",
};
-static int break_f(BlockDriverState *bs, int argc, char **argv)
+static int break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
- ret = bdrv_debug_breakpoint(bs, argv[1], argv[2]);
+ ret = bdrv_debug_breakpoint(blk_bs(blk), argv[1], argv[2]);
if (ret < 0) {
printf("Could not set breakpoint: %s\n", strerror(-ret));
}
@@ -1966,11 +1978,11 @@ static int break_f(BlockDriverState *bs, int argc, char **argv)
return 0;
}
-static int remove_break_f(BlockDriverState *bs, int argc, char **argv)
+static int remove_break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
- ret = bdrv_debug_remove_breakpoint(bs, argv[1]);
+ ret = bdrv_debug_remove_breakpoint(blk_bs(blk), argv[1]);
if (ret < 0) {
printf("Could not remove breakpoint %s: %s\n", argv[1], strerror(-ret));
}
@@ -1997,11 +2009,11 @@ static const cmdinfo_t remove_break_cmd = {
.oneline = "remove a breakpoint by tag",
};
-static int resume_f(BlockDriverState *bs, int argc, char **argv)
+static int resume_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
- ret = bdrv_debug_resume(bs, argv[1]);
+ ret = bdrv_debug_resume(blk_bs(blk), argv[1]);
if (ret < 0) {
printf("Could not resume request: %s\n", strerror(-ret));
}
@@ -2018,10 +2030,10 @@ static const cmdinfo_t resume_cmd = {
.oneline = "resumes the request tagged as tag",
};
-static int wait_break_f(BlockDriverState *bs, int argc, char **argv)
+static int wait_break_f(BlockBackend *blk, int argc, char **argv)
{
- while (!bdrv_debug_is_suspended(bs, argv[1])) {
- aio_poll(bdrv_get_aio_context(bs), true);
+ while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
+ aio_poll(blk_get_aio_context(blk), true);
}
return 0;
@@ -2036,7 +2048,7 @@ static const cmdinfo_t wait_break_cmd = {
.oneline = "waits for the suspension of a request",
};
-static int abort_f(BlockDriverState *bs, int argc, char **argv)
+static int abort_f(BlockBackend *blk, int argc, char **argv)
{
abort();
}
@@ -2062,7 +2074,7 @@ static void sigraise_help(void)
"\n", SIGTERM);
}
-static int sigraise_f(BlockDriverState *bs, int argc, char **argv);
+static int sigraise_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t sigraise_cmd = {
.name = "sigraise",
@@ -2075,7 +2087,7 @@ static const cmdinfo_t sigraise_cmd = {
.help = sigraise_help,
};
-static int sigraise_f(BlockDriverState *bs, int argc, char **argv)
+static int sigraise_f(BlockBackend *blk, int argc, char **argv)
{
int sig = cvtnum(argv[1]);
if (sig < 0) {
@@ -2099,7 +2111,7 @@ static void sleep_cb(void *opaque)
*expired = true;
}
-static int sleep_f(BlockDriverState *bs, int argc, char **argv)
+static int sleep_f(BlockBackend *blk, int argc, char **argv)
{
char *endptr;
long ms;
@@ -2168,7 +2180,7 @@ static void help_all(void)
printf("\nUse 'help commandname' for extended help.\n");
}
-static int help_f(BlockDriverState *bs, int argc, char **argv)
+static int help_f(BlockBackend *blk, int argc, char **argv)
{
const cmdinfo_t *ct;
@@ -2198,7 +2210,7 @@ static const cmdinfo_t help_cmd = {
.oneline = "help for one or all commands",
};
-bool qemuio_command(BlockDriverState *bs, const char *cmd)
+bool qemuio_command(BlockBackend *blk, const char *cmd)
{
char *input;
const cmdinfo_t *ct;
@@ -2211,7 +2223,7 @@ bool qemuio_command(BlockDriverState *bs, const char *cmd)
if (c) {
ct = find_command(v[0]);
if (ct) {
- done = command(bs, ct, c, v);
+ done = command(blk, ct, c, v);
} else {
fprintf(stderr, "command \"%s\" not found\n", v[0]);
}
diff --git a/qemu-io.c b/qemu-io.c
index 0237ecb..bde97f8 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -36,7 +36,7 @@ static char **cmdline;
static ReadLineState *readline_state;
-static int close_f(BlockDriverState *bs, int argc, char **argv)
+static int close_f(BlockBackend *blk, int argc, char **argv)
{
blk_unref(qemuio_blk);
qemuio_blk = NULL;
@@ -90,7 +90,7 @@ static void open_help(void)
"\n");
}
-static int open_f(BlockDriverState *bs, int argc, char **argv);
+static int open_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t open_cmd = {
.name = "open",
@@ -114,7 +114,7 @@ static QemuOptsList empty_opts = {
},
};
-static int open_f(BlockDriverState *bs, int argc, char **argv)
+static int open_f(BlockBackend *blk, int argc, char **argv)
{
int flags = 0;
int readonly = 0;
@@ -164,7 +164,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
}
}
-static int quit_f(BlockDriverState *bs, int argc, char **argv)
+static int quit_f(BlockBackend *blk, int argc, char **argv)
{
return 1;
}
@@ -301,7 +301,7 @@ static void command_loop(void)
char *input;
for (i = 0; !done && i < ncmdline; i++) {
- done = qemuio_command(qemuio_bs, cmdline[i]);
+ done = qemuio_command(qemuio_blk, cmdline[i]);
}
if (cmdline) {
g_free(cmdline);
@@ -326,7 +326,7 @@ static void command_loop(void)
if (input == NULL) {
break;
}
- done = qemuio_command(qemuio_bs, input);
+ done = qemuio_command(qemuio_blk, input);
g_free(input);
prompted = 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (10 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 17:15 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS Max Reitz
` (3 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
BlockBackend is used as the interface between the block layer and guest
devices. It should therefore assure that all requests are clamped to the
image size.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 543edaa..96a5bc6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -31,6 +31,16 @@ struct BlockBackend {
void *dev_opaque;
};
+typedef struct BlockBackendAIOCB {
+ BlockAIOCB common;
+ QEMUBH *bh;
+ int ret;
+} BlockBackendAIOCB;
+
+static const AIOCBInfo block_backend_aiocb_info = {
+ .aiocb_size = sizeof(BlockBackendAIOCB),
+};
+
static void drive_info_del(DriveInfo *dinfo);
/* All the BlockBackends (except for hidden ones) */
@@ -428,39 +438,137 @@ void blk_iostatus_enable(BlockBackend *blk)
bdrv_iostatus_enable(blk->bs);
}
+static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
+ size_t size)
+{
+ int64_t len;
+
+ if (size > INT_MAX) {
+ return -EIO;
+ }
+
+ if (!blk_is_inserted(blk)) {
+ return -ENOMEDIUM;
+ }
+
+ len = blk_getlength(blk);
+ if (len < 0) {
+ return len;
+ }
+
+ if (offset < 0) {
+ return -EIO;
+ }
+
+ if (offset > len || len - offset < size) {
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int blk_check_request(BlockBackend *blk, int64_t sector_num,
+ int nb_sectors)
+{
+ if (sector_num < 0 || sector_num > INT64_MAX / BDRV_SECTOR_SIZE) {
+ return -EIO;
+ }
+
+ if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
+ return -EIO;
+ }
+
+ return blk_check_byte_request(blk, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE);
+}
+
int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_read(blk->bs, sector_num, buf, nb_sectors);
}
int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_read_unthrottled(blk->bs, sector_num, buf, nb_sectors);
}
int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_write(blk->bs, sector_num, buf, nb_sectors);
}
+static void error_callback_bh(void *opaque)
+{
+ struct BlockBackendAIOCB *acb = opaque;
+ qemu_bh_delete(acb->bh);
+ acb->common.cb(acb->common.opaque, acb->ret);
+ qemu_aio_unref(acb);
+}
+
+static BlockAIOCB *abort_aio_request(BlockBackend *blk, BlockCompletionFunc *cb,
+ void *opaque, int ret)
+{
+ struct BlockBackendAIOCB *acb;
+ QEMUBH *bh;
+
+ acb = blk_aio_get(&block_backend_aiocb_info, blk, cb, opaque);
+ acb->ret = ret;
+
+ bh = aio_bh_new(blk_get_aio_context(blk), error_callback_bh, acb);
+ acb->bh = bh;
+ qemu_bh_schedule(bh);
+
+ return &acb->common;
+}
+
BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
int nb_sectors, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return abort_aio_request(blk, cb, opaque, ret);
+ }
+
return bdrv_aio_write_zeroes(blk->bs, sector_num, nb_sectors, flags,
cb, opaque);
}
int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
{
+ int ret = blk_check_byte_request(blk, offset, count);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_pread(blk->bs, offset, buf, count);
}
int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count)
{
+ int ret = blk_check_byte_request(blk, offset, count);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_pwrite(blk->bs, offset, buf, count);
}
@@ -478,6 +586,11 @@ BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return abort_aio_request(blk, cb, opaque, ret);
+ }
+
return bdrv_aio_readv(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
}
@@ -485,6 +598,11 @@ BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return abort_aio_request(blk, cb, opaque, ret);
+ }
+
return bdrv_aio_writev(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
}
@@ -498,6 +616,11 @@ BlockAIOCB *blk_aio_discard(BlockBackend *blk,
int64_t sector_num, int nb_sectors,
BlockCompletionFunc *cb, void *opaque)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return abort_aio_request(blk, cb, opaque, ret);
+ }
+
return bdrv_aio_discard(blk->bs, sector_num, nb_sectors, cb, opaque);
}
@@ -513,6 +636,15 @@ void blk_aio_cancel_async(BlockAIOCB *acb)
int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs)
{
+ int i, ret;
+
+ for (i = 0; i < num_reqs; i++) {
+ ret = blk_check_request(blk, reqs[i].sector, reqs[i].nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
return bdrv_aio_multiwrite(blk->bs, reqs, num_reqs);
}
@@ -529,6 +661,11 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_co_discard(blk->bs, sector_num, nb_sectors);
}
@@ -701,12 +838,22 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t sector_num,
int nb_sectors, BdrvRequestFlags flags)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_co_write_zeroes(blk->bs, sector_num, nb_sectors, flags);
}
int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
const uint8_t *buf, int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_write_compressed(blk->bs, sector_num, buf, nb_sectors);
}
@@ -717,6 +864,11 @@ int blk_truncate(BlockBackend *blk, int64_t offset)
int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
{
+ int ret = blk_check_request(blk, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+
return bdrv_discard(blk->bs, sector_num, nb_sectors);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (11 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 17:29 ` Eric Blake
2015-02-02 19:46 ` Kevin Wolf
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value Max Reitz
` (2 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Now that request clamping is done in the BlockBackend, the "growable"
field can be removed from the BlockDriverState. All BDSs are now treated
as being "growable" (that is, they are allowed to grow; they are not
necessarily actually able to).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 24 +++++-------------------
block/qcow2.c | 6 ------
block/raw-posix.c | 2 +-
block/raw-win32.c | 2 +-
block/sheepdog.c | 2 +-
include/block/block_int.h | 3 ---
6 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/block.c b/block.c
index d45e4dd..356a857 100644
--- a/block.c
+++ b/block.c
@@ -970,7 +970,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
bs->zero_beyond_eof = true;
open_flags = bdrv_open_flags(bs, flags);
bs->read_only = !(open_flags & BDRV_O_RDWR);
- bs->growable = !!(flags & BDRV_O_PROTOCOL);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
error_setg(errp,
@@ -1885,7 +1884,6 @@ void bdrv_close(BlockDriverState *bs)
bs->encrypted = 0;
bs->valid_key = 0;
bs->sg = 0;
- bs->growable = 0;
bs->zero_beyond_eof = false;
QDECREF(bs->options);
bs->options = NULL;
@@ -2645,25 +2643,13 @@ exit:
static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
size_t size)
{
- int64_t len;
-
if (size > INT_MAX) {
return -EIO;
}
- if (!bdrv_is_inserted(bs))
+ if (!bdrv_is_inserted(bs)) {
return -ENOMEDIUM;
-
- if (bs->growable)
- return 0;
-
- len = bdrv_getlength(bs);
-
- if (offset < 0)
- return -EIO;
-
- if ((offset > len) || (len - offset < size))
- return -EIO;
+ }
return 0;
}
@@ -3045,10 +3031,10 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
}
/* Forward the request to the BlockDriver */
- if (!(bs->zero_beyond_eof && bs->growable)) {
+ if (!bs->zero_beyond_eof) {
ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
} else {
- /* Read zeros after EOF of growable BDSes */
+ /* Read zeros after EOF */
int64_t total_sectors, max_nb_sectors;
total_sectors = bdrv_nb_sectors(bs);
@@ -3328,7 +3314,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
block_acct_highest_sector(&bs->stats, sector_num, nb_sectors);
- if (bs->growable && ret >= 0) {
+ if (ret >= 0) {
bs->total_sectors = MAX(bs->total_sectors, sector_num + nb_sectors);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index dbaf016..2b31dac 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2521,15 +2521,12 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
{
BDRVQcowState *s = bs->opaque;
int64_t total_sectors = bs->total_sectors;
- int growable = bs->growable;
bool zero_beyond_eof = bs->zero_beyond_eof;
int ret;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
- bs->growable = 1;
bs->zero_beyond_eof = false;
ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
- bs->growable = growable;
bs->zero_beyond_eof = zero_beyond_eof;
/* bdrv_co_do_writev will have increased the total_sectors value to include
@@ -2544,15 +2541,12 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int64_t pos, int size)
{
BDRVQcowState *s = bs->opaque;
- int growable = bs->growable;
bool zero_beyond_eof = bs->zero_beyond_eof;
int ret;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
- bs->growable = 1;
bs->zero_beyond_eof = false;
ret = bdrv_pread(bs, qcow2_vm_state_offset(s) + pos, buf, size);
- bs->growable = growable;
bs->zero_beyond_eof = zero_beyond_eof;
return ret;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..5debc72 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -984,7 +984,7 @@ static int aio_worker(void *arg)
switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) {
case QEMU_AIO_READ:
ret = handle_aiocb_rw(aiocb);
- if (ret >= 0 && ret < aiocb->aio_nbytes && aiocb->bs->growable) {
+ if (ret >= 0 && ret < aiocb->aio_nbytes) {
iov_memset(aiocb->aio_iov, aiocb->aio_niov, ret,
0, aiocb->aio_nbytes - ret);
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 06243d7..dae5d2f 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -101,7 +101,7 @@ static int aio_worker(void *arg)
switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) {
case QEMU_AIO_READ:
count = handle_aiocb_rw(aiocb);
- if (count < aiocb->aio_nbytes && aiocb->bs->growable) {
+ if (count < aiocb->aio_nbytes) {
/* A short read means that we have reached EOF. Pad the buffer
* with zeros for bytes after EOF. */
iov_memset(aiocb->aio_iov, aiocb->aio_niov, count,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index be3176f..b84e215 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2117,7 +2117,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
BDRVSheepdogState *s = bs->opaque;
- if (bs->growable && offset > s->inode.vdi_size) {
+ if (offset > s->inode.vdi_size) {
ret = sd_truncate(bs, offset);
if (ret < 0) {
return ret;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7ad1950..b340e7e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -369,9 +369,6 @@ struct BlockDriverState {
/* I/O Limits */
BlockLimits bl;
- /* Whether the disk can expand beyond total_sectors */
- int growable;
-
/* Whether produces zeros when read beyond eof */
bool zero_beyond_eof;
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (12 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS Max Reitz
@ 2015-01-26 15:00 ` Max Reitz
2015-01-27 17:36 ` Eric Blake
2015-01-26 15:49 ` [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Stefano Stabellini
2015-02-02 19:50 ` Kevin Wolf
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-26 15:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Markus Armbruster, Stefan Hajnoczi,
Stefano Stabellini
Do not throw away the value returned by bdrv_check_request() and
bdrv_check_byte_request().
Fix up some coding style issues in the proximity of the affected hunks.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index 356a857..4a5f8fc 100644
--- a/block.c
+++ b/block.c
@@ -3096,8 +3096,10 @@ static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
if (!drv) {
return -ENOMEDIUM;
}
- if (bdrv_check_byte_request(bs, offset, bytes)) {
- return -EIO;
+
+ ret = bdrv_check_byte_request(bs, offset, bytes);
+ if (ret < 0) {
+ return ret;
}
if (bs->copy_on_read) {
@@ -3343,8 +3345,10 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
if (bs->read_only) {
return -EACCES;
}
- if (bdrv_check_byte_request(bs, offset, bytes)) {
- return -EIO;
+
+ ret = bdrv_check_byte_request(bs, offset, bytes);
+ if (ret < 0) {
+ return ret;
}
/* throttling disk I/O */
@@ -4168,12 +4172,18 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors)
{
BlockDriver *drv = bs->drv;
- if (!drv)
+ int ret;
+
+ if (!drv) {
return -ENOMEDIUM;
- if (!drv->bdrv_write_compressed)
+ }
+ if (!drv->bdrv_write_compressed) {
return -ENOTSUP;
- if (bdrv_check_request(bs, sector_num, nb_sectors))
- return -EIO;
+ }
+ ret = bdrv_check_request(bs, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
assert(QLIST_EMPTY(&bs->dirty_bitmaps));
@@ -5091,12 +5101,15 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque)
int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
- int max_discard;
+ int max_discard, ret;
if (!bs->drv) {
return -ENOMEDIUM;
- } else if (bdrv_check_request(bs, sector_num, nb_sectors)) {
- return -EIO;
+ }
+
+ ret = bdrv_check_request(bs, sector_num, nb_sectors);
+ if (ret < 0) {
+ return ret;
} else if (bs->read_only) {
return -EROFS;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (13 preceding siblings ...)
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value Max Reitz
@ 2015-01-26 15:49 ` Stefano Stabellini
2015-02-02 19:50 ` Kevin Wolf
15 siblings, 0 replies; 49+ messages in thread
From: Stefano Stabellini @ 2015-01-26 15:49 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Stefano Stabellini, qemu-devel, Stefan Hajnoczi,
Markus Armbruster
On Mon, 26 Jan 2015, Max Reitz wrote:
> This series removes the "growable" field from the BlockDriverState
> object. Its use was to clamp guest requests against the limits of the
> BDS; however, this can now be done more easily by moving those checks
> into the BlockBackend functions.
>
> In a future series, "growable" may be reintroduced (maybe with a
> different name); it will then signify whether a BDS is able to grow (in
> contrast to the current "growable", which signifies whether it is
> allowed to). Maybe I will add it to the BlockDriver instead of the BDS,
> though.
>
> To be able to remove that field, qemu-io needs to be converted to
> BlockBackend, which is done by this series as well. While working on
> that I decided to convert blk_new_with_bs()+bdrv_open() to
> blk_new_open(). I was skeptical about that decision at first, but it
> seems good now that I was able to replace nearly every blk_new_with_bs()
> call by blk_new_open(). In a future series I may try to convert some
> remaining bdrv_open() calls to blk_new_open() as well. (And, in fact, in
> a future series I *will* replace the last remaining blk_new_with_bs()
> outside of blk_new_open() by blk_new_open().)
>
> Finally, the question needs to be asked: If, after this series, every
> BDS is allowed to grow, are there any users which do not use BB, but
> should still be disallowed from reading/writing beyond a BDS's limits?
> The only users I could see were the block jobs. Some of them should
> indeed be converted to BB; but none of them takes a user-supplied offset
> or size, all work on the full BDS (or only on parts which have been
> modified, etc.). Therefore, it is by design impossible for them to
> exceed the BDS's limits, which makes making all BDS's growable safe.
>
>
> v3:
> - Rebased (onto Stefan's branch rebased onto master)
> - Fixed patch 4 [Stefano]
Thanks! It builds correctly and seems to work OK.
> v2:
> - Rebased [Kevin]
> - Patch 2: Added a TODO comment about removing @filename and @flags from
> blk_new_open() when possible [Kevin]
>
>
> git-backport-diff against v2:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/14:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
> 002/14:[----] [--] 'block: Add blk_new_open()'
> 003/14:[----] [--] 'blockdev: Use blk_new_open() in blockdev_init()'
> 004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
> 005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
> 006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
> 007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
> 008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
> 009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
> 010/14:[----] [--] 'qemu-io: Remove "growable" option'
> 011/14:[----] [--] 'qemu-io: Use BlockBackend'
> 012/14:[----] [--] 'block: Clamp BlockBackend requests'
> 013/14:[----] [--] 'block: Remove "growable" from BDS'
> 014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
>
>
> git-backport-diff against v1:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/14:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
> 002/14:[0006] [FC] 'block: Add blk_new_open()'
> 003/14:[----] [-C] 'blockdev: Use blk_new_open() in blockdev_init()'
> 004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
> 005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
> 006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
> 007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
> 008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
> 009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
> 010/14:[0002] [FC] 'qemu-io: Remove "growable" option'
> 011/14:[----] [--] 'qemu-io: Use BlockBackend'
> 012/14:[----] [--] 'block: Clamp BlockBackend requests'
> 013/14:[----] [--] 'block: Remove "growable" from BDS'
> 014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
>
>
> Max Reitz (14):
> block: Lift some BDS functions to the BlockBackend
> block: Add blk_new_open()
> blockdev: Use blk_new_open() in blockdev_init()
> block/xen: Use blk_new_open() in blk_connect()
> qemu-img: Use blk_new_open() in img_open()
> qemu-img: Use blk_new_open() in img_rebase()
> qemu-img: Use BlockBackend as far as possible
> qemu-nbd: Use blk_new_open() in main()
> qemu-io: Use blk_new_open() in openfile()
> qemu-io: Remove "growable" option
> qemu-io: Use BlockBackend
> block: Clamp BlockBackend requests
> block: Remove "growable" from BDS
> block: Keep bdrv_check*_request()'s return value
>
> block.c | 59 +++++-----
> block/block-backend.c | 219 +++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 6 --
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 2 +-
> block/sheepdog.c | 2 +-
> blockdev.c | 92 ++++++++--------
> hmp.c | 9 +-
> hw/block/xen_disk.c | 28 +++--
> include/block/block_int.h | 3 -
> include/qemu-io.h | 4 +-
> include/sysemu/block-backend.h | 12 +++
> qemu-img.c | 171 ++++++++++++++---------------
> qemu-io-cmds.c | 238 ++++++++++++++++++++++-------------------
> qemu-io.c | 58 ++++------
> qemu-nbd.c | 25 ++---
> tests/qemu-iotests/016 | 73 -------------
> tests/qemu-iotests/016.out | 23 ----
> tests/qemu-iotests/051.out | 60 +++++------
> tests/qemu-iotests/087.out | 8 +-
> tests/qemu-iotests/group | 1 -
> 21 files changed, 592 insertions(+), 503 deletions(-)
> delete mode 100755 tests/qemu-iotests/016
> delete mode 100644 tests/qemu-iotests/016.out
>
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend Max Reitz
@ 2015-01-26 21:48 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-26 21:48 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 694 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Create the blk_* counterparts for the following bdrv_* functions (which
> make sense to call on the BlockBackend level):
> - bdrv_co_write_zeroes()
> - bdrv_write_compressed()
> - bdrv_truncate()
> - bdrv_discard()
> - bdrv_load_vmstate()
> - bdrv_save_vmstate()
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/block-backend.c | 33 +++++++++++++++++++++++++++++++++
> include/sysemu/block-backend.h | 9 +++++++++
> 2 files changed, 42 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open() Max Reitz
@ 2015-01-26 21:56 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-26 21:56 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> blk_new_with_bs() creates a BlockBackend with an empty BlockDriverState
> attached to it. Empty BDSs are not nice, therefore add an alternative
> function which combines blk_new_with_bs() with bdrv_open().
>
> Note: In contrast to bdrv_open() which takes a BlockDriver parameter,
> blk_new_open() does not take such a parameter. This is because
> bdrv_open() opens a BlockDriverState, therefore it is naturally to be
s/naturally/natural/
> able to set the BlockDriver for that BDS. The fact that bdrv_open() can
> open more than a single BDS is merely some form of a byproduct.
>
> blk_new_open() on the other hand is intended to be used to create a
> whole tree of BlockDriverStates. Therefore, setting a single BlockDriver
> does not make much sense. Instead, the drivers to be used for each of
> the nodes must be configured through the "options" QDict; including the
> driver of the root BDS.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/block-backend.c | 34 ++++++++++++++++++++++++++++++++++
> include/sysemu/block-backend.h | 3 +++
> 2 files changed, 37 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init() Max Reitz
@ 2015-01-26 22:37 ` Eric Blake
2015-01-27 2:08 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Eric Blake @ 2015-01-26 22:37 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 2048 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Due to different error propagation, this breaks tests 051 and 087; fix
> their output.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> blockdev.c | 92 +++++++++++++++++++++-------------------------
> tests/qemu-iotests/051.out | 60 +++++++++++++++---------------
> tests/qemu-iotests/087.out | 8 ++--
> 3 files changed, 76 insertions(+), 84 deletions(-)
>
> Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: Cannot specify both 'driver' and 'format'
Is it possible to specify driver=qcow2,format=qcow2? Should it be?
Either way, are we testing the outcome of that? (that is, there is a
difference between two competing options, and two spellings of the same
option - I could go for either rejecting the duplication, or for
allowing it when the two are the same, whichever is easier, but would
like to make sure it is tested so we know if we change our minds later
whether we are risking a regression).
>
>
> === Specifying both an option and its legacy alias ===
> @@ -323,13 +323,13 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' a
> === Parsing protocol from file name ===
>
> Testing: -hda foo:bar
> -QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
> +QEMU_PROG: -hda foo:bar: Unknown protocol
Not the fault of this patch, but can this error message be improved?
Even 'Unknown protocol: foo' would read better.
All of the other shorter error messages still seem to read fine, and the
decrease in verbosity could be argued as a feature. So overall, I'm
fine with this patch.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect() Max Reitz
@ 2015-01-26 22:46 ` Eric Blake
2015-02-02 18:27 ` Kevin Wolf
1 sibling, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-26 22:46 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 371 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> hw/block/xen_disk.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open() Max Reitz
@ 2015-01-26 22:47 ` Eric Blake
2015-02-02 18:35 ` Kevin Wolf
1 sibling, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-26 22:47 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 353 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()
2015-01-26 22:37 ` Eric Blake
@ 2015-01-27 2:08 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-27 2:08 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
On 2015-01-26 at 17:37, Eric Blake wrote:
> On 01/26/2015 08:00 AM, Max Reitz wrote:
>> Due to different error propagation, this breaks tests 051 and 087; fix
>> their output.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> blockdev.c | 92 +++++++++++++++++++++-------------------------
>> tests/qemu-iotests/051.out | 60 +++++++++++++++---------------
>> tests/qemu-iotests/087.out | 8 ++--
>> 3 files changed, 76 insertions(+), 84 deletions(-)
>>
>> Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
>> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice
>> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: Cannot specify both 'driver' and 'format'
> Is it possible to specify driver=qcow2,format=qcow2? Should it be?
No, it isn't, and in my opinion it shouldn't be (just a personal
feeling, though).
> Either way, are we testing the outcome of that? (that is, there is a
> difference between two competing options, and two spellings of the same
> option - I could go for either rejecting the duplication, or for
> allowing it when the two are the same, whichever is easier, but would
> like to make sure it is tested so we know if we change our minds later
> whether we are risking a regression).
No, we aren't yet. Albeit not really related to this series, it is a
good point, so I'll probably just add a test case in v4.
>>
>>
>> === Specifying both an option and its legacy alias ===
>> @@ -323,13 +323,13 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' a
>> === Parsing protocol from file name ===
>>
>> Testing: -hda foo:bar
>> -QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
>> +QEMU_PROG: -hda foo:bar: Unknown protocol
> Not the fault of this patch, but can this error message be improved?
> Even 'Unknown protocol: foo' would read better.
Indeed, I'll fix it.
> All of the other shorter error messages still seem to read fine, and the
> decrease in verbosity could be argued as a feature. So overall, I'm
> fine with this patch.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Thank you!
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase() Max Reitz
@ 2015-01-27 3:05 ` Eric Blake
2015-01-27 15:01 ` Max Reitz
2015-02-02 19:00 ` Kevin Wolf
1 sibling, 1 reply; 49+ messages in thread
From: Eric Blake @ 2015-01-27 3:05 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 712 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 57 ++++++++++++++++++++++++---------------------------------
> 1 file changed, 24 insertions(+), 33 deletions(-)
>
> -
> /* For safe rebasing we need to compare old and new backing file */
> if (!unsafe) {
> char backing_name[PATH_MAX];
> + QDict *options = NULL;
> +
> + if (!unsafe && bs->backing_format[0] != '\0') {
Useless conditional ('!unsafe' is always true here).
With that fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible Max Reitz
@ 2015-01-27 3:38 ` Eric Blake
2015-01-27 15:07 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Eric Blake @ 2015-01-27 3:38 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 4851 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Although qemu-img already creates BlockBackends, it does not do accesses
> to the images through them. This patch converts all of the bdrv_* calls
> for which this is currently possible to blk_* calls. Most of the
> remaining calls will probably stay bdrv_* calls because they really do
> operate on the BDS level instead of the BB level.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 98 ++++++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 54 insertions(+), 44 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 0b23c87..8b4139e 100644
> --- a/qemu-img.c
> @@ -1130,22 +1130,26 @@ static int img_compare(int argc, char **argv)
> }
> bs2 = blk_bs(blk2);
>
> - buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
> - buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
> - total_sectors1 = bdrv_nb_sectors(bs1);
> + buf1 = blk_blockalign(blk1, IO_BUF_SIZE);
> + buf2 = blk_blockalign(blk2, IO_BUF_SIZE);
> + total_sectors1 = blk_getlength(blk1);
> if (total_sectors1 < 0) {
> error_report("Can't get size of %s: %s",
> filename1, strerror(-total_sectors1));
> ret = 4;
> goto out;
> }
> - total_sectors2 = bdrv_nb_sectors(bs2);
> + total_sectors2 = blk_getlength(blk2);
The naming feels awkward; your conversion is now using bytes while the
old code was using sectors, so 'total_sectors2' feels weird...
> if (total_sectors2 < 0) {
> error_report("Can't get size of %s: %s",
> filename2, strerror(-total_sectors2));
> ret = 4;
> goto out;
> }
> +
> + total_sectors1 /= BDRV_SECTOR_SIZE;
> + total_sectors2 /= BDRV_SECTOR_SIZE;
...at least you end up converting to sectors after all. But it makes me
wonder if you should have blk_nb_sectors(), and/or temporary
intermediate variables to avoid cross-unit confusion.
> @@ -1476,13 +1480,14 @@ static int img_convert(int argc, char **argv)
> goto out;
> }
> bs[bs_i] = blk_bs(blk[bs_i]);
> - bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]);
> + bs_sectors[bs_i] = blk_getlength(blk[bs_i]);
> if (bs_sectors[bs_i] < 0) {
> error_report("Could not get size of %s: %s",
> argv[optind + bs_i], strerror(-bs_sectors[bs_i]));
> ret = -1;
> goto out;
> }
> + bs_sectors[bs_i] /= BDRV_SECTOR_SIZE;
Another instance of the confusion.
> total_sectors += bs_sectors[bs_i];
> }
>
> @@ -1625,16 +1630,19 @@ static int img_convert(int argc, char **argv)
> out_bs->bl.discard_alignment))
> );
>
> - buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
> + buf = blk_blockalign(out_blk, bufsectors * BDRV_SECTOR_SIZE);
>
> if (skip_create) {
> - int64_t output_sectors = bdrv_nb_sectors(out_bs);
> + int64_t output_sectors = blk_getlength(out_blk);
> if (output_sectors < 0) {
> error_report("unable to get output image length: %s\n",
> strerror(-output_sectors));
> ret = -1;
> goto out;
> - } else if (output_sectors < total_sectors) {
> + }
> +
> + output_sectors /= BDRV_SECTOR_SIZE;
> + if (output_sectors < total_sectors) {
And another.
> @@ -2585,17 +2591,17 @@ static int img_rebase(int argc, char **argv)
> uint8_t * buf_new;
> float local_progress = 0;
>
> - buf_old = qemu_blockalign(bs, IO_BUF_SIZE);
> - buf_new = qemu_blockalign(bs, IO_BUF_SIZE);
> + buf_old = blk_blockalign(blk, IO_BUF_SIZE);
> + buf_new = blk_blockalign(blk, IO_BUF_SIZE);
>
> - num_sectors = bdrv_nb_sectors(bs);
> + num_sectors = blk_getlength(blk);
> if (num_sectors < 0) {
...
> - if (bs_new_backing) {
> - new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing);
> +
> + num_sectors /= BDRV_SECTOR_SIZE;
> + old_backing_num_sectors /= BDRV_SECTOR_SIZE;
and another.
I did not closely audit if there were any other conversions that should
have been made. Also, I suspect that a blk_nb_sectors() as a pre-req
patch would make this one feel cleaner if you respin and rebase. But if
we don't add blk_nb_sectors(), at least this version of the patch
appears to be clean with what it does, so you can consider this to be a
rather weak:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main() Max Reitz
@ 2015-01-27 4:59 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 4:59 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 358 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-nbd.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase()
2015-01-27 3:05 ` Eric Blake
@ 2015-01-27 15:01 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-27 15:01 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
On 2015-01-26 at 22:05, Eric Blake wrote:
> On 01/26/2015 08:00 AM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-img.c | 57 ++++++++++++++++++++++++---------------------------------
>> 1 file changed, 24 insertions(+), 33 deletions(-)
>>
>> -
>> /* For safe rebasing we need to compare old and new backing file */
>> if (!unsafe) {
>> char backing_name[PATH_MAX];
>> + QDict *options = NULL;
>> +
>> + if (!unsafe && bs->backing_format[0] != '\0') {
> Useless conditional ('!unsafe' is always true here).
Oh, right, thanks.
Max
> With that fixed,
> Reviewed-by: Eric Blake <eblake@redhat.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible
2015-01-27 3:38 ` Eric Blake
@ 2015-01-27 15:07 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-01-27 15:07 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
On 2015-01-26 at 22:38, Eric Blake wrote:
> On 01/26/2015 08:00 AM, Max Reitz wrote:
>> Although qemu-img already creates BlockBackends, it does not do accesses
>> to the images through them. This patch converts all of the bdrv_* calls
>> for which this is currently possible to blk_* calls. Most of the
>> remaining calls will probably stay bdrv_* calls because they really do
>> operate on the BDS level instead of the BB level.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-img.c | 98 ++++++++++++++++++++++++++++++++++----------------------------
>> 1 file changed, 54 insertions(+), 44 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 0b23c87..8b4139e 100644
>> --- a/qemu-img.c
>> @@ -1130,22 +1130,26 @@ static int img_compare(int argc, char **argv)
>> }
>> bs2 = blk_bs(blk2);
>>
>> - buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
>> - buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
>> - total_sectors1 = bdrv_nb_sectors(bs1);
>> + buf1 = blk_blockalign(blk1, IO_BUF_SIZE);
>> + buf2 = blk_blockalign(blk2, IO_BUF_SIZE);
>> + total_sectors1 = blk_getlength(blk1);
>> if (total_sectors1 < 0) {
>> error_report("Can't get size of %s: %s",
>> filename1, strerror(-total_sectors1));
>> ret = 4;
>> goto out;
>> }
>> - total_sectors2 = bdrv_nb_sectors(bs2);
>> + total_sectors2 = blk_getlength(blk2);
> The naming feels awkward; your conversion is now using bytes while the
> old code was using sectors, so 'total_sectors2' feels weird...
>
>> if (total_sectors2 < 0) {
>> error_report("Can't get size of %s: %s",
>> filename2, strerror(-total_sectors2));
>> ret = 4;
>> goto out;
>> }
>> +
>> + total_sectors1 /= BDRV_SECTOR_SIZE;
>> + total_sectors2 /= BDRV_SECTOR_SIZE;
> ...at least you end up converting to sectors after all. But it makes me
> wonder if you should have blk_nb_sectors(), and/or temporary
> intermediate variables to avoid cross-unit confusion.
>
>> @@ -1476,13 +1480,14 @@ static int img_convert(int argc, char **argv)
>> goto out;
>> }
>> bs[bs_i] = blk_bs(blk[bs_i]);
>> - bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]);
>> + bs_sectors[bs_i] = blk_getlength(blk[bs_i]);
>> if (bs_sectors[bs_i] < 0) {
>> error_report("Could not get size of %s: %s",
>> argv[optind + bs_i], strerror(-bs_sectors[bs_i]));
>> ret = -1;
>> goto out;
>> }
>> + bs_sectors[bs_i] /= BDRV_SECTOR_SIZE;
> Another instance of the confusion.
>
>> total_sectors += bs_sectors[bs_i];
>> }
>>
>> @@ -1625,16 +1630,19 @@ static int img_convert(int argc, char **argv)
>> out_bs->bl.discard_alignment))
>> );
>>
>> - buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
>> + buf = blk_blockalign(out_blk, bufsectors * BDRV_SECTOR_SIZE);
>>
>> if (skip_create) {
>> - int64_t output_sectors = bdrv_nb_sectors(out_bs);
>> + int64_t output_sectors = blk_getlength(out_blk);
>> if (output_sectors < 0) {
>> error_report("unable to get output image length: %s\n",
>> strerror(-output_sectors));
>> ret = -1;
>> goto out;
>> - } else if (output_sectors < total_sectors) {
>> + }
>> +
>> + output_sectors /= BDRV_SECTOR_SIZE;
>> + if (output_sectors < total_sectors) {
> And another.
>
>> @@ -2585,17 +2591,17 @@ static int img_rebase(int argc, char **argv)
>> uint8_t * buf_new;
>> float local_progress = 0;
>>
>> - buf_old = qemu_blockalign(bs, IO_BUF_SIZE);
>> - buf_new = qemu_blockalign(bs, IO_BUF_SIZE);
>> + buf_old = blk_blockalign(blk, IO_BUF_SIZE);
>> + buf_new = blk_blockalign(blk, IO_BUF_SIZE);
>>
>> - num_sectors = bdrv_nb_sectors(bs);
>> + num_sectors = blk_getlength(blk);
>> if (num_sectors < 0) {
> ...
>
>> - if (bs_new_backing) {
>> - new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing);
>> +
>> + num_sectors /= BDRV_SECTOR_SIZE;
>> + old_backing_num_sectors /= BDRV_SECTOR_SIZE;
> and another.
>
> I did not closely audit if there were any other conversions that should
> have been made. Also, I suspect that a blk_nb_sectors() as a pre-req
> patch would make this one feel cleaner if you respin and rebase.
The reason why I don't want to add blk_nb_sectors() is simply that we
want to convert all the code to using bytes at a later point anyway, so
I felt like it'd be a step backwards to introduce blk_nb_sectors().
However, I do see your point and intermediate variables probably don't
make this any nicer. Also, if we get to convert the code to bytes,
finding all instances of {blk,bdrv}_nb_sectors() will be one of the
easier tasks, so I'll just introduce blk_nb_sectors().
Max
> But if
> we don't add blk_nb_sectors(), at least this version of the patch
> appears to be clean with what it does, so you can consider this to be a
> rather weak:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile() Max Reitz
@ 2015-01-27 16:23 ` Eric Blake
2015-02-02 19:34 ` Kevin Wolf
1 sibling, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 16:23 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-io.c | 31 ++++++++++++-------------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option Max Reitz
@ 2015-01-27 16:59 ` Eric Blake
2015-01-27 17:04 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Eric Blake @ 2015-01-27 16:59 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 896 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Remove "growable" option from the "open" command and from the qemu-io
> command line. qemu-io is about to be converted to BlockBackend which
> will make sure that no request exceeds the image size, so the only way
> to keep "growable" would be to use BlockBackend if it is not given and
> to directly access the BDS if it is.
>
> qemu-io is a debugging tool, therefore removing a rarely used option
> will have only a very small impact, if any. There was only one
> qemu-iotest which used the option; since it is not critical, this patch
> just removes it.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
Do we want to ever reuse the test number that you are deleting?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-27 16:59 ` Eric Blake
@ 2015-01-27 17:04 ` Max Reitz
2015-01-27 17:10 ` Eric Blake
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-27 17:04 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
On 2015-01-27 at 11:59, Eric Blake wrote:
> On 01/26/2015 08:00 AM, Max Reitz wrote:
>> Remove "growable" option from the "open" command and from the qemu-io
>> command line. qemu-io is about to be converted to BlockBackend which
>> will make sure that no request exceeds the image size, so the only way
>> to keep "growable" would be to use BlockBackend if it is not given and
>> to directly access the BDS if it is.
>>
>> qemu-io is a debugging tool, therefore removing a rarely used option
>> will have only a very small impact, if any. There was only one
>> qemu-iotest which used the option; since it is not critical, this patch
>> just removes it.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Do we want to ever reuse the test number that you are deleting?
Good question, I think I have talked about that with Kevin before. It
would not hurt too much if we were to accidentally reuse the test case
number, most certainly not here in upstream.
However, for all downstream versions of qemu, this might make adding the
new test 16 difficult; but certainly not impossible (if someone is
affected by this issue, he/she can just use 999 or something). So we may
want to keep in mind not to reuse number 16, but if someone does, so be it.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend Max Reitz
@ 2015-01-27 17:08 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 17:08 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> qemu-io should behave like a guest, therefore it should use BlockBackend
> to access the block layer.
>
> There are a couple of places where that is infeasible: First, the
> bdrv_debug_* functions could theoretically be mirrored in the
> BlockBackend, but since these are functions internal to the block layer,
> they should not be visible externally (qemu-io as a test tool is excempt
s/excempt/exempt/
> from this).
>
> Second, bdrv_get_info() and bdrv_get_specific_info() work on a single
> BDS alone, therefore they should stay BDS-specific.
>
> Third, bdrv_is_allocated() mainly works on a single BDS as well. Some
> data may be passed through from the BDS's file (if sectors which are
> apparently allocated in the file are not really allocated there but just
> zero).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> -static int map_f(BlockDriverState *bs, int argc, char **argv)
> +static int map_f(BlockBackend *blk, int argc, char **argv)
> {
> int64_t offset;
> - int64_t nb_sectors;
> + int64_t nb_sectors, total_sectors;
> char s1[64];
> int64_t num;
> int ret;
> const char *retstr;
>
> offset = 0;
> - nb_sectors = bs->total_sectors;
> + total_sectors = blk_getlength(blk);
> + if (total_sectors < 0) {
> + error_report("Failed to query image length: %s",
> + strerror(-total_sectors));
> + return 0;
> + }
> +
> + total_sectors /= BDRV_SECTOR_SIZE;
Another spot that would benefit from a (possibly temporary)
bdrv_nb_sectors().
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-27 17:04 ` Max Reitz
@ 2015-01-27 17:10 ` Eric Blake
2015-01-27 17:11 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Eric Blake @ 2015-01-27 17:10 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
On 01/27/2015 10:04 AM, Max Reitz wrote:
> On 2015-01-27 at 11:59, Eric Blake wrote:
>> On 01/26/2015 08:00 AM, Max Reitz wrote:
>>> Remove "growable" option from the "open" command and from the qemu-io
>>> command line. qemu-io is about to be converted to BlockBackend which
>>> will make sure that no request exceeds the image size, so the only way
>>> to keep "growable" would be to use BlockBackend if it is not given and
>>> to directly access the BDS if it is.
>>>
>>> qemu-io is a debugging tool, therefore removing a rarely used option
>>> will have only a very small impact, if any. There was only one
>>> qemu-iotest which used the option; since it is not critical, this patch
>>> just removes it.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> Do we want to ever reuse the test number that you are deleting?
>
> Good question, I think I have talked about that with Kevin before. It
> would not hurt too much if we were to accidentally reuse the test case
> number, most certainly not here in upstream.
>
> However, for all downstream versions of qemu, this might make adding the
> new test 16 difficult; but certainly not impossible (if someone is
> affected by this issue, he/she can just use 999 or something). So we may
> want to keep in mind not to reuse number 16, but if someone does, so be it.
Is it worth a placeholder file that has a comment mentioning that the
test number is intentionally reserved (and if someone attempts to run,
always passes)?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-27 17:10 ` Eric Blake
@ 2015-01-27 17:11 ` Max Reitz
2015-02-02 19:36 ` Kevin Wolf
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2015-01-27 17:11 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
On 2015-01-27 at 12:10, Eric Blake wrote:
> On 01/27/2015 10:04 AM, Max Reitz wrote:
>> On 2015-01-27 at 11:59, Eric Blake wrote:
>>> On 01/26/2015 08:00 AM, Max Reitz wrote:
>>>> Remove "growable" option from the "open" command and from the qemu-io
>>>> command line. qemu-io is about to be converted to BlockBackend which
>>>> will make sure that no request exceeds the image size, so the only way
>>>> to keep "growable" would be to use BlockBackend if it is not given and
>>>> to directly access the BDS if it is.
>>>>
>>>> qemu-io is a debugging tool, therefore removing a rarely used option
>>>> will have only a very small impact, if any. There was only one
>>>> qemu-iotest which used the option; since it is not critical, this patch
>>>> just removes it.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>
>>> Do we want to ever reuse the test number that you are deleting?
>> Good question, I think I have talked about that with Kevin before. It
>> would not hurt too much if we were to accidentally reuse the test case
>> number, most certainly not here in upstream.
>>
>> However, for all downstream versions of qemu, this might make adding the
>> new test 16 difficult; but certainly not impossible (if someone is
>> affected by this issue, he/she can just use 999 or something). So we may
>> want to keep in mind not to reuse number 16, but if someone does, so be it.
> Is it worth a placeholder file that has a comment mentioning that the
> test number is intentionally reserved (and if someone attempts to run,
> always passes)?
Seems good to me. It's a minor effort now and may avert some hassle later.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests Max Reitz
@ 2015-01-27 17:15 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 17:15 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> BlockBackend is used as the interface between the block layer and guest
> devices. It should therefore assure that all requests are clamped to the
> image size.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/block-backend.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS Max Reitz
@ 2015-01-27 17:29 ` Eric Blake
2015-02-02 19:46 ` Kevin Wolf
1 sibling, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 17:29 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Now that request clamping is done in the BlockBackend, the "growable"
> field can be removed from the BlockDriverState. All BDSs are now treated
> as being "growable" (that is, they are allowed to grow; they are not
> necessarily actually able to).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> +++ b/include/block/block_int.h
> @@ -369,9 +369,6 @@ struct BlockDriverState {
> /* I/O Limits */
> BlockLimits bl;
>
> - /* Whether the disk can expand beyond total_sectors */
> - int growable;
> -
Yay - getting rid of an 'int' that was only used to hold 'bool' values :)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value Max Reitz
@ 2015-01-27 17:36 ` Eric Blake
0 siblings, 0 replies; 49+ messages in thread
From: Eric Blake @ 2015-01-27 17:36 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
On 01/26/2015 08:00 AM, Max Reitz wrote:
> Do not throw away the value returned by bdrv_check_request() and
> bdrv_check_byte_request().
>
> Fix up some coding style issues in the proximity of the affected hunks.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect() Max Reitz
2015-01-26 22:46 ` Eric Blake
@ 2015-02-02 18:27 ` Kevin Wolf
2015-02-02 19:41 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 18:27 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> hw/block/xen_disk.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 21842a0..1b0257c 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -40,6 +40,8 @@
> #include "xen_blkif.h"
> #include "sysemu/blockdev.h"
> #include "sysemu/block-backend.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qstring.h"
>
> /* ------------------------------------------------------------- */
>
> @@ -897,30 +899,24 @@ static int blk_connect(struct XenDevice *xendev)
> blkdev->dinfo = drive_get(IF_XEN, 0, index);
> if (!blkdev->dinfo) {
> Error *local_err = NULL;
> - BlockBackend *blk;
> - BlockDriver *drv;
> - BlockDriverState *bs;
> + QDict *options = NULL;
>
> - /* setup via xenbus -> create new block driver instance */
> - xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
> - blk = blk_new_with_bs(blkdev->dev, NULL);
> - if (!blk) {
> - return -1;
> + if (strcmp(blkdev->fileproto, "<unset>")) {
xen_disk's usage of the string "<unset>" to mark configurations where no
driver is specific is quite ugly. I think it's possible for users to
pass this string as the driver name, and we would end up probing the
driver instead of returning an error.
Which was actually how any invalid driver name was handled before this
patch: bdrv_find_whitelisted_format() would return NULL, and instead of
erroring out, NULL would be passed to bdrv_open(), which probed the
driver then.
This patch improves the situation: Now any value that is not "<unset>"
is passed as the "driver" option, so invalid drivers will produce an
error now. Should be mentioned in the commit message.
However, if the user passes "<unset>", that still means probing. Ugly.
Not sure why blkdev->fileproto == NULL wasn't used to specify that no
driver was given. We could change that in a patch before this one if we
wanted to clean it up. Or we could just feel reassured that xen_disk is
horrible code, that this patch already fixes most of it and leave it
alone.
> + options = qdict_new();
> + qdict_put_obj(options, "driver",
> + QOBJECT(qstring_from_str(blkdev->fileproto)));
> }
> - blkdev->blk = blk;
>
> - bs = blk_bs(blk);
> - drv = bdrv_find_whitelisted_format(blkdev->fileproto, readonly);
> - if (bdrv_open(&bs, blkdev->filename, NULL, NULL, qflags,
> - drv, &local_err) != 0) {
> + /* setup via xenbus -> create new block driver instance */
> + xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
> + blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
> + qflags, &local_err);
> + if (!blkdev->blk) {
> xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
> error_get_pretty(local_err));
> error_free(local_err);
> - blk_unref(blk);
> - blkdev->blk = NULL;
> return -1;
> }
> - assert(bs == blk_bs(blk));
> } else {
> /* setup via qemu cmdline -> already setup for us */
> xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open() Max Reitz
2015-01-26 22:47 ` Eric Blake
@ 2015-02-02 18:35 ` Kevin Wolf
2015-02-02 19:42 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 18:35 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 4e9a7f5..be1953d 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -291,32 +291,24 @@ static BlockBackend *img_open(const char *id, const char *filename,
> {
> BlockBackend *blk;
> BlockDriverState *bs;
> - BlockDriver *drv;
> char password[256];
> Error *local_err = NULL;
> - int ret;
> -
> - blk = blk_new_with_bs(id, &error_abort);
> - bs = blk_bs(blk);
> + QDict *options = NULL;
>
> if (fmt) {
> - drv = bdrv_find_format(fmt);
> - if (!drv) {
> - error_report("Unknown file format '%s'", fmt);
> - goto fail;
> - }
> - } else {
> - drv = NULL;
> + options = qdict_new();
> + qdict_put_obj(options, "driver", QOBJECT(qstring_from_str(fmt)));
I'm only noticing it here, though you did the same in the previous
patches. This can be written shorter as:
qdict_put(options, "driver", qstring_from_str(fmt));
> }
>
> - ret = bdrv_open(&bs, filename, NULL, NULL, flags, drv, &local_err);
> - if (ret < 0) {
> + blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
> + if (!blk) {
> error_report("Could not open '%s': %s", filename,
> error_get_pretty(local_err));
> error_free(local_err);
> goto fail;
> }
>
> + bs = blk_bs(blk);
> if (bdrv_is_encrypted(bs) && require_io) {
> qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
> if (read_password(password, sizeof(password)) < 0) {
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase() Max Reitz
2015-01-27 3:05 ` Eric Blake
@ 2015-02-02 19:00 ` Kevin Wolf
2015-02-02 19:47 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 19:00 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 57 ++++++++++++++++++++++++---------------------------------
> 1 file changed, 24 insertions(+), 33 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index be1953d..0b23c87 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2430,7 +2430,6 @@ static int img_rebase(int argc, char **argv)
> {
> BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
> BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL;
> - BlockDriver *old_backing_drv, *new_backing_drv;
> char *filename;
> const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
> int c, flags, src_flags, ret;
> @@ -2524,54 +2523,46 @@ static int img_rebase(int argc, char **argv)
> }
> bs = blk_bs(blk);
>
> - /* Find the right drivers for the backing files */
> - old_backing_drv = NULL;
> - new_backing_drv = NULL;
> -
> - if (!unsafe && bs->backing_format[0] != '\0') {
> - old_backing_drv = bdrv_find_format(bs->backing_format);
> - if (old_backing_drv == NULL) {
> - error_report("Invalid format name: '%s'", bs->backing_format);
> - ret = -1;
> - goto out;
> - }
> - }
> -
> - if (out_basefmt != NULL) {
> - new_backing_drv = bdrv_find_format(out_basefmt);
> - if (new_backing_drv == NULL) {
> - error_report("Invalid format name: '%s'", out_basefmt);
> - ret = -1;
> - goto out;
> - }
> - }
You're removing the validity check of the new backing file format:
[master] $ ./qemu-img rebase -u -b /tmp/backing.qcow2 -F foobar /tmp/test.qcow2
qemu-img: Invalid format name: 'foobar'
[master] $
[growable-v3] $ ./qemu-img rebase -u -b /tmp/backing.qcow2 -F foobar /tmp/test.qcow2
[growable-v3] $
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile()
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile() Max Reitz
2015-01-27 16:23 ` Eric Blake
@ 2015-02-02 19:34 ` Kevin Wolf
2015-02-02 19:51 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 19:34 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-io.c | 31 ++++++++++++-------------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/qemu-io.c b/qemu-io.c
> index 91a445a..81f8f64 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -39,7 +39,6 @@ static ReadLineState *readline_state;
> static int close_f(BlockDriverState *bs, int argc, char **argv)
> {
> blk_unref(qemuio_blk);
> - qemuio_bs = NULL;
> qemuio_blk = NULL;
> return 0;
> }
This doesn't seem to be correct at this point in the series. qemuio_bs
is still passed to qemuio_command(), which checks whether non-global
commands are allowed to run using init_check_command().
At the end of the series, please remove qemuio_bs altogether. This
version of the series leaves it around without any reader.
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-01-27 17:11 ` Max Reitz
@ 2015-02-02 19:36 ` Kevin Wolf
2015-02-02 19:52 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 19:36 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 27.01.2015 um 18:11 hat Max Reitz geschrieben:
> On 2015-01-27 at 12:10, Eric Blake wrote:
> >On 01/27/2015 10:04 AM, Max Reitz wrote:
> >>On 2015-01-27 at 11:59, Eric Blake wrote:
> >>>On 01/26/2015 08:00 AM, Max Reitz wrote:
> >>>>Remove "growable" option from the "open" command and from the qemu-io
> >>>>command line. qemu-io is about to be converted to BlockBackend which
> >>>>will make sure that no request exceeds the image size, so the only way
> >>>>to keep "growable" would be to use BlockBackend if it is not given and
> >>>>to directly access the BDS if it is.
> >>>>
> >>>>qemu-io is a debugging tool, therefore removing a rarely used option
> >>>>will have only a very small impact, if any. There was only one
> >>>>qemu-iotest which used the option; since it is not critical, this patch
> >>>>just removes it.
> >>>>
> >>>>Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>>>---
> >>>Reviewed-by: Eric Blake <eblake@redhat.com>
> >>>
> >>>Do we want to ever reuse the test number that you are deleting?
> >>Good question, I think I have talked about that with Kevin before. It
> >>would not hurt too much if we were to accidentally reuse the test case
> >>number, most certainly not here in upstream.
> >>
> >>However, for all downstream versions of qemu, this might make adding the
> >>new test 16 difficult; but certainly not impossible (if someone is
> >>affected by this issue, he/she can just use 999 or something). So we may
> >>want to keep in mind not to reuse number 16, but if someone does, so be it.
> >Is it worth a placeholder file that has a comment mentioning that the
> >test number is intentionally reserved (and if someone attempts to run,
> >always passes)?
>
> Seems good to me. It's a minor effort now and may avert some hassle later.
How about just keeping a comment line in group?
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect()
2015-02-02 18:27 ` Kevin Wolf
@ 2015-02-02 19:41 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:41 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 13:27, Kevin Wolf wrote:
> Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> hw/block/xen_disk.c | 28 ++++++++++++----------------
>> 1 file changed, 12 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
>> index 21842a0..1b0257c 100644
>> --- a/hw/block/xen_disk.c
>> +++ b/hw/block/xen_disk.c
>> @@ -40,6 +40,8 @@
>> #include "xen_blkif.h"
>> #include "sysemu/blockdev.h"
>> #include "sysemu/block-backend.h"
>> +#include "qapi/qmp/qdict.h"
>> +#include "qapi/qmp/qstring.h"
>>
>> /* ------------------------------------------------------------- */
>>
>> @@ -897,30 +899,24 @@ static int blk_connect(struct XenDevice *xendev)
>> blkdev->dinfo = drive_get(IF_XEN, 0, index);
>> if (!blkdev->dinfo) {
>> Error *local_err = NULL;
>> - BlockBackend *blk;
>> - BlockDriver *drv;
>> - BlockDriverState *bs;
>> + QDict *options = NULL;
>>
>> - /* setup via xenbus -> create new block driver instance */
>> - xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
>> - blk = blk_new_with_bs(blkdev->dev, NULL);
>> - if (!blk) {
>> - return -1;
>> + if (strcmp(blkdev->fileproto, "<unset>")) {
> xen_disk's usage of the string "<unset>" to mark configurations where no
> driver is specific is quite ugly. I think it's possible for users to
> pass this string as the driver name, and we would end up probing the
> driver instead of returning an error.
>
> Which was actually how any invalid driver name was handled before this
> patch: bdrv_find_whitelisted_format() would return NULL, and instead of
> erroring out, NULL would be passed to bdrv_open(), which probed the
> driver then.
>
> This patch improves the situation: Now any value that is not "<unset>"
> is passed as the "driver" option, so invalid drivers will produce an
> error now. Should be mentioned in the commit message.
>
> However, if the user passes "<unset>", that still means probing. Ugly.
> Not sure why blkdev->fileproto == NULL wasn't used to specify that no
> driver was given. We could change that in a patch before this one if we
> wanted to clean it up. Or we could just feel reassured that xen_disk is
> horrible code, that this patch already fixes most of it and leave it
> alone.
I was glad enough that I made it out alive after digging just a bit into
the code. I'm fully in favor of leaving this as-is and adding a note to
the commit message.
Thank you for reviewing!
Max
>> + options = qdict_new();
>> + qdict_put_obj(options, "driver",
>> + QOBJECT(qstring_from_str(blkdev->fileproto)));
>> }
>> - blkdev->blk = blk;
>>
>> - bs = blk_bs(blk);
>> - drv = bdrv_find_whitelisted_format(blkdev->fileproto, readonly);
>> - if (bdrv_open(&bs, blkdev->filename, NULL, NULL, qflags,
>> - drv, &local_err) != 0) {
>> + /* setup via xenbus -> create new block driver instance */
>> + xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
>> + blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
>> + qflags, &local_err);
>> + if (!blkdev->blk) {
>> xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
>> error_get_pretty(local_err));
>> error_free(local_err);
>> - blk_unref(blk);
>> - blkdev->blk = NULL;
>> return -1;
>> }
>> - assert(bs == blk_bs(blk));
>> } else {
>> /* setup via qemu cmdline -> already setup for us */
>> xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
> Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open()
2015-02-02 18:35 ` Kevin Wolf
@ 2015-02-02 19:42 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:42 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 13:35, Kevin Wolf wrote:
> Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-img.c | 20 ++++++--------------
>> 1 file changed, 6 insertions(+), 14 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 4e9a7f5..be1953d 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -291,32 +291,24 @@ static BlockBackend *img_open(const char *id, const char *filename,
>> {
>> BlockBackend *blk;
>> BlockDriverState *bs;
>> - BlockDriver *drv;
>> char password[256];
>> Error *local_err = NULL;
>> - int ret;
>> -
>> - blk = blk_new_with_bs(id, &error_abort);
>> - bs = blk_bs(blk);
>> + QDict *options = NULL;
>>
>> if (fmt) {
>> - drv = bdrv_find_format(fmt);
>> - if (!drv) {
>> - error_report("Unknown file format '%s'", fmt);
>> - goto fail;
>> - }
>> - } else {
>> - drv = NULL;
>> + options = qdict_new();
>> + qdict_put_obj(options, "driver", QOBJECT(qstring_from_str(fmt)));
> I'm only noticing it here, though you did the same in the previous
> patches. This can be written shorter as:
>
> qdict_put(options, "driver", qstring_from_str(fmt));
Indeed, will do.
Max
>> }
>>
>> - ret = bdrv_open(&bs, filename, NULL, NULL, flags, drv, &local_err);
>> - if (ret < 0) {
>> + blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
>> + if (!blk) {
>> error_report("Could not open '%s': %s", filename,
>> error_get_pretty(local_err));
>> error_free(local_err);
>> goto fail;
>> }
>>
>> + bs = blk_bs(blk);
>> if (bdrv_is_encrypted(bs) && require_io) {
>> qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
>> if (read_password(password, sizeof(password)) < 0) {
> Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS Max Reitz
2015-01-27 17:29 ` Eric Blake
@ 2015-02-02 19:46 ` Kevin Wolf
2015-02-02 19:54 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 19:46 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> Now that request clamping is done in the BlockBackend, the "growable"
> field can be removed from the BlockDriverState. All BDSs are now treated
> as being "growable" (that is, they are allowed to grow; they are not
> necessarily actually able to).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 24 +++++-------------------
> block/qcow2.c | 6 ------
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 2 +-
> block/sheepdog.c | 2 +-
> include/block/block_int.h | 3 ---
> 6 files changed, 8 insertions(+), 31 deletions(-)
>
> diff --git a/block.c b/block.c
> index d45e4dd..356a857 100644
> --- a/block.c
> +++ b/block.c
> @@ -970,7 +970,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> bs->zero_beyond_eof = true;
> open_flags = bdrv_open_flags(bs, flags);
> bs->read_only = !(open_flags & BDRV_O_RDWR);
> - bs->growable = !!(flags & BDRV_O_PROTOCOL);
>
> if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
> error_setg(errp,
> @@ -1885,7 +1884,6 @@ void bdrv_close(BlockDriverState *bs)
> bs->encrypted = 0;
> bs->valid_key = 0;
> bs->sg = 0;
> - bs->growable = 0;
> bs->zero_beyond_eof = false;
> QDECREF(bs->options);
> bs->options = NULL;
> @@ -2645,25 +2643,13 @@ exit:
> static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
> size_t size)
> {
> - int64_t len;
> -
> if (size > INT_MAX) {
> return -EIO;
> }
>
> - if (!bdrv_is_inserted(bs))
> + if (!bdrv_is_inserted(bs)) {
> return -ENOMEDIUM;
> -
> - if (bs->growable)
> - return 0;
> -
> - len = bdrv_getlength(bs);
> -
> - if (offset < 0)
> - return -EIO;
Wouldn't it be better to keep this one, even though bs->growable used to
disable it?
Kevin
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase()
2015-02-02 19:00 ` Kevin Wolf
@ 2015-02-02 19:47 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:47 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 14:00, Kevin Wolf wrote:
> Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-img.c | 57 ++++++++++++++++++++++++---------------------------------
>> 1 file changed, 24 insertions(+), 33 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index be1953d..0b23c87 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -2430,7 +2430,6 @@ static int img_rebase(int argc, char **argv)
>> {
>> BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
>> BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL;
>> - BlockDriver *old_backing_drv, *new_backing_drv;
>> char *filename;
>> const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
>> int c, flags, src_flags, ret;
>> @@ -2524,54 +2523,46 @@ static int img_rebase(int argc, char **argv)
>> }
>> bs = blk_bs(blk);
>>
>> - /* Find the right drivers for the backing files */
>> - old_backing_drv = NULL;
>> - new_backing_drv = NULL;
>> -
>> - if (!unsafe && bs->backing_format[0] != '\0') {
>> - old_backing_drv = bdrv_find_format(bs->backing_format);
>> - if (old_backing_drv == NULL) {
>> - error_report("Invalid format name: '%s'", bs->backing_format);
>> - ret = -1;
>> - goto out;
>> - }
>> - }
>> -
>> - if (out_basefmt != NULL) {
>> - new_backing_drv = bdrv_find_format(out_basefmt);
>> - if (new_backing_drv == NULL) {
>> - error_report("Invalid format name: '%s'", out_basefmt);
>> - ret = -1;
>> - goto out;
>> - }
>> - }
> You're removing the validity check of the new backing file format:
>
> [master] $ ./qemu-img rebase -u -b /tmp/backing.qcow2 -F foobar /tmp/test.qcow2
> qemu-img: Invalid format name: 'foobar'
> [master] $
>
> [growable-v3] $ ./qemu-img rebase -u -b /tmp/backing.qcow2 -F foobar /tmp/test.qcow2
> [growable-v3] $
Oops, right, I missed that this changes the unsafe path. Thanks for
catching it! I'll fix it in v4.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open()
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
` (14 preceding siblings ...)
2015-01-26 15:49 ` [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Stefano Stabellini
@ 2015-02-02 19:50 ` Kevin Wolf
15 siblings, 0 replies; 49+ messages in thread
From: Kevin Wolf @ 2015-02-02 19:50 UTC (permalink / raw)
To: Max Reitz
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
> This series removes the "growable" field from the BlockDriverState
> object. Its use was to clamp guest requests against the limits of the
> BDS; however, this can now be done more easily by moving those checks
> into the BlockBackend functions.
>
> In a future series, "growable" may be reintroduced (maybe with a
> different name); it will then signify whether a BDS is able to grow (in
> contrast to the current "growable", which signifies whether it is
> allowed to). Maybe I will add it to the BlockDriver instead of the BDS,
> though.
>
> To be able to remove that field, qemu-io needs to be converted to
> BlockBackend, which is done by this series as well. While working on
> that I decided to convert blk_new_with_bs()+bdrv_open() to
> blk_new_open(). I was skeptical about that decision at first, but it
> seems good now that I was able to replace nearly every blk_new_with_bs()
> call by blk_new_open(). In a future series I may try to convert some
> remaining bdrv_open() calls to blk_new_open() as well. (And, in fact, in
> a future series I *will* replace the last remaining blk_new_with_bs()
> outside of blk_new_open() by blk_new_open().)
>
> Finally, the question needs to be asked: If, after this series, every
> BDS is allowed to grow, are there any users which do not use BB, but
> should still be disallowed from reading/writing beyond a BDS's limits?
> The only users I could see were the block jobs. Some of them should
> indeed be converted to BB; but none of them takes a user-supplied offset
> or size, all work on the full BDS (or only on parts which have been
> modified, etc.). Therefore, it is by design impossible for them to
> exceed the BDS's limits, which makes making all BDS's growable safe.
Patches that I didn't comment on (all except 4, 5, 6, 9, 10, 13) are,
assuming that you address Eric's comments, if any:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile()
2015-02-02 19:34 ` Kevin Wolf
@ 2015-02-02 19:51 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:51 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 14:34, Kevin Wolf wrote:
> Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-io.c | 31 ++++++++++++-------------------
>> 1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/qemu-io.c b/qemu-io.c
>> index 91a445a..81f8f64 100644
>> --- a/qemu-io.c
>> +++ b/qemu-io.c
>> @@ -39,7 +39,6 @@ static ReadLineState *readline_state;
>> static int close_f(BlockDriverState *bs, int argc, char **argv)
>> {
>> blk_unref(qemuio_blk);
>> - qemuio_bs = NULL;
>> qemuio_blk = NULL;
>> return 0;
>> }
> This doesn't seem to be correct at this point in the series. qemuio_bs
> is still passed to qemuio_command(), which checks whether non-global
> commands are allowed to run using init_check_command().
$ ./qemu-io -c close -c 'read 0 64k' test.qcow2
read failed: No medium found
Yep, will fix. Thanks for catching it!
> At the end of the series, please remove qemuio_bs altogether. This
> version of the series leaves it around without any reader.
Okay, will do.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option
2015-02-02 19:36 ` Kevin Wolf
@ 2015-02-02 19:52 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:52 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 14:36, Kevin Wolf wrote:
> Am 27.01.2015 um 18:11 hat Max Reitz geschrieben:
>> On 2015-01-27 at 12:10, Eric Blake wrote:
>>> On 01/27/2015 10:04 AM, Max Reitz wrote:
>>>> On 2015-01-27 at 11:59, Eric Blake wrote:
>>>>> On 01/26/2015 08:00 AM, Max Reitz wrote:
>>>>>> Remove "growable" option from the "open" command and from the qemu-io
>>>>>> command line. qemu-io is about to be converted to BlockBackend which
>>>>>> will make sure that no request exceeds the image size, so the only way
>>>>>> to keep "growable" would be to use BlockBackend if it is not given and
>>>>>> to directly access the BDS if it is.
>>>>>>
>>>>>> qemu-io is a debugging tool, therefore removing a rarely used option
>>>>>> will have only a very small impact, if any. There was only one
>>>>>> qemu-iotest which used the option; since it is not critical, this patch
>>>>>> just removes it.
>>>>>>
>>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>>>> ---
>>>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>>>
>>>>> Do we want to ever reuse the test number that you are deleting?
>>>> Good question, I think I have talked about that with Kevin before. It
>>>> would not hurt too much if we were to accidentally reuse the test case
>>>> number, most certainly not here in upstream.
>>>>
>>>> However, for all downstream versions of qemu, this might make adding the
>>>> new test 16 difficult; but certainly not impossible (if someone is
>>>> affected by this issue, he/she can just use 999 or something). So we may
>>>> want to keep in mind not to reuse number 16, but if someone does, so be it.
>>> Is it worth a placeholder file that has a comment mentioning that the
>>> test number is intentionally reserved (and if someone attempts to run,
>>> always passes)?
>> Seems good to me. It's a minor effort now and may avert some hassle later.
> How about just keeping a comment line in group?
Oh, that would be too simple.
Or maybe just simple enough for me; yes, that seems nicer indeed.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS
2015-02-02 19:46 ` Kevin Wolf
@ 2015-02-02 19:54 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2015-02-02 19:54 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefano Stabellini, qemu-devel, Stefan Hajnoczi, Markus Armbruster
On 2015-02-02 at 14:46, Kevin Wolf wrote:
> Am 26.01.2015 um 16:00 hat Max Reitz geschrieben:
>> Now that request clamping is done in the BlockBackend, the "growable"
>> field can be removed from the BlockDriverState. All BDSs are now treated
>> as being "growable" (that is, they are allowed to grow; they are not
>> necessarily actually able to).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block.c | 24 +++++-------------------
>> block/qcow2.c | 6 ------
>> block/raw-posix.c | 2 +-
>> block/raw-win32.c | 2 +-
>> block/sheepdog.c | 2 +-
>> include/block/block_int.h | 3 ---
>> 6 files changed, 8 insertions(+), 31 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index d45e4dd..356a857 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -970,7 +970,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>> bs->zero_beyond_eof = true;
>> open_flags = bdrv_open_flags(bs, flags);
>> bs->read_only = !(open_flags & BDRV_O_RDWR);
>> - bs->growable = !!(flags & BDRV_O_PROTOCOL);
>>
>> if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
>> error_setg(errp,
>> @@ -1885,7 +1884,6 @@ void bdrv_close(BlockDriverState *bs)
>> bs->encrypted = 0;
>> bs->valid_key = 0;
>> bs->sg = 0;
>> - bs->growable = 0;
>> bs->zero_beyond_eof = false;
>> QDECREF(bs->options);
>> bs->options = NULL;
>> @@ -2645,25 +2643,13 @@ exit:
>> static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
>> size_t size)
>> {
>> - int64_t len;
>> -
>> if (size > INT_MAX) {
>> return -EIO;
>> }
>>
>> - if (!bdrv_is_inserted(bs))
>> + if (!bdrv_is_inserted(bs)) {
>> return -ENOMEDIUM;
>> -
>> - if (bs->growable)
>> - return 0;
>> -
>> - len = bdrv_getlength(bs);
>> -
>> - if (offset < 0)
>> - return -EIO;
> Wouldn't it be better to keep this one, even though bs->growable used to
> disable it?
Indeed, there shouldn't be any harm in keeping it.
Max
^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2015-02-02 19:54 UTC | newest]
Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-26 15:00 [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 01/14] block: Lift some BDS functions to the BlockBackend Max Reitz
2015-01-26 21:48 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 02/14] block: Add blk_new_open() Max Reitz
2015-01-26 21:56 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init() Max Reitz
2015-01-26 22:37 ` Eric Blake
2015-01-27 2:08 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 04/14] block/xen: Use blk_new_open() in blk_connect() Max Reitz
2015-01-26 22:46 ` Eric Blake
2015-02-02 18:27 ` Kevin Wolf
2015-02-02 19:41 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 05/14] qemu-img: Use blk_new_open() in img_open() Max Reitz
2015-01-26 22:47 ` Eric Blake
2015-02-02 18:35 ` Kevin Wolf
2015-02-02 19:42 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 06/14] qemu-img: Use blk_new_open() in img_rebase() Max Reitz
2015-01-27 3:05 ` Eric Blake
2015-01-27 15:01 ` Max Reitz
2015-02-02 19:00 ` Kevin Wolf
2015-02-02 19:47 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 07/14] qemu-img: Use BlockBackend as far as possible Max Reitz
2015-01-27 3:38 ` Eric Blake
2015-01-27 15:07 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 08/14] qemu-nbd: Use blk_new_open() in main() Max Reitz
2015-01-27 4:59 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 09/14] qemu-io: Use blk_new_open() in openfile() Max Reitz
2015-01-27 16:23 ` Eric Blake
2015-02-02 19:34 ` Kevin Wolf
2015-02-02 19:51 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 10/14] qemu-io: Remove "growable" option Max Reitz
2015-01-27 16:59 ` Eric Blake
2015-01-27 17:04 ` Max Reitz
2015-01-27 17:10 ` Eric Blake
2015-01-27 17:11 ` Max Reitz
2015-02-02 19:36 ` Kevin Wolf
2015-02-02 19:52 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 11/14] qemu-io: Use BlockBackend Max Reitz
2015-01-27 17:08 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 12/14] block: Clamp BlockBackend requests Max Reitz
2015-01-27 17:15 ` Eric Blake
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 13/14] block: Remove "growable" from BDS Max Reitz
2015-01-27 17:29 ` Eric Blake
2015-02-02 19:46 ` Kevin Wolf
2015-02-02 19:54 ` Max Reitz
2015-01-26 15:00 ` [Qemu-devel] [PATCH v3 14/14] block: Keep bdrv_check*_request()'s return value Max Reitz
2015-01-27 17:36 ` Eric Blake
2015-01-26 15:49 ` [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() Stefano Stabellini
2015-02-02 19:50 ` 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.