On Mon, Nov 02, 2015 at 14:34:43 +0300, rodinasop...@gmail.com wrote:
> From: sonya <rodinasop...@gmail.com>

The commit message is used to explain the changes.

> 
> ---
>  tools/Makefile.am       |   1 +
>  tools/virsh-domain.c    | 195 
> ++++++++++++++++++++++++++++++++++++------------
>  tools/virsh-edit.c      |  97 +++++-------------------
>  tools/virsh-interface.c |  66 ++++++++++++----
>  tools/virsh-network.c   |  66 ++++++++++++----
>  tools/virsh-nwfilter.c  |  67 +++++++++++++----
>  tools/virsh-pool.c      |  66 ++++++++++++----
>  tools/virsh-snapshot.c  |  78 ++++++++++++++-----
>  8 files changed, 437 insertions(+), 199 deletions(-)
> 
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index d5638d9..88bb1b2 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -205,6 +205,7 @@ virsh_SOURCES =                                           
>         \
>               virsh-domain.c virsh-domain.h                   \
>               virsh-domain-monitor.c virsh-domain-monitor.h   \
>               virsh-host.c virsh-host.h                       \
> +             virsh-edit.c virsh-edit.h                       \
>               virsh-interface.c virsh-interface.h             \
>               virsh-network.c virsh-network.h                 \
>               virsh-nodedev.c virsh-nodedev.h                 \
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 12e85e3..648b086 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -56,6 +56,7 @@
>  #include "virtime.h"
>  #include "virtypedparam.h"
>  #include "virxml.h"
> +#include "virsh-edit.h"

This file is not created in this commit, so this will fail to compile.

>  
>  /* Gnulib doesn't guarantee SA_SIGINFO support.  */
>  #ifndef SA_SIGINFO
> @@ -4622,6 +4623,45 @@ static const vshCmdOptDef opts_save_image_edit[] = {
>      {.name = NULL}
>  };
>  
> +typedef struct editDomainUniversalStruct

We tend to prefix struct names with 'vir', 'vsh' or 'virsh' according to
the section it's used in.

> +{
> +     vshControl *ctl;
> +     virshControlPtr priv;
> +     const char *file;
> +     virDomainPtr dom;
> +     unsigned int getxml_flags;
> +     bool *ret;
> +     unsigned int *define_flags;
> +     const char *key;
> +     virDomainPtr *dom_edited;
> +} editDomainUniversalStruct;

This complete patch has broken formatting. Please use 4 spaces per level
to indent code. (See our contributors documentation for guidelines)

> +
> +static char*

the pointer should be separated with a space

> +editDomainSaveImageGetXML(void *args)

No prefix.

> +{
> +     editDomainUniversalStruct *str = (editDomainUniversalStruct*)args;

This typecast will be done automatically.

> +     return virDomainSaveImageGetXMLDesc(str->priv->conn, str->file, 
> str->getxml_flags);
> +}
> +

At this point, formatting and the fact that this doesn't compile is
enough to break the review here. Please post a fixed version and make
sure that 'make check' and 'make syntax-check' pass after every single
commit. Also plese fill the commit message and fix the formatting
according to guidelines.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to