[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v20 07/10] Introduce new APIs to do replication
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v20 07/10] Introduce new APIs to do replication operation |
Date: |
Wed, 8 Jun 2016 07:41:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/07/2016 07:11 PM, Changlong Xie wrote:
> Signed-off-by: Wen Congyang <address@hidden>
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> Signed-off-by: Changlong Xie <address@hidden>
No mention of the API names in the commit message? Grepping 'git log'
is easier if there is something useful to grep for.
> ---
> Makefile.objs | 1 +
> qapi/block-core.json | 13 ++++
> replication.c | 105 ++++++++++++++++++++++++++++++
> replication.h | 176
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 295 insertions(+)
> create mode 100644 replication.c
> create mode 100644 replication.h
>
> +++ b/qapi/block-core.json
> @@ -2032,6 +2032,19 @@
> '*read-pattern': 'QuorumReadPattern' } }
>
> ##
> +# @ReplicationMode
> +#
> +# An enumeration of replication modes.
> +#
> +# @primary: Primary mode, the vm's state will be sent to secondary QEMU.
> +#
> +# @secondary: Secondary mode, receive the vm's state from primary QEMU.
> +#
> +# Since: 2.7
> +##
> +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
> +
This part is fine, from an interface point of view. However, I have not
closely reviewed the rest of the patch or series. That said, here's
some quick things that caught my eye.
> +++ b/replication.c
> @@ -0,0 +1,105 @@
> +/*
> + * Replication filter
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 Intel Corporation
> + * Copyright (c) 2016 FUJITSU LIMITED
> + *
> + * Author:
> + * Changlong Xie <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "replication.h"
All new .c files must include "qemu/osdep.h" first.
> +++ b/replication.h
> @@ -0,0 +1,176 @@
> +/*
> + * Replication filter
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + * Copyright (c) 2016 Intel Corporation
> + * Copyright (c) 2016 FUJITSU LIMITED
> + *
> + * Author:
> + * Changlong Xie <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef REPLICATION_H
> +#define REPLICATION_H
> +
> +#include "qemu/osdep.h"
And .h files must NOT include osdep.h.
> +#include "qapi/error.h"
Do you really need the full error.h, or is typedefs.h enough to get the
forward declaration of Error?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v20 00/10] Block replication for continuous checkpoints, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 01/10] unblock backup operations in backing file, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 02/10] Backup: clear all bitmap when doing block checkpoint, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 04/10] Link backup into block core, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 05/10] docs: block replication's description, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 03/10] Backup: export interfaces for extra serialization, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 07/10] Introduce new APIs to do replication operation, Changlong Xie, 2016/06/07
- Re: [Qemu-devel] [PATCH v20 07/10] Introduce new APIs to do replication operation,
Eric Blake <=
- [Qemu-devel] [PATCH v20 09/10] tests: add unit test case for replication, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 10/10] support replication driver in blockdev-add, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 06/10] auto complete active commit, Changlong Xie, 2016/06/07
- [Qemu-devel] [PATCH v20 08/10] Implement new driver for block replication, Changlong Xie, 2016/06/07
- Re: [Qemu-devel] [PATCH v20 00/10] Block replication for continuous checkpoints, Eric Blake, 2016/06/08
- Re: [Qemu-devel] [PATCH v20 00/10] Block replication for continuous checkpoints, Michael Tokarev, 2016/06/10