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? > > 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. Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
