On 7/11/22 14:07, Denis V. Lunev wrote:
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.
I'm not sure that 5f76a7aac156ca is really the corner stone.. But yes,
libvirt moved to "blockdev era", which means that we don't use old -drive,
instead block nodes are created by -blockdev / qmp: blockdev-add, and attached
to block devices by node-name.
And if I understand correctly blockdev_init() is called only on -drive path.
I have some questions:
1. After this patch, don't we call block_acct_setup() twice on old path with
-drive? That seems safe as block_acct_setup just assign fields of
BlockAcctStats.. But that's doesn't look good.
2. Do we really need these options? Could we instead just enable accounting
invalid and failed ops unconditionally? I doubt that someone will learn that
these new options appeared and will use them to disable the failed/invalid
accounting again.
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. It adds
account_invalid/account_failed properties to BlockConf and setups them
accordingly.
Signed-off-by: Denis V. Lunev <d...@openvz.org>
CC: Peter Krempa <pkre...@redhat.com>
CC: Markus Armbruster <arm...@redhat.com>
CC: John Snow <js...@redhat.com>
CC: Kevin Wolf <kw...@redhat.com>
CC: Hanna Reitz <hre...@redhat.com>
---
hw/block/block.c | 2 +
include/hw/block/block.h | 7 +++-
tests/qemu-iotests/172.out | 76 ++++++++++++++++++++++++++++++++++++++
3 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/hw/block/block.c b/hw/block/block.c
index 25f45df723..53b100cdc3 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -205,6 +205,8 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool
readonly,
BlockdevOnError rerror;
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..ffd439fc83 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;
+ bool 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_BOOL("account-invalid", _state, \
+ _conf.account_invalid, true), \
+ DEFINE_PROP_BOOL("account-failed", _state, \
+ _conf.account_failed, true)
#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..a6c451e098 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -28,6 +28,8 @@ Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=737280
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+ account-invalid = true
+ account-failed = true
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 = true
+ account-failed = true
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 = true
+ account-failed = true
drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
@@ -104,6 +110,8 @@ Testing: -fdb TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
[..]
--
Best regards,
Vladimir
