[libvirt] [PATCH] Don't overwrite virRun error messages
Eric Blake
eblake at redhat.com
Wed Mar 9 03:27:02 UTC 2011
On 03/08/2011 11:50 AM, Cole Robinson wrote:
> virRun gives pretty useful error output, let's not overwrite it unless there
> is a good reason. Some places were providing more information about what
> the commands were _attempting_ to do, however that's usually less useful from
> a debugging POV than what actually happened.
> ---
> src/lxc/veth.c | 45 +++------------------------------
> src/openvz/openvz_driver.c | 20 --------------
> src/qemu/qemu_driver.c | 4 ---
> src/storage/storage_backend.c | 3 --
> src/storage/storage_backend_logical.c | 3 --
> src/vmware/vmware_driver.c | 2 -
> 6 files changed, 4 insertions(+), 73 deletions(-)
Hmm, I wonder if we should be using virCommand instead of virRun. But
that's a question for a separate patch. Meanwhile, I agree with your
argument that virRun's diagnosis is usually better.
> @@ -122,13 +121,7 @@ int vethCreate(char** veth1, char** veth2)
> argv[8] = *veth2;
>
> VIR_DEBUG("veth1: %s veth2: %s", *veth1, *veth2);
> - rc = virRun(argv, &cmdResult);
> -
> - if (rc != 0 ||
> - (WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
> - vethError(VIR_ERR_INTERNAL_ERROR,
> - _("Failed to create veth device pair '%s', '%s': %d"),
> - *veth1, *veth2, WEXITSTATUS(cmdResult));
> + if (virRun(argv, NULL) < 0) {
Not to mention that it fixes real bugs! Here, in the old code, rc can be
0 (we successfully waited on the child), while WIFEXITED(cmdResult) is
false (such as if the child command had a bug, and died due to a signal
such as SIGSEGV [that never happens, right?]) - but in that situation we
didn't report vethError or take the cleanup path. In the new code,
virRun() refuses to return 0 if the child does not exit normally, so we
always diagnose failure, whether due to non-zero status or to a signal.
ACK!
--
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: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110308/a5775512/attachment-0001.sig>
More information about the libvir-list
mailing list