* [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-25 9:03 ` Changlong Xie
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper zhanghailiang
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst,
zhanghailiang, Zhang Chen
Introuduce the scenario of shared-disk block replication
and how to use it.
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>
---
docs/block-replication.txt | 131 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 127 insertions(+), 4 deletions(-)
diff --git a/docs/block-replication.txt b/docs/block-replication.txt
index 6bde673..97fcfc1 100644
--- a/docs/block-replication.txt
+++ b/docs/block-replication.txt
@@ -24,7 +24,7 @@ only dropped at next checkpoint time. To reduce the network transportation
effort during a vmstate checkpoint, the disk modification operations of
the Primary disk are asynchronously forwarded to the Secondary node.
-== Workflow ==
+== Non-shared disk workflow ==
The following is the image of block replication workflow:
+----------------------+ +------------------------+
@@ -57,7 +57,7 @@ The following is the image of block replication workflow:
4) Secondary write requests will be buffered in the Disk buffer and it
will overwrite the existing sector content in the buffer.
-== Architecture ==
+== None-shared disk architecture ==
We are going to implement block replication from many basic
blocks that are already in QEMU.
@@ -106,6 +106,74 @@ any state that would otherwise be lost by the speculative write-through
of the NBD server into the secondary disk. So before block replication,
the primary disk and secondary disk should contain the same data.
+== Shared Disk Mode Workflow ==
+The following is the image of block replication workflow:
+
+ +----------------------+ +------------------------+
+ |Primary Write Requests| |Secondary Write Requests|
+ +----------------------+ +------------------------+
+ | |
+ | (4)
+ | V
+ | /-------------\
+ | (2)Forward and write through | |
+ | +--------------------------> | Disk Buffer |
+ | | | |
+ | | \-------------/
+ | |(1)read |
+ | | |
+ (3)write | | | backing file
+ V | |
+ +-----------------------------+ |
+ | Shared Disk | <-----+
+ +-----------------------------+
+
+ 1) Primary writes will read original data and forward it to Secondary
+ QEMU.
+ 2) Before Primary write requests are written to Shared disk, the
+ original sector content will be read from Shared disk and
+ forwarded and buffered in the Disk buffer on the secondary site,
+ but it will not overwrite the existing
+ sector content(it could be from either "Secondary Write Requests" or
+ previous COW of "Primary Write Requests") in the Disk buffer.
+ 3) Primary write requests will be written to Shared disk.
+ 4) Secondary write requests will be buffered in the Disk buffer and it
+ will overwrite the existing sector content in the buffer.
+
+== Shared Disk Mode Architecture ==
+We are going to implement block replication from many basic
+blocks that are already in QEMU.
+ virtio-blk || .----------
+ / || | Secondary
+ / || '----------
+ / || virtio-blk
+ / || |
+ | || replication(5)
+ | NBD --------> NBD (2) |
+ | client || server ---> hidden disk <-- active disk(4)
+ | ^ || |
+ | replication(1) || |
+ | | || |
+ | +-----------------' || |
+ (3) |drive-backup sync=none || |
+--------. | +-----------------+ || |
+Primary | | | || backing |
+--------' | | || |
+ V | |
+ +-------------------------------------------+ |
+ | shared disk | <----------+
+ +-------------------------------------------+
+
+
+ 1) Primary writes will read original data and forward it to Secondary
+ QEMU.
+ 2) The hidden-disk buffers the original content that is modified by the
+ primary VM. It should also be an empty disk, and
+ the driver supports bdrv_make_empty() and backing file.
+ 3) Primary write requests will be written to Shared disk.
+ 4) Secondary write requests will be buffered in the active disk and it
+ will overwrite the existing sector content in the buffer.
+
== Failure Handling ==
There are 7 internal errors when block replication is running:
1. I/O error on primary disk
@@ -145,7 +213,7 @@ d. replication_stop_all()
things except failover. The caller must hold the I/O mutex lock if it is
in migration/checkpoint thread.
-== Usage ==
+== Non-shared disk usage ==
Primary:
-drive if=xxx,driver=quorum,read-pattern=fifo,id=colo1,vote-threshold=1,\
children.0.file.filename=1.raw,\
@@ -234,6 +302,61 @@ Secondary:
The primary host is down, so we should do the following thing:
{ 'execute': 'nbd-server-stop' }
+== Shared disk usage ==
+Primary:
+ -drive if=virtio,id=primary_disk0,file.filename=1.raw,driver=raw
+
+Issue qmp command:
+ {'execute': 'human-monitor-command',
+ 'arguments': {
+ 'command-line': 'drive_add-nbuddydriver=replication,
+ mode=primary,
+ file.driver=nbd,
+ file.host=9.42.3.17,
+ file.port=9998,
+ file.export=hidden_disk0,
+ shared-disk-id=primary_disk0,
+ shared-disk=on,
+ node-name=rep'
+ }
+ }
+ -drive if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,\
+ backing.driver=raw,backing.file.filename=1.raw \
+ -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
+
+Issue qmp command:
+1. {'execute': 'nbd-server-start',
+ 'arguments': {
+ 'addr': {
+ 'type': 'inet',
+ 'data': {
+ 'host': '0',
+ 'port': '9998'
+ }
+ }
+ }
+ }
+2. {
+ 'execute': 'nbd-server-add',
+ 'arguments': {
+ 'device': 'hidden_disk0',
+ 'writable': true
+ }
+ }
+
+After Failover:
+Primary:
+{'execute': 'human-monitor-command',
+ 'arguments': {
+ 'command-line': 'drive_delrep'
+ }
+}
+
+Secondary:
+ {'execute': 'nbd-server-stop' }
+
TODO:
1. Continuous block replication
-2. Shared disk
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case zhanghailiang
@ 2016-10-25 9:03 ` Changlong Xie
2016-11-28 5:13 ` Hailiang Zhang
0 siblings, 1 reply; 24+ messages in thread
From: Changlong Xie @ 2016-10-25 9:03 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen
On 10/20/2016 09:57 PM, zhanghailiang wrote:
> Introuduce the scenario of shared-disk block replication
> and how to use it.
>
> 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>
> ---
> docs/block-replication.txt | 131 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 127 insertions(+), 4 deletions(-)
>
> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
> index 6bde673..97fcfc1 100644
> --- a/docs/block-replication.txt
> +++ b/docs/block-replication.txt
> @@ -24,7 +24,7 @@ only dropped at next checkpoint time. To reduce the network transportation
> effort during a vmstate checkpoint, the disk modification operations of
> the Primary disk are asynchronously forwarded to the Secondary node.
>
> -== Workflow ==
> +== Non-shared disk workflow ==
> The following is the image of block replication workflow:
>
> +----------------------+ +------------------------+
> @@ -57,7 +57,7 @@ The following is the image of block replication workflow:
> 4) Secondary write requests will be buffered in the Disk buffer and it
> will overwrite the existing sector content in the buffer.
>
> -== Architecture ==
> +== None-shared disk architecture ==
s/None-shared/Non-shared/g
> We are going to implement block replication from many basic
> blocks that are already in QEMU.
>
> @@ -106,6 +106,74 @@ any state that would otherwise be lost by the speculative write-through
> of the NBD server into the secondary disk. So before block replication,
> the primary disk and secondary disk should contain the same data.
>
> +== Shared Disk Mode Workflow ==
> +The following is the image of block replication workflow:
> +
> + +----------------------+ +------------------------+
> + |Primary Write Requests| |Secondary Write Requests|
> + +----------------------+ +------------------------+
> + | |
> + | (4)
> + | V
> + | /-------------\
> + | (2)Forward and write through | |
> + | +--------------------------> | Disk Buffer |
> + | | | |
> + | | \-------------/
> + | |(1)read |
> + | | |
> + (3)write | | | backing file
> + V | |
> + +-----------------------------+ |
> + | Shared Disk | <-----+
> + +-----------------------------+
> +
> + 1) Primary writes will read original data and forward it to Secondary
> + QEMU.
> + 2) Before Primary write requests are written to Shared disk, the
> + original sector content will be read from Shared disk and
> + forwarded and buffered in the Disk buffer on the secondary site,
> + but it will not overwrite the existing
extra spaces at the end of line
> + sector content(it could be from either "Secondary Write Requests" or
Need a space before "(" for better style.
> + previous COW of "Primary Write Requests") in the Disk buffer.
> + 3) Primary write requests will be written to Shared disk.
> + 4) Secondary write requests will be buffered in the Disk buffer and it
> + will overwrite the existing sector content in the buffer.
> +
> +== Shared Disk Mode Architecture ==
> +We are going to implement block replication from many basic
> +blocks that are already in QEMU.
> + virtio-blk || .----------
> + / || | Secondary
> + / || '----------
> + / || virtio-blk
> + / || |
> + | || replication(5)
> + | NBD --------> NBD (2) |
> + | client || server ---> hidden disk <-- active disk(4)
> + | ^ || |
> + | replication(1) || |
> + | | || |
> + | +-----------------' || |
> + (3) |drive-backup sync=none || |
> +--------. | +-----------------+ || |
> +Primary | | | || backing |
> +--------' | | || |
> + V | |
> + +-------------------------------------------+ |
> + | shared disk | <----------+
> + +-------------------------------------------+
> +
> +
> + 1) Primary writes will read original data and forward it to Secondary
> + QEMU.
> + 2) The hidden-disk buffers the original content that is modified by the
> + primary VM. It should also be an empty disk, and
extra spaces at end of line
> + the driver supports bdrv_make_empty() and backing file.
> + 3) Primary write requests will be written to Shared disk.
> + 4) Secondary write requests will be buffered in the active disk and it
> + will overwrite the existing sector content in the buffer.
> +
> == Failure Handling ==
> There are 7 internal errors when block replication is running:
> 1. I/O error on primary disk
> @@ -145,7 +213,7 @@ d. replication_stop_all()
> things except failover. The caller must hold the I/O mutex lock if it is
> in migration/checkpoint thread.
>
> -== Usage ==
> +== Non-shared disk usage ==
> Primary:
> -drive if=xxx,driver=quorum,read-pattern=fifo,id=colo1,vote-threshold=1,\
> children.0.file.filename=1.raw,\
> @@ -234,6 +302,61 @@ Secondary:
> The primary host is down, so we should do the following thing:
> { 'execute': 'nbd-server-stop' }
>
> +== Shared disk usage ==
Keep the some coding style with "== Non-shared disk usage ==" part is
good to me.
> +Primary:
> + -drive if=virtio,id=primary_disk0,file.filename=1.raw,driver=raw
> +
> +Issue qmp command:
> + {'execute': 'human-monitor-command',
two space indentation for the whole "{...}" part
> + 'arguments': {
> + 'command-line': 'drive_add-nbuddydriver=replication,
missing spaces
> + mode=primary,
> + file.driver=nbd,
> + file.host=9.42.3.17,
> + file.port=9998,
> + file.export=hidden_disk0,
> + shared-disk-id=primary_disk0,
> + shared-disk=on,
> + node-name=rep'
Keep the whole commands after "command-line" in one line, or you can
execute it correctly. IIRC
> + }
> + }
Secondary:
> + -drive if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,\
> + backing.driver=raw,backing.file.filename=1.raw \
> + -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
> +
> +Issue qmp command:
> +1. {'execute': 'nbd-server-start',
> + 'arguments': {
> + 'addr': {
> + 'type': 'inet',
> + 'data': {
> + 'host': '0',
s/0/9.42.3.17/g, since you use designated ip address above
> + 'port': '9998'
> + }
> + }
> + }
> + }
> +2. {
> + 'execute': 'nbd-server-add',
> + 'arguments': {
> + 'device': 'hidden_disk0',
> + 'writable': true
> + }
> + }
> +
> +After Failover:
> +Primary:
> +{'execute': 'human-monitor-command',
> + 'arguments': {
> + 'command-line': 'drive_delrep'
drive_del rep
> + }
> +}
> +
> +Secondary:
> + {'execute': 'nbd-server-stop' }
> +
> TODO:
> 1. Continuous block replication
> -2. Shared disk
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case
2016-10-25 9:03 ` Changlong Xie
@ 2016-11-28 5:13 ` Hailiang Zhang
2016-11-28 6:00 ` Changlong Xie
0 siblings, 1 reply; 24+ messages in thread
From: Hailiang Zhang @ 2016-11-28 5:13 UTC (permalink / raw)
To: Changlong Xie, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen
On 2016/10/25 17:03, Changlong Xie wrote:
> On 10/20/2016 09:57 PM, zhanghailiang wrote:
>> Introuduce the scenario of shared-disk block replication
>> and how to use it.
>>
>> 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>
>> ---
>> docs/block-replication.txt | 131 +++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 127 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>> index 6bde673..97fcfc1 100644
>> --- a/docs/block-replication.txt
>> +++ b/docs/block-replication.txt
>> @@ -24,7 +24,7 @@ only dropped at next checkpoint time. To reduce the network transportation
>> effort during a vmstate checkpoint, the disk modification operations of
>> the Primary disk are asynchronously forwarded to the Secondary node.
>>
>> -== Workflow ==
>> +== Non-shared disk workflow ==
>> The following is the image of block replication workflow:
>>
>> +----------------------+ +------------------------+
>> @@ -57,7 +57,7 @@ The following is the image of block replication workflow:
>> 4) Secondary write requests will be buffered in the Disk buffer and it
>> will overwrite the existing sector content in the buffer.
>>
>> -== Architecture ==
>> +== None-shared disk architecture ==
>
> s/None-shared/Non-shared/g
>
>> We are going to implement block replication from many basic
>> blocks that are already in QEMU.
>>
>> @@ -106,6 +106,74 @@ any state that would otherwise be lost by the speculative write-through
>> of the NBD server into the secondary disk. So before block replication,
>> the primary disk and secondary disk should contain the same data.
>>
>> +== Shared Disk Mode Workflow ==
>> +The following is the image of block replication workflow:
>> +
>> + +----------------------+ +------------------------+
>> + |Primary Write Requests| |Secondary Write Requests|
>> + +----------------------+ +------------------------+
>> + | |
>> + | (4)
>> + | V
>> + | /-------------\
>> + | (2)Forward and write through | |
>> + | +--------------------------> | Disk Buffer |
>> + | | | |
>> + | | \-------------/
>> + | |(1)read |
>> + | | |
>> + (3)write | | | backing file
>> + V | |
>> + +-----------------------------+ |
>> + | Shared Disk | <-----+
>> + +-----------------------------+
>> +
>> + 1) Primary writes will read original data and forward it to Secondary
>> + QEMU.
>> + 2) Before Primary write requests are written to Shared disk, the
>> + original sector content will be read from Shared disk and
>> + forwarded and buffered in the Disk buffer on the secondary site,
>> + but it will not overwrite the existing
>
> extra spaces at the end of line
>
>> + sector content(it could be from either "Secondary Write Requests" or
>
> Need a space before "(" for better style.
>
>> + previous COW of "Primary Write Requests") in the Disk buffer.
>> + 3) Primary write requests will be written to Shared disk.
>> + 4) Secondary write requests will be buffered in the Disk buffer and it
>> + will overwrite the existing sector content in the buffer.
>> +
>> +== Shared Disk Mode Architecture ==
>> +We are going to implement block replication from many basic
>> +blocks that are already in QEMU.
>> + virtio-blk || .----------
>> + / || | Secondary
>> + / || '----------
>> + / || virtio-blk
>> + / || |
>> + | || replication(5)
>> + | NBD --------> NBD (2) |
>> + | client || server ---> hidden disk <-- active disk(4)
>> + | ^ || |
>> + | replication(1) || |
>> + | | || |
>> + | +-----------------' || |
>> + (3) |drive-backup sync=none || |
>> +--------. | +-----------------+ || |
>> +Primary | | | || backing |
>> +--------' | | || |
>> + V | |
>> + +-------------------------------------------+ |
>> + | shared disk | <----------+
>> + +-------------------------------------------+
>> +
>> +
>> + 1) Primary writes will read original data and forward it to Secondary
>> + QEMU.
>> + 2) The hidden-disk buffers the original content that is modified by the
>> + primary VM. It should also be an empty disk, and
>
> extra spaces at end of line
>
>> + the driver supports bdrv_make_empty() and backing file.
>> + 3) Primary write requests will be written to Shared disk.
>> + 4) Secondary write requests will be buffered in the active disk and it
>> + will overwrite the existing sector content in the buffer.
>> +
>> == Failure Handling ==
>> There are 7 internal errors when block replication is running:
>> 1. I/O error on primary disk
>> @@ -145,7 +213,7 @@ d. replication_stop_all()
>> things except failover. The caller must hold the I/O mutex lock if it is
>> in migration/checkpoint thread.
>>
>> -== Usage ==
>> +== Non-shared disk usage ==
>> Primary:
>> -drive if=xxx,driver=quorum,read-pattern=fifo,id=colo1,vote-threshold=1,\
>> children.0.file.filename=1.raw,\
>> @@ -234,6 +302,61 @@ Secondary:
>> The primary host is down, so we should do the following thing:
>> { 'execute': 'nbd-server-stop' }
>>
>> +== Shared disk usage ==
>
> Keep the some coding style with "== Non-shared disk usage ==" part is
> good to me.
>
>> +Primary:
>> + -drive if=virtio,id=primary_disk0,file.filename=1.raw,driver=raw
>> +
>> +Issue qmp command:
>> + {'execute': 'human-monitor-command',
>
> two space indentation for the whole "{...}" part
>
>> + 'arguments': {
>> + 'command-line': 'drive_add-nbuddydriver=replication,
>
> missing spaces
>
>> + mode=primary,
>> + file.driver=nbd,
>> + file.host=9.42.3.17,
>> + file.port=9998,
>> + file.export=hidden_disk0,
>> + shared-disk-id=primary_disk0,
>> + shared-disk=on,
>> + node-name=rep'
>
> Keep the whole commands after "command-line" in one line, or you can
> execute it correctly. IIRC
>
Hmm, i will change this hmp command to qmp 'blockdev-add' command in next
version, because it is supported now, though it is ready for production.
>> + }
>> + }
>
> Secondary:
>
>> + -drive if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,\
>> + backing.driver=raw,backing.file.filename=1.raw \
>> + -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
>> +
>> +Issue qmp command:
>> +1. {'execute': 'nbd-server-start',
>> + 'arguments': {
>> + 'addr': {
>> + 'type': 'inet',
>> + 'data': {
>> + 'host': '0',
>
> s/0/9.42.3.17/g, since you use designated ip address above
>
>> + 'port': '9998'
>> + }
>> + }
>> + }
>> + }
>> +2. {
>> + 'execute': 'nbd-server-add',
>> + 'arguments': {
>> + 'device': 'hidden_disk0',
>> + 'writable': true
>> + }
>> + }
>> +
>> +After Failover:
>> +Primary:
>> +{'execute': 'human-monitor-command',
>> + 'arguments': {
>> + 'command-line': 'drive_delrep'
>
> drive_del rep
>
I'll use the qmp command instead here.
>> + }
>> +}
>> +
>> +Secondary:
>> + {'execute': 'nbd-server-stop' }
>> +
>> TODO:
>> 1. Continuous block replication
>> -2. Shared disk
>>
>
I will fix all the above problems in next version, thanks.
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case
2016-11-28 5:13 ` Hailiang Zhang
@ 2016-11-28 6:00 ` Changlong Xie
2016-11-28 5:58 ` Hailiang Zhang
0 siblings, 1 reply; 24+ messages in thread
From: Changlong Xie @ 2016-11-28 6:00 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen, Markus Armbruster
On 11/28/2016 01:13 PM, Hailiang Zhang wrote:
>
> On 2016/10/25 17:03, Changlong Xie wrote:
>> On 10/20/2016 09:57 PM, zhanghailiang wrote:
>>> Introuduce the scenario of shared-disk block replication
>>> and how to use it.
>>>
>>> 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>
>>> ---
>>> docs/block-replication.txt | 131
>>> +++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 127 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>>> index 6bde673..97fcfc1 100644
>>> --- a/docs/block-replication.txt
>>> +++ b/docs/block-replication.txt
>>> @@ -24,7 +24,7 @@ only dropped at next checkpoint time. To reduce the
>>> network transportation
>>> effort during a vmstate checkpoint, the disk modification
>>> operations of
>>> the Primary disk are asynchronously forwarded to the Secondary node.
>>>
>>> -== Workflow ==
>>> +== Non-shared disk workflow ==
>>> The following is the image of block replication workflow:
>>>
>>> +----------------------+
>>> +------------------------+
>>> @@ -57,7 +57,7 @@ The following is the image of block replication
>>> workflow:
>>> 4) Secondary write requests will be buffered in the Disk
>>> buffer and it
>>> will overwrite the existing sector content in the buffer.
>>>
>>> -== Architecture ==
>>> +== None-shared disk architecture ==
>>
>> s/None-shared/Non-shared/g
>>
>
>>> We are going to implement block replication from many basic
>>> blocks that are already in QEMU.
>>>
>>> @@ -106,6 +106,74 @@ any state that would otherwise be lost by the
>>> speculative write-through
>>> of the NBD server into the secondary disk. So before block
>>> replication,
>>> the primary disk and secondary disk should contain the same data.
>>>
>>> +== Shared Disk Mode Workflow ==
>>> +The following is the image of block replication workflow:
>>> +
>>> + +----------------------+ +------------------------+
>>> + |Primary Write Requests| |Secondary Write Requests|
>>> + +----------------------+ +------------------------+
>>> + | |
>>> + | (4)
>>> + | V
>>> + | /-------------\
>>> + | (2)Forward and write through | |
>>> + | +--------------------------> | Disk Buffer |
>>> + | | | |
>>> + | | \-------------/
>>> + | |(1)read |
>>> + | | |
>>> + (3)write | | | backing file
>>> + V | |
>>> + +-----------------------------+ |
>>> + | Shared Disk | <-----+
>>> + +-----------------------------+
>>> +
>>> + 1) Primary writes will read original data and forward it to
>>> Secondary
>>> + QEMU.
>>> + 2) Before Primary write requests are written to Shared disk, the
>>> + original sector content will be read from Shared disk and
>>> + forwarded and buffered in the Disk buffer on the secondary site,
>>> + but it will not overwrite the existing
>>
>> extra spaces at the end of line
>>
>
>>> + sector content(it could be from either "Secondary Write
>>> Requests" or
>>
>> Need a space before "(" for better style.
>>
>
>>> + previous COW of "Primary Write Requests") in the Disk buffer.
>>> + 3) Primary write requests will be written to Shared disk.
>>> + 4) Secondary write requests will be buffered in the Disk buffer
>>> and it
>>> + will overwrite the existing sector content in the buffer.
>>> +
>>> +== Shared Disk Mode Architecture ==
>>> +We are going to implement block replication from many basic
>>> +blocks that are already in QEMU.
>>> + virtio-blk
>>> || .----------
>>> + /
>>> || | Secondary
>>> + /
>>> || '----------
>>> + /
>>> || virtio-blk
>>> + /
>>> || |
>>> + |
>>> || replication(5)
>>> + | NBD --------> NBD
>>> (2) |
>>> + | client || server ---> hidden
>>> disk <-- active disk(4)
>>> + | ^ || |
>>> + | replication(1) || |
>>> + | | || |
>>> + | +-----------------' || |
>>> + (3) |drive-backup sync=none || |
>>> +--------. | +-----------------+ || |
>>> +Primary | | | || backing |
>>> +--------' | | || |
>>> + V | |
>>> + +-------------------------------------------+ |
>>> + | shared disk | <----------+
>>> + +-------------------------------------------+
>>> +
>>> +
>>> + 1) Primary writes will read original data and forward it to
>>> Secondary
>>> + QEMU.
>>> + 2) The hidden-disk buffers the original content that is modified
>>> by the
>>> + primary VM. It should also be an empty disk, and
>>
>> extra spaces at end of line
>>
>
>>> + the driver supports bdrv_make_empty() and backing file.
>>> + 3) Primary write requests will be written to Shared disk.
>>> + 4) Secondary write requests will be buffered in the active disk
>>> and it
>>> + will overwrite the existing sector content in the buffer.
>>> +
>>> == Failure Handling ==
>>> There are 7 internal errors when block replication is running:
>>> 1. I/O error on primary disk
>>> @@ -145,7 +213,7 @@ d. replication_stop_all()
>>> things except failover. The caller must hold the I/O mutex lock
>>> if it is
>>> in migration/checkpoint thread.
>>>
>>> -== Usage ==
>>> +== Non-shared disk usage ==
>>> Primary:
>>> -drive
>>> if=xxx,driver=quorum,read-pattern=fifo,id=colo1,vote-threshold=1,\
>>> children.0.file.filename=1.raw,\
>>> @@ -234,6 +302,61 @@ Secondary:
>>> The primary host is down, so we should do the following thing:
>>> { 'execute': 'nbd-server-stop' }
>>>
>>> +== Shared disk usage ==
>>
>> Keep the some coding style with "== Non-shared disk usage ==" part is
>> good to me.
>>
>
>>> +Primary:
>>> + -drive if=virtio,id=primary_disk0,file.filename=1.raw,driver=raw
>>> +
>>> +Issue qmp command:
>>> + {'execute': 'human-monitor-command',
>>
>> two space indentation for the whole "{...}" part
>>
>>> + 'arguments': {
>>> + 'command-line': 'drive_add-nbuddydriver=replication,
>>
>> missing spaces
>>
>>> + mode=primary,
>>> + file.driver=nbd,
>>> + file.host=9.42.3.17,
>>> + file.port=9998,
>>> + file.export=hidden_disk0,
>>> + shared-disk-id=primary_disk0,
>>> + shared-disk=on,
>>> + node-name=rep'
>>
>
>> Keep the whole commands after "command-line" in one line, or you can
>> execute it correctly. IIRC
>>
>
> Hmm, i will change this hmp command to qmp 'blockdev-add' command in next
> version, because it is supported now, though it is ready for production.
>
It's a good start, but i'm not sure here.
http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg01062.html
Thanks
-Xie
>>> + }
>>> + }
>>
>> Secondary:
>>
>>> + -drive
>>> if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,\
>>>
>>> + backing.driver=raw,backing.file.filename=1.raw \
>>> + -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
>>> +
>>> +Issue qmp command:
>>> +1. {'execute': 'nbd-server-start',
>>> + 'arguments': {
>>> + 'addr': {
>>> + 'type': 'inet',
>>> + 'data': {
>>> + 'host': '0',
>>
>> s/0/9.42.3.17/g, since you use designated ip address above
>>
>
>>> + 'port': '9998'
>>> + }
>>> + }
>>> + }
>>> + }
>>> +2. {
>>> + 'execute': 'nbd-server-add',
>>> + 'arguments': {
>>> + 'device': 'hidden_disk0',
>>> + 'writable': true
>>> + }
>>> + }
>>> +
>>> +After Failover:
>>> +Primary:
>>> +{'execute': 'human-monitor-command',
>>> + 'arguments': {
>>> + 'command-line': 'drive_delrep'
>>
>> drive_del rep
>>
>
> I'll use the qmp command instead here.
>
>>> + }
>>> +}
>>> +
>>> +Secondary:
>>> + {'execute': 'nbd-server-stop' }
>>> +
>>> TODO:
>>> 1. Continuous block replication
>>> -2. Shared disk
>>>
>>
>
> I will fix all the above problems in next version, thanks.
>
>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case
2016-11-28 6:00 ` Changlong Xie
@ 2016-11-28 5:58 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-11-28 5:58 UTC (permalink / raw)
To: Changlong Xie, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen, Markus Armbruster
On 2016/11/28 14:00, Changlong Xie wrote:
> On 11/28/2016 01:13 PM, Hailiang Zhang wrote:
>>
>> On 2016/10/25 17:03, Changlong Xie wrote:
>>> On 10/20/2016 09:57 PM, zhanghailiang wrote:
>>>> Introuduce the scenario of shared-disk block replication
>>>> and how to use it.
>>>>
>>>> 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>
>>>> ---
>>>> docs/block-replication.txt | 131
>>>> +++++++++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 127 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>>>> index 6bde673..97fcfc1 100644
>>>> --- a/docs/block-replication.txt
>>>> +++ b/docs/block-replication.txt
>>>> @@ -24,7 +24,7 @@ only dropped at next checkpoint time. To reduce the
>>>> network transportation
>>>> effort during a vmstate checkpoint, the disk modification
>>>> operations of
>>>> the Primary disk are asynchronously forwarded to the Secondary node.
>>>>
>>>> -== Workflow ==
>>>> +== Non-shared disk workflow ==
>>>> The following is the image of block replication workflow:
>>>>
>>>> +----------------------+
>>>> +------------------------+
>>>> @@ -57,7 +57,7 @@ The following is the image of block replication
>>>> workflow:
>>>> 4) Secondary write requests will be buffered in the Disk
>>>> buffer and it
>>>> will overwrite the existing sector content in the buffer.
>>>>
>>>> -== Architecture ==
>>>> +== None-shared disk architecture ==
>>>
>>> s/None-shared/Non-shared/g
>>>
>>
>>>> We are going to implement block replication from many basic
>>>> blocks that are already in QEMU.
>>>>
>>>> @@ -106,6 +106,74 @@ any state that would otherwise be lost by the
>>>> speculative write-through
>>>> of the NBD server into the secondary disk. So before block
>>>> replication,
>>>> the primary disk and secondary disk should contain the same data.
>>>>
>>>> +== Shared Disk Mode Workflow ==
>>>> +The following is the image of block replication workflow:
>>>> +
>>>> + +----------------------+ +------------------------+
>>>> + |Primary Write Requests| |Secondary Write Requests|
>>>> + +----------------------+ +------------------------+
>>>> + | |
>>>> + | (4)
>>>> + | V
>>>> + | /-------------\
>>>> + | (2)Forward and write through | |
>>>> + | +--------------------------> | Disk Buffer |
>>>> + | | | |
>>>> + | | \-------------/
>>>> + | |(1)read |
>>>> + | | |
>>>> + (3)write | | | backing file
>>>> + V | |
>>>> + +-----------------------------+ |
>>>> + | Shared Disk | <-----+
>>>> + +-----------------------------+
>>>> +
>>>> + 1) Primary writes will read original data and forward it to
>>>> Secondary
>>>> + QEMU.
>>>> + 2) Before Primary write requests are written to Shared disk, the
>>>> + original sector content will be read from Shared disk and
>>>> + forwarded and buffered in the Disk buffer on the secondary site,
>>>> + but it will not overwrite the existing
>>>
>>> extra spaces at the end of line
>>>
>>
>>>> + sector content(it could be from either "Secondary Write
>>>> Requests" or
>>>
>>> Need a space before "(" for better style.
>>>
>>
>>>> + previous COW of "Primary Write Requests") in the Disk buffer.
>>>> + 3) Primary write requests will be written to Shared disk.
>>>> + 4) Secondary write requests will be buffered in the Disk buffer
>>>> and it
>>>> + will overwrite the existing sector content in the buffer.
>>>> +
>>>> +== Shared Disk Mode Architecture ==
>>>> +We are going to implement block replication from many basic
>>>> +blocks that are already in QEMU.
>>>> + virtio-blk
>>>> || .----------
>>>> + /
>>>> || | Secondary
>>>> + /
>>>> || '----------
>>>> + /
>>>> || virtio-blk
>>>> + /
>>>> || |
>>>> + |
>>>> || replication(5)
>>>> + | NBD --------> NBD
>>>> (2) |
>>>> + | client || server ---> hidden
>>>> disk <-- active disk(4)
>>>> + | ^ || |
>>>> + | replication(1) || |
>>>> + | | || |
>>>> + | +-----------------' || |
>>>> + (3) |drive-backup sync=none || |
>>>> +--------. | +-----------------+ || |
>>>> +Primary | | | || backing |
>>>> +--------' | | || |
>>>> + V | |
>>>> + +-------------------------------------------+ |
>>>> + | shared disk | <----------+
>>>> + +-------------------------------------------+
>>>> +
>>>> +
>>>> + 1) Primary writes will read original data and forward it to
>>>> Secondary
>>>> + QEMU.
>>>> + 2) The hidden-disk buffers the original content that is modified
>>>> by the
>>>> + primary VM. It should also be an empty disk, and
>>>
>>> extra spaces at end of line
>>>
>>
>>>> + the driver supports bdrv_make_empty() and backing file.
>>>> + 3) Primary write requests will be written to Shared disk.
>>>> + 4) Secondary write requests will be buffered in the active disk
>>>> and it
>>>> + will overwrite the existing sector content in the buffer.
>>>> +
>>>> == Failure Handling ==
>>>> There are 7 internal errors when block replication is running:
>>>> 1. I/O error on primary disk
>>>> @@ -145,7 +213,7 @@ d. replication_stop_all()
>>>> things except failover. The caller must hold the I/O mutex lock
>>>> if it is
>>>> in migration/checkpoint thread.
>>>>
>>>> -== Usage ==
>>>> +== Non-shared disk usage ==
>>>> Primary:
>>>> -drive
>>>> if=xxx,driver=quorum,read-pattern=fifo,id=colo1,vote-threshold=1,\
>>>> children.0.file.filename=1.raw,\
>>>> @@ -234,6 +302,61 @@ Secondary:
>>>> The primary host is down, so we should do the following thing:
>>>> { 'execute': 'nbd-server-stop' }
>>>>
>>>> +== Shared disk usage ==
>>>
>>> Keep the some coding style with "== Non-shared disk usage ==" part is
>>> good to me.
>>>
>>
>>>> +Primary:
>>>> + -drive if=virtio,id=primary_disk0,file.filename=1.raw,driver=raw
>>>> +
>>>> +Issue qmp command:
>>>> + {'execute': 'human-monitor-command',
>>>
>>> two space indentation for the whole "{...}" part
>>>
>>>> + 'arguments': {
>>>> + 'command-line': 'drive_add-nbuddydriver=replication,
>>>
>>> missing spaces
>>>
>>>> + mode=primary,
>>>> + file.driver=nbd,
>>>> + file.host=9.42.3.17,
>>>> + file.port=9998,
>>>> + file.export=hidden_disk0,
>>>> + shared-disk-id=primary_disk0,
>>>> + shared-disk=on,
>>>> + node-name=rep'
>>>
>>
>>> Keep the whole commands after "command-line" in one line, or you can
>>> execute it correctly. IIRC
>>>
>>
>> Hmm, i will change this hmp command to qmp 'blockdev-add' command in next
>> version, because it is supported now, though it is ready for production.
>>
>
> It's a good start, but i'm not sure here.
>
> http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg01062.html
>
Yes, i noticed that, but for COLO, it is not ready for production either.
So I think it is OK to use it here ...
> Thanks
> -Xie
>>>> + }
>>>> + }
>>>
>>> Secondary:
>>>
>>>> + -drive
>>>> if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,\
>>>>
>>>> + backing.driver=raw,backing.file.filename=1.raw \
>>>> + -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
>>>> +
>>>> +Issue qmp command:
>>>> +1. {'execute': 'nbd-server-start',
>>>> + 'arguments': {
>>>> + 'addr': {
>>>> + 'type': 'inet',
>>>> + 'data': {
>>>> + 'host': '0',
>>>
>>> s/0/9.42.3.17/g, since you use designated ip address above
>>>
>>
>>>> + 'port': '9998'
>>>> + }
>>>> + }
>>>> + }
>>>> + }
>>>> +2. {
>>>> + 'execute': 'nbd-server-add',
>>>> + 'arguments': {
>>>> + 'device': 'hidden_disk0',
>>>> + 'writable': true
>>>> + }
>>>> + }
>>>> +
>>>> +After Failover:
>>>> +Primary:
>>>> +{'execute': 'human-monitor-command',
>>>> + 'arguments': {
>>>> + 'command-line': 'drive_delrep'
>>>
>>> drive_del rep
>>>
>>
>> I'll use the qmp command instead here.
>>
>>>> + }
>>>> +}
>>>> +
>>>> +Secondary:
>>>> + {'execute': 'nbd-server-stop' }
>>>> +
>>>> TODO:
>>>> 1. Continuous block replication
>>>> -2. Shared disk
>>>>
>>>
>>
>> I will fix all the above problems in next version, thanks.
>>
>>>
>>>
>>> .
>>>
>>
>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-25 9:58 ` Changlong Xie
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options zhanghailiang
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst, zhanghailiang
With this helper function, we can get the BdrvChild struct
from BlockBackend
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
block/block-backend.c | 5 +++++
include/sysemu/block-backend.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 1a724a8..66387f0 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -389,6 +389,11 @@ BlockDriverState *blk_bs(BlockBackend *blk)
return blk->root ? blk->root->bs : NULL;
}
+BdrvChild *blk_root(BlockBackend *blk)
+{
+ return blk->root;
+}
+
static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
{
BdrvChild *child;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index b07159b..867f9f5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -99,6 +99,7 @@ void blk_remove_bs(BlockBackend *blk);
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
bool bdrv_has_blk(BlockDriverState *bs);
bool bdrv_is_root_node(BlockDriverState *bs);
+BdrvChild *blk_root(BlockBackend *blk);
void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
void blk_iostatus_enable(BlockBackend *blk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper zhanghailiang
@ 2016-10-25 9:58 ` Changlong Xie
2016-12-05 2:41 ` Hailiang Zhang
0 siblings, 1 reply; 24+ messages in thread
From: Changlong Xie @ 2016-10-25 9:58 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency
I know you need blk->root in the next patch, but we strongly don't
recommend your current solution. Please refer Kevin's cf2ab8fc
1409 /* XXX Ugly way to get blk->root, but that's a feature, not a
bug. This
1410 * hack makes it obvious that vhdx_write_header() bypasses the
BlockBackend
1411 * here, which it really shouldn't be doing. */
1412 child = QLIST_FIRST(&bs->parents);
1413 assert(!QLIST_NEXT(child, next_parent));
Then you can drop this commit.
On 10/20/2016 09:57 PM, zhanghailiang wrote:
> With this helper function, we can get the BdrvChild struct
> from BlockBackend
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> block/block-backend.c | 5 +++++
> include/sysemu/block-backend.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 1a724a8..66387f0 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -389,6 +389,11 @@ BlockDriverState *blk_bs(BlockBackend *blk)
> return blk->root ? blk->root->bs : NULL;
> }
>
> +BdrvChild *blk_root(BlockBackend *blk)
> +{
> + return blk->root;
> +}
> +
> static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
> {
> BdrvChild *child;
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index b07159b..867f9f5 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -99,6 +99,7 @@ void blk_remove_bs(BlockBackend *blk);
> void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
> bool bdrv_has_blk(BlockDriverState *bs);
> bool bdrv_is_root_node(BlockDriverState *bs);
> +BdrvChild *blk_root(BlockBackend *blk);
>
> void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
> void blk_iostatus_enable(BlockBackend *blk);
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper
2016-10-25 9:58 ` Changlong Xie
@ 2016-12-05 2:41 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-12-05 2:41 UTC (permalink / raw)
To: Changlong Xie, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency
On 2016/10/25 17:58, Changlong Xie wrote:
> I know you need blk->root in the next patch, but we strongly don't
> recommend your current solution. Please refer Kevin's cf2ab8fc
>
> 1409 /* XXX Ugly way to get blk->root, but that's a feature, not a
> bug. This
> 1410 * hack makes it obvious that vhdx_write_header() bypasses the
> BlockBackend
> 1411 * here, which it really shouldn't be doing. */
> 1412 child = QLIST_FIRST(&bs->parents);
> 1413 assert(!QLIST_NEXT(child, next_parent));
>
> Then you can drop this commit.
>
OK, got it, I'll drop this patch in next version, thanks.
> On 10/20/2016 09:57 PM, zhanghailiang wrote:
>> With this helper function, we can get the BdrvChild struct
>> from BlockBackend
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>> block/block-backend.c | 5 +++++
>> include/sysemu/block-backend.h | 1 +
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index 1a724a8..66387f0 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -389,6 +389,11 @@ BlockDriverState *blk_bs(BlockBackend *blk)
>> return blk->root ? blk->root->bs : NULL;
>> }
>>
>> +BdrvChild *blk_root(BlockBackend *blk)
>> +{
>> + return blk->root;
>> +}
>> +
>> static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
>> {
>> BdrvChild *child;
>> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
>> index b07159b..867f9f5 100644
>> --- a/include/sysemu/block-backend.h
>> +++ b/include/sysemu/block-backend.h
>> @@ -99,6 +99,7 @@ void blk_remove_bs(BlockBackend *blk);
>> void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
>> bool bdrv_has_blk(BlockDriverState *bs);
>> bool bdrv_is_root_node(BlockDriverState *bs);
>> +BdrvChild *blk_root(BlockBackend *blk);
>>
>> void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
>> void blk_iostatus_enable(BlockBackend *blk);
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 1/7] docs/block-replication: Add description for shared-disk case zhanghailiang
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 2/7] block-backend: Introduce blk_root() helper zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-25 10:01 ` Changlong Xie
2016-10-26 1:58 ` Changlong Xie
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint() zhanghailiang
` (5 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst,
zhanghailiang, Zhang Chen
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>
---
block/replication.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/block/replication.c b/block/replication.c
index 3bd1cf1..2a2fdb2 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,8 @@ 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;
ret = -EINVAL;
opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
@@ -114,6 +130,22 @@ 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");
+ 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;
+ }
+ s->primary_disk = blk_root(blk);
+ }
s->rs = replication_new(bs, &replication_ops);
@@ -130,6 +162,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);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options zhanghailiang
@ 2016-10-25 10:01 ` Changlong Xie
2016-12-05 3:08 ` Hailiang Zhang
2016-10-26 1:58 ` Changlong Xie
1 sibling, 1 reply; 24+ messages in thread
From: Changlong Xie @ 2016-10-25 10:01 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen
On 10/20/2016 09:57 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>
> ---
> block/replication.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/block/replication.c b/block/replication.c
> index 3bd1cf1..2a2fdb2 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,8 @@ 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;
>
> ret = -EINVAL;
> opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
> @@ -114,6 +130,22 @@ 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");
> + 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);
g_free(s->shared_disk_id);
> + goto fail;
> + }
> + s->primary_disk = blk_root(blk);
> + }
>
> s->rs = replication_new(bs, &replication_ops);
>
> @@ -130,6 +162,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);
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options
2016-10-25 10:01 ` Changlong Xie
@ 2016-12-05 3:08 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-12-05 3:08 UTC (permalink / raw)
To: Changlong Xie, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen
On 2016/10/25 18:01, Changlong Xie wrote:
> On 10/20/2016 09:57 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>
>> ---
>> block/replication.c | 33 +++++++++++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/block/replication.c b/block/replication.c
>> index 3bd1cf1..2a2fdb2 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,8 @@ 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;
>>
>> ret = -EINVAL;
>> opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
>> @@ -114,6 +130,22 @@ 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");
>> + 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);
>
> g_free(s->shared_disk_id);
>
Will fix in next version， thanks
>> + goto fail;
>> + }
>> + s->primary_disk = blk_root(blk);
>> + }
>>
>> s->rs = replication_new(bs, &replication_ops);
>>
>> @@ -130,6 +162,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);
>> }
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options zhanghailiang
2016-10-25 10:01 ` Changlong Xie
@ 2016-10-26 1:58 ` Changlong Xie
1 sibling, 0 replies; 24+ messages in thread
From: Changlong Xie @ 2016-10-26 1:58 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Zhang Chen
On 10/20/2016 09:57 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>
> ---
> block/replication.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/block/replication.c b/block/replication.c
> index 3bd1cf1..2a2fdb2 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,8 @@ 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;
>
> ret = -EINVAL;
> opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
> @@ -114,6 +130,22 @@ static int replication_open(BlockDriverState *bs, QDict *options,
> "The option mode's value should be primary or secondary");
> goto fail;
> }
Now we have four runtime options
"mode"/"top-id"/"shared-disk"/"shared-disk-id". But the current checking
logic is too weak, i think you need enhance it to avoid opts misusage.
> + s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
> + false);
Missing one space.
> + 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");
> + 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;
> + }
> + s->primary_disk = blk_root(blk);
> + }
>
> s->rs = replication_new(bs, &replication_ops);
>
> @@ -130,6 +162,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);
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (2 preceding siblings ...)
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 3/7] replication: add shared-disk and shared-disk-id options zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-26 1:40 ` Changlong Xie
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 5/7] replication: fix code logic with the new shared_disk option zhanghailiang
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst, zhanghailiang
The helper backup_do_checkpoint() will be used for primary related
codes. Here we split it out from secondary_do_checkpoint().
Besides, it is unnecessary to call backup_do_checkpoint() in
replication starting and normally stop replication path.
We only need call it while do real checkpointing.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
block/replication.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index 2a2fdb2..d687ffc 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs,
static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
{
- Error *local_err = NULL;
int ret;
- if (!s->secondary_disk->bs->job) {
- error_setg(errp, "Backup job was cancelled unexpectedly");
- return;
- }
-
- backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs);
if (ret < 0) {
error_setg(errp, "Cannot make active disk empty");
@@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
aio_context_release(aio_context);
return;
}
+
+ secondary_do_checkpoint(s, errp);
break;
default:
aio_context_release(aio_context);
@@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
s->replication_state = BLOCK_REPLICATION_RUNNING;
- if (s->mode == REPLICATION_MODE_SECONDARY) {
- secondary_do_checkpoint(s, errp);
- }
-
s->error = 0;
aio_context_release(aio_context);
}
@@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
BlockDriverState *bs = rs->opaque;
BDRVReplicationState *s;
AioContext *aio_context;
+ Error *local_err = NULL;
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
s = bs->opaque;
- if (s->mode == REPLICATION_MODE_SECONDARY) {
+ switch (s->mode) {
+ case REPLICATION_MODE_PRIMARY:
+ break;
+ case REPLICATION_MODE_SECONDARY:
+ if (!s->secondary_disk->bs->job) {
+ error_setg(errp, "Backup job was cancelled unexpectedly");
+ break;
+ }
+ backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ break;
+ }
secondary_do_checkpoint(s, errp);
+ break;
+ default:
+ abort();
}
aio_context_release(aio_context);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint() zhanghailiang
@ 2016-10-26 1:40 ` Changlong Xie
2016-12-05 3:41 ` Hailiang Zhang
0 siblings, 1 reply; 24+ messages in thread
From: Changlong Xie @ 2016-10-26 1:40 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency
On 10/20/2016 09:57 PM, zhanghailiang wrote:
> The helper backup_do_checkpoint() will be used for primary related
> codes. Here we split it out from secondary_do_checkpoint().
>
> Besides, it is unnecessary to call backup_do_checkpoint() in
> replication starting and normally stop replication path.
This patch is unnecessary. We *really* need clean
backup_job->done_bitmap in replication_start/stop path.
> We only need call it while do real checkpointing.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> block/replication.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/block/replication.c b/block/replication.c
> index 2a2fdb2..d687ffc 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs,
>
> static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
> {
> - Error *local_err = NULL;
> int ret;
>
> - if (!s->secondary_disk->bs->job) {
> - error_setg(errp, "Backup job was cancelled unexpectedly");
> - return;
> - }
> -
> - backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs);
> if (ret < 0) {
> error_setg(errp, "Cannot make active disk empty");
> @@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
> aio_context_release(aio_context);
> return;
> }
> +
> + secondary_do_checkpoint(s, errp);
> break;
> default:
> aio_context_release(aio_context);
> @@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>
> s->replication_state = BLOCK_REPLICATION_RUNNING;
>
> - if (s->mode == REPLICATION_MODE_SECONDARY) {
> - secondary_do_checkpoint(s, errp);
> - }
> -
> s->error = 0;
> aio_context_release(aio_context);
> }
> @@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
> BlockDriverState *bs = rs->opaque;
> BDRVReplicationState *s;
> AioContext *aio_context;
> + Error *local_err = NULL;
>
> aio_context = bdrv_get_aio_context(bs);
> aio_context_acquire(aio_context);
> s = bs->opaque;
>
> - if (s->mode == REPLICATION_MODE_SECONDARY) {
> + switch (s->mode) {
> + case REPLICATION_MODE_PRIMARY:
> + break;
> + case REPLICATION_MODE_SECONDARY:
> + if (!s->secondary_disk->bs->job) {
> + error_setg(errp, "Backup job was cancelled unexpectedly");
> + break;
> + }
> + backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + break;
> + }
> secondary_do_checkpoint(s, errp);
> + break;
> + default:
> + abort();
> }
> aio_context_release(aio_context);
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
2016-10-26 1:40 ` Changlong Xie
@ 2016-12-05 3:41 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-12-05 3:41 UTC (permalink / raw)
To: Changlong Xie, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency
On 2016/10/26 9:40, Changlong Xie wrote:
> On 10/20/2016 09:57 PM, zhanghailiang wrote:
>> The helper backup_do_checkpoint() will be used for primary related
>> codes. Here we split it out from secondary_do_checkpoint().
>>
>> Besides, it is unnecessary to call backup_do_checkpoint() in
>> replication starting and normally stop replication path.
>
> This patch is unnecessary. We *really* need clean
> backup_job->done_bitmap in replication_start/stop path.
>
After we support internal job ('BLOCK_JOB_INTERNAL'), do we still need
to call backup_do?
IMHO, we don't have to clean 'done_bitmap', because
its initial value is zero, and we don't have to call it in
stop path either, the backup job will be cleaned.
>> We only need call it while do real checkpointing.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>> block/replication.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/block/replication.c b/block/replication.c
>> index 2a2fdb2..d687ffc 100644
>> --- a/block/replication.c
>> +++ b/block/replication.c
>> @@ -320,20 +320,8 @@ static bool replication_recurse_is_first_non_filter(BlockDriverState *bs,
>>
>> static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
>> {
>> - Error *local_err = NULL;
>> int ret;
>>
>> - if (!s->secondary_disk->bs->job) {
>> - error_setg(errp, "Backup job was cancelled unexpectedly");
>> - return;
>> - }
>> -
>> - backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
>> - if (local_err) {
>> - error_propagate(errp, local_err);
>> - return;
>> - }
>> -
>> ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs);
>> if (ret < 0) {
>> error_setg(errp, "Cannot make active disk empty");
>> @@ -539,6 +527,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>> aio_context_release(aio_context);
>> return;
>> }
>> +
>> + secondary_do_checkpoint(s, errp);
>> break;
>> default:
>> aio_context_release(aio_context);
>> @@ -547,10 +537,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>>
>> s->replication_state = BLOCK_REPLICATION_RUNNING;
>>
>> - if (s->mode == REPLICATION_MODE_SECONDARY) {
>> - secondary_do_checkpoint(s, errp);
>> - }
>> -
>> s->error = 0;
>> aio_context_release(aio_context);
>> }
>> @@ -560,13 +546,29 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
>> BlockDriverState *bs = rs->opaque;
>> BDRVReplicationState *s;
>> AioContext *aio_context;
>> + Error *local_err = NULL;
>>
>> aio_context = bdrv_get_aio_context(bs);
>> aio_context_acquire(aio_context);
>> s = bs->opaque;
>>
>> - if (s->mode == REPLICATION_MODE_SECONDARY) {
>> + switch (s->mode) {
>> + case REPLICATION_MODE_PRIMARY:
>> + break;
>> + case REPLICATION_MODE_SECONDARY:
>> + if (!s->secondary_disk->bs->job) {
>> + error_setg(errp, "Backup job was cancelled unexpectedly");
>> + break;
>> + }
>> + backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + break;
>> + }
>> secondary_do_checkpoint(s, errp);
>> + break;
>> + default:
>> + abort();
>> }
>> aio_context_release(aio_context);
>> }
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH RFC 5/7] replication: fix code logic with the new shared_disk option
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (3 preceding siblings ...)
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 4/7] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint() zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 6/7] replication: Implement block replication for shared disk case zhanghailiang
` (3 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst, zhanghailiang
Some code logic only be needed in non-shared disk, here
we adjust these codes to prepare for shared disk scenario.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
block/replication.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index d687ffc..39c616d 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -517,15 +517,21 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
bdrv_op_block_all(top_bs, s->blocker);
bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
- backup_start("replication-backup", s->secondary_disk->bs,
- s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, false,
- BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
- backup_job_completed, s, NULL, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- backup_job_cleanup(s);
- aio_context_release(aio_context);
- return;
+ /*
+ * Only in the case of non-shared disk,
+ * the backup job is in the Slave side
+ */
+ if (!s->is_shared_disk) {
+ backup_start("replication-backup", s->secondary_disk->bs,
+ s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, false,
+ BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
+ backup_job_completed, s, NULL, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ backup_job_cleanup(s);
+ aio_context_release(aio_context);
+ return;
+ }
}
secondary_do_checkpoint(s, errp);
@@ -556,14 +562,16 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
case REPLICATION_MODE_PRIMARY:
break;
case REPLICATION_MODE_SECONDARY:
- if (!s->secondary_disk->bs->job) {
- error_setg(errp, "Backup job was cancelled unexpectedly");
- break;
- }
- backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- break;
+ if (!s->is_shared_disk) {
+ if (!s->secondary_disk->bs->job) {
+ error_setg(errp, "Backup job was cancelled unexpectedly");
+ break;
+ }
+ backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ break;
+ }
}
secondary_do_checkpoint(s, errp);
break;
@@ -644,7 +652,7 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
* before the BDS is closed, because we will access hidden
* disk, secondary disk in backup_job_completed().
*/
- if (s->secondary_disk->bs->job) {
+ if (!s->is_shared_disk && s->secondary_disk->bs->job) {
block_job_cancel_sync(s->secondary_disk->bs->job);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH RFC 6/7] replication: Implement block replication for shared disk case
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (4 preceding siblings ...)
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 5/7] replication: fix code logic with the new shared_disk option zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver zhanghailiang
` (2 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst,
zhanghailiang, Zhang Chen
Just as the scenario of non-shared disk block replication,
we are going to implement block replication from many basic
blocks that are already in QEMU.
The architecture is:
virtio-blk || .----------
/ || | Secondary
/ || '----------
/ || virtio-blk
/ || |
| || replication(5)
| NBD --------> NBD (2) |
| client || server ---> hidden disk <-- active disk(4)
| ^ || |
| replication(1) || |
| | || |
| +-----------------' || |
(3) |drive-backup sync=none || |
--------. | +-----------------+ || |
Primary | | | || backing |
--------' | | || |
V | |
+-------------------------------------------+ |
| shared disk | <----------+
+-------------------------------------------+
1) Primary writes will read original data and forward it to Secondary
QEMU.
2) The hidden-disk is created automatically. It buffers the original content
that is modified by the primary VM. It should also be an empty disk, and
the driver supports bdrv_make_empty() and backing file.
3) Primary write requests will be written to Shared disk.
4) Secondary write requests will be buffered in the active disk and it
will overwrite the existing sector content in the buffer.
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>
---
block/replication.c | 45 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index 39c616d..e66b1ca 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -221,7 +221,7 @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs,
QEMUIOVector *qiov)
{
BDRVReplicationState *s = bs->opaque;
- BdrvChild *child = s->secondary_disk;
+ BdrvChild *child = s->is_shared_disk ? s->primary_disk : s->secondary_disk;
BlockJob *job = NULL;
CowRequest req;
int ret;
@@ -398,8 +398,12 @@ static void backup_job_completed(void *opaque, int ret)
/* The backup job is cancelled unexpectedly */
s->error = -EIO;
}
-
- backup_job_cleanup(s);
+ if (s->mode == REPLICATION_MODE_PRIMARY) {
+ s->replication_state = BLOCK_REPLICATION_DONE;
+ s->error = 0;
+ } else {
+ backup_job_cleanup(s);
+ }
}
static bool check_top_bs(BlockDriverState *top_bs, BlockDriverState *bs)
@@ -450,6 +454,15 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
switch (s->mode) {
case REPLICATION_MODE_PRIMARY:
+ if (s->is_shared_disk) {
+ backup_start("replication-backup", s->primary_disk->bs, bs, 0,
+ MIRROR_SYNC_MODE_NONE, NULL, false, BLOCKDEV_ON_ERROR_REPORT,
+ BLOCKDEV_ON_ERROR_REPORT, backup_job_completed,
+ s, NULL, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+ }
break;
case REPLICATION_MODE_SECONDARY:
s->active_disk = bs->file;
@@ -468,7 +481,8 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
}
s->secondary_disk = s->hidden_disk->bs->backing;
- if (!s->secondary_disk->bs || !bdrv_has_blk(s->secondary_disk->bs)) {
+ if (!s->secondary_disk->bs ||
+ (!s->is_shared_disk && !bdrv_has_blk(s->secondary_disk->bs))) {
error_setg(errp, "The secondary disk doesn't have block backend");
aio_context_release(aio_context);
return;
@@ -560,11 +574,24 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
switch (s->mode) {
case REPLICATION_MODE_PRIMARY:
+ if (s->is_shared_disk) {
+ if (!s->primary_disk->bs->job) {
+ error_setg(errp, "Primary backup job was cancelled"
+ " unexpectedly");
+ break;
+ }
+
+ backup_do_checkpoint(s->primary_disk->bs->job, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+ }
break;
case REPLICATION_MODE_SECONDARY:
if (!s->is_shared_disk) {
if (!s->secondary_disk->bs->job) {
- error_setg(errp, "Backup job was cancelled unexpectedly");
+ error_setg(errp, "Secondary backup job was cancelled"
+ " unexpectedly");
break;
}
backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
@@ -643,8 +670,12 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
switch (s->mode) {
case REPLICATION_MODE_PRIMARY:
- s->replication_state = BLOCK_REPLICATION_DONE;
- s->error = 0;
+ if (s->is_shared_disk && s->primary_disk->bs->job) {
+ block_job_cancel(s->primary_disk->bs->job);
+ } else {
+ s->replication_state = BLOCK_REPLICATION_DONE;
+ s->error = 0;
+ }
break;
case REPLICATION_MODE_SECONDARY:
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (5 preceding siblings ...)
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 6/7] replication: Implement block replication for shared disk case zhanghailiang
@ 2016-10-20 13:57 ` zhanghailiang
2016-10-20 15:34 ` Eric Blake
2016-10-26 2:06 ` [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case Changlong Xie
2016-11-22 10:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
8 siblings, 1 reply; 24+ messages in thread
From: zhanghailiang @ 2016-10-20 13:57 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, xiecl.fnst, zhanghailiang
Without this callback, there will be an error reports in the primary side:
"qemu-system-x86_64: Couldn't determine the cluster size of the target image,
which has no backing file: Operation not supported
Aborting, since this may create an unusable destination image"
For nbd driver, it doesn't have cluster size, so here we return
a fake value for it.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
block/nbd.c | 12 ++++++++++++
block/replication.c | 6 ++++++
2 files changed, 18 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index 6bc06d6..96d7023 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -40,6 +40,8 @@
#define EN_OPTSTR ":exportname="
+#define NBD_FAKE_CLUSTER_SIZE 512
+
typedef struct BDRVNBDState {
NbdClientSession client;
@@ -483,6 +485,13 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+ bdi->cluster_size = NBD_FAKE_CLUSTER_SIZE;
+
+ return 0;
+}
+
static BlockDriver bdrv_nbd = {
.format_name = "nbd",
.protocol_name = "nbd",
@@ -499,6 +508,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_get_info = nbd_get_info,
};
static BlockDriver bdrv_nbd_tcp = {
@@ -517,6 +527,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_get_info = nbd_get_info,
};
static BlockDriver bdrv_nbd_unix = {
@@ -535,6 +546,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_get_info = nbd_get_info,
};
static void bdrv_nbd_init(void)
diff --git a/block/replication.c b/block/replication.c
index e66b1ca..14c718e 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -707,6 +707,11 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
aio_context_release(aio_context);
}
+static int replication_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+ return bdrv_get_info(bs->file->bs, bdi);
+}
+
BlockDriver bdrv_replication = {
.format_name = "replication",
.protocol_name = "replication",
@@ -719,6 +724,7 @@ BlockDriver bdrv_replication = {
.bdrv_co_readv = replication_co_readv,
.bdrv_co_writev = replication_co_writev,
+ .bdrv_get_info = replication_get_info,
.is_filter = true,
.bdrv_recurse_is_first_non_filter = replication_recurse_is_first_non_filter,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver zhanghailiang
@ 2016-10-20 15:34 ` Eric Blake
2016-10-24 2:44 ` Hailiang Zhang
0 siblings, 1 reply; 24+ messages in thread
From: Eric Blake @ 2016-10-20 15:34 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: kwolf, xiecl.fnst, mreitz, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]
On 10/20/2016 08:57 AM, zhanghailiang wrote:
> Without this callback, there will be an error reports in the primary side:
> "qemu-system-x86_64: Couldn't determine the cluster size of the target image,
> which has no backing file: Operation not supported
> Aborting, since this may create an unusable destination image"
>
> For nbd driver, it doesn't have cluster size, so here we return
> a fake value for it.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> block/nbd.c | 12 ++++++++++++
> block/replication.c | 6 ++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 6bc06d6..96d7023 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -40,6 +40,8 @@
>
> #define EN_OPTSTR ":exportname="
>
> +#define NBD_FAKE_CLUSTER_SIZE 512
Why 512? NBD allows byte-addressable operations (even if it is more
efficient on aligned I/O); and I've been working hard to convert things
to the point that NBD does not enforce alignment on other layers.
Wouldn't 1 be better?
> +static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> +{
> + bdi->cluster_size = NBD_FAKE_CLUSTER_SIZE;
I also have patches written (but waiting for NBD write zeroes support to
be reviewed first) that add support for the experimental NBD block info,
that lets a server advertise actual sizes to the client rather than
having to guess. Here's the last time I posted a preview of it:
https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg03567.html
It would be nice to use that instead of just faking things.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver
2016-10-20 15:34 ` Eric Blake
@ 2016-10-24 2:44 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-10-24 2:44 UTC (permalink / raw)
To: Eric Blake, qemu-devel, qemu-block
Cc: kwolf, xiecl.fnst, mreitz, stefanha, pbonzini
Hi,
On 2016/10/20 23:34, Eric Blake wrote:
> On 10/20/2016 08:57 AM, zhanghailiang wrote:
>> Without this callback, there will be an error reports in the primary side:
>> "qemu-system-x86_64: Couldn't determine the cluster size of the target image,
>> which has no backing file: Operation not supported
>> Aborting, since this may create an unusable destination image"
>>
>> For nbd driver, it doesn't have cluster size, so here we return
>> a fake value for it.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> block/nbd.c | 12 ++++++++++++
>> block/replication.c | 6 ++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 6bc06d6..96d7023 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -40,6 +40,8 @@
>>
>> #define EN_OPTSTR ":exportname="
>>
>> +#define NBD_FAKE_CLUSTER_SIZE 512
>
> Why 512? NBD allows byte-addressable operations (even if it is more
> efficient on aligned I/O); and I've been working hard to convert things
> to the point that NBD does not enforce alignment on other layers.
> Wouldn't 1 be better?
>
Yes, that makes no difference for block replication driver. :)
>> +static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>> +{
>> + bdi->cluster_size = NBD_FAKE_CLUSTER_SIZE;
>
> I also have patches written (but waiting for NBD write zeroes support to
> be reviewed first) that add support for the experimental NBD block info,
> that lets a server advertise actual sizes to the client rather than
> having to guess. Here's the last time I posted a preview of it:
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg03567.html
>
> It would be nice to use that instead of just faking things.
>
That's great, that's what we really want, here it is just a
temporary solution, I'll drop this patch after you nbd patch been merged.
Thanks,
Hailiang
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (6 preceding siblings ...)
2016-10-20 13:57 ` [Qemu-devel] [PATCH RFC 7/7] nbd/replication: implement .bdrv_get_info() for nbd and replication driver zhanghailiang
@ 2016-10-26 2:06 ` Changlong Xie
2016-11-22 10:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
8 siblings, 0 replies; 24+ messages in thread
From: Changlong Xie @ 2016-10-26 2:06 UTC (permalink / raw)
To: zhanghailiang, qemu-devel, qemu-block
Cc: stefanha, kwolf, mreitz, pbonzini, wency, Juan Quintela,
Amit Shah, Dr . David Alan Gilbert
I did't review p5/p6, I think you can merge p5/p6 into a single one.
Also don't forget update qapi/block-core.json with p3.
Thanks
-Xie
On 10/20/2016 09:57 PM, zhanghailiang wrote:
> COLO block replication doesn't support the shared disk case,
> Here we try to implement it.
>
> Just as the scenario of non-shared disk block replication,
> we are going to implement block replication from many basic
> blocks that are already in QEMU.
> The architecture is:
>
> virtio-blk || .----------
> / || | Secondary
> / || '----------
> / || virtio-blk
> / || |
> | || replication(5)
> | NBD --------> NBD (2) |
> | client || server ---> hidden disk <-- active disk(4)
> | ^ || |
> | replication(1) || |
> | | || |
> | +-----------------' || |
> (3) |drive-backup sync=none || |
> --------. | +-----------------+ || |
> Primary | | | || backing |
> --------' | | || |
> V | |
> +-------------------------------------------+ |
> | shared disk | <----------+
> +-------------------------------------------+
> 1) Primary writes will read original data and forward it to Secondary
> QEMU.
> 2) The hidden-disk will buffers the original content that is modified
> by the primary VM. It should also be an empty disk, and
> the driver supports bdrv_make_empty() and backing file.
> 3) Primary write requests will be written to Shared disk.
> 4) Secondary write requests will be buffered in the active disk and it
> will overwrite the existing sector content in the buffe
>
> For more details, please refer to patch 1.
>
> The complete codes can be found from the link:
> https://github.com/coloft/qemu/tree/colo-v5.1-developing-COLO-frame-v21-with-shared-disk
>
> Test steps:
> 1. Secondary:
> # x86_64-softmmu/qemu-system-x86_64 -boot c -m 2048 -smp 2 -qmp stdio -vnc :9 -name secondary -enable-kvm -cpu qemu64,+kvmclock -device piix3-usb-uhci -drive if=none,driver=qcow2,file.filename=/mnt/ramfs/hidden_disk.img,id=hidden_disk0,backing.driver=raw,backing.file.filename=/work/kvm/suse11_sp3_64 -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 -incoming tcp:0:9999
>
> Issue qmp commands:
> {'execute':'qmp_capabilities'}
> {'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet', 'data': {'host': '0', 'port': '9998'} } } }
> {'execute': 'nbd-server-add', 'arguments': {'device': 'hidden_disk0', 'writable': true } }
>
> 2.Primary:
> # x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 2048 -smp 2 -qmp stdio -vnc :9 -name primary -cpu qemu64,+kvmclock -device piix3-usb-uhci -drive if=virtio,id=primary_disk0,file.filename=/work/kvm/suse11_sp3_64,driver=raw -S
>
> Issue qmp commands:
> {'execute':'qmp_capabilities'}
> {'execute': 'human-monitor-command', 'arguments': {'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=9.42.3.17,file.port=9998,file.export=hidden_disk0,shared-disk-id=primary_disk0,shared-disk=on,node-name=rep'}}
> {'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> {'execute': 'migrate', 'arguments': {'uri': 'tcp:9.42.3.17:9999' } }
>
> 3. Failover
> Secondary side:
> Issue qmp commands:
> { 'execute': 'nbd-server-stop' }
> { "execute": "x-colo-lost-heartbeat" }
>
> Please review and any commits are welcomed.
>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> Cc: Dr. David Alan Gilbert (git) <dgilbert@redhat.com>
>
> zhanghailiang (7):
> docs/block-replication: Add description for shared-disk case
> block-backend: Introduce blk_root() helper
> replication: add shared-disk and shared-disk-id options
> replication: Split out backup_do_checkpoint() from
> secondary_do_checkpoint()
> replication: fix code logic with the new shared_disk option
> replication: Implement block replication for shared disk case
> nbd/replication: implement .bdrv_get_info() for nbd and replication
> driver
>
> block/block-backend.c | 5 ++
> block/nbd.c | 12 ++++
> block/replication.c | 146 +++++++++++++++++++++++++++++++----------
> docs/block-replication.txt | 131 ++++++++++++++++++++++++++++++++++--
> include/sysemu/block-backend.h | 1 +
> 5 files changed, 258 insertions(+), 37 deletions(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH RFC 0/7] COLO block replication supports shared disk case
2016-10-20 13:57 [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case zhanghailiang
` (7 preceding siblings ...)
2016-10-26 2:06 ` [Qemu-devel] [PATCH RFC 0/7] COLO block replication supports shared disk case Changlong Xie
@ 2016-11-22 10:33 ` Stefan Hajnoczi
2016-11-23 1:47 ` Hailiang Zhang
8 siblings, 1 reply; 24+ messages in thread
From: Stefan Hajnoczi @ 2016-11-22 10:33 UTC (permalink / raw)
To: zhanghailiang
Cc: qemu-devel, qemu-block, kwolf, xiecl.fnst, wency, Juan Quintela,
Dr . David Alan Gilbert, mreitz, stefanha, Amit Shah, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]
On Thu, Oct 20, 2016 at 09:57:33PM +0800, zhanghailiang wrote:
> COLO block replication doesn't support the shared disk case,
> Here we try to implement it.
>
> Just as the scenario of non-shared disk block replication,
> we are going to implement block replication from many basic
> blocks that are already in QEMU.
> The architecture is:
>
> virtio-blk || .----------
> / || | Secondary
> / || '----------
> / || virtio-blk
> / || |
> | || replication(5)
> | NBD --------> NBD (2) |
> | client || server ---> hidden disk <-- active disk(4)
> | ^ || |
> | replication(1) || |
> | | || |
> | +-----------------' || |
> (3) |drive-backup sync=none || |
> --------. | +-----------------+ || |
> Primary | | | || backing |
> --------' | | || |
> V | |
> +-------------------------------------------+ |
> | shared disk | <----------+
> +-------------------------------------------+
> 1) Primary writes will read original data and forward it to Secondary
> QEMU.
> 2) The hidden-disk will buffers the original content that is modified
> by the primary VM. It should also be an empty disk, and
> the driver supports bdrv_make_empty() and backing file.
> 3) Primary write requests will be written to Shared disk.
> 4) Secondary write requests will be buffered in the active disk and it
> will overwrite the existing sector content in the buffe
This design looks good. I have not reviewed the patches in detail but
will review the next revision.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH RFC 0/7] COLO block replication supports shared disk case
2016-11-22 10:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2016-11-23 1:47 ` Hailiang Zhang
0 siblings, 0 replies; 24+ messages in thread
From: Hailiang Zhang @ 2016-11-23 1:47 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, qemu-block, kwolf, xiecl.fnst, wency, Juan Quintela,
Dr . David Alan Gilbert, mreitz, stefanha, Amit Shah, pbonzini
On 2016/11/22 18:33, Stefan Hajnoczi wrote:
> On Thu, Oct 20, 2016 at 09:57:33PM +0800, zhanghailiang wrote:
>> COLO block replication doesn't support the shared disk case,
>> Here we try to implement it.
>>
>> Just as the scenario of non-shared disk block replication,
>> we are going to implement block replication from many basic
>> blocks that are already in QEMU.
>> The architecture is:
>>
>> virtio-blk || .----------
>> / || | Secondary
>> / || '----------
>> / || virtio-blk
>> / || |
>> | || replication(5)
>> | NBD --------> NBD (2) |
>> | client || server ---> hidden disk <-- active disk(4)
>> | ^ || |
>> | replication(1) || |
>> | | || |
>> | +-----------------' || |
>> (3) |drive-backup sync=none || |
>> --------. | +-----------------+ || |
>> Primary | | | || backing |
>> --------' | | || |
>> V | |
>> +-------------------------------------------+ |
>> | shared disk | <----------+
>> +-------------------------------------------+
>> 1) Primary writes will read original data and forward it to Secondary
>> QEMU.
>> 2) The hidden-disk will buffers the original content that is modified
>> by the primary VM. It should also be an empty disk, and
>> the driver supports bdrv_make_empty() and backing file.
>> 3) Primary write requests will be written to Shared disk.
>> 4) Secondary write requests will be buffered in the active disk and it
>> will overwrite the existing sector content in the buffe
>
> This design looks good. I have not reviewed the patches in detail but
> will review the next revision.
>
Thanks very much, I'll update it with the recent upstream. :)
> Stefan
>
^ permalink raw reply [flat|nested] 24+ messages in thread