[Kimchi-devel] [PATCH V2] Avoid useless libvirt error log produced by featuretests

[Aline Manera]

On 01/24/2014 01:54 PM, Sheldon wrote:
> On 01/24/2014 12:48 PM, me,apporc wrote:
>> huh, i have checked that the error code is just 1, which is "internal 
>> error". This is a generic error, and we can not depend on it here.
>> Any ideas?
>>
> http://libvirt.org/html/libvirt-virterror.html#virErrorNumber
>
> Sure, maybe the the virErrorNumber can not distinguish all kinds of error.
> Can we log all errors on a special file,  kimchi_log.log?
>
> IMO, we just do not want to print these errors on stderr, but we 
> really want some errors info?
>
> http://libvirt.org/html/libvirt-virterror.html#virErrorLevel
> we may just log error message.
>

I agree, Sheldon!
We can "return False" in all cases and log the error messages in the 
kimchi log.

>>
>> On Fri, Jan 24, 2014 at 11:04 AM, Aline Manera 
>> <alinefm at linux.vnet.ibm.com <mailto:alinefm at linux.vnet.ibm.com>> wrote:
>>
>>     On 01/24/2014 12:22 AM, me,apporc wrote:
>>>     I mean a 'False' can't tell us whether it's because of 'protocol
>>>     type error' or not, Maybe it's another error from libvirt, for
>>>     example, the ISO_STREAM_XML is deprecated and can not work with
>>>     new version of ibvirt. So i don't think the backend can handle
>>>     this correctly.
>>>
>>>     Frist, this exception got raised is not a 'protocol type error'
>>>     and we don't know what it is. Second, the error handler of
>>>     libvirt is got replaced, and the error message will not show
>>>     now, then we won't see it.
>>>     As Cristian pointed out, it's good to just hide the "protocol"
>>>     error", but not them all.
>>>
>>>
>>
>>     Ok ok.
>>     But please, check the error code instead of comparing strings.
>>
>>
>>>
>>>     On Fri, Jan 24, 2014 at 2:23 AM, Aline Manera
>>>     <alinefm at linux.vnet.ibm.com <mailto:alinefm at linux.vnet.ibm.com>>
>>>     wrote:
>>>
>>>         On 01/23/2014 08:16 AM, apporc wrote:
>>>
>>>             When running feature tests, we get bunch of error
>>>             messages as below:
>>>
>>>             *** Running feature tests ***
>>>             libvirt: Domain Config error : internal error unknown
>>>             protocol type 'http'
>>>             libvirt: Domain Config error : internal error unknown
>>>             protocol type 'https'
>>>             libvirt: Domain Config error : internal error unknown
>>>             protocol type 'ftp'
>>>             libvirt: Domain Config error : internal error unknown
>>>             protocol type 'ftps'
>>>             libvirt: Domain Config error : internal error unknown
>>>             protocol type 'tftp'
>>>             *** Feature tests completed ***
>>>
>>>             By replacing default error handler of libvirt, this
>>>             patch succeeded to
>>>             avoid the error. After Featuretest, the default error
>>>             handler will be restored.
>>>             The default error handler(in libvirtmod.so) just do one
>>>             thing, it prints the error message to
>>>             stderr before we catch the exception in python.
>>>
>>>             Signed-off-by: apporc <appleorchard2000 at gmail.com
>>>             <mailto:appleorchard2000 at gmail.com>>
>>>             ---
>>>               src/kimchi/featuretests.py |   14 ++++++++++++--
>>>               1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>>             diff --git a/src/kimchi/featuretests.py
>>>             b/src/kimchi/featuretests.py
>>>             index a5755a2..d68fcb8 100644
>>>             --- a/src/kimchi/featuretests.py
>>>             +++ b/src/kimchi/featuretests.py
>>>             @@ -57,16 +57,26 @@ class FeatureTests(object):
>>>
>>>                   @staticmethod
>>>                   def libvirt_supports_iso_stream(protocol):
>>>             +        def libvirt_errorhandler(userdata, error):
>>>             +            # A libvirt error handler to ignore
>>>             annoying messages in stderr
>>>             +            pass
>>>             +
>>>                       xml = ISO_STREAM_XML % {'protocol': protocol}
>>>                       conn = None
>>>                       try:
>>>             +            # Register the error handler to hide
>>>             libvirt error in stderr
>>>             +  libvirt.registerErrorHandler(f=libvirt_errorhandler,
>>>             ctx=None)
>>>                           conn = libvirt.open('qemu:///system')
>>>                           dom = conn.defineXML(xml)
>>>                           dom.undefine()
>>>                           return True
>>>             -        except libvirt.libvirtError:
>>>             -            return False
>>>
>>>
>>>             +  except libvirt.libvirtError, e:
>>>             +            if e.message == "internal error unknown
>>>             protocol type '%s'" % protocol:
>>>             +                return False
>>>             +            else:
>>>             +                raise e
>>>
>>>
>>>         Independent of the exception we should not raise it.
>>>         It is a feature test, so if something failed we just return
>>>         'False' and backend will handle it correctly.
>>>
>>>
>>>             finally:
>>>             +  libvirt.registerErrorHandler(f=None, ctx=None)
>>>                           conn is None or conn.close()
>>>
>>>                   @staticmethod
>>>
>>>
>>>
>>>
>>>
>>>     -- 
>>>     Regards,
>>>     apporc
>>
>>
>>
>>
>> -- 
>> Regards,
>> apporc
>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
>
> -- 
> Thanks and best regards!
>
> Sheldon Feng(???)<shaohef at linux.vnet.ibm.com>
> IBM Linux Technology Center

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140130/b7c6b832/attachment.html>