[virt-tools-list] [PATCH virt-viewer 3/3] ovirt-foreign-menu: Bypass errors from Host/Cluster/Data Center
Christophe Fergeau
cfergeau at redhat.com
Tue Jul 17 14:48:12 UTC 2018
On Fri, Jul 06, 2018 at 09:59:23AM -0300, Eduardo Lima (Etrunko) wrote:
> When accessing ovirt as a regular user, it may happen that queries to
> Hosts, Clusters and Data Centers return errors due to insufficient
> permissions, while they will work fine if access is done by admin user.
> In this case, we skip the errors and fallback to the old method.
>
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> ---
> src/ovirt-foreign-menu.c | 43 +++++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index 70a0b50..8ed08c9 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -624,12 +624,20 @@ static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu,
>
> #ifdef HAVE_OVIRT_DATA_CENTER
> static gboolean storage_domain_attached_to_data_center(OvirtStorageDomain *domain,
> - OvirtDataCenter *data_center)
> + OvirtDataCenter *data_center)
> {
> GStrv data_center_ids;
> char *data_center_guid;
> gboolean match;
>
> + /* For some reason we did not get data center information, so just return
> + * TRUE as it will work like a fallback to old method, where we did not
> + * check relationship with data center and storage domain.*/
> + if (data_center == NULL) {
> + g_debug("Could not get data center info, considering storage domain is attached to it");
> + return TRUE;
> + }
> +
> g_object_get(domain, "data-center-ids", &data_center_ids, NULL);
> g_object_get(data_center, "guid", &data_center_guid, NULL);
> match = g_strv_contains((const gchar * const *) data_center_ids, data_center_guid);
> @@ -743,9 +751,6 @@ static void data_center_fetched_cb(GObject *source_object,
> ovirt_resource_refresh_finish(resource, result, &error);
> if (error != NULL) {
> g_debug("failed to fetch Data Center: %s", error->message);
> - g_task_return_error(task, error);
> - g_object_unref(task);
> - return;
I don't think the hunk just above
(storage_domain_attached_to_data_center) takes this situation into
account, menu->priv->data_center is non-null, but it will be 'empty', so
storage_domain_attached_to_data_center is probably not going to do the
right thing.
> }
>
> ovirt_foreign_menu_next_async_step(menu, task, STATE_DATA_CENTER);
> @@ -760,6 +765,12 @@ static void ovirt_foreign_menu_fetch_data_center_async(OvirtForeignMenu *menu,
> g_return_if_fail(OVIRT_IS_CLUSTER(menu->priv->cluster));
>
> menu->priv->data_center = ovirt_cluster_get_data_center(menu->priv->cluster);
> +
> + if (menu->priv->data_center == NULL) {
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_DATA_CENTER);
> + return;
> + }
> +
> ovirt_resource_refresh_async(OVIRT_RESOURCE(menu->priv->data_center),
> menu->priv->proxy,
> g_task_get_cancellable(task),
> @@ -780,9 +791,6 @@ static void cluster_fetched_cb(GObject *source_object,
> ovirt_resource_refresh_finish(resource, result, &error);
> if (error != NULL) {
> g_debug("failed to fetch Cluster: %s", error->message);
> - g_task_return_error(task, error);
> - g_object_unref(task);
> - return;
> }
>
> ovirt_foreign_menu_next_async_step(menu, task, STATE_CLUSTER);
> @@ -794,9 +802,14 @@ static void ovirt_foreign_menu_fetch_cluster_async(OvirtForeignMenu *menu,
> {
> g_return_if_fail(OVIRT_IS_FOREIGN_MENU(menu));
> g_return_if_fail(OVIRT_IS_PROXY(menu->priv->proxy));
> - g_return_if_fail(OVIRT_IS_HOST(menu->priv->host));
> + g_return_if_fail(OVIRT_IS_VM(menu->priv->vm));
Maybe keep these 2, but in the right block in the if (menu->priv->host
== NULL) below?
> +
> + /* If there is no host information, we get cluster from the VM */
> + if (menu->priv->host == NULL)
> + menu->priv->cluster = ovirt_vm_get_cluster(menu->priv->vm);
> + else
> + menu->priv->cluster = ovirt_host_get_cluster(menu->priv->host);
>
I think we should make 100% sure menu->priv->cluster is not NULL at this
point.
Christophe
> - menu->priv->cluster = ovirt_host_get_cluster(menu->priv->host);
> ovirt_resource_refresh_async(OVIRT_RESOURCE(menu->priv->cluster),
> menu->priv->proxy,
> g_task_get_cancellable(task),
> @@ -817,9 +830,6 @@ static void host_fetched_cb(GObject *source_object,
> ovirt_resource_refresh_finish(resource, result, &error);
> if (error != NULL) {
> g_debug("failed to fetch Host: %s", error->message);
> - g_task_return_error(task, error);
> - g_object_unref(task);
> - return;
> }
>
> ovirt_foreign_menu_next_async_step(menu, task, STATE_HOST);
> @@ -834,6 +844,15 @@ static void ovirt_foreign_menu_fetch_host_async(OvirtForeignMenu *menu,
> g_return_if_fail(OVIRT_IS_VM(menu->priv->vm));
>
> menu->priv->host = ovirt_vm_get_host(menu->priv->vm);
> +
> + /* In some cases the VM XML does not include host information, so we just
> + * skip to the next step
> + */
> + if (menu->priv->host == NULL) {
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_HOST);
> + return;
> + }
> +
> ovirt_resource_refresh_async(OVIRT_RESOURCE(menu->priv->host),
> menu->priv->proxy,
> g_task_get_cancellable(task),
> --
> 2.14.4
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20180717/45b0e742/attachment.sig>
More information about the virt-tools-list
mailing list