Re: [Qemu-devel] [PATCH v1] migration: add incremental mode on drive-mirror
I redescribe details on cover letter and send a new mail in December 20, 2018.
Please help me review again, thank you.
在 2018/12/19 下午10:48，“Eric Blake” 写入:
On 12/19/18 12:50 AM, mahaocong wrote:
> From: mahaocong
>
> Signed-off-by: mahaocong
The subject line tells "what", but you are missing a commit message body
that gives the "why". Details about the new feature, and how it is
useful, go a long way to making the code easier to review. How much of
your cover letter should be here as well?
At a first glance, I'm just going to focus on the interface:
> ---
> block/dirty-bitmap.c | 14 ++
> block/mirror.c | 63
+++-
> blockdev.c | 36 +++--
> include/block/block_int.h| 3 ++-
> include/block/dirty-bitmap.h | 3 +++
> include/qemu/hbitmap.h | 2 ++
> qapi/block-core.json | 4 ++-
> util/hbitmap.c | 28
> 8 files changed, 136 insertions(+), 17 deletions(-)
>
> +++ b/qapi/block-core.json
> @@ -1727,6 +1727,8 @@
> #(all the disk, only the sectors allocated in the topmost
image, or
> #only new I/O).
> #
> +# @bitmap-name: the bitmap to be used in incremental mode.
Missing a '(since 4.0)' tag. And doesn't really explain what
incremental mode in a mirroring operation actually means.
> +#
> # @granularity: granularity of the dirty bitmap, default is 64K
> # if the image format doesn't have clusters, 4K if the
clusters
> # are smaller than that, else the cluster size. Must be a
> @@ -1768,7 +1770,7 @@
> { 'struct': 'DriveMirror',
> 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
> '*format': 'str', '*node-name': 'str', '*replaces': 'str',
> -'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
> +'sync': 'MirrorSyncMode', '*bitmap-name': 'str', '*mode':
'NewImageMode',
Can this be named just 'bitmap', instead of the longer 'bitmap-name'?
> '*speed': 'int', '*granularity': 'uint32',
> '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
> '*on-target-error': 'BlockdevOnError',
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 8d402c5..7ae2fc2 100644
> --- a/util/hbitmap.c
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v1] migration: add incremental mode on drive-mirror
On 12/19/18 12:50 AM, mahaocong wrote:
From: mahaocong
Signed-off-by: mahaocong
The subject line tells "what", but you are missing a commit message body
that gives the "why". Details about the new feature, and how it is
useful, go a long way to making the code easier to review. How much of
your cover letter should be here as well?
At a first glance, I'm just going to focus on the interface:
---
block/dirty-bitmap.c | 14 ++
block/mirror.c | 63 +++-
blockdev.c | 36 +++--
include/block/block_int.h| 3 ++-
include/block/dirty-bitmap.h | 3 +++
include/qemu/hbitmap.h | 2 ++
qapi/block-core.json | 4 ++-
util/hbitmap.c | 28
8 files changed, 136 insertions(+), 17 deletions(-)
+++ b/qapi/block-core.json
@@ -1727,6 +1727,8 @@
#(all the disk, only the sectors allocated in the topmost image, or
#only new I/O).
#
+# @bitmap-name: the bitmap to be used in incremental mode.
Missing a '(since 4.0)' tag. And doesn't really explain what
incremental mode in a mirroring operation actually means.
+#
# @granularity: granularity of the dirty bitmap, default is 64K
# if the image format doesn't have clusters, 4K if the clusters
# are smaller than that, else the cluster size. Must be a
@@ -1768,7 +1770,7 @@
{ 'struct': 'DriveMirror',
'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
'*format': 'str', '*node-name': 'str', '*replaces': 'str',
-'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
+'sync': 'MirrorSyncMode', '*bitmap-name': 'str', '*mode':
'NewImageMode',
Can this be named just 'bitmap', instead of the longer 'bitmap-name'?
'*speed': 'int', '*granularity': 'uint32',
'*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 8d402c5..7ae2fc2 100644
--- a/util/hbitmap.c
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
