Re: [PATCH v2 virt-manager] osdict: handle libosinfo lookup failure

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

 



On 03/26/2014 10:39 AM, Giuseppe Scrivano wrote:
> Cole Robinson <crobinso@xxxxxxxxxx> writes:
> 
>> On 03/26/2014 09:41 AM, Giuseppe Scrivano wrote:
>>> Cole Robinson <crobinso@xxxxxxxxxx> writes:
>>>
>>>> On 03/26/2014 08:51 AM, Giuseppe Scrivano wrote:
>>>>> Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx>
>>>>> ---
>>>>> OK to include this patch into the series?
>>>>>
>>>>>  virtinst/osdict.py | 7 +++++--
>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/virtinst/osdict.py b/virtinst/osdict.py
>>>>> index 13f6670..f37ccab 100644
>>>>> --- a/virtinst/osdict.py
>>>>> +++ b/virtinst/osdict.py
>>>>> @@ -165,9 +165,12 @@ def get_recommended_resources(variant, arch):
>>>>>  
>>>>>  
>>>>>  def lookup_os_by_media(location):
>>>>> -    media = libosinfo.Media.create_from_location(location, None)
>>>>> +    try:
>>>>> +        media = libosinfo.Media.create_from_location(location, None)
>>>>> +    except:
>>>>> +        return None
>>>>
>>>> I'd like to see the error here logged, it either shouldn't happen so it will
>>>> rarely trigger, or its potentially informative. ACK otherwise
>>>
>>> I've preferred to just mute it as the error was that the location could
>>> not be opened, and that makes sense when we pass a URL.  Should this be
>>> changed?
>>>
>>
>> It's still informative, for the logs at least. There could be another error
>> going on in libosinfo and we would want to track that at least. But we could
>> avoid the URL error by not passing a URL location to libosinfo, just skip if
>> distroinstaller._is_url is False
> 
> ok thanks, I didn't see that function, ok to push this version (and
> enable again the exception in osdict.py)?
> 
> --- a/virtinst/distroinstaller.py
> +++ b/virtinst/distroinstaller.py
> @@ -473,6 +474,16 @@ class DistroInstaller(Installer):
>  
>      def detect_distro(self, guest):
>          try:
> +            if not _is_url(self.conn, self.location):
> +                name = osdict.lookup_os_by_media(self.location)
> +                if name:
> +                    logging.debug("installer.detect_distro returned=%s", name)
> +                    return name
> +        except:
> +            logging.debug("libosinfo detect failed", exc_info=True)
> +
> +        try:
> 
> Giuseppe
> 

ACK

- Cole

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list




[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux