We use these two options to identify which disk is
shared
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
v4:
- Add proper comment for primary_disk (Stefan)
v2:
- Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
- Fix comments for these two options
---
block/replication.c | 43 +++++++++++++++++++++++++++++++++++++++++--
qapi/block-core.json | 10 +++++++++-
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index bf3c395..418b81b 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -25,9 +25,12 @@
typedef struct BDRVReplicationState {
ReplicationMode mode;
int replication_state;
+ bool is_shared_disk;
+ char *shared_disk_id;
BdrvChild *active_disk;
BdrvChild *hidden_disk;
BdrvChild *secondary_disk;
+ BdrvChild *primary_disk;
char *top_id;
ReplicationState *rs;
Error *blocker;
@@ -53,6 +56,9 @@ static void replication_stop(ReplicationState *rs, bool failover,
#define REPLICATION_MODE "mode"
#define REPLICATION_TOP_ID "top-id"
+#define REPLICATION_SHARED_DISK "shared-disk"
+#define REPLICATION_SHARED_DISK_ID "shared-disk-id"
+
static QemuOptsList replication_runtime_opts = {
.name = "replication",
.head = QTAILQ_HEAD_INITIALIZER(replication_runtime_opts.head),
@@ -65,6 +71,14 @@ static QemuOptsList replication_runtime_opts = {
.name = REPLICATION_TOP_ID,
.type = QEMU_OPT_STRING,
},
+ {
+ .name = REPLICATION_SHARED_DISK_ID,
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = REPLICATION_SHARED_DISK,
+ .type = QEMU_OPT_BOOL,
+ },
{ /* end of list */ }
},
};
@@ -85,6 +99,9 @@ static int replication_open(BlockDriverState *bs, QDict *options,
QemuOpts *opts = NULL;
const char *mode;
const char *top_id;
+ const char *shared_disk_id;
+ BlockBackend *blk;
+ BlockDriverState *tmp_bs;
bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
false, errp);
@@ -125,12 +142,33 @@ static int replication_open(BlockDriverState *bs, QDict *options,
"The option mode's value should be primary or secondary");
goto fail;
}
+ s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
+ false);
+ if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
+ shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
+ if (!shared_disk_id) {
+ error_setg(&local_err, "Missing shared disk blk option");
+ goto fail;
+ }
+ s->shared_disk_id = g_strdup(shared_disk_id);
+ blk = blk_by_name(s->shared_disk_id);
+ if (!blk) {
+ error_setg(&local_err, "There is no %s block", s->shared_disk_id);
+ goto fail;
+ }
+ /* We have a BlockBackend for the primary disk but use BdrvChild for
+ * consistency - active_disk, secondary_disk, etc are also BdrvChild.
+ */
+ tmp_bs = blk_bs(blk);
+ s->primary_disk = QLIST_FIRST(&tmp_bs->parents);
+ }
s->rs = replication_new(bs, &replication_ops);
- ret = 0;
-
+ qemu_opts_del(opts);
+ return 0;
fail:
+ g_free(s->shared_disk_id);
qemu_opts_del(opts);
error_propagate(errp, local_err);
@@ -141,6 +179,7 @@ static void replication_close(BlockDriverState *bs)
{
BDRVReplicationState *s = bs->opaque;
+ g_free(s->shared_disk_id);
if (s->replication_state == BLOCK_REPLICATION_RUNNING) {
replication_stop(s->rs, false, NULL);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 033457c..361c932 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2661,12 +2661,20 @@
# node who owns the replication node chain. Must not be given in
# primary mode.
#
+# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
+# is true, this option is required (Since: 2.10)
+#
+# @shared-disk: To indicate whether or not a disk is shared by primary VM
+# and secondary VM. (The default is false) (Since: 2.10)
+#
# Since: 2.9
##
{ 'struct': 'BlockdevOptionsReplication',
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
- '*top-id': 'str' } }
+ '*top-id': 'str',
+ '*shared-disk-id': 'str',
+ '*shared-disk': 'bool' } }
##
# @NFSTransport:
--
1.8.3.1
On 04/12/2017 09:05 AM, zhanghailiang wrote:
> We use these two options to identify which disk is
> shared
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> v4:
> - Add proper comment for primary_disk (Stefan)
> v2:
> - Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
> - Fix comments for these two options
> ---
> +++ b/qapi/block-core.json
> @@ -2661,12 +2661,20 @@
> # node who owns the replication node chain. Must not be given in
> # primary mode.
> #
> +# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
> +# is true, this option is required (Since: 2.10)
> +#
> +# @shared-disk: To indicate whether or not a disk is shared by primary VM
> +# and secondary VM. (The default is false) (Since: 2.10)
> +#
> # Since: 2.9
> ##
> { 'struct': 'BlockdevOptionsReplication',
> 'base': 'BlockdevOptionsGenericFormat',
> 'data': { 'mode': 'ReplicationMode',
> - '*top-id': 'str' } }
> + '*top-id': 'str',
> + '*shared-disk-id': 'str',
> + '*shared-disk': 'bool' } }
Do we need a separate bool and string? Or is it sufficient to say that
if shared-disk is omitted, we are not sharing, and that if a shared-disk
string is present, then we are sharing and it names the id of the shared
disk.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 2017/4/12 22:28, Eric Blake wrote:
> On 04/12/2017 09:05 AM, zhanghailiang wrote:
>> We use these two options to identify which disk is
>> shared
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> v4:
>> - Add proper comment for primary_disk (Stefan)
>> v2:
>> - Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
>> - Fix comments for these two options
>> ---
>> +++ b/qapi/block-core.json
>> @@ -2661,12 +2661,20 @@
>> # node who owns the replication node chain. Must not be given in
>> # primary mode.
>> #
>> +# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
>> +# is true, this option is required (Since: 2.10)
>> +#
>> +# @shared-disk: To indicate whether or not a disk is shared by primary VM
>> +# and secondary VM. (The default is false) (Since: 2.10)
>> +#
>> # Since: 2.9
>> ##
>> { 'struct': 'BlockdevOptionsReplication',
>> 'base': 'BlockdevOptionsGenericFormat',
>> 'data': { 'mode': 'ReplicationMode',
>> - '*top-id': 'str' } }
>> + '*top-id': 'str',
>> + '*shared-disk-id': 'str',
>> + '*shared-disk': 'bool' } }
> Do we need a separate bool and string? Or is it sufficient to say that
> if shared-disk is omitted, we are not sharing, and that if a shared-disk
> string is present, then we are sharing and it names the id of the shared
> disk.
Er, Yes, We need both of them, the command line of secondary sides is like:
-drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\
file.driver=qcow2,top-id=active-disk0,\
file.file.filename=/mnt/ramfs/active_disk.img,\
file.backing=hidden_disk0,shared-disk=on
We only need the bool shared-disk to indicate whether disk is sharing or not, but
for primary side, we need to the blockdev-add command to tell primary which disk is shared.
{ 'execute': 'blockdev-add',
'arguments': {
'driver': 'replication',
'node-name': 'rep',
'mode': 'primary',
'shared-disk-id': 'primary_disk0',
'shared-disk': true,
'file': {
'driver': 'nbd',
'export': 'hidden_disk0',
'server': {
'type': 'inet',
'data': {
'host': 'xxx.xxx.xxx.xxx',
'port': 'yyy'
}
}
}
}
}
On 04/12/2017 10:05 PM, zhanghailiang wrote:
> We use these two options to identify which disk is
> shared
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> v4:
> - Add proper comment for primary_disk (Stefan)
> v2:
> - Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
> - Fix comments for these two options
> ---
> block/replication.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> qapi/block-core.json | 10 +++++++++-
> 2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/block/replication.c b/block/replication.c
> index bf3c395..418b81b 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -25,9 +25,12 @@
> typedef struct BDRVReplicationState {
> ReplicationMode mode;
> int replication_state;
> + bool is_shared_disk;
> + char *shared_disk_id;
> BdrvChild *active_disk;
> BdrvChild *hidden_disk;
> BdrvChild *secondary_disk;
> + BdrvChild *primary_disk;
> char *top_id;
> ReplicationState *rs;
> Error *blocker;
> @@ -53,6 +56,9 @@ static void replication_stop(ReplicationState *rs, bool failover,
>
> #define REPLICATION_MODE "mode"
> #define REPLICATION_TOP_ID "top-id"
> +#define REPLICATION_SHARED_DISK "shared-disk"
> +#define REPLICATION_SHARED_DISK_ID "shared-disk-id"
> +
> static QemuOptsList replication_runtime_opts = {
> .name = "replication",
> .head = QTAILQ_HEAD_INITIALIZER(replication_runtime_opts.head),
> @@ -65,6 +71,14 @@ static QemuOptsList replication_runtime_opts = {
> .name = REPLICATION_TOP_ID,
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = REPLICATION_SHARED_DISK_ID,
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = REPLICATION_SHARED_DISK,
> + .type = QEMU_OPT_BOOL,
> + },
> { /* end of list */ }
> },
> };
> @@ -85,6 +99,9 @@ static int replication_open(BlockDriverState *bs, QDict *options,
> QemuOpts *opts = NULL;
> const char *mode;
> const char *top_id;
> + const char *shared_disk_id;
> + BlockBackend *blk;
> + BlockDriverState *tmp_bs;
>
> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
> false, errp);
> @@ -125,12 +142,33 @@ static int replication_open(BlockDriverState *bs, QDict *options,
> "The option mode's value should be primary or secondary");
> goto fail;
> }
> + s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
> +
What If secondary side is supplied with 'REPLICATION_SHARED_DISK_ID'?
Pls refer f4f2539bc to pefect the logical.
> false);
> + if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
> + shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
> + if (!shared_disk_id) {
> + error_setg(&local_err, "Missing shared disk blk option");
> + goto fail;
> + }
> + s->shared_disk_id = g_strdup(shared_disk_id);
> + blk = blk_by_name(s->shared_disk_id);
> + if (!blk) {
> + error_setg(&local_err, "There is no %s block", s->shared_disk_id);
> + goto fail;
> + }
> + /* We have a BlockBackend for the primary disk but use BdrvChild for
> + * consistency - active_disk, secondary_disk, etc are also BdrvChild.
> + */
> + tmp_bs = blk_bs(blk);
> + s->primary_disk = QLIST_FIRST(&tmp_bs->parents);
> + }
>
> s->rs = replication_new(bs, &replication_ops);
>
> - ret = 0;
> -
> + qemu_opts_del(opts);
> + return 0;
> fail:
> + g_free(s->shared_disk_id);
> qemu_opts_del(opts);
> error_propagate(errp, local_err);
>
> @@ -141,6 +179,7 @@ static void replication_close(BlockDriverState *bs)
> {
> BDRVReplicationState *s = bs->opaque;
>
> + g_free(s->shared_disk_id);
> if (s->replication_state == BLOCK_REPLICATION_RUNNING) {
> replication_stop(s->rs, false, NULL);
> }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 033457c..361c932 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2661,12 +2661,20 @@
> # node who owns the replication node chain. Must not be given in
> # primary mode.
> #
> +# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
> +# is true, this option is required (Since: 2.10)
> +#
Further explanations:
For @shared-disk-id, it must/only be given when @shared-disk enable on
Primary side.
> +# @shared-disk: To indicate whether or not a disk is shared by primary VM
> +# and secondary VM. (The default is false) (Since: 2.10)
> +#
Further explanations:
For @shared-disk, it must be given or not-given on both side at the same
time.
> # Since: 2.9
> ##
> { 'struct': 'BlockdevOptionsReplication',
> 'base': 'BlockdevOptionsGenericFormat',
> 'data': { 'mode': 'ReplicationMode',
> - '*top-id': 'str' } }
> + '*top-id': 'str',
> + '*shared-disk-id': 'str',
> + '*shared-disk': 'bool' } }
>
> ##
> # @NFSTransport:
On 2017/4/18 13:59, Xie Changlong wrote:
>
> On 04/12/2017 10:05 PM, zhanghailiang wrote:
>> We use these two options to identify which disk is
>> shared
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> v4:
>> - Add proper comment for primary_disk (Stefan)
>> v2:
>> - Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
>> - Fix comments for these two options
>> ---
>> block/replication.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>> qapi/block-core.json | 10 +++++++++-
>> 2 files changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/replication.c b/block/replication.c
>> index bf3c395..418b81b 100644
>> --- a/block/replication.c
>> +++ b/block/replication.c
>> @@ -25,9 +25,12 @@
>> typedef struct BDRVReplicationState {
>> ReplicationMode mode;
>> int replication_state;
>> + bool is_shared_disk;
>> + char *shared_disk_id;
>> BdrvChild *active_disk;
>> BdrvChild *hidden_disk;
>> BdrvChild *secondary_disk;
>> + BdrvChild *primary_disk;
>> char *top_id;
>> ReplicationState *rs;
>> Error *blocker;
>> @@ -53,6 +56,9 @@ static void replication_stop(ReplicationState *rs, bool failover,
>>
>> #define REPLICATION_MODE "mode"
>> #define REPLICATION_TOP_ID "top-id"
>> +#define REPLICATION_SHARED_DISK "shared-disk"
>> +#define REPLICATION_SHARED_DISK_ID "shared-disk-id"
>> +
>> static QemuOptsList replication_runtime_opts = {
>> .name = "replication",
>> .head = QTAILQ_HEAD_INITIALIZER(replication_runtime_opts.head),
>> @@ -65,6 +71,14 @@ static QemuOptsList replication_runtime_opts = {
>> .name = REPLICATION_TOP_ID,
>> .type = QEMU_OPT_STRING,
>> },
>> + {
>> + .name = REPLICATION_SHARED_DISK_ID,
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = REPLICATION_SHARED_DISK,
>> + .type = QEMU_OPT_BOOL,
>> + },
>> { /* end of list */ }
>> },
>> };
>> @@ -85,6 +99,9 @@ static int replication_open(BlockDriverState *bs, QDict *options,
>> QemuOpts *opts = NULL;
>> const char *mode;
>> const char *top_id;
>> + const char *shared_disk_id;
>> + BlockBackend *blk;
>> + BlockDriverState *tmp_bs;
>>
>> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
>> false, errp);
>> @@ -125,12 +142,33 @@ static int replication_open(BlockDriverState *bs, QDict *options,
>> "The option mode's value should be primary or secondary");
>> goto fail;
>> }
>> + s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
>> +
> What If secondary side is supplied with 'REPLICATION_SHARED_DISK_ID'?
> Pls refer f4f2539bc to pefect the logical.
Hmm, we should not configure it for secondary side, i'll fix it in next version.
>> false);
>> + if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
>> + shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
>> + if (!shared_disk_id) {
>> + error_setg(&local_err, "Missing shared disk blk option");
>> + goto fail;
>> + }
>> + s->shared_disk_id = g_strdup(shared_disk_id);
>> + blk = blk_by_name(s->shared_disk_id);
>> + if (!blk) {
>> + error_setg(&local_err, "There is no %s block", s->shared_disk_id);
>> + goto fail;
>> + }
>> + /* We have a BlockBackend for the primary disk but use BdrvChild for
>> + * consistency - active_disk, secondary_disk, etc are also BdrvChild.
>> + */
>> + tmp_bs = blk_bs(blk);
>> + s->primary_disk = QLIST_FIRST(&tmp_bs->parents);
>> + }
>>
>> s->rs = replication_new(bs, &replication_ops);
>>
>> - ret = 0;
>> -
>> + qemu_opts_del(opts);
>> + return 0;
>> fail:
>> + g_free(s->shared_disk_id);
>> qemu_opts_del(opts);
>> error_propagate(errp, local_err);
>>
>> @@ -141,6 +179,7 @@ static void replication_close(BlockDriverState *bs)
>> {
>> BDRVReplicationState *s = bs->opaque;
>>
>> + g_free(s->shared_disk_id);
>> if (s->replication_state == BLOCK_REPLICATION_RUNNING) {
>> replication_stop(s->rs, false, NULL);
>> }
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 033457c..361c932 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2661,12 +2661,20 @@
>> # node who owns the replication node chain. Must not be given in
>> # primary mode.
>> #
>> +# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
>> +# is true, this option is required (Since: 2.10)
>> +#
> Further explanations:
>
> For @shared-disk-id, it must/only be given when @shared-disk enable on
> Primary side.
OK.
>> +# @shared-disk: To indicate whether or not a disk is shared by primary VM
>> +# and secondary VM. (The default is false) (Since: 2.10)
>> +#
> Further explanations:
>
> For @shared-disk, it must be given or not-given on both side at the same
> time.
OK, will fix it, thanks.
>> # Since: 2.9
>> ##
>> { 'struct': 'BlockdevOptionsReplication',
>> 'base': 'BlockdevOptionsGenericFormat',
>> 'data': { 'mode': 'ReplicationMode',
>> - '*top-id': 'str' } }
>> + '*top-id': 'str',
>> + '*shared-disk-id': 'str',
>> + '*shared-disk': 'bool' } }
>>
>> ##
>> # @NFSTransport:
>
>
>
> .
>
On Wed, Apr 12, 2017 at 10:05:17PM +0800, zhanghailiang wrote: > We use these two options to identify which disk is > shared > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > v4: > - Add proper comment for primary_disk (Stefan) > v2: > - Move g_free(s->shared_disk_id) to the common fail process place (Stefan) > - Fix comments for these two options > --- > block/replication.c | 43 +++++++++++++++++++++++++++++++++++++++++-- > qapi/block-core.json | 10 +++++++++- > 2 files changed, 50 insertions(+), 3 deletions(-) Aside from the ongoing discussion about this patch... Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 2017/5/12 3:08, Stefan Hajnoczi wrote: > On Wed, Apr 12, 2017 at 10:05:17PM +0800, zhanghailiang wrote: >> We use these two options to identify which disk is >> shared >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> v4: >> - Add proper comment for primary_disk (Stefan) >> v2: >> - Move g_free(s->shared_disk_id) to the common fail process place (Stefan) >> - Fix comments for these two options >> --- >> block/replication.c | 43 +++++++++++++++++++++++++++++++++++++++++-- >> qapi/block-core.json | 10 +++++++++- >> 2 files changed, 50 insertions(+), 3 deletions(-) > Aside from the ongoing discussion about this patch... > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Thanks, I'll fix the related problems found by changlong.
© 2016 - 2026 Red Hat, Inc.