Re: [libvirt] [PATCH] Better error reporting in virsh.
On 04/05/2010 11:37 AM, Chris Lalancette wrote:
This patch remedies the situation somewhat by
printing out the errors inside the command methods
themselves when we know it will go through a cleanup
path that will lose the error.
Sounds reasonable for purposes of better reporting, but it also sounds a
bit fragile - is it always easy to tell what might be clearing the error
later? Is this something where we might inadvertently break things by
rewriting a cleanup: label, and end up with duplicate messages down the
road?
@@ -2623,9 +2623,8 @@ cmdDomXMLFromNative(vshControl *ctl, const
vshCmd *cmd)
format = vshCommandOptString(cmd, "format", NULL);
configFile = vshCommandOptString(cmd, "config", NULL);
- if (virFileReadAll(configFile, 1024*1024, &configData) < 0) {
+ if (virFileReadAll(configFile, 1024*1024, &configData) < 0)
return FALSE;
- }
Why the unrelated no-op change here, and in the next couple of hunks?
@@ -5427,6 +5424,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd
*cmd)
}
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ virshReportError(ctl);
virStoragePoolFree(pool);
return FALSE;
Should we add comments that remind us that we know that
virStoragePoolFree will clear any errors? Likewise for the other
changes in this file?
At any rate, ACK that it improves the virsh experience.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 323 bytes)