[libvirt] [PATCH] Better error reporting in virsh.

Eric Blake eblake at redhat.com
Mon Apr 5 21:20:47 UTC 2010


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 323 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100405/13bad4ca/attachment-0001.sig>


More information about the libvir-list mailing list