[Kimchi-devel] [PATCH] [Kimchi] Handle URLError exception when creating Template.

[Aline Manera]

On 06/20/2016 03:02 PM, Paulo Ricardo Paz Vital wrote:
> On Jun 20 12:48PM, Aline Manera wrote:
>>
>> On 06/14/2016 07:26 PM, pvital at linux.vnet.ibm.com wrote:
>>> From: Paulo Vital <pvital at linux.vnet.ibm.com>
>>>
>>> urllib2 raised URLError when could not access a remote file due to unauthorized
>>> access, for example. This patch gives to VMTemplate the ability of handle this
>>> exception and raises a relevant error message to user.
>>>
>>> This patch fixes Issue #959
>>>
>>> Signed-off-by: Paulo Vital <pvital at linux.vnet.ibm.com>
>>> ---
>>>    i18n.py       | 1 +
>>>    vmtemplate.py | 3 +++
>>>    2 files changed, 4 insertions(+)
>>>
>>> diff --git a/i18n.py b/i18n.py
>>> index 2d8390f..b33c6ec 100644
>>> --- a/i18n.py
>>> +++ b/i18n.py
>>> @@ -52,6 +52,7 @@ messages = {
>>>                         "to file access control lists for '%(user)s' user if possible, or add the "
>>>                         "'%(user)s' to the ISO path group, or (not recommended) 'chmod -R o+x 'path_to_iso'."
>>>                         "Details: %(err)s" ),
>>> +    "KCHISO0009E": _("Unable to access remote ISO. Details: %(err)s"),
>>>
>>>        "KCHIMG0001E": _("An error occurred when probing image OS information."),
>>>        "KCHIMG0003E": _("Unable to read image file %(filename)s"),
>>> diff --git a/vmtemplate.py b/vmtemplate.py
>>> index ef92914..6faf4c5 100644
>>> --- a/vmtemplate.py
>>> +++ b/vmtemplate.py
>>> @@ -24,6 +24,7 @@ import urlparse
>>>    import uuid
>>>    from lxml import etree
>>>    from lxml.builder import E
>>> +from urllib2 import URLError
>>>
>>>    from wok.exception import InvalidParameter, ImageFormatError, IsoFormatError
>>>    from wok.exception import MissingParameter, OperationFailed
>>> @@ -172,6 +173,8 @@ class VMTemplate(object):
>>>                return iso_img.probe()
>>>            except IsoFormatError:
>>>                raise InvalidParameter("KCHISO0001E", {'filename': iso})
>>> +        except URLError as e:
>>> +            raise OperationFailed("KCHISO0009E", {'err': e})
>> The IsoImage() should raise a IsoFormatError. We need to check why it is not
>> being done.
>> Is there proper error handlers there?
>>
> No. If I trace from where the exception is coming, the results is:
>
> IsoImage()->__init__()->_scan()->_get_iso_data() line 422
>
> and none of these methods are handling exceptions, specially the exceptions or
> errors from urllib2 module.
>
> I can add a try/exception block in _get_iso_data() and raise something, but IMO
> IsoFormatError is not the correct name for this error - the problem is not
> related to an ISO format, but not able to access the remote link. Because of
> this difference on the kind of error raised, I coded on vmtemplate.
>

Ok. I agree IsoFormatError is not the appropriated one.

But from my view, isoinfo.py should proper get the exception there 
(_get_iso_data() line 422) and raise an OperationFailed exception.
That OperationFailed exception should be get in vmtemplate.py and raised.
That way, we make the code in isoinfo.py saver for anyone to use it.

What do you think?

>>>        def _get_cdrom_xml(self, libvirt_stream_protocols):
>>>            if 'cdrom' not in self.info:
>>> --
>>> 2.5.5
> --
> Paulo Ricardo Paz Vital
> Linux Technology Center, IBM Systems
> http://www.ibm.com/linux/ltc/
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>