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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list