Bug 1086726
| Summary: | Kill all uses of __FUNCTION__ in error messages | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Community] Virtualization Tools | Reporter: | Shanzhi Yu <shyu> | ||||||
| Component: | libvirt | Assignee: | Eric Blake <eblake> | ||||||
| Status: | CLOSED UPSTREAM | QA Contact: | |||||||
| Severity: | low | Docs Contact: | |||||||
| Priority: | low | ||||||||
| Version: | unspecified | CC: | ashu.noella207, dyuan, eblake, jdenemar, mprivozn, mzhan, rbalakri | ||||||
| Target Milestone: | --- | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2015-04-03 12:27:43 UTC | Type: | Bug | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Attachments: |
|
||||||||
|
Description
Shanzhi Yu
2014-04-11 11:39:59 UTC
(In reply to Shanzhi Yu from comment #0) > #virsh snapshot-revert rhel6 s1 --running --paused > error: running and paused flags in virDomainRevertToSnapshot are mutually > exclusive > > Expected results: > > The error output can be more friendly. > There is no need to show up virDomainRevertToSnapshot to user. What exactly do you propose? I'm not in the mood to duplicate error checking that normally occurs in virDomainRevertToSnapshot to also occur in virsh - one of the fundamental principles of virsh is that it should ALLOW conflicting flags to be passed into underlying API so that we can prove the underlying API is properly rejecting the conflicting flags. I'm inclined to close this as WONTFIX. Alternatively, we could use this bug as a reason to kill all use of __FUNCTION__ in the body of error messages in src/libvirt.c - after all, 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 and thus bug reports like this. Hello everyone, I will love to work on this bug. Looking at /src/util/virerror.* I'm thinking of reworking some of the functions to remove references to the __FUNC__ arguments for string message errors. i would like to know which particular functions where this is necessary. Thanks, Noella I am working on the above bug to remove all references to __FUNC__ in error messages. While trying to rework virError to remove __FUNC__ args in error message. I searched for src/libvirt* files containing __FUNC__ args and come across libvirt_domain.c only with references to this. While following +Eric's comments to kill all __FUNC__ references in src/libvirt.c. Only virReportErrorHelper has references to __FUNC__ arguments. Dunno if it's ok to rework only these(libvirt-domain.c, libvirt.c) or is there anything else i may be missing? Created attachment 1009206 [details]
Reworked some error messages
Created attachment 1009645 [details]
__FUNCTION__ discard from error mesages
this should fix libvirt.c libvirt-domain.c libvirt-domain-snapshot.c
I've just pushed the patch upstream:
commit c4db8c5ee3e7bc4559ccde83587ff5318e115865
Author: Noella Ashu <ashu.noella207>
AuthorDate: Fri Apr 3 11:33:44 2015 +0100
Commit: Michal Privoznik <mprivozn>
CommitDate: Fri Apr 3 14:02:54 2015 +0200
libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
The error output of snapshot-revert should be more friendly.
There is no need to show virDomainRevertToSnapshot to user.
virReportError 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
v1.2.14-45-gc4db8c5
|