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