[libvirt] [PATCH] virsh: Make self-test failures noisy
Peter Krempa
pkrempa at redhat.com
Tue Mar 12 12:14:52 UTC 2019
On Tue, Mar 12, 2019 at 06:40:42 -0500, Eric Blake wrote:
> On 3/12/19 2:50 AM, Erik Skultety wrote:
> > On Mon, Mar 11, 2019 at 09:18:44PM -0500, Eric Blake wrote:
> >> In local testing, I accidentally introduced a self-test failure,
> >> and spent way too much time debugging it. Make sure the testsuite
> >> log includes some hint as to why command option validation failed.
> >>
> >> Signed-off-by: Eric Blake <eblake at redhat.com>
>
> >> - if (opt->flags || !opt->help)
> >> + if (opt->flags || !opt->help) {
> >> + vshError(ctl, _("command '%s' has incorrect alias option"),
> >> + cmd->name);
> >> return -1; /* alias options are tracked by the original name */
> >> + }
> >> if ((p = strchr(name, '=')) &&
> >> - VIR_STRNDUP(name, name, p - name) < 0)
> >> + VIR_STRNDUP(name, name, p - name) < 0) {
> >> + vshError(ctl, _("allocation failure while checking command '%s'"),
> >> + cmd->name);
> >
> > I think ^this one can be dropped, if there was an allocation failure, we have
> > much bigger problems and it's likely it will fail again which vshError will
> > transitively try to do if you look at vshOutputLogFile for example.
>
> Agreed. We can't use assert() in libvirt.so, but virsh has other places
> where it fits, so I'll switch this one to assert.
assert() is for debug-builds only, isn't it? Did you mean abort() ?
Still the construction proposed in the patch looks much better than
either of those.
Also for local output the printing is (partially) done into a file
descriptor, so you'll at least get the string 'error' printed.
Note also that virsh uses exit(EXIT_FAILURE) on allocation failure, so
neither abort nor assert should be used.
I think the appropriate solution is vshErrorOOM though, which by the way
is called also if the allocation in vshError fails.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190312/e2c3a072/attachment-0001.sig>
More information about the libvir-list
mailing list