On Fri, Apr 3, 2015 at 1:13 PM, Michal Privoznik <mprivozn@redhat.com> wrote:

On 03.04.2015 12:33, Noella Ashu wrote:
> The error output of snapshot-revert should be more friendly. There is no
> need to show up virDomainRevertToSnapshot to user. virError already includes
> __FUNCTION__ information in a separate member of the struct, so repeating
> it in the message is redundant and leads to situations where higher level
> code ends up reporting the lower level name We correctly converted the
> error output making it more succinct and user-friendly.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726
> ---
> src/libvirt-domain-snapshot.c | 30 +++----
> src/libvirt-domain.c | 201 ++++++++++++++++++------------------------
> 2 files changed, 96 insertions(+), 135 deletions(-)

I guess I should have been more accurate: you need to make sure the code compiles after your patch too. 'make syntax-check' and 'make check' are needed too, but make no sense without pure 'make all'. What's needed is something like following:

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f9db3ad..0acfd13 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1438,7 +1438,7 @@ virDomainScreenshot(virDomainPtr domain,
virCheckReadOnlyGoto(domain->conn->flags, error);

if (domain->conn != stream->conn) {
- virReportInvalidArg(stream, "%s",
+ virReportInvalidArg(stream,
_("stream must match connection of domain '%s'"),
domain->name);
goto error;
@@ -2177,7 +2177,7 @@ virDomainGetMemoryParameters(virDomainPtr domain,
/* At most one of these two flags should be set. */
if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags, "%s",
+ virReportInvalidArg(flags, "%s",
_("flags 'affect live' and 'affect config' are mutually exclusive"));
goto error;
}
....

Anyway, I've fixed all the compilation errors, ACKed and pushed. Congratulations on your first libvirt contribution!

Michal