Re: [PATCH v2 20/20] virlog: Split parsing and setting priority

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

 



On 21/09/16 22:01, John Ferlan wrote:
> 
> 
> On 08/18/2016 07:47 AM, Erik Skultety wrote:
>> Handling of outputs and filters has been changed in a way that splits
>> parsing and defining. Do the same thing for logging priority as well, this
>> however, doesn't need much of a preparation.
>> ---
>>  src/util/virlog.c | 21 +++++++++------------
>>  tests/eventtest.c |  3 ++-
>>  2 files changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/util/virlog.c b/src/util/virlog.c
>> index 713cd0c..683cf6b 100644
>> --- a/src/util/virlog.c
>> +++ b/src/util/virlog.c
>> @@ -220,7 +220,9 @@ int
>>  virLogSetDefaultPriority(virLogPriority priority)
>>  {
>>      if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) {
>> -        VIR_WARN("Ignoring invalid log level setting.");
>> +        virReportError(VIR_ERR_INVALID_ARG,
>> +                       _("Failed to set logging priority, argument '%u' is "
>> +                         "invalid"), priority);
> 
> By this point I was too lazy to check, but we're not going to start
> seeing strange failures from some rogue/incorrect setting that
> previously essentially ignored this.
> 

No, we won't. The only thing that changed is that now we'll see issues
logged as errors rather than warnings, but so far, each of the original
callers ignores the outcome of the function, so no unexpected
crashes/behaviour related to incorrect setting shouldn't take place, if
the new setting is incorrect, the original setting will stay intact.

Erik

>>          return -1;
>>      }
>>      if (virLogInitialize() < 0)
>> @@ -1158,20 +1160,15 @@ virLogGetNbOutputs(void)
> 
> 
> You'll need to update the comments to this function about treturn values...
> 
> Conditional ACK w/ adjustment and assurance about the above usage of
> virReportError vs. VIR_WARN
> 
> 
> John


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]