* [PULL 01/18] qcow2: fix memory leak in qcow2_read_extensions
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 02/18] block/qcow2-bitmap: Add missing cast to silent GCC error Kevin Wolf
` (17 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: lu zhipeng <luzhipeng@cestc.cn>
Free feature_table if it is failed in bdrv_pread.
Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
Message-Id: <20220921144515.1166-1-luzhipeng@cestc.cn>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index c6c6692fb7..c8fc3a6160 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -275,6 +275,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
if (ret < 0) {
error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
"Could not read table");
+ g_free(feature_table);
return ret;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 02/18] block/qcow2-bitmap: Add missing cast to silent GCC error
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
2022-09-30 16:52 ` [PULL 01/18] qcow2: fix memory leak in qcow2_read_extensions Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 03/18] qemu-img: Wean documentation and help output off '?' for help Kevin Wolf
` (16 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Commit d1258dd0c8 ("qcow2: autoloading dirty bitmaps") added the
set_readonly_helper() GFunc handler, correctly casting the gpointer
user_data in both the g_slist_foreach() caller and the handler.
Few commits later (commit 1b6b0562db), the handler is reused in
qcow2_reopen_bitmaps_rw() but missing the gpointer cast, resulting
in the following error when using Homebrew GCC 12.2.0:
[2/658] Compiling C object libblock.fa.p/block_qcow2-bitmap.c.o
../../block/qcow2-bitmap.c: In function 'qcow2_reopen_bitmaps_rw':
../../block/qcow2-bitmap.c:1211:60: error: incompatible type for argument 3 of 'g_slist_foreach'
1211 | g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
| ^~~~~
| |
| _Bool
In file included from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gmain.h:26,
from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/giochannel.h:33,
from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib.h:54,
from /Users/philmd/source/qemu/include/glib-compat.h:32,
from /Users/philmd/source/qemu/include/qemu/osdep.h:144,
from ../../block/qcow2-bitmap.c:28:
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gslist.h:127:61: note: expected 'gpointer' {aka 'void *'} but argument is of type '_Bool'
127 | gpointer user_data);
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~
At top level:
FAILED: libblock.fa.p/block_qcow2-bitmap.c.o
Fix by adding the missing gpointer cast.
Fixes: 1b6b0562db ("qcow2: support .bdrv_reopen_bitmaps_rw")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20220919182755.51967-1-f4bug@amsat.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-bitmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index ff3309846c..7197754843 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1208,7 +1208,7 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
}
}
- g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
+ g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, (gpointer)false);
ret = 0;
out:
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 03/18] qemu-img: Wean documentation and help output off '?' for help
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
2022-09-30 16:52 ` [PULL 01/18] qcow2: fix memory leak in qcow2_read_extensions Kevin Wolf
2022-09-30 16:52 ` [PULL 02/18] block/qcow2-bitmap: Add missing cast to silent GCC error Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 04/18] block: pass OnOffAuto instead of bool to block_acct_setup() Kevin Wolf
` (15 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
'?' for help is deprecated since commit c8057f951d "Support 'help' as
a synonym for '?' in command line options", v1.2.0. We neglected to
update output of qemu-img --help and the manual. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220908130842.641410-1-armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/tools/qemu-img.rst | 2 +-
qemu-img.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 85a6e05b35..15aeddc6d8 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -57,7 +57,7 @@ cases. See below for a description of the supported disk formats.
*OUTPUT_FMT* is the destination format.
*OPTIONS* is a comma separated list of format specific options in a
-name=value format. Use ``-o ?`` for an overview of the options supported
+name=value format. Use ``-o help`` for an overview of the options supported
by the used format or see the format descriptions below for details.
*SNAPSHOT_PARAM* is param used for internal snapshot, format is
diff --git a/qemu-img.c b/qemu-img.c
index 7d4b33b3da..cab9776f42 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -164,8 +164,8 @@ void help(void)
" 'output_filename' is the destination disk image filename\n"
" 'output_fmt' is the destination format\n"
" 'options' is a comma separated list of format specific options in a\n"
- " name=value format. Use -o ? for an overview of the options supported by the\n"
- " used format\n"
+ " name=value format. Use -o help for an overview of the options supported by\n"
+ " the used format\n"
" 'snapshot_param' is param used for internal snapshot, format\n"
" is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
" '[ID_OR_NAME]'\n"
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 04/18] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2022-09-30 16:52 ` [PULL 03/18] qemu-img: Wean documentation and help output off '?' for help Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 05/18] block: add missed block_acct_setup with new block device init procedure Kevin Wolf
` (14 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: "Denis V. Lunev" <den@openvz.org>
We would have one more place for block_acct_setup() calling, which should
not corrupt original value.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Peter Krempa <pkrempa@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220824095044.166009-2-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/accounting.h | 6 +++---
block/accounting.c | 22 ++++++++++++++++++----
blockdev.c | 17 ++++++++++++++---
3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 878b4c3581..b9caad60d5 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -27,7 +27,7 @@
#include "qemu/timed-average.h"
#include "qemu/thread.h"
-#include "qapi/qapi-builtin-types.h"
+#include "qapi/qapi-types-common.h"
typedef struct BlockAcctTimedStats BlockAcctTimedStats;
typedef struct BlockAcctStats BlockAcctStats;
@@ -100,8 +100,8 @@ typedef struct BlockAcctCookie {
} BlockAcctCookie;
void block_acct_init(BlockAcctStats *stats);
-void block_acct_setup(BlockAcctStats *stats, bool account_invalid,
- bool account_failed);
+void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
+ enum OnOffAuto account_failed);
void block_acct_cleanup(BlockAcctStats *stats);
void block_acct_add_interval(BlockAcctStats *stats, unsigned interval_length);
BlockAcctTimedStats *block_acct_interval_next(BlockAcctStats *stats,
diff --git a/block/accounting.c b/block/accounting.c
index 2030851d79..6b300c5129 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -40,11 +40,25 @@ void block_acct_init(BlockAcctStats *stats)
}
}
-void block_acct_setup(BlockAcctStats *stats, bool account_invalid,
- bool account_failed)
+static bool bool_from_onoffauto(OnOffAuto val, bool def)
{
- stats->account_invalid = account_invalid;
- stats->account_failed = account_failed;
+ switch (val) {
+ case ON_OFF_AUTO_AUTO:
+ return def;
+ case ON_OFF_AUTO_ON:
+ return true;
+ case ON_OFF_AUTO_OFF:
+ return false;
+ default:
+ abort();
+ }
+}
+
+void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
+ enum OnOffAuto account_failed)
+{
+ stats->account_invalid = bool_from_onoffauto(account_invalid, true);
+ stats->account_failed = bool_from_onoffauto(account_failed, true);
}
void block_acct_cleanup(BlockAcctStats *stats)
diff --git a/blockdev.c b/blockdev.c
index 9230888e34..392d9476e6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -455,6 +455,17 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
}
}
+static OnOffAuto account_get_opt(QemuOpts *opts, const char *name)
+{
+ if (!qemu_opt_find(opts, name)) {
+ return ON_OFF_AUTO_AUTO;
+ }
+ if (qemu_opt_get_bool(opts, name, true)) {
+ return ON_OFF_AUTO_ON;
+ }
+ return ON_OFF_AUTO_OFF;
+}
+
/* Takes the ownership of bs_opts */
static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
Error **errp)
@@ -462,7 +473,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
const char *buf;
int bdrv_flags = 0;
int on_read_error, on_write_error;
- bool account_invalid, account_failed;
+ OnOffAuto account_invalid, account_failed;
bool writethrough, read_only;
BlockBackend *blk;
BlockDriverState *bs;
@@ -496,8 +507,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
/* extract parameters */
snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
- account_invalid = qemu_opt_get_bool(opts, "stats-account-invalid", true);
- account_failed = qemu_opt_get_bool(opts, "stats-account-failed", true);
+ account_invalid = account_get_opt(opts, "stats-account-invalid");
+ account_failed = account_get_opt(opts, "stats-account-failed");
writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 05/18] block: add missed block_acct_setup with new block device init procedure
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2022-09-30 16:52 ` [PULL 04/18] block: pass OnOffAuto instead of bool to block_acct_setup() Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 06/18] block: use bdrv_is_sg() helper instead of raw bs->sg reading Kevin Wolf
` (13 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: "Denis V. Lunev" <den@openvz.org>
Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
the first glance, but it has changed things a lot. 'libvirt' uses it to
detect that it should follow new initialization way and this changes
things considerably. With this procedure followed, blockdev_init() is
not called anymore and thus block_acct_setup() helper is not called.
This means in particular that defaults for block accounting statistics
are changed and account_invalid/account_failed are actually initialized
as false instead of true originally.
This commit changes things to match original world. There are the following
constraints:
* new default value in block_acct_init() is set to true
* block_acct_setup() inside blockdev_init() is called before
blkconf_apply_backend_options()
* thus newly created option in block device properties has precedence if
specified
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Peter Krempa <pkrempa@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220824095044.166009-3-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/hw/block/block.h | 7 +++-
block/accounting.c | 8 +++-
hw/block/block.c | 2 +
tests/qemu-iotests/172.out | 76 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/227.out | 4 +-
5 files changed, 92 insertions(+), 5 deletions(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 5902c0440a..15fff66435 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -31,6 +31,7 @@ typedef struct BlockConf {
uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
+ OnOffAuto account_invalid, account_failed;
BlockdevOnError rerror;
BlockdevOnError werror;
} BlockConf;
@@ -61,7 +62,11 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
_conf.discard_granularity, -1), \
DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
ON_OFF_AUTO_AUTO), \
- DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
+ DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false), \
+ DEFINE_PROP_ON_OFF_AUTO("account-invalid", _state, \
+ _conf.account_invalid, ON_OFF_AUTO_AUTO), \
+ DEFINE_PROP_ON_OFF_AUTO("account-failed", _state, \
+ _conf.account_failed, ON_OFF_AUTO_AUTO)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
diff --git a/block/accounting.c b/block/accounting.c
index 6b300c5129..2829745377 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -38,6 +38,8 @@ void block_acct_init(BlockAcctStats *stats)
if (qtest_enabled()) {
clock_type = QEMU_CLOCK_VIRTUAL;
}
+ stats->account_invalid = true;
+ stats->account_failed = true;
}
static bool bool_from_onoffauto(OnOffAuto val, bool def)
@@ -57,8 +59,10 @@ static bool bool_from_onoffauto(OnOffAuto val, bool def)
void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
enum OnOffAuto account_failed)
{
- stats->account_invalid = bool_from_onoffauto(account_invalid, true);
- stats->account_failed = bool_from_onoffauto(account_failed, true);
+ stats->account_invalid = bool_from_onoffauto(account_invalid,
+ stats->account_invalid);
+ stats->account_failed = bool_from_onoffauto(account_failed,
+ stats->account_failed);
}
void block_acct_cleanup(BlockAcctStats *stats)
diff --git a/hw/block/block.c b/hw/block/block.c
index 04279166ee..f9c4fe6767 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -205,6 +205,8 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
blk_set_enable_write_cache(blk, wce);
blk_set_on_error(blk, rerror, werror);
+ block_acct_setup(blk_get_stats(blk), conf->account_invalid,
+ conf->account_failed);
return true;
}
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 9479b92185..07eebf3583 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -28,6 +28,8 @@ Testing:
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
@@ -55,6 +57,8 @@ Testing: -fda TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -92,6 +96,8 @@ Testing: -fdb TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -104,6 +110,8 @@ Testing: -fdb TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -145,6 +153,8 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -157,6 +167,8 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -199,6 +211,8 @@ Testing: -fdb
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
dev: floppy, id ""
unit = 0 (0x0)
@@ -211,6 +225,8 @@ Testing: -fdb
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
@@ -238,6 +254,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -275,6 +293,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -287,6 +307,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -328,6 +350,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -340,6 +364,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -385,6 +411,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -422,6 +450,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -459,6 +489,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -471,6 +503,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -522,6 +556,8 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -534,6 +570,8 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -576,6 +614,8 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -588,6 +628,8 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -630,6 +672,8 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 1 (0x1)
@@ -642,6 +686,8 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -684,6 +730,8 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 1 (0x1)
@@ -696,6 +744,8 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -747,6 +797,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -759,6 +811,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -801,6 +855,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -813,6 +869,8 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/device[N]
@@ -861,6 +919,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -928,6 +988,8 @@ Testing: -device floppy
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
Testing: -device floppy,drive-type=120
@@ -952,6 +1014,8 @@ Testing: -device floppy,drive-type=120
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "120"
Testing: -device floppy,drive-type=144
@@ -976,6 +1040,8 @@ Testing: -device floppy,drive-type=144
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
Testing: -device floppy,drive-type=288
@@ -1000,6 +1066,8 @@ Testing: -device floppy,drive-type=288
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
@@ -1027,6 +1095,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "120"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -1064,6 +1134,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "288"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -1104,6 +1176,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
@@ -1141,6 +1215,8 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = "auto"
+ account-failed = "auto"
drive-type = "144"
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/peripheral-anon/device[N]
diff --git a/tests/qemu-iotests/227.out b/tests/qemu-iotests/227.out
index 9c09ee3917..378c1b8fb1 100644
--- a/tests/qemu-iotests/227.out
+++ b/tests/qemu-iotests/227.out
@@ -188,7 +188,7 @@ Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device virtio-b
],
"failed_unmap_operations": 0,
"failed_flush_operations": 0,
- "account_invalid": false,
+ "account_invalid": true,
"rd_total_time_ns": 0,
"invalid_unmap_operations": 0,
"flush_operations": 0,
@@ -198,7 +198,7 @@ Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device virtio-b
"rd_bytes": 0,
"unmap_total_time_ns": 0,
"invalid_flush_operations": 0,
- "account_failed": false,
+ "account_failed": true,
"rd_operations": 0,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 06/18] block: use bdrv_is_sg() helper instead of raw bs->sg reading
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2022-09-30 16:52 ` [PULL 05/18] block: add missed block_acct_setup with new block device init procedure Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 07/18] block: make serializing requests functions 'void' Kevin Wolf
` (12 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: "Denis V. Lunev" <den@openvz.org>
I believe that if the helper exists, it must be used always for reading
of the value. It breaks expectations in the other case.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <fam@euphon.net>
CC: Ronnie Sahlberg <ronniesahlberg@gmail.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Peter Lieven <pl@kamp.de>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20220817083736.40981-2-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 2 +-
block/iscsi.c | 2 +-
block/raw-format.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 48cd096624..256de1f456 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1295,7 +1295,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
}
#endif
- if (bs->sg || S_ISBLK(st.st_mode)) {
+ if (bdrv_is_sg(bs) || S_ISBLK(st.st_mode)) {
int ret = hdev_get_max_hw_transfer(s->fd, &st);
if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
diff --git a/block/iscsi.c b/block/iscsi.c
index d707d0b354..612de127e5 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2065,7 +2065,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff;
unsigned int block_size = MAX(BDRV_SECTOR_SIZE, iscsilun->block_size);
- assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bs->sg);
+ assert(iscsilun->block_size >= BDRV_SECTOR_SIZE || bdrv_is_sg(bs));
bs->bl.request_alignment = block_size;
diff --git a/block/raw-format.c b/block/raw-format.c
index 69fd650eaf..c7278e348e 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -463,7 +463,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- bs->sg = bs->file->bs->sg;
+ bs->sg = bdrv_is_sg(bs->file->bs);
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
(BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
@@ -489,7 +489,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}
- if (bs->sg && (s->offset || s->has_size)) {
+ if (bdrv_is_sg(bs) && (s->offset || s->has_size)) {
error_setg(errp, "Cannot use offset/size with SCSI generic devices");
return -EINVAL;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 07/18] block: make serializing requests functions 'void'
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2022-09-30 16:52 ` [PULL 06/18] block: use bdrv_is_sg() helper instead of raw bs->sg reading Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 08/18] gluster: stop using .bdrv_needs_filename Kevin Wolf
` (11 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: "Denis V. Lunev" <den@openvz.org>
Return codes of the following functions are never used in the code:
* bdrv_wait_serialising_requests_locked
* bdrv_wait_serialising_requests
* bdrv_make_request_serialising
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <fam@euphon.net>
CC: Ronnie Sahlberg <ronniesahlberg@gmail.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Peter Lieven <pl@kamp.de>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20220817083736.40981-3-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int-io.h | 2 +-
block/io.c | 23 +++++++----------------
2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 91cdd61692..4b0b3e17ef 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -73,7 +73,7 @@ static inline int coroutine_fn bdrv_co_pwrite(BdrvChild *child,
return bdrv_co_pwritev(child, offset, bytes, &qiov, flags);
}
-bool coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req,
+void coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req,
uint64_t align);
BdrvTrackedRequest *coroutine_fn bdrv_co_get_self_request(BlockDriverState *bs);
diff --git a/block/io.c b/block/io.c
index 0a8cbefe86..51d8f943a4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -828,20 +828,16 @@ bdrv_find_conflicting_request(BdrvTrackedRequest *self)
}
/* Called with self->bs->reqs_lock held */
-static bool coroutine_fn
+static void coroutine_fn
bdrv_wait_serialising_requests_locked(BdrvTrackedRequest *self)
{
BdrvTrackedRequest *req;
- bool waited = false;
while ((req = bdrv_find_conflicting_request(self))) {
self->waiting_for = req;
qemu_co_queue_wait(&req->wait_queue, &self->bs->reqs_lock);
self->waiting_for = NULL;
- waited = true;
}
-
- return waited;
}
/* Called with req->bs->reqs_lock held */
@@ -934,36 +930,31 @@ void bdrv_dec_in_flight(BlockDriverState *bs)
bdrv_wakeup(bs);
}
-static bool coroutine_fn bdrv_wait_serialising_requests(BdrvTrackedRequest *self)
+static void coroutine_fn
+bdrv_wait_serialising_requests(BdrvTrackedRequest *self)
{
BlockDriverState *bs = self->bs;
- bool waited = false;
if (!qatomic_read(&bs->serialising_in_flight)) {
- return false;
+ return;
}
qemu_co_mutex_lock(&bs->reqs_lock);
- waited = bdrv_wait_serialising_requests_locked(self);
+ bdrv_wait_serialising_requests_locked(self);
qemu_co_mutex_unlock(&bs->reqs_lock);
-
- return waited;
}
-bool coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req,
+void coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req,
uint64_t align)
{
- bool waited;
IO_CODE();
qemu_co_mutex_lock(&req->bs->reqs_lock);
tracked_request_set_serialising(req, align);
- waited = bdrv_wait_serialising_requests_locked(req);
+ bdrv_wait_serialising_requests_locked(req);
qemu_co_mutex_unlock(&req->bs->reqs_lock);
-
- return waited;
}
int bdrv_check_qiov_request(int64_t offset, int64_t bytes,
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 08/18] gluster: stop using .bdrv_needs_filename
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (6 preceding siblings ...)
2022-09-30 16:52 ` [PULL 07/18] block: make serializing requests functions 'void' Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 09/18] block/qcow2: Keep auto_backing_file if possible Kevin Wolf
` (10 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
The gluster protocol driver used to parse URIs (filenames) but was
extended with a richer JSON syntax in commit 6c7189bb29de
("block/gluster: add support for multiple gluster servers"). The gluster
drivers that have JSON parsing set .bdrv_needs_filename to false.
The gluster+unix and gluster+rdma drivers still to require a filename
even though the JSON parser is equipped to parse the same
volume/path/sockaddr details as the URI parser. Let's allow JSON parsing
for these drivers too.
Note that the gluster+rdma driver actually uses TCP because RDMA support
is not available, so the JSON server.type field must be "inet".
Drop .bdrv_needs_filename since both the filename and the JSON parsers
can handle gluster+unix and gluster+rdma. This change is in preparation
for eventually removing .bdrv_needs_filename across the entire codebase.
Cc: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220811164905.430834-1-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/gluster.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index b60213ab80..bb1144cf6a 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1555,7 +1555,6 @@ static BlockDriver bdrv_gluster = {
.format_name = "gluster",
.protocol_name = "gluster",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = false,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
@@ -1585,7 +1584,6 @@ static BlockDriver bdrv_gluster_tcp = {
.format_name = "gluster",
.protocol_name = "gluster+tcp",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = false,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
@@ -1615,7 +1613,6 @@ static BlockDriver bdrv_gluster_unix = {
.format_name = "gluster",
.protocol_name = "gluster+unix",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = true,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
@@ -1651,7 +1648,6 @@ static BlockDriver bdrv_gluster_rdma = {
.format_name = "gluster",
.protocol_name = "gluster+rdma",
.instance_size = sizeof(BDRVGlusterState),
- .bdrv_needs_filename = true,
.bdrv_file_open = qemu_gluster_open,
.bdrv_reopen_prepare = qemu_gluster_reopen_prepare,
.bdrv_reopen_commit = qemu_gluster_reopen_commit,
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 09/18] block/qcow2: Keep auto_backing_file if possible
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (7 preceding siblings ...)
2022-09-30 16:52 ` [PULL 08/18] gluster: stop using .bdrv_needs_filename Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 10/18] block/qed: " Kevin Wolf
` (9 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Hanna Reitz <hreitz@redhat.com>
qcow2_do_open() is used by qcow2_co_invalidate_cache(), i.e. may be run
on an image that has been opened before. When reading the backing file
string from the image header, compare it against the existing
bs->backing_file, and update bs->auto_backing_file only if they differ.
auto_backing_file should ideally contain the filename the backing BDS
will actually have after opening, i.e. a post-bdrv_refresh_filename()
version of what is in the image header. So for example, if the image
header reports the following backing file string:
json:{"driver": "qcow2", "file": {
"driver": "file", "filename": "/tmp/backing.qcow2"
}}
Then auto_backing_file should contain simply "/tmp/backing.qcow2".
Because bdrv_refresh_filename() only works on existing BDSs, though, the
way how we get this auto_backing_file value is to have the format driver
set it to whatever is in the image header, and when the backing BDS is
opened based on that, we update it with the filename the backing BDS
actually got.
However, qcow2's qcow2_co_invalidate_cache() implementation breaks this
because it just resets auto_backing_file to whatever is in the image
file without opening a BDS based on it, so we never get
auto_backing_file back to the "refreshed" version, and in the example
above, it would stay "json:{...}".
Then, bs->backing->bs->filename will differ from bs->auto_backing_file,
making bdrv_backing_overridden(bs) return true, which will lead
bdrv_refresh_filename(bs) to generate a json:{} filename for bs, even
though that may not have been necessary. This is reported in the issue
linked below.
Therefore, skip updating auto_backing_file if nothing has changed in the
image header since we last read it.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1117
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220803144446.20723-2-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index c8fc3a6160..6c8c8b2b5a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1697,16 +1697,27 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
ret = -EINVAL;
goto fail;
}
+
+ s->image_backing_file = g_malloc(len + 1);
ret = bdrv_pread(bs->file, header.backing_file_offset, len,
- bs->auto_backing_file, 0);
+ s->image_backing_file, 0);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not read backing file name");
goto fail;
}
- bs->auto_backing_file[len] = '\0';
- pstrcpy(bs->backing_file, sizeof(bs->backing_file),
- bs->auto_backing_file);
- s->image_backing_file = g_strdup(bs->auto_backing_file);
+ s->image_backing_file[len] = '\0';
+
+ /*
+ * Update only when something has changed. This function is called by
+ * qcow2_co_invalidate_cache(), and we do not want to reset
+ * auto_backing_file unless necessary.
+ */
+ if (!g_str_equal(s->image_backing_file, bs->backing_file)) {
+ pstrcpy(bs->backing_file, sizeof(bs->backing_file),
+ s->image_backing_file);
+ pstrcpy(bs->auto_backing_file, sizeof(bs->auto_backing_file),
+ s->image_backing_file);
+ }
}
/*
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 10/18] block/qed: Keep auto_backing_file if possible
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (8 preceding siblings ...)
2022-09-30 16:52 ` [PULL 09/18] block/qcow2: Keep auto_backing_file if possible Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 11/18] iotests/backing-file-invalidation: Add new test Kevin Wolf
` (8 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Hanna Reitz <hreitz@redhat.com>
Just like qcow2, qed invokes its open function in its
.bdrv_co_invalidate_cache() implementation. Therefore, just like done
for qcow2 in HEAD^, update auto_backing_file only if the backing file
string in the image header differs from the one we have read before.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220803144446.20723-3-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qed.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index 40943e679b..324ca0e95a 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -445,6 +445,8 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
}
if ((s->header.features & QED_F_BACKING_FILE)) {
+ g_autofree char *backing_file_str = NULL;
+
if ((uint64_t)s->header.backing_filename_offset +
s->header.backing_filename_size >
s->header.cluster_size * s->header.header_size) {
@@ -452,16 +454,21 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
return -EINVAL;
}
+ backing_file_str = g_malloc(sizeof(bs->backing_file));
ret = qed_read_string(bs->file, s->header.backing_filename_offset,
s->header.backing_filename_size,
- bs->auto_backing_file,
- sizeof(bs->auto_backing_file));
+ backing_file_str, sizeof(bs->backing_file));
if (ret < 0) {
error_setg(errp, "Failed to read backing filename");
return ret;
}
- pstrcpy(bs->backing_file, sizeof(bs->backing_file),
- bs->auto_backing_file);
+
+ if (!g_str_equal(backing_file_str, bs->backing_file)) {
+ pstrcpy(bs->backing_file, sizeof(bs->backing_file),
+ backing_file_str);
+ pstrcpy(bs->auto_backing_file, sizeof(bs->auto_backing_file),
+ backing_file_str);
+ }
if (s->header.features & QED_F_BACKING_FORMAT_NO_PROBE) {
pstrcpy(bs->backing_format, sizeof(bs->backing_format), "raw");
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 11/18] iotests/backing-file-invalidation: Add new test
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (9 preceding siblings ...)
2022-09-30 16:52 ` [PULL 10/18] block/qed: " Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 12/18] block: move bdrv_qiov_is_aligned to file-posix Kevin Wolf
` (7 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Hanna Reitz <hreitz@redhat.com>
Add a new test to see what happens when you migrate a VM with a backing
chain that has json:{} backing file strings, which, when opened, will be
resolved to plain filenames.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220803144446.20723-4-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
.../tests/backing-file-invalidation | 152 ++++++++++++++++++
.../tests/backing-file-invalidation.out | 5 +
2 files changed, 157 insertions(+)
create mode 100755 tests/qemu-iotests/tests/backing-file-invalidation
create mode 100644 tests/qemu-iotests/tests/backing-file-invalidation.out
diff --git a/tests/qemu-iotests/tests/backing-file-invalidation b/tests/qemu-iotests/tests/backing-file-invalidation
new file mode 100755
index 0000000000..4eccc80153
--- /dev/null
+++ b/tests/qemu-iotests/tests/backing-file-invalidation
@@ -0,0 +1,152 @@
+#!/usr/bin/env python3
+# group: rw migration
+#
+# Migrate a VM with a BDS with backing nodes, which runs
+# bdrv_invalidate_cache(), which for qcow2 and qed triggers reading the
+# backing file string from the image header. Check whether this
+# interferes with bdrv_backing_overridden().
+#
+# Copyright (C) 2022 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/>.
+#
+
+import json
+import os
+from typing import Optional
+
+import iotests
+from iotests import qemu_img_create, qemu_img_info
+
+
+image_size = 1 * 1024 * 1024
+imgs = [os.path.join(iotests.test_dir, f'{i}.img') for i in range(0, 4)]
+
+mig_sock = os.path.join(iotests.sock_dir, 'mig.sock')
+
+
+class TestPostMigrateFilename(iotests.QMPTestCase):
+ vm_s: Optional[iotests.VM] = None
+ vm_d: Optional[iotests.VM] = None
+
+ def setUp(self) -> None:
+ # Create backing chain of three images, where the backing file strings
+ # are json:{} filenames
+ qemu_img_create('-f', iotests.imgfmt, imgs[0], str(image_size))
+ for i in range(1, 3):
+ backing = {
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': imgs[i - 1]
+ }
+ }
+ qemu_img_create('-f', iotests.imgfmt, '-F', iotests.imgfmt,
+ '-b', 'json:' + json.dumps(backing),
+ imgs[i], str(image_size))
+
+ def tearDown(self) -> None:
+ if self.vm_s is not None:
+ self.vm_s.shutdown()
+ if self.vm_d is not None:
+ self.vm_d.shutdown()
+
+ for img in imgs:
+ try:
+ os.remove(img)
+ except OSError:
+ pass
+ try:
+ os.remove(mig_sock)
+ except OSError:
+ pass
+
+ def test_migration(self) -> None:
+ """
+ Migrate a VM with the backing chain created in setUp() attached. At
+ the end of the migration process, the destination will run
+ bdrv_invalidate_cache(), which for some image formats (qcow2 and qed)
+ means the backing file string is re-read from the image header. If
+ this overwrites bs->auto_backing_file, doing so may cause
+ bdrv_backing_overridden() to become true: The image header reports a
+ json:{} filename, but when opening it, bdrv_refresh_filename() will
+ simplify it to a plain simple filename; and when bs->auto_backing_file
+ and bs->backing->bs->filename differ, bdrv_backing_overridden() becomes
+ true.
+ If bdrv_backing_overridden() is true, the BDS will be forced to get a
+ json:{} filename, which in general is not the end of the world, but not
+ great. Check whether that happens, i.e. whether migration changes the
+ node's filename.
+ """
+
+ blockdev = {
+ 'node-name': 'node0',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': imgs[2]
+ }
+ }
+
+ self.vm_s = iotests.VM(path_suffix='a') \
+ .add_blockdev(json.dumps(blockdev))
+ self.vm_d = iotests.VM(path_suffix='b') \
+ .add_blockdev(json.dumps(blockdev)) \
+ .add_incoming(f'unix:{mig_sock}')
+
+ assert self.vm_s is not None
+ assert self.vm_d is not None
+
+ self.vm_s.launch()
+ self.vm_d.launch()
+
+ pre_mig_filename = self.vm_s.node_info('node0')['file']
+
+ self.vm_s.qmp('migrate', uri=f'unix:{mig_sock}')
+
+ # Wait for migration to be done
+ self.vm_s.event_wait('STOP')
+ self.vm_d.event_wait('RESUME')
+
+ post_mig_filename = self.vm_d.node_info('node0')['file']
+
+ # Verify that the filename hasn't changed from before the migration
+ self.assertEqual(pre_mig_filename, post_mig_filename)
+
+ self.vm_s.shutdown()
+ self.vm_s = None
+
+ # For good measure, try creating an overlay and check its backing
+ # chain below. This is how the issue was originally found.
+ result = self.vm_d.qmp('blockdev-snapshot-sync',
+ format=iotests.imgfmt,
+ snapshot_file=imgs[3],
+ node_name='node0',
+ snapshot_node_name='node0-overlay')
+ self.assert_qmp(result, 'return', {})
+
+ self.vm_d.shutdown()
+ self.vm_d = None
+
+ # Check the newly created overlay's backing chain
+ chain = qemu_img_info('--backing-chain', imgs[3])
+ for index, image in enumerate(chain):
+ self.assertEqual(image['filename'], imgs[3 - index])
+
+
+if __name__ == '__main__':
+ # These are the image formats that run their open() function from their
+ # .bdrv_co_invaliate_cache() implementations, so test them
+ iotests.main(supported_fmts=['qcow2', 'qed'],
+ supported_protocols=['file'])
diff --git a/tests/qemu-iotests/tests/backing-file-invalidation.out b/tests/qemu-iotests/tests/backing-file-invalidation.out
new file mode 100644
index 0000000000..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/tests/backing-file-invalidation.out
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 12/18] block: move bdrv_qiov_is_aligned to file-posix
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (10 preceding siblings ...)
2022-09-30 16:52 ` [PULL 11/18] iotests/backing-file-invalidation: Add new test Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 13/18] block: use the request length for iov alignment Kevin Wolf
` (6 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Keith Busch <kbusch@kernel.org>
There is only user of bdrv_qiov_is_aligned(), so move the alignment
function to there and make it static.
Signed-off-by: Keith Busch <kbusch@kernel.org>
Message-Id: <20220929200523.3218710-2-kbusch@meta.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-io.h | 1 -
block/file-posix.c | 21 +++++++++++++++++++++
block/io.c | 21 ---------------------
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index fd25ffa9be..492f95fc05 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -150,7 +150,6 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size);
void *qemu_blockalign0(BlockDriverState *bs, size_t size);
void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
void *qemu_try_blockalign0(BlockDriverState *bs, size_t size);
-bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
diff --git a/block/file-posix.c b/block/file-posix.c
index 256de1f456..989dfc4586 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2061,6 +2061,27 @@ static int coroutine_fn raw_thread_pool_submit(BlockDriverState *bs,
return thread_pool_submit_co(pool, func, arg);
}
+/*
+ * Check if all memory in this vector is sector aligned.
+ */
+static bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
+{
+ int i;
+ size_t alignment = bdrv_min_mem_align(bs);
+ IO_CODE();
+
+ for (i = 0; i < qiov->niov; i++) {
+ if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
+ return false;
+ }
+ if (qiov->iov[i].iov_len % alignment) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int type)
{
diff --git a/block/io.c b/block/io.c
index 51d8f943a4..c3200bcdff 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3227,27 +3227,6 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
return mem;
}
-/*
- * Check if all memory in this vector is sector aligned.
- */
-bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
-{
- int i;
- size_t alignment = bdrv_min_mem_align(bs);
- IO_CODE();
-
- for (i = 0; i < qiov->niov; i++) {
- if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
- return false;
- }
- if (qiov->iov[i].iov_len % alignment) {
- return false;
- }
- }
-
- return true;
-}
-
void bdrv_io_plug(BlockDriverState *bs)
{
BdrvChild *child;
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 13/18] block: use the request length for iov alignment
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (11 preceding siblings ...)
2022-09-30 16:52 ` [PULL 12/18] block: move bdrv_qiov_is_aligned to file-posix Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 14/18] piix_ide_reset: Use pci_set_* functions instead of direct access Kevin Wolf
` (5 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Keith Busch <kbusch@kernel.org>
An iov length needs to be aligned to the logical block size, which may
be larger than the memory alignment.
Tested-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Message-Id: <20220929200523.3218710-3-kbusch@meta.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 989dfc4586..66fdb07820 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2068,13 +2068,14 @@ static bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
{
int i;
size_t alignment = bdrv_min_mem_align(bs);
+ size_t len = bs->bl.request_alignment;
IO_CODE();
for (i = 0; i < qiov->niov; i++) {
if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
return false;
}
- if (qiov->iov[i].iov_len % alignment) {
+ if (qiov->iov[i].iov_len % len) {
return false;
}
}
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 14/18] piix_ide_reset: Use pci_set_* functions instead of direct access
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (12 preceding siblings ...)
2022-09-30 16:52 ` [PULL 13/18] block: use the request length for iov alignment Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 15/18] tests/qtest/ide-test.c: Create disk image for use as a secondary Kevin Wolf
` (4 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Lev Kujawski <lkujaw@member.fsf.org>
Eliminate the remaining TODOs in hw/ide/piix.c by:
* Using pci_set_{size} functions to write the PIIX PCI configuration
space instead of manipulating it directly as an array; and
* Documenting the default register values by reference to the
controlling specification.
Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707031140.158958-1-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/ide/piix.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 9a9b28078e..de1f4f0efb 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -21,6 +21,10 @@
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
+ *
+ * References:
+ * [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
+ * 290550-002, Intel Corporation, April 1997.
*/
#include "qemu/osdep.h"
@@ -114,14 +118,11 @@ static void piix_ide_reset(DeviceState *dev)
ide_bus_reset(&d->bus[i]);
}
- /* TODO: this is the default. do not override. */
- pci_conf[PCI_COMMAND] = 0x00;
- /* TODO: this is the default. do not override. */
- pci_conf[PCI_COMMAND + 1] = 0x00;
- /* TODO: use pci_set_word */
- pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
- pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
- pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+ /* PCI command register default value (0000h) per [1, p.48]. */
+ pci_set_word(pci_conf + PCI_COMMAND, 0x0000);
+ pci_set_word(pci_conf + PCI_STATUS,
+ PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK);
+ pci_set_byte(pci_conf + 0x20, 0x01); /* BMIBA: 20-23h */
}
static int pci_piix_init_ports(PCIIDEState *d)
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 15/18] tests/qtest/ide-test.c: Create disk image for use as a secondary
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (13 preceding siblings ...)
2022-09-30 16:52 ` [PULL 14/18] piix_ide_reset: Use pci_set_* functions instead of direct access Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 16/18] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC Kevin Wolf
` (3 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Lev Kujawski <lkujaw@member.fsf.org>
Change 'tmp_path' into an array of two members to accommodate another
disk image of size TEST_IMAGE_SIZE. This facilitates testing ATA
protocol aspects peculiar to secondary devices on the same controller.
Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707031140.158958-2-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/ide-test.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 4ea89c26c9..93b4416023 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -121,7 +121,7 @@ enum {
static QPCIBus *pcibus = NULL;
static QGuestAllocator guest_malloc;
-static char *tmp_path;
+static char *tmp_path[2];
static char *debug_path;
static QTestState *ide_test_start(const char *cmdline_fmt, ...)
@@ -310,7 +310,7 @@ static QTestState *test_bmdma_setup(void)
qts = ide_test_start(
"-drive file=%s,if=ide,cache=writeback,format=raw "
"-global ide-hd.serial=%s -global ide-hd.ver=%s",
- tmp_path, "testdisk", "version");
+ tmp_path[0], "testdisk", "version");
qtest_irq_intercept_in(qts, "ioapic");
return qts;
@@ -574,7 +574,7 @@ static void test_identify(void)
qts = ide_test_start(
"-drive file=%s,if=ide,cache=writeback,format=raw "
"-global ide-hd.serial=%s -global ide-hd.ver=%s",
- tmp_path, "testdisk", "version");
+ tmp_path[0], "testdisk", "version");
dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
@@ -662,7 +662,7 @@ static void test_flush(void)
qts = ide_test_start(
"-drive file=blkdebug::%s,if=ide,cache=writeback,format=raw",
- tmp_path);
+ tmp_path[0]);
dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
@@ -713,7 +713,7 @@ static void test_pci_retry_flush(void)
qts = ide_test_start(
"-drive file=blkdebug:%s:%s,if=ide,cache=writeback,format=raw,"
"rerror=stop,werror=stop",
- debug_path, tmp_path);
+ debug_path, tmp_path[0]);
dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
@@ -892,14 +892,14 @@ static void cdrom_pio_impl(int nblocks)
/* Prepopulate the CDROM with an interesting pattern */
generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
- fh = fopen(tmp_path, "wb+");
+ fh = fopen(tmp_path[0], "wb+");
ret = fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh);
g_assert_cmpint(ret, ==, patt_blocks);
fclose(fh);
qts = ide_test_start(
"-drive if=none,file=%s,media=cdrom,format=raw,id=sr0,index=0 "
- "-device ide-cd,drive=sr0,bus=ide.0", tmp_path);
+ "-device ide-cd,drive=sr0,bus=ide.0", tmp_path[0]);
dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
qtest_irq_intercept_in(qts, "ioapic");
@@ -985,7 +985,7 @@ static void test_cdrom_dma(void)
qts = ide_test_start(
"-drive if=none,file=%s,media=cdrom,format=raw,id=sr0,index=0 "
- "-device ide-cd,drive=sr0,bus=ide.0", tmp_path);
+ "-device ide-cd,drive=sr0,bus=ide.0", tmp_path[0]);
qtest_irq_intercept_in(qts, "ioapic");
guest_buf = guest_alloc(&guest_malloc, len);
@@ -993,7 +993,7 @@ static void test_cdrom_dma(void)
prdt[0].size = cpu_to_le32(len | PRDT_EOT);
generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE);
- fh = fopen(tmp_path, "wb+");
+ fh = fopen(tmp_path[0], "wb+");
ret = fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh);
g_assert_cmpint(ret, ==, 16);
fclose(fh);
@@ -1012,6 +1012,7 @@ static void test_cdrom_dma(void)
int main(int argc, char **argv)
{
const char *base;
+ int i;
int fd;
int ret;
@@ -1035,12 +1036,14 @@ int main(int argc, char **argv)
close(fd);
/* Create a temporary raw image */
- tmp_path = g_strdup_printf("%s/qtest.XXXXXX", base);
- fd = g_mkstemp(tmp_path);
- g_assert(fd >= 0);
- ret = ftruncate(fd, TEST_IMAGE_SIZE);
- g_assert(ret == 0);
- close(fd);
+ for (i = 0; i < 2; ++i) {
+ tmp_path[i] = g_strdup_printf("%s/qtest.XXXXXX", base);
+ fd = g_mkstemp(tmp_path[i]);
+ g_assert(fd >= 0);
+ ret = ftruncate(fd, TEST_IMAGE_SIZE);
+ g_assert(ret == 0);
+ close(fd);
+ }
/* Run the tests */
g_test_init(&argc, &argv, NULL);
@@ -1064,8 +1067,10 @@ int main(int argc, char **argv)
ret = g_test_run();
/* Cleanup */
- unlink(tmp_path);
- g_free(tmp_path);
+ for (i = 0; i < 2; ++i) {
+ unlink(tmp_path[i]);
+ g_free(tmp_path[i]);
+ }
unlink(debug_path);
g_free(debug_path);
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 16/18] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (14 preceding siblings ...)
2022-09-30 16:52 ` [PULL 15/18] tests/qtest/ide-test.c: Create disk image for use as a secondary Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 17/18] tests/qtest/ide-test: Verify that DIAGNOSTIC clears DEV to zero Kevin Wolf
` (2 subsequent siblings)
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Lev Kujawski <lkujaw@member.fsf.org>
Prior to this patch, cmd_exec_dev_diagnostic relied upon
ide_set_signature to clear the device register. While the
preservation of the drive bit by ide_set_signature is necessary for
the DEVICE RESET, IDENTIFY DEVICE, and READ SECTOR commands,
ATA/ATAPI-6 specifies that "DEV shall be cleared to zero" for EXECUTE
DEVICE DIAGNOSTIC.
This deviation was uncovered by the ATACT Device Testing Program
written by Hale Landis.
Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707031140.158958-3-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/ide/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7cbc0a54a7..b747191ebf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1704,8 +1704,14 @@ static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
return false;
}
+/* EXECUTE DEVICE DIAGNOSTIC */
static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
{
+ /*
+ * Clear the device register per the ATA (v6) specification,
+ * because ide_set_signature does not clear LBA or drive bits.
+ */
+ s->select = (ATA_DEV_ALWAYS_ON);
ide_set_signature(s);
if (s->drive_kind == IDE_CD) {
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 17/18] tests/qtest/ide-test: Verify that DIAGNOSTIC clears DEV to zero
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (15 preceding siblings ...)
2022-09-30 16:52 ` [PULL 16/18] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-09-30 16:52 ` [PULL 18/18] hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS command Kevin Wolf
2022-10-03 23:02 ` [PULL 00/18] Block layer patches Stefan Hajnoczi
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Lev Kujawski <lkujaw@member.fsf.org>
Verify correction of EXECUTE DEVICE DIAGNOSTIC introduced in commit
72423831c3 (hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE
DIAGNOSTIC, 2022-05-28).
Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707031140.158958-4-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/ide-test.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 93b4416023..dbe1563b23 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -90,6 +90,7 @@ enum {
enum {
CMD_DSM = 0x06,
+ CMD_DIAGNOSE = 0x90,
CMD_READ_DMA = 0xc8,
CMD_WRITE_DMA = 0xca,
CMD_FLUSH_CACHE = 0xe7,
@@ -614,6 +615,36 @@ static void test_identify(void)
free_pci_device(dev);
}
+static void test_diagnostic(void)
+{
+ QTestState *qts;
+ QPCIDevice *dev;
+ QPCIBar bmdma_bar, ide_bar;
+ uint8_t data;
+
+ qts = ide_test_start(
+ "-blockdev driver=file,node-name=hda,filename=%s "
+ "-blockdev driver=file,node-name=hdb,filename=%s "
+ "-device ide-hd,drive=hda,bus=ide.0,unit=0 "
+ "-device ide-hd,drive=hdb,bus=ide.0,unit=1 ",
+ tmp_path[0], tmp_path[1]);
+
+ dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
+
+ /* DIAGNOSE command on device 1 */
+ qpci_io_writeb(dev, ide_bar, reg_device, DEV);
+ data = qpci_io_readb(dev, ide_bar, reg_device);
+ g_assert_cmphex(data & DEV, ==, DEV);
+ qpci_io_writeb(dev, ide_bar, reg_command, CMD_DIAGNOSE);
+
+ /* Verify that DEVICE is now 0 */
+ data = qpci_io_readb(dev, ide_bar, reg_device);
+ g_assert_cmphex(data & DEV, ==, 0);
+
+ ide_test_quit(qts);
+ free_pci_device(dev);
+}
+
/*
* Write sector 1 with random data to make IDE storage dirty
* Needed for flush tests so that flushes actually go though the block layer
@@ -1050,6 +1081,8 @@ int main(int argc, char **argv)
qtest_add_func("/ide/identify", test_identify);
+ qtest_add_func("/ide/diagnostic", test_diagnostic);
+
qtest_add_func("/ide/bmdma/simple_rw", test_bmdma_simple_rw);
qtest_add_func("/ide/bmdma/trim", test_bmdma_trim);
qtest_add_func("/ide/bmdma/various_prdts", test_bmdma_various_prdts);
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PULL 18/18] hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS command
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (16 preceding siblings ...)
2022-09-30 16:52 ` [PULL 17/18] tests/qtest/ide-test: Verify that DIAGNOSTIC clears DEV to zero Kevin Wolf
@ 2022-09-30 16:52 ` Kevin Wolf
2022-10-03 23:02 ` [PULL 00/18] Block layer patches Stefan Hajnoczi
18 siblings, 0 replies; 20+ messages in thread
From: Kevin Wolf @ 2022-09-30 16:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, qemu-devel
From: Lev Kujawski <lkujaw@member.fsf.org>
CHS-based disk utilities and operating systems may adjust the logical
geometry of a hard drive to cope with the expectations or limitations
of software using the ATA INITIALIZE_DEVICE_PARAMETERS command.
Prior to this patch, INITIALIZE_DEVICE_PARAMETERS was a nop that
always returned success, raising the possibility of data loss or
corruption if the CHS<->LBA translation redirected a write to the
wrong sector.
* hw/ide/core.c
ide_reset():
Reset the logical CHS geometry of the hard disk when the power-on
defaults feature is enabled.
cmd_specify():
a) New function implementing INITIALIZE_DEVICE_PARAMETERS.
b) Ignore calls for empty or ATAPI devices.
cmd_set_features():
Implement the power-on defaults enable and disable features.
struct ide_cmd_table:
Switch WIN_SPECIFY from cmd_nop() to cmd_specify().
ide_init_drive():
Set new fields 'drive_heads' and 'drive_sectors' based upon the
actual disk geometry.
* include/hw/ide/internal.h
struct IDEState:
a) Store the actual drive CHS values within the new fields
'drive_heads' and 'drive_sectors.'
b) Track whether a soft IDE reset should also reset the logical CHS
geometry of the hard disk within the new field 'reset_reverts'.
Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707031140.158958-7-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/hw/ide/internal.h | 3 +++
hw/ide/core.c | 29 ++++++++++++++++++++++++++---
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 97e7e59dc5..b17f36df95 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -375,6 +375,7 @@ struct IDEState {
uint8_t unit;
/* ide config */
IDEDriveKind drive_kind;
+ int drive_heads, drive_sectors;
int cylinders, heads, sectors, chs_trans;
int64_t nb_sectors;
int mult_sectors;
@@ -401,6 +402,8 @@ struct IDEState {
uint8_t select;
uint8_t status;
+ bool reset_reverts;
+
/* set for lba48 access */
uint8_t lba48;
BlockBackend *blk;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index b747191ebf..39afdc0006 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1340,6 +1340,11 @@ static void ide_reset(IDEState *s)
s->pio_aiocb = NULL;
}
+ if (s->reset_reverts) {
+ s->reset_reverts = false;
+ s->heads = s->drive_heads;
+ s->sectors = s->drive_sectors;
+ }
if (s->drive_kind == IDE_CFATA)
s->mult_sectors = 0;
else
@@ -1618,6 +1623,20 @@ static bool cmd_check_power_mode(IDEState *s, uint8_t cmd)
return true;
}
+/* INITIALIZE DEVICE PARAMETERS */
+static bool cmd_specify(IDEState *s, uint8_t cmd)
+{
+ if (s->blk && s->drive_kind != IDE_CD) {
+ s->heads = (s->select & (ATA_DEV_HS)) + 1;
+ s->sectors = s->nsector;
+ ide_set_irq(s->bus);
+ } else {
+ ide_abort_command(s);
+ }
+
+ return true;
+}
+
static bool cmd_set_features(IDEState *s, uint8_t cmd)
{
uint16_t *identify_data;
@@ -1641,7 +1660,11 @@ static bool cmd_set_features(IDEState *s, uint8_t cmd)
ide_flush_cache(s);
return false;
case 0xcc: /* reverting to power-on defaults enable */
+ s->reset_reverts = true;
+ return true;
case 0x66: /* reverting to power-on defaults disable */
+ s->reset_reverts = false;
+ return true;
case 0xaa: /* read look-ahead enable */
case 0x55: /* read look-ahead disable */
case 0x05: /* set advanced power management mode */
@@ -2051,7 +2074,7 @@ static const struct {
[WIN_SEEK] = { cmd_seek, HD_CFA_OK | SET_DSC },
[CFA_TRANSLATE_SECTOR] = { cmd_cfa_translate_sector, CFA_OK },
[WIN_DIAGNOSE] = { cmd_exec_dev_diagnostic, ALL_OK },
- [WIN_SPECIFY] = { cmd_nop, HD_CFA_OK | SET_DSC },
+ [WIN_SPECIFY] = { cmd_specify, HD_CFA_OK | SET_DSC },
[WIN_STANDBYNOW2] = { cmd_nop, HD_CFA_OK },
[WIN_IDLEIMMEDIATE2] = { cmd_nop, HD_CFA_OK },
[WIN_STANDBY2] = { cmd_nop, HD_CFA_OK },
@@ -2541,8 +2564,8 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
blk_get_geometry(blk, &nb_sectors);
s->cylinders = cylinders;
- s->heads = heads;
- s->sectors = secs;
+ s->heads = s->drive_heads = heads;
+ s->sectors = s->drive_sectors = secs;
s->chs_trans = chs_trans;
s->nb_sectors = nb_sectors;
s->wwn = wwn;
--
2.37.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PULL 00/18] Block layer patches
2022-09-30 16:52 [PULL 00/18] Block layer patches Kevin Wolf
` (17 preceding siblings ...)
2022-09-30 16:52 ` [PULL 18/18] hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS command Kevin Wolf
@ 2022-10-03 23:02 ` Stefan Hajnoczi
18 siblings, 0 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2022-10-03 23:02 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, kwolf, stefanha, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 115 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/7.2 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread