Re: [PATCH] virsh: Make self-test failures noisy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

>> -            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.


>>          case VSH_OT_ARGV:
>> -            if (cmd->opts[i + 1].name)
>> +            if (cmd->opts[i + 1].name) {
>> +                vshError(ctl, _("command '%s' has option after argv"),
>> +                         cmd->name);
> 
> The commentary below actually gives me better idea about the error than the
> error itself, can we use that text instead?

Switched to 'does not list argv option last'

> 
> Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
> 

Thanks; adjusted and pushed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux