[virt-tools-list] [PATCH virt-viewer v3] ovirt-foreign-menu: Bypass errors from Host/Cluster/Data Center

[Eduardo Lima (Etrunko)]

On 6/7/19 10:37 AM, Christophe Fergeau wrote:
> On Fri, Jun 07, 2019 at 10:04:19AM -0300, Eduardo Lima (Etrunko) wrote:
>> On 6/6/19 1:22 PM, Christophe Fergeau wrote:
>>> Hey,
>>>
>>> I'm not really comfortable with that patch, which ignores some errors,
>>> and adds some code not to crash when we do that, in the hope that the
>>> end result will make sense. I'm under the impression that if the oVirt
>>> instance does not have the permissions issues that you mention, but if
>>> instead we get a temporary networking issue at the wrong time (for
>>> example), then rather than doing the proper checks, we'll just ignore
>>> the error, and assume the image we got is accessible to our VM?
>>
>> There is no crash avoidance, what this patch does is to replicate the
>> previous behavior when the code only did queries for VM and storage
>> domains, which depending on the setup can lead to showing wrong ISO
>> images for that VM. But those setups are not very common, and most of
>> the cases it was fine.
>>
>>>
>>> Isn't this a problem in the oVirt REST API if you can access a
>>> datacenter, a VM, but have no way to know if they are related?
>>>
>>
>> That's it, we need to know the common thing between the VM and a storage
>> domain, and that is the datacenter. It would indeed be much better if
>> the information about it was available right away in the VM xml, but at
>> the moment we need to send a few other requests to get to it:
>>
>> VM -> Host -> Cluster -> Data Center.
> 
> Is there a bug against the oVirt REST API for this?
> 

Not that I am aware of.

>>
>> Anyway, IMO, it is better to fallback to the old method and show the
>> available ISO images, than to not show any at all and  break in the
>> middle with an error message about insufficient permissions.
> 
> My complaint is that *all* errors are being ignored, even the ones which
> might be meaningful, not just the permissions errors.

I understand, but currently we get a generic "400 Bad Request" error,
which is also used in other cases, while it should be something like a
"401 Unauthorized". Using g_error_matches() would not help here, I think.

< HTTP/1.1 400 Bad Request
< Soup-Debug-Timestamp: 1559921229
< Soup-Debug: SoupMessage 1 (0x5570ecf0e8e0)
< Date: Fri, 07 Jun 2019 15:27:09 GMT
< Server: Apache/2.4.6 (Red Hat Enterprise Linux) OpenSSL/1.0.2k-fips
mod_auth_gssapi/1.5.1 mod_nss/1.0.14 NSS/3.28.4 mod_wsgi/3.4 Python/2.7.5
< Content-Type: application/xml;charset=UTF-8
< Content-Length: 188
< Correlation-Id: 4417d860-9453-4e5e-94dc-fb1f7c964db8
< Connection: close
<
< <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
< <fault>
<     <reason>Operation Failed</reason>
<     <detail>query execution failed due to insufficient
permissions.</detail>
< </fault>





> 
> Christophe
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com