* [PATCH v2] block: add missed block_acct_setup with new block device init procedure
@ 2022-08-17 9:28 Denis V. Lunev
2022-08-17 9:28 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
2022-08-17 9:28 ` [PATCH 2/2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
0 siblings, 2 replies; 4+ messages in thread
From: Denis V. Lunev @ 2022-08-17 9:28 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: Denis V . Lunev, Peter Krempa, Markus Armbruster, John Snow,
Kevin Wolf, Hanna Reitz, Vladimir Sementsov-Ogievskiy
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
Changes from v1:
* set account_invalid/account_failed to true by default
* pass OnOffAuto to block_acct_init() to handle double initialization (patch 1)
* changed properties on BLK device to OnOffAuto
Signed-off-by: Denis V. Lunev <den@openvz.org>
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>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-08-17 9:28 [PATCH v2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
@ 2022-08-17 9:28 ` Denis V. Lunev
2022-08-19 16:15 ` Vladimir Sementsov-Ogievskiy
2022-08-17 9:28 ` [PATCH 2/2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
1 sibling, 1 reply; 4+ messages in thread
From: Denis V. Lunev @ 2022-08-17 9:28 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: Denis V. Lunev, Peter Krempa, Markus Armbruster, John Snow,
Kevin Wolf, Hanna Reitz, Vladimir Sementsov-Ogievskiy
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>
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>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/accounting.c | 24 ++++++++++++++++++++----
blockdev.c | 17 ++++++++++++++---
include/block/accounting.h | 6 +++---
3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 2030851d79..73ac639656 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -40,11 +40,27 @@ 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,
+ stats->account_invalid);
+ stats->account_failed = bool_from_onoffauto(account_failed,
+ stats->account_failed);
}
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,
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] block: add missed block_acct_setup with new block device init procedure
2022-08-17 9:28 [PATCH v2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
2022-08-17 9:28 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
@ 2022-08-17 9:28 ` Denis V. Lunev
1 sibling, 0 replies; 4+ messages in thread
From: Denis V. Lunev @ 2022-08-17 9:28 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: Denis V. Lunev, Peter Krempa, Markus Armbruster, John Snow,
Kevin Wolf, Hanna Reitz, Vladimir Sementsov-Ogievskiy
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>
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>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/accounting.c | 2 +
hw/block/block.c | 2 +
include/hw/block/block.h | 7 +++-
qapi/misc.json | 9 +----
tests/qemu-iotests/172.out | 76 ++++++++++++++++++++++++++++++++++++++
5 files changed, 87 insertions(+), 9 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 73ac639656..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)
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/qapi/misc.json b/qapi/misc.json
index 27ef5a2b20..f469cd0ded 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -210,12 +210,6 @@
#
# @cpu-index: The CPU to use for commands that require an implicit CPU
#
-# Features:
-# @savevm-monitor-nodes: If present, HMP command savevm only snapshots
-# monitor-owned nodes if they have no parents.
-# This allows the use of 'savevm' with
-# -blockdev. (since 4.2)
-#
# Returns: the output of the command as a string
#
# Since: 0.14
@@ -243,8 +237,7 @@
##
{ 'command': 'human-monitor-command',
'data': {'command-line': 'str', '*cpu-index': 'int'},
- 'returns': 'str',
- 'features': [ 'savevm-monitor-nodes' ] }
+ 'returns': 'str' }
##
# @getfd:
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 @@ Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=737280
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]
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-08-17 9:28 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
@ 2022-08-19 16:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-08-19 16:15 UTC (permalink / raw)
To: Denis V. Lunev, qemu-block, qemu-devel
Cc: Peter Krempa, Markus Armbruster, John Snow, Kevin Wolf, Hanna Reitz
On 8/17/22 12:28, Denis V. Lunev wrote:
> 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>
> 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>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> block/accounting.c | 24 ++++++++++++++++++++----
> blockdev.c | 17 ++++++++++++++---
> include/block/accounting.h | 6 +++---
> 3 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/block/accounting.c b/block/accounting.c
> index 2030851d79..73ac639656 100644
> --- a/block/accounting.c
> +++ b/block/accounting.c
> @@ -40,11 +40,27 @@ 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,
> + stats->account_invalid);
> + stats->account_failed = bool_from_onoffauto(account_failed,
> + stats->account_failed);
> }
>
> 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");
Hm. Actually here you change default behavior from true to false.
In the next patch you fix it back, but better is to avoid the extra change somehow.
>
> 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,
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-19 16:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 9:28 [PATCH v2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
2022-08-17 9:28 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
2022-08-19 16:15 ` Vladimir Sementsov-Ogievskiy
2022-08-17 9:28 ` [PATCH 2/2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.