From 38f034e728e5a60c27c3ac43ed2bc4ae7a6659b0 Mon Sep 17 00:00:00 2001 From: lu zhipeng Date: Wed, 21 Sep 2022 22:45:15 +0800 Subject: [PATCH 01/18] qcow2: fix memory leak in qcow2_read_extensions Free feature_table if it is failed in bdrv_pread. Signed-off-by: lu zhipeng Message-Id: <20220921144515.1166-1-luzhipeng@cestc.cn> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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; } From 8485563aa6b0e32f8509184b202d285fce2169bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 19 Sep 2022 20:27:55 +0200 Subject: [PATCH 02/18] block/qcow2-bitmap: Add missing cast to silent GCC error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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é Message-Id: <20220919182755.51967-1-f4bug@amsat.org> Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf --- 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: From 7f118b433a2e8c98fe5a8d73f0dd529aada327d1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 8 Sep 2022 15:08:42 +0200 Subject: [PATCH 03/18] qemu-img: Wean documentation and help output off '?' for help '?' 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 Message-Id: <20220908130842.641410-1-armbru@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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" From b2aaf354773417f034fdc1a56cdb76ad79de6e19 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 24 Aug 2022 11:50:43 +0200 Subject: [PATCH 04/18] block: pass OnOffAuto instead of bool to block_acct_setup() We would have one more place for block_acct_setup() calling, which should not corrupt original value. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Peter Krempa CC: Markus Armbruster CC: John Snow CC: Kevin Wolf CC: Hanna Reitz Message-Id: <20220824095044.166009-2-den@openvz.org> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/accounting.c | 22 ++++++++++++++++++---- blockdev.c | 17 ++++++++++++++--- include/block/accounting.h | 6 +++--- 3 files changed, 35 insertions(+), 10 deletions(-) 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); 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, From 62a6c300f18a9f6994baf6d767985425d48de427 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 24 Aug 2022 11:50:44 +0200 Subject: [PATCH 05/18] block: add missed block_acct_setup with new block device init procedure 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 Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Peter Krempa CC: Markus Armbruster CC: John Snow CC: Kevin Wolf CC: Hanna Reitz Message-Id: <20220824095044.166009-3-den@openvz.org> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/accounting.c | 8 +++- hw/block/block.c | 2 + include/hw/block/block.h | 7 +++- tests/qemu-iotests/172.out | 76 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/227.out | 4 +- 5 files changed, 92 insertions(+), 5 deletions(-) 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/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/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 From 006e1962449f8ea9f0822b838d9fdf13c98eb147 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 17 Aug 2022 10:37:35 +0200 Subject: [PATCH 06/18] block: use bdrv_is_sg() helper instead of raw bs->sg reading 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 CC: Kevin Wolf CC: Hanna Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Ronnie Sahlberg CC: Paolo Bonzini CC: Peter Lieven CC: Vladimir Sementsov-Ogievskiy Message-Id: <20220817083736.40981-2-den@openvz.org> Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf --- 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; } From 131498f775ea18852992dc56441eba33de3fcde5 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 17 Aug 2022 10:37:36 +0200 Subject: [PATCH 07/18] block: make serializing requests functions 'void' 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 CC: Kevin Wolf CC: Hanna Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Ronnie Sahlberg CC: Paolo Bonzini CC: Peter Lieven CC: Vladimir Sementsov-Ogievskiy Message-Id: <20220817083736.40981-3-den@openvz.org> Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf --- block/io.c | 23 +++++++---------------- include/block/block_int-io.h | 2 +- 2 files changed, 8 insertions(+), 17 deletions(-) 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, 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); From 9a891a91a19a9b999fec00ce6e27bc71c93273ec Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 11 Aug 2022 12:49:05 -0400 Subject: [PATCH 08/18] gluster: stop using .bdrv_needs_filename 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 Signed-off-by: Stefan Hajnoczi Message-Id: <20220811164905.430834-1-stefanha@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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, From ec64b1ca08138b5f57d270a5756a9138f897ee5a Mon Sep 17 00:00:00 2001 From: Hanna Reitz Date: Wed, 3 Aug 2022 16:44:44 +0200 Subject: [PATCH 09/18] block/qcow2: Keep auto_backing_file if possible 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 Message-Id: <20220803144446.20723-2-hreitz@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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); + } } /* From dc70638ff616ef0a1a6e4f61ce1f3f5282d2c942 Mon Sep 17 00:00:00 2001 From: Hanna Reitz Date: Wed, 3 Aug 2022 16:44:45 +0200 Subject: [PATCH 10/18] block/qed: Keep auto_backing_file if possible 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 Message-Id: <20220803144446.20723-3-hreitz@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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"); From 283153f10ab474cb2f14cc2b3f272edcf13aedaf Mon Sep 17 00:00:00 2001 From: Hanna Reitz Date: Wed, 3 Aug 2022 16:44:46 +0200 Subject: [PATCH 11/18] iotests/backing-file-invalidation: Add new test 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 Message-Id: <20220803144446.20723-4-hreitz@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- .../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 . +# + +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 From a7c5f67a78569f8c275ea4ea9962e9c79b9d03cb Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Thu, 29 Sep 2022 13:05:22 -0700 Subject: [PATCH 12/18] block: move bdrv_qiov_is_aligned to file-posix 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 Message-Id: <20220929200523.3218710-2-kbusch@meta.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/file-posix.c | 21 +++++++++++++++++++++ block/io.c | 21 --------------------- include/block/block-io.h | 1 - 3 files changed, 21 insertions(+), 22 deletions(-) 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; 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); From 25474d90aa50bd32e0de395a33d8de42dd6f2aef Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Thu, 29 Sep 2022 13:05:23 -0700 Subject: [PATCH 13/18] block: use the request length for iov alignment An iov length needs to be aligned to the logical block size, which may be larger than the memory alignment. Tested-by: Jens Axboe Signed-off-by: Keith Busch Message-Id: <20220929200523.3218710-3-kbusch@meta.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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; } } From 4851a986b205ac8fb133164cd15d9bb983d8502d Mon Sep 17 00:00:00 2001 From: Lev Kujawski Date: Thu, 7 Jul 2022 03:11:34 +0000 Subject: [PATCH 14/18] piix_ide_reset: Use pci_set_* functions instead of direct access 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 Message-Id: <20220707031140.158958-1-lkujaw@member.fsf.org> Signed-off-by: Kevin Wolf --- 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) From ecfcf71314bc1548f387a035e1061296ec8f4ec3 Mon Sep 17 00:00:00 2001 From: Lev Kujawski Date: Thu, 7 Jul 2022 03:11:35 +0000 Subject: [PATCH 15/18] tests/qtest/ide-test.c: Create disk image for use as a secondary 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 Message-Id: <20220707031140.158958-2-lkujaw@member.fsf.org> Signed-off-by: Kevin Wolf --- 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); From 3195c9e6abe96b367a671474e850ae3f89781880 Mon Sep 17 00:00:00 2001 From: Lev Kujawski Date: Thu, 7 Jul 2022 03:11:36 +0000 Subject: [PATCH 16/18] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC 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 Message-Id: <20220707031140.158958-3-lkujaw@member.fsf.org> Signed-off-by: Kevin Wolf --- 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) { From 2cc38a02e61e0f4d858df2cfa5388f72bddcd634 Mon Sep 17 00:00:00 2001 From: Lev Kujawski Date: Thu, 7 Jul 2022 03:11:37 +0000 Subject: [PATCH 17/18] tests/qtest/ide-test: Verify that DIAGNOSTIC clears DEV to zero 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 Message-Id: <20220707031140.158958-4-lkujaw@member.fsf.org> Signed-off-by: Kevin Wolf --- 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); From 176e4961bb33d559da1af441fb0ee2e0cb8245ae Mon Sep 17 00:00:00 2001 From: Lev Kujawski Date: Thu, 7 Jul 2022 03:11:40 +0000 Subject: [PATCH 18/18] hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS command 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 Message-Id: <20220707031140.158958-7-lkujaw@member.fsf.org> Signed-off-by: Kevin Wolf --- hw/ide/core.c | 29 ++++++++++++++++++++++++++--- include/hw/ide/internal.h | 3 +++ 2 files changed, 29 insertions(+), 3 deletions(-) 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; 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;