block: add block device shared field
PATCH 1/1 block: add block device shared field
This adds a boolean option called 'shared' to block devices. It defaults to off/false. When enabled for a particular block device, the 'shared' option causes the block migration code to skip over syncing of that device. This allows controlling exactly which block devices get synced during a migration.
--- block.c | 7 +++++++ block/qapi.c | 2 ++ include/block/block.h | 1 + include/block/block_int.h | 3 +++ migration/block.c | 4 ++++ qapi/block-core.json | 2 +- 6 files changed, 18 insertions(+), 1 deletion(-)
block.c +7 -0
@@ -2466,6 +2466,13 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,bdrv_backing_options (&flags, options, flags, options); } +/* Check for shared flag */ +/* See cautionary note on accessing @options above */ + bs->shared = +g_strcmp0 (qdict_get_try_str (options,BDRV_OPT_SHARED ),"on" ) ==0 || +qdict_get_try_bool (options,BDRV_OPT_SHARED , false); +qdict_del (options,BDRV_OPT_SHARED ); + bs->open_flags = flags; bs->options = options; options =qdict_clone_shallow (options);
block/qapi.c +2 -0
@@ -66,6 +66,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, info->detect_zeroes = bs->detect_zeroes; + info->shared = bs->shared; +if (blk &&blk_get_public (blk)->throttle_state) {ThrottleConfig cfg;
include/block/block.h +1 -0
@@ -110,6 +110,7 @@ typedef struct HDGeometry {#define BDRV_OPT_READ_ONLY "read-only"#define BDRV_OPT_DISCARD "discard"#define BDRV_OPT_FORCE_SHARE "force-share" +#define BDRV_OPT_SHARED "shared"#define BDRV_SECTOR_BITS 9
include/block/block_int.h +3 -0
@@ -672,6 +672,9 @@ struct BlockDriverState {/* Only read/written by whoever has set active_flush_req to true. */ unsigned int flushed_gen;/* Flushed write generation */ + +/* Shared devices are not migrated. */ +bool shared; };struct BlockBackendRootState {
migration/block.c +4 -0
@@ -402,6 +402,10 @@ static int init_blk_migration(QEMUFile *f) bmds_bs =g_malloc0 (num_bs *sizeof (*bmds_bs));for (i =0 , bs =bdrv_first (&it); bs; bs =bdrv_next (&it), i++) { +if (bs->shared) { +continue ; + } +if (bdrv_is_read_only (bs)) {continue ; }
qapi/block-core.json +1 -1
@@ -365,7 +365,7 @@ '*bps_wr_max_length': 'int', '*iops_max_length': 'int', '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int', '*iops_size': 'int', '*group': 'str', 'cache': 'BlockdevCacheInfo', - 'write_threshold': 'int' } } + 'write_threshold': 'int', 'shared': 'bool' } } ## # @BlockDeviceIoStatus:
Signature · 1 line
2.13.1
Quoted 18 lines · Brian Steffens
This adds a boolean option called 'shared' to block devices. It defaults to off/false. When enabled for a particular block device, the 'shared' option causes the block migration code to skip over syncing of that device. This allows controlling exactly which block devices get synced during a migration.
Signed-off-by Brian Steffens <[email protected]>--- block.c | 7 +++++++ block/qapi.c | 2 ++ include/block/block.h | 1 + include/block/block_int.h | 3 +++ migration/block.c | 4 ++++ qapi/block-core.json | 2 +- 6 files changed, 18 insertions(+), 1 deletion(-)+++
b/qapi/block-core.json
qapi/block-core.json line 368
Quoted 5 lines
qapi/block-core.json +1
-1@@ -365,7 +365,7 @@ '*bps_wr_max_length': 'int', '*iops_max_length': 'int', '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int', '*iops_size': 'int', '*group': 'str', 'cache': 'BlockdevCacheInfo', - 'write_threshold': 'int' } } + 'write_threshold': 'int', 'shared': 'bool' } }
Missing documentation, which should include '(since 2.11)'. The new parameter should be optional on input; but it looks like you've only modified a structure that is present on output - so you're missing a QAPI edit that mirrors the command-line edit.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 6 lines · Brian Steffens
This adds a boolean option called 'shared' to block devices. It defaults to off/false. When enabled for a particular block device, the 'shared' option causes the block migration code to skip over syncing of that device. This allows controlling exactly which block devices get synced during a migration.
Signed-off-by Brian Steffens <[email protected]>
Quoted 8 lines
--- block.c | 7 +++++++ block/qapi.c | 2 ++ include/block/block.h | 1 + include/block/block_int.h | 3 +++ migration/block.c | 4 ++++ qapi/block-core.json | 2 +- 6 files changed, 18 insertions(+), 1 deletion(-)
Thanks for the patch! Please email the relevant maintainers:
$ scripts/get_maintainer.pl -f block.c Kevin Wolf <[email protected]> (supporter:Block layer core) Max Reitz <[email protected]> (supporter:Block layer core) [email protected] (open list:Block layer core) [email protected] (open list:All patches CC here)
I have CCed them so they see this email.
migration/block.c static int init_blk_migration(QEMUFile *f) · line 408
Quoted 12 lines
migration/block.c +4
-0@@ -402,6 +402,10 @@ static int init_blk_migration(QEMUFile *f) bmds_bs =g_malloc0 (num_bs *sizeof (*bmds_bs));for (i =0 , bs =bdrv_first (&it); bs; bs =bdrv_next (&it), i++) { +if (bs->shared) { +continue ; + } +
Have you considered extending the 'migrate' command with a list of drives instead?
{ 'command': 'migrate',
'data': {'*blk': 'bool',
'*drives': ['str'], <--- new!
...}}That way the set of drives doesn't need to be decided until migration time. It avoids adding new state to BlockDriverState that is used only by the legacy block/migration.c code.
In case you haven't seen it, the preferred approach for non-shared
storage migration is drive-mirror + NBD. The block/migration.c code is
an older feature that is less flexible. More info on drive-mirror +
NBD:
http://wiki.libvirt.org/page/NBD_storage_migration
Slide 29 in http://events.linuxfoundation.org/sites/events/files/slides/A-Practical-Look-at-QEMU-Block-Layer-Primitives.pdf
Stefan
Thanks for taking a look at the patch and fixing the missing CC addresses!
Quoted 2 lines
Have you considered extending the 'migrate' command with a list of drives instead?
That was my original plan but I thought having information on whether a device is shared or not could potentially be useful to other systems. If not then you're right, it would be a lighter touch to add it only to the 'migrate' command.
Quoted 4 lines
In case you haven't seen it, the preferred approach for non-shared storage migration is drive-mirror + NBD. The
block/migration.ccode is an older feature that is less flexible. More info on drive-mirror + NBD:
I didn't realize drive-mirror + NBD was the preferred approach. Are there any plans to drop support for the block/migration.c style?
Quoted 33 lines · Stefan Hajnoczi
Quoted 6 lines · Brian Steffens
This adds a boolean option called 'shared' to block devices. It defaults to off/false. When enabled for a particular block device, the 'shared' option causes the block migration code to skip over syncing of that device. This allows controlling exactly which block devices get synced during a migration.
Signed-off-by Brian Steffens <[email protected]>Quoted 8 lines
--- block.c | 7 +++++++ block/qapi.c | 2 ++ include/block/block.h | 1 + include/block/block_int.h | 3 +++ migration/block.c | 4 ++++ qapi/block-core.json | 2 +- 6 files changed, 18 insertions(+), 1 deletion(-)Thanks for the patch! Please email the relevant maintainers:
$ scripts/get_maintainer.pl -f block.c Kevin Wolf <[email protected]> (supporter:Block layer core) Max Reitz <[email protected]> (supporter:Block layer core) [email protected] (open list:Block layer core) [email protected] (open list:All patches CC here)I have CCed them so they see this email.
Quoted 7 lines
migration/block.c +0
-0@@ -402,6 +402,10 @@ static int init_blk_migration(QEMUFile *f) bmds_bs =g_malloc0 (num_bs *sizeof (*bmds_bs));
Quoted 2 lines
migration/block.c static int init_blk_migration(QEMUFile *f) · line 405
Quoted 2 lines
migration/block.c +1
-0for (i =0 , bs =bdrv_first (&it); bs; bs =bdrv_next (&it), i++) { +if (bs->shared) {
If we go with this approach, I'd prefer a more specific name like bs->skip_block_migration; "shared" has a lot meanings so is less readable.
Fam
Quoted 3 lines
Quoted 3 lines
migration/block.c +3
-0+continue ; + } +
Quoted 7 lines · Brian Steffens
Quoted 2 lines
Have you considered extending the 'migrate' command with a list of drives instead?
That was my original plan but I thought having information on whether a device is shared or not could potentially be useful to other systems. If not then you're right, it would be a lighter touch to add it only to the 'migrate' command.
I agree with Fam that bs->shared doesn't necessarily have the same meaning to everyone. Therefore I suggest extending the 'migrate' command instead.
Quoted 7 lines
Quoted 4 lines
In case you haven't seen it, the preferred approach for non-shared storage migration is drive-mirror + NBD. The
block/migration.ccode is an older feature that is less flexible. More info on drive-mirror + NBD:I didn't realize drive-mirror + NBD was the preferred approach. Are there any plans to drop support for the
block/migration.cstyle?
migration/block.c is not formally deprecated yet. It's still used by libvirt in situations where NBD connections are not desirable. Eventually it will probably be deprecated and then removed after 2 more QEMU releases according to the deprecation policy (see qemu-doc.texi).
Stefan