Re: [virt-tools-list] [PATCH virt-viewer 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-19 Thread Eduardo Lima (Etrunko) 
On 07/19/2016 12:32 PM, Christophe Fergeau wrote:
> On Sun, Jul 17, 2016 at 11:13:04PM -0300, Eduardo Lima (Etrunko) wrote:
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/ovirt-foreign-menu.c | 49 
>> ++--
>>  src/ovirt-foreign-menu.h |  5 -
>>  2 files changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index b071e27..2446239 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -47,6 +47,7 @@ static void 
>> ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
>>  static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
>>  static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
>> *menu);
>>  static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
>> +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
>> gpointer user_data);
>>  
>>  G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
>>  
>> @@ -85,7 +86,7 @@ enum {
>>  };
>>  
>>  
>> -static char *
>> +char *
>>  ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
>>  {
>>  char *name;
>> @@ -100,6 +101,36 @@ 
>> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
>>  }
>>  
>>  
>> +void
>> +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, 
>> char *name)
>> +{
> 
> For what it's worth, this is a bit misleading as this going to trigger
> an async update of the ISO name, and this sets "next_iso_name" more than
> "current_iso_name". I think you need to expose this an async method
> anyway, so that you can catch failures to change the ISO (ie the REST
> API call failed).
> 

Okay, I will work on it. I really did it the laziest way possible,
without any error treatment indeed.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com



signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-19 Thread Christophe Fergeau 
On Sun, Jul 17, 2016 at 11:13:04PM -0300, Eduardo Lima (Etrunko) wrote:
> Signed-off-by: Eduardo Lima (Etrunko) 
> ---
>  src/ovirt-foreign-menu.c | 49 
> ++--
>  src/ovirt-foreign-menu.h |  5 -
>  2 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index b071e27..2446239 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -47,6 +47,7 @@ static void 
> ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
>  static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
>  static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
> *menu);
>  static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
> +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
> gpointer user_data);
>  
>  G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
>  
> @@ -85,7 +86,7 @@ enum {
>  };
>  
>  
> -static char *
> +char *
>  ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
>  {
>  char *name;
> @@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
> *foreign_menu)
>  }
>  
>  
> +void
> +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
> *name)
> +{

For what it's worth, this is a bit misleading as this going to trigger
an async update of the ISO name, and this sets "next_iso_name" more than
"current_iso_name". I think you need to expose this an async method
anyway, so that you can catch failures to change the ISO (ie the REST
API call failed).

Christophe

> +g_return_if_fail(foreign_menu->priv->cdrom != NULL);
> +g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
> +
> +if (name) {
> +g_debug("Updating VM cdrom image to '%s'", name);
> +foreign_menu->priv->next_iso_name = g_strdup(name);
> +} else {
> +g_debug("Removing current cdrom image");
> +foreign_menu->priv->next_iso_name = NULL;
> +}
> +
> +g_object_set(foreign_menu->priv->cdrom,
> + "file", name,
> + NULL);
> +ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
> + foreign_menu->priv->proxy, NULL,
> + updated_cdrom_cb, foreign_menu);
> +}
> +
> +
> +GList*
> +ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
> +{
> +return foreign_menu->priv->iso_names;
> +}
> +
> +
>  static void
>  ovirt_foreign_menu_get_property(GObject *object, guint property_id,
> GValue *value, GParamSpec *pspec)
> @@ -385,7 +416,7 @@ static void
>  ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer 
> user_data)
>  {
>  OvirtForeignMenu *foreign_menu;
> -const char *iso_name;
> +const char *iso_name = NULL;
>  gboolean checked;
>  
>  checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
> @@ -405,19 +436,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem 
> *menuitem, gpointer user_data)
>  
>  if (checked) {
>  iso_name = gtk_menu_item_get_label(menuitem);
> -g_debug("Updating VM cdrom image to '%s'", iso_name);
> -foreign_menu->priv->next_iso_name = g_strdup(iso_name);
> -} else {
> -g_debug("Removing current cdrom image");
> -iso_name = NULL;
> -foreign_menu->priv->next_iso_name = NULL;
>  }
> -g_object_set(foreign_menu->priv->cdrom,
> - "file", iso_name,
> - NULL);
> -ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
> - foreign_menu->priv->proxy, NULL,
> - updated_cdrom_cb, foreign_menu);
> +
> +ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
>  }
>  
>  
> diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
> index cf18b52..f1a1ddb 100644
> --- a/src/ovirt-foreign-menu.h
> +++ b/src/ovirt-foreign-menu.h
> @@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy 
> *proxy);
>  OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self);
>  void ovirt_foreign_menu_start(OvirtForeignMenu *menu);
>  
> -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu);
> +char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu);
> +void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char 
> *name);
> +
> +GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu);
>  
>  G_END_DECLS
>  
> -- 
> 2.7.4
> 
> ___
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list


signature.asc
Description: PGP signature
___
virt-tools-list mailing list

[virt-tools-list] [PATCH virt-viewer 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-17 Thread Eduardo Lima (Etrunko) 
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 49 ++--
 src/ovirt-foreign-menu.h |  5 -
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index b071e27..2446239 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -47,6 +47,7 @@ static void 
ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
+static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -85,7 +86,7 @@ enum {
 };
 
 
-static char *
+char *
 ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
 {
 char *name;
@@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
*foreign_menu)
 }
 
 
+void
+ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
*name)
+{
+g_return_if_fail(foreign_menu->priv->cdrom != NULL);
+g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
+
+if (name) {
+g_debug("Updating VM cdrom image to '%s'", name);
+foreign_menu->priv->next_iso_name = g_strdup(name);
+} else {
+g_debug("Removing current cdrom image");
+foreign_menu->priv->next_iso_name = NULL;
+}
+
+g_object_set(foreign_menu->priv->cdrom,
+ "file", name,
+ NULL);
+ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
+ foreign_menu->priv->proxy, NULL,
+ updated_cdrom_cb, foreign_menu);
+}
+
+
+GList*
+ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
+{
+return foreign_menu->priv->iso_names;
+}
+
+
 static void
 ovirt_foreign_menu_get_property(GObject *object, guint property_id,
GValue *value, GParamSpec *pspec)
@@ -385,7 +416,7 @@ static void
 ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
 {
 OvirtForeignMenu *foreign_menu;
-const char *iso_name;
+const char *iso_name = NULL;
 gboolean checked;
 
 checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
@@ -405,19 +436,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, 
gpointer user_data)
 
 if (checked) {
 iso_name = gtk_menu_item_get_label(menuitem);
-g_debug("Updating VM cdrom image to '%s'", iso_name);
-foreign_menu->priv->next_iso_name = g_strdup(iso_name);
-} else {
-g_debug("Removing current cdrom image");
-iso_name = NULL;
-foreign_menu->priv->next_iso_name = NULL;
 }
-g_object_set(foreign_menu->priv->cdrom,
- "file", iso_name,
- NULL);
-ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
- foreign_menu->priv->proxy, NULL,
- updated_cdrom_cb, foreign_menu);
+
+ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
 }
 
 
diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
index cf18b52..f1a1ddb 100644
--- a/src/ovirt-foreign-menu.h
+++ b/src/ovirt-foreign-menu.h
@@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy);
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self);
 void ovirt_foreign_menu_start(OvirtForeignMenu *menu);
 
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu);
+char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu);
+void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char 
*name);
+
+GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu);
 
 G_END_DECLS
 
-- 
2.7.4

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