* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
* [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-08-24 9:50 [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
@ 2022-08-24 9:50 ` Denis V. Lunev
0 siblings, 0 replies; 7+ messages in thread
From: Denis V. Lunev @ 2022-08-24 9:50 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: Denis V. Lunev, Vladimir Sementsov-Ogievskiy, Peter Krempa,
Markus Armbruster, John Snow, Kevin Wolf, Hanna Reitz
We would have one more place for block_acct_setup() calling, which should
not corrupt original value.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Peter Krempa <pkrempa@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
---
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,
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-08-23 9:57 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
@ 2022-08-23 16:40 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-08-23 16:40 UTC (permalink / raw)
To: Denis V. Lunev, qemu-block, qemu-devel
Cc: Peter Krempa, Markus Armbruster, John Snow, Kevin Wolf, Hanna Reitz
On 8/23/22 12:57, 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>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
2022-08-23 9:57 [PATCH v4 0/2] " Denis V. Lunev
@ 2022-08-23 9:57 ` Denis V. Lunev
2022-08-23 16:40 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 7+ messages in thread
From: Denis V. Lunev @ 2022-08-23 9:57 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 | 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,
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-24 11:58 UTC | newest]
Thread overview: 7+ 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
2022-08-23 9:57 [PATCH v4 0/2] " Denis V. Lunev
2022-08-23 9:57 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() Denis V. Lunev
2022-08-23 16:40 ` Vladimir Sementsov-Ogievskiy
2022-08-24 9:50 [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure Denis V. Lunev
2022-08-24 9:50 ` [PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup() 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.