On Mon, Nov 02, 2015 at 14:34:43 +0300, rodinasophie@xxxxxxxxx wrote:
> From: sonya <rodinasophie@xxxxxxxxx>
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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list