Re: [libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages

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