[libvirt] [PATCH 1/4] General structures and interfaces were added

[Peter Krempa]

On Mon, Nov 02, 2015 at 14:34:43 +0300, rodinasophie at gmail.com wrote:
> From: sonya <rodinasophie at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151102/acc43e42/attachment-0001.sig>