On 06/07/2016 07:11 PM, Changlong Xie wrote: > Signed-off-by: Wen Congyang <[email protected]> > Signed-off-by: zhanghailiang <[email protected]> > Signed-off-by: Gonglei <[email protected]> > Signed-off-by: Changlong Xie <[email protected]>
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 <[email protected]>
> + *
> + * 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 <[email protected]>
> + *
> + * 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
