[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