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

Wednesday, 1 April 2015

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(-)

diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 9feb669..ac858ba 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -222,26 +222,20 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
 
     if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) &&
         !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
-        virReportInvalidArg(flags,
-                            _("use of 'current' flag in %s requires "
-                              "'redefine' flag"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("use of 'current' flag in requires
'redefine' flag"));
         goto error;
     }
     if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
         (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
-        virReportInvalidArg(flags,
-                            _("'redefine' and 'no metadata' flags in
%s are "
-                              "mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("'redefine' and 'no metadata' flags in
are mutually exclusive"));
         goto error;
     }
     if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
         (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
-        virReportInvalidArg(flags,
-                            _("'redefine' and 'halt' flags in %s are
mutually "
-                              "exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("'redefine' and 'halt' flags are
mutually exclusive"));
         goto error;
     }
 
@@ -1084,10 +1078,8 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
 
     if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
         (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-        virReportInvalidArg(flags,
-                            _("running and paused flags in %s are mutually "
-                              "exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("running and paused flags are mutually
exclusive"));
         goto error;
     }
 
@@ -1146,10 +1138,8 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
 
     if ((flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) &&
         (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
-        virReportInvalidArg(flags,
-                            _("children and children_only flags in %s are "
-                              "mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("children and children_only flags are mutually
exclusive"));
         goto error;
     }
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4a45b9e..78b7a93 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -385,9 +385,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char *uuidstr)
     virCheckNonNullArgGoto(uuidstr, error);
 
     if (virUUIDParse(uuidstr, uuid) < 0) {
-        virReportInvalidArg(uuidstr,
-                            _("uuidstr in %s must be a valid UUID"),
-                            __FUNCTION__);
+        virReportInvalidArg(uuidstr, "%s", _("Invalid UUID"));
         goto error;
     }
 
@@ -1440,9 +1438,9 @@ virDomainScreenshot(virDomainPtr domain,
     virCheckReadOnlyGoto(domain->conn->flags, error);
 
     if (domain->conn != stream->conn) {
-        virReportInvalidArg(stream,
-                            _("stream in %s must match connection of domain
'%s'"),
-                            __FUNCTION__, domain->name);
+        virReportInvalidArg(stream, "%s", 
+                            _("stream must match connection of domain
'%s'"),
+                            domain->name);
         goto error;
     }
 
@@ -2179,10 +2177,8 @@ 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,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s", 
+                            _("flags 'affect live' and 'affect
config' are mutually exclusive"));
         goto error;
     }
     conn = domain->conn;
@@ -2423,10 +2419,8 @@ virDomainGetBlkioParameters(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,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s", 
+                            _("flags 'affect live' and 'affect
config' are mutually exclusive"));
         goto error;
     }
     conn = domain->conn;
@@ -3343,9 +3337,8 @@ virDomainMigratePeer2PeerFull(virDomainPtr domain,
     if (!(tempuri = virURIParse(dconnuri)))
         return -1;
     if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
-        virReportInvalidArg(dconnuri,
-                            _("unable to parse server from dconnuri in %s"),
-                            __FUNCTION__);
+        virReportInvalidArg(dconnuri, "%s",
+                            _("unable to parse server from dconnuri"));
         virURIFree(tempuri);
         return -1;
     }
@@ -3580,10 +3573,9 @@ virDomainMigrate(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
-                            _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("flags 'shared disk' and 'shared
incremental'"
+                              " are mutually exclusive"));
         goto error;
     }
 
@@ -3809,10 +3801,9 @@ virDomainMigrate2(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
+        virReportInvalidArg(flags, "%s",
                             _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -3989,10 +3980,9 @@ virDomainMigrate3(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
+        virReportInvalidArg(flags, "%s",
                             _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+                              "are mutually exclusive"));
         goto error;
     }
     if (flags & VIR_MIGRATE_PEER2PEER) {
@@ -4212,10 +4202,9 @@ virDomainMigrateToURI(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
+        virReportInvalidArg(flags, "%s",
                             _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -4372,10 +4361,9 @@ virDomainMigrateToURI2(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
+        virReportInvalidArg(flags, "%s",
                             _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -4481,10 +4469,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
 
     if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
         flags & VIR_MIGRATE_NON_SHARED_INC) {
-        virReportInvalidArg(flags,
+        virReportInvalidArg(flags, "%s",
                             _("flags 'shared disk' and 'shared
incremental' "
-                              "in %s are mutually exclusive"),
-                            __FUNCTION__);
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -4791,9 +4778,8 @@ virDomainMigratePrepareTunnel(virConnectPtr conn,
     virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn != st->conn) {
-        virReportInvalidArg(conn,
-                            _("conn in %s must match stream connection"),
-                            __FUNCTION__);
+        virReportInvalidArg(conn, "%s",
+                            _("conn must match stream connection"));
         goto error;
     }
 
@@ -4937,9 +4923,8 @@ virDomainMigratePrepareTunnel3(virConnectPtr conn,
     virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn != st->conn) {
-        virReportInvalidArg(conn,
-                            _("conn in %s must match stream connection"),
-                            __FUNCTION__);
+        virReportInvalidArg(conn, "%s", 
+                            _("conn must match stream connection"));
         goto error;
     }
 
@@ -5221,8 +5206,7 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
 
     if (conn != st->conn) {
         virReportInvalidArg(conn,
-                            _("conn in %s must match stream connection"),
-                            __FUNCTION__);
+                            _("conn must match stream connection"));
         goto error;
     }
 
@@ -5529,10 +5513,9 @@ virDomainGetSchedulerParametersFlags(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,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("flags 'affect live' and 'affect
config' in "
+                              "are mutually exclusive"));
         goto error;
     }
     conn = domain->conn;
@@ -5709,9 +5692,9 @@ virDomainBlockStats(virDomainPtr dom, const char *disk,
     virCheckNonNullArgGoto(disk, error);
     virCheckNonNullArgGoto(stats, error);
     if (size > sizeof(stats2)) {
-        virReportInvalidArg(size,
-                            _("size in %s must not exceed %zu"),
-                            __FUNCTION__, sizeof(stats2));
+        virReportInvalidArg(size, "%s",
+                            _("size must not exceed %zu"),
+                            sizeof(stats2));
         goto error;
     }
     conn = dom->conn;
@@ -5850,9 +5833,9 @@ virDomainInterfaceStats(virDomainPtr dom, const char *path,
     virCheckNonNullArgGoto(path, error);
     virCheckNonNullArgGoto(stats, error);
     if (size > sizeof(stats2)) {
-        virReportInvalidArg(size,
-                            _("size in %s must not exceed %zu"),
-                            __FUNCTION__, sizeof(stats2));
+        virReportInvalidArg(size, "%s",
+                            _("size must not exceed %zu"),
+                            sizeof(stats2));
         goto error;
     }
 
@@ -6297,10 +6280,9 @@ virDomainMemoryPeek(virDomainPtr dom,
 
     /* Exactly one of these two flags must be set.  */
     if (!(flags & VIR_MEMORY_VIRTUAL) == !(flags & VIR_MEMORY_PHYSICAL)) {
-        virReportInvalidArg(flags,
-                            _("flags in %s must include VIR_MEMORY_VIRTUAL or
"
-                              "VIR_MEMORY_PHYSICAL"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("flags must include VIR_MEMORY_VIRTUAL or "
+                              "VIR_MEMORY_PHYSICAL"));
         goto error;
     }
 
@@ -7144,9 +7126,9 @@ virDomainSendKey(virDomainPtr domain,
     virCheckPositiveArgGoto(nkeycodes, error);
 
     if (nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
-        virReportInvalidArg(nkeycodes,
-                            _("nkeycodes in %s must be <= %d"),
-                            __FUNCTION__, VIR_DOMAIN_SEND_KEY_MAX_KEYS);
+        virReportInvalidArg(nkeycodes, "%s", 
+                            _("nkeycodes must be <= %d"),
+                            VIR_DOMAIN_SEND_KEY_MAX_KEYS);
         goto error;
     }
 
@@ -7344,8 +7326,8 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
         flags & VIR_DOMAIN_VCPU_MAXIMUM) {
         virReportInvalidArg(flags,
                             _("flags 'VIR_DOMAIN_VCPU_MAXIMUM' and "
-                              "'VIR_DOMAIN_VCPU_GUEST' in '%s' are
mutually "
-                              "exclusive"), __FUNCTION__);
+                              "'VIR_DOMAIN_VCPU_GUEST' in are mutually
"
+                              "exclusive"));
         goto error;
     }
 
@@ -7419,10 +7401,9 @@ virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
     /* At most one of these two flags should be set.  */
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
-        virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s", 
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -7626,9 +7607,8 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps,
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
         virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -7756,9 +7736,8 @@ virDomainGetEmulatorPinInfo(virDomainPtr domain, unsigned char
*cpumap,
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
         virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
     conn = domain->conn;
@@ -7925,10 +7904,9 @@ virDomainGetIOThreadInfo(virDomainPtr dom,
     /* At most one of these two flags should be set.  */
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
-        virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s",
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -8181,10 +8159,9 @@ virDomainSetMetadata(virDomainPtr domain,
     switch (type) {
     case VIR_DOMAIN_METADATA_TITLE:
         if (metadata && strchr(metadata, '\n')) {
-            virReportInvalidArg(metadata,
-                                _("metadata title in %s can't contain "
-                                  "newlines"),
-                                __FUNCTION__);
+            virReportInvalidArg(metadata, "%s", 
+                                _("metadata title can't contain "
+                                  "newlines"));
             goto error;
         }
         /* fallthrough */
@@ -8259,10 +8236,9 @@ virDomainGetMetadata(virDomainPtr domain,
 
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
-        virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s", 
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
 
@@ -9199,18 +9175,18 @@ virConnectDomainEventRegisterAny(virConnectPtr conn,
     if (dom) {
         virCheckDomainGoto(dom, error);
         if (dom->conn != conn) {
-            virReportInvalidArg(dom,
-                                _("domain '%s' in %s must match
connection"),
-                                dom->name, __FUNCTION__);
+            virReportInvalidArg(dom, "%s", 
+                                _("domain '%s' must match
connection"),
+                                dom->name);
             goto error;
         }
     }
     virCheckNonNullArgGoto(cb, error);
     virCheckNonNegativeArgGoto(eventID, error);
     if (eventID >= VIR_DOMAIN_EVENT_ID_LAST) {
-        virReportInvalidArg(eventID,
-                            _("eventID in %s must be less than %d"),
-                            __FUNCTION__, VIR_DOMAIN_EVENT_ID_LAST);
+        virReportInvalidArg(eventID, "%s", 
+                            _("eventID must be less than %d"),
+                            VIR_DOMAIN_EVENT_ID_LAST);
         goto error;
     }
 
@@ -9311,9 +9287,8 @@ virDomainManagedSave(virDomainPtr dom, unsigned int flags)
 
     if ((flags & VIR_DOMAIN_SAVE_RUNNING) && (flags &
VIR_DOMAIN_SAVE_PAUSED)) {
         virReportInvalidArg(flags,
-                            _("running and paused flags in %s are mutually "
-                              "exclusive"),
-                            __FUNCTION__);
+                            _("running and paused flags are mutually "
+                              "exclusive"));
         goto error;
     }
 
@@ -9466,9 +9441,9 @@ virDomainOpenConsole(virDomainPtr dom,
     virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn != st->conn) {
-        virReportInvalidArg(st,
-                            _("stream in %s must match connection of domain
'%s'"),
-                            __FUNCTION__, dom->name);
+        virReportInvalidArg(st, "%s", 
+                            _("stream must match connection of domain
'%s'"),
+                            dom->name);
         goto error;
     }
 
@@ -9531,8 +9506,8 @@ virDomainOpenChannel(virDomainPtr dom,
 
     if (conn != st->conn) {
         virReportInvalidArg(st,
-                            _("stream in %s must match connection of domain
'%s'"),
-                            __FUNCTION__, dom->name);
+                            _("stream must match connection of domain
'%s'"),
+                            dom->name);
         goto error;
     }
 
@@ -9955,9 +9930,8 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
                         VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
                         VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
                         VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
-        virReportInvalidArg(flags,
-                            _("use of flags in %s requires a copy job"),
-                            __FUNCTION__);
+        virReportInvalidArg(flags, "%s", 
+                            _("use of flags requires a copy job"));
         goto error;
     }
 
@@ -10276,9 +10250,9 @@ virDomainOpenGraphics(virDomainPtr dom,
     }
 
     if (!S_ISSOCK(sb.st_mode)) {
-        virReportInvalidArg(fd,
-                          _("fd %d in %s must be a socket"),
-                            fd, __FUNCTION__);
+        virReportInvalidArg(fd, "%s", 
+                            _("fd %d must be a socket"),
+                            fd);
         goto error;
     }
 
@@ -10490,9 +10464,8 @@ virDomainGetBlockIoTune(virDomainPtr dom,
     if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
         (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
         virReportInvalidArg(flags,
-                            _("flags 'affect live' and 'affect
config' in %s "
-                              "are mutually exclusive"),
-                            __FUNCTION__);
+                            _("flags 'affect live' and 'affect
config' "
+                              "are mutually exclusive"));
         goto error;
     }
     conn = dom->conn;
@@ -10615,8 +10588,7 @@ virDomainGetCPUStats(virDomainPtr domain,
     if (start_cpu == -1) {
         if (ncpus != 1) {
             virReportInvalidArg(start_cpu,
-                                _("ncpus in %s must be 1 when start_cpu is
-1"),
-                                __FUNCTION__);
+                                _("ncpus must be 1 when start_cpu is -1"));
             goto error;
         }
     } else {
@@ -11284,9 +11256,8 @@ virDomainListGetStats(virDomainPtr *doms,
     virCheckNonNullArgGoto(retStats, cleanup);
 
     if (!*doms) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("doms array must contain at least one domain in %s"),
-                       __FUNCTION__);
+        virReportError(VIR_ERR_INVALID_ARG, "%s", 
+                       _("doms array must contain at least one domain"));
         goto cleanup;
     }
 
@@ -11304,9 +11275,9 @@ virDomainListGetStats(virDomainPtr *doms,
         virCheckDomainGoto(dom, cleanup);
 
         if (dom->conn != conn) {
-            virReportError(VIR_ERR_INVALID_ARG,
+            virReportError(VIR_ERR_INVALID_ARG, "%s", 
                            _("domains in 'doms' array must belong to a
"
-                             "single connection in %s"), __FUNCTION__);
+                             "single connection"));
             goto cleanup;
         }
 
-- 
2.1.0


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

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

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

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