Patchwork [v2] gtk: Rename File to Machine menu and add stop, reset and power-down items

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 22, 2013, 4:29 p.m.
Message ID <51279CFC.2060601@siemens.com>
Download mbox | patch
Permalink /patch/222553/
State New
Headers show

Comments

Jan Kiszka - Feb. 22, 2013, 4:29 p.m.
This adds basic guest control commands to the "Machine" menu - a nice
added-value for the GTK UI. We pick up the stock stop accelerators (if
any) but not its image as we add a check item.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - File -> Machine menu
 - stock accels for stop

 ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 71 insertions(+), 11 deletions(-)
Anthony Liguori - Feb. 22, 2013, 5:16 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> This adds basic guest control commands to the "Machine" menu - a nice
> added-value for the GTK UI. We pick up the stock stop accelerators (if
> any) but not its image as we add a check item.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v2:
>  - File -> Machine menu
>  - stock accels for stop
>
>  ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 71 insertions(+), 11 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index bc4ec67..5e8a158 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>  
>      GtkAccelGroup *accel_group;
>  
> -    GtkWidget *file_menu_item;
> -    GtkWidget *file_menu;
> +    GtkWidget *machine_menu_item;
> +    GtkWidget *machine_menu;
> +    GtkWidget *stop_item;
> +    GtkWidget *reset_item;
> +    GtkWidget *powerdown_item;
>      GtkWidget *quit_item;
>  
>      GtkWidget *view_menu_item;
> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>      GdkCursor *null_cursor;
>      Notifier mouse_mode_notifier;
>      gboolean free_scale;
> +
> +    bool external_stop_update;
>  } GtkDisplayState;
>  
>  static GtkDisplayState *global_state;
> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>      const char *status = "";
>      gchar *title;
>      const char *grab = "";
> +    bool is_stopped = !runstate_is_running();
>  
>      if (gd_is_grab_active(s)) {
>          grab = " - Press Ctrl+Alt+G to release grab";
>      }
>  
> -    if (!runstate_is_running()) {
> +    if (is_stopped) {
>          status = " [Stopped]";
>      }
> +    s->external_stop_update = true;
> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
> +                                   is_stopped);
> +    s->external_stop_update = false;
>  
>      if (qemu_name) {
>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>  
>  /** Window Menu Actions **/
>  
> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
> +{
> +    GtkDisplayState *s = opaque;
> +
> +    if (s->external_stop_update) {
> +        return;
> +    }
> +    if (runstate_is_running()) {
> +        qmp_stop(NULL);
> +    } else {
> +        qmp_cont(NULL);
> +    }
> +}
> +
> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
> +{
> +    qmp_system_reset(NULL);
> +}
> +
> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
> +{
> +    qmp_system_powerdown(NULL);
> +}
> +
>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>  {
>      qmp_quit(NULL);
> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>      g_signal_connect(s->drawing_area, "key-release-event",
>                       G_CALLBACK(gd_key_event), s);
>  
> +    g_signal_connect(s->stop_item, "activate",
> +                     G_CALLBACK(gd_menu_stop), s);
> +    g_signal_connect(s->reset_item, "activate",
> +                     G_CALLBACK(gd_menu_reset), s);
> +    g_signal_connect(s->powerdown_item, "activate",
> +                     G_CALLBACK(gd_menu_powerdown), s);
>      g_signal_connect(s->quit_item, "activate",
>                       G_CALLBACK(gd_menu_quit), s);
>      g_signal_connect(s->full_screen_item, "activate",
> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>      int i;
>  
>      accel_group = gtk_accel_group_new();
> -    s->file_menu = gtk_menu_new();
> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
> +    s->machine_menu = gtk_menu_new();
> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
> +
> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
> +                                 "<QEMU>/Machine/Stop");
> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
> +
> +    separator = gtk_separator_menu_item_new();
> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
> +
> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
> +
> +    s->powerdown_item =
> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));

'Power Down' please.  I can make this change if you don't object.

Regards,

Anthony Liguori
Jan Kiszka - Feb. 22, 2013, 5:17 p.m.
On 2013-02-22 18:16, Anthony Liguori wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> This adds basic guest control commands to the "Machine" menu - a nice
>> added-value for the GTK UI. We pick up the stock stop accelerators (if
>> any) but not its image as we add a check item.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>>  - File -> Machine menu
>>  - stock accels for stop
>>
>>  ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>  1 files changed, 71 insertions(+), 11 deletions(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index bc4ec67..5e8a158 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
>> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>>  
>>      GtkAccelGroup *accel_group;
>>  
>> -    GtkWidget *file_menu_item;
>> -    GtkWidget *file_menu;
>> +    GtkWidget *machine_menu_item;
>> +    GtkWidget *machine_menu;
>> +    GtkWidget *stop_item;
>> +    GtkWidget *reset_item;
>> +    GtkWidget *powerdown_item;
>>      GtkWidget *quit_item;
>>  
>>      GtkWidget *view_menu_item;
>> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>>      GdkCursor *null_cursor;
>>      Notifier mouse_mode_notifier;
>>      gboolean free_scale;
>> +
>> +    bool external_stop_update;
>>  } GtkDisplayState;
>>  
>>  static GtkDisplayState *global_state;
>> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>>      const char *status = "";
>>      gchar *title;
>>      const char *grab = "";
>> +    bool is_stopped = !runstate_is_running();
>>  
>>      if (gd_is_grab_active(s)) {
>>          grab = " - Press Ctrl+Alt+G to release grab";
>>      }
>>  
>> -    if (!runstate_is_running()) {
>> +    if (is_stopped) {
>>          status = " [Stopped]";
>>      }
>> +    s->external_stop_update = true;
>> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
>> +                                   is_stopped);
>> +    s->external_stop_update = false;
>>  
>>      if (qemu_name) {
>>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
>> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>>  
>>  /** Window Menu Actions **/
>>  
>> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
>> +{
>> +    GtkDisplayState *s = opaque;
>> +
>> +    if (s->external_stop_update) {
>> +        return;
>> +    }
>> +    if (runstate_is_running()) {
>> +        qmp_stop(NULL);
>> +    } else {
>> +        qmp_cont(NULL);
>> +    }
>> +}
>> +
>> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
>> +{
>> +    qmp_system_reset(NULL);
>> +}
>> +
>> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
>> +{
>> +    qmp_system_powerdown(NULL);
>> +}
>> +
>>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>>  {
>>      qmp_quit(NULL);
>> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>>      g_signal_connect(s->drawing_area, "key-release-event",
>>                       G_CALLBACK(gd_key_event), s);
>>  
>> +    g_signal_connect(s->stop_item, "activate",
>> +                     G_CALLBACK(gd_menu_stop), s);
>> +    g_signal_connect(s->reset_item, "activate",
>> +                     G_CALLBACK(gd_menu_reset), s);
>> +    g_signal_connect(s->powerdown_item, "activate",
>> +                     G_CALLBACK(gd_menu_powerdown), s);
>>      g_signal_connect(s->quit_item, "activate",
>>                       G_CALLBACK(gd_menu_quit), s);
>>      g_signal_connect(s->full_screen_item, "activate",
>> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>>      int i;
>>  
>>      accel_group = gtk_accel_group_new();
>> -    s->file_menu = gtk_menu_new();
>> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
>> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
>> +    s->machine_menu = gtk_menu_new();
>> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
>> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
>> +
>> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
>> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
>> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
>> +                                 "<QEMU>/Machine/Stop");
>> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
>> +
>> +    separator = gtk_separator_menu_item_new();
>> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
>> +
>> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
>> +
>> +    s->powerdown_item =
>> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
> 
> 'Power Down' please.  I can make this change if you don't object.

You also mentioned picking up the label string for Stop. Can do both and
send v3.

Jan
Anthony Liguori - Feb. 22, 2013, 5:53 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2013-02-22 18:16, Anthony Liguori wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> This adds basic guest control commands to the "Machine" menu - a nice
>>> added-value for the GTK UI. We pick up the stock stop accelerators (if
>>> any) but not its image as we add a check item.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> Changes in v2:
>>>  - File -> Machine menu
>>>  - stock accels for stop
>>>
>>>  ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>>  1 files changed, 71 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>> index bc4ec67..5e8a158 100644
>>> --- a/ui/gtk.c
>>> +++ b/ui/gtk.c
>>> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>>>  
>>>      GtkAccelGroup *accel_group;
>>>  
>>> -    GtkWidget *file_menu_item;
>>> -    GtkWidget *file_menu;
>>> +    GtkWidget *machine_menu_item;
>>> +    GtkWidget *machine_menu;
>>> +    GtkWidget *stop_item;
>>> +    GtkWidget *reset_item;
>>> +    GtkWidget *powerdown_item;
>>>      GtkWidget *quit_item;
>>>  
>>>      GtkWidget *view_menu_item;
>>> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>>>      GdkCursor *null_cursor;
>>>      Notifier mouse_mode_notifier;
>>>      gboolean free_scale;
>>> +
>>> +    bool external_stop_update;
>>>  } GtkDisplayState;
>>>  
>>>  static GtkDisplayState *global_state;
>>> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>>>      const char *status = "";
>>>      gchar *title;
>>>      const char *grab = "";
>>> +    bool is_stopped = !runstate_is_running();
>>>  
>>>      if (gd_is_grab_active(s)) {
>>>          grab = " - Press Ctrl+Alt+G to release grab";
>>>      }
>>>  
>>> -    if (!runstate_is_running()) {
>>> +    if (is_stopped) {
>>>          status = " [Stopped]";
>>>      }
>>> +    s->external_stop_update = true;
>>> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
>>> +                                   is_stopped);
>>> +    s->external_stop_update = false;
>>>  
>>>      if (qemu_name) {
>>>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
>>> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>>>  
>>>  /** Window Menu Actions **/
>>>  
>>> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    GtkDisplayState *s = opaque;
>>> +
>>> +    if (s->external_stop_update) {
>>> +        return;
>>> +    }
>>> +    if (runstate_is_running()) {
>>> +        qmp_stop(NULL);
>>> +    } else {
>>> +        qmp_cont(NULL);
>>> +    }
>>> +}
>>> +
>>> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    qmp_system_reset(NULL);
>>> +}
>>> +
>>> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    qmp_system_powerdown(NULL);
>>> +}
>>> +
>>>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>>>  {
>>>      qmp_quit(NULL);
>>> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>>>      g_signal_connect(s->drawing_area, "key-release-event",
>>>                       G_CALLBACK(gd_key_event), s);
>>>  
>>> +    g_signal_connect(s->stop_item, "activate",
>>> +                     G_CALLBACK(gd_menu_stop), s);
>>> +    g_signal_connect(s->reset_item, "activate",
>>> +                     G_CALLBACK(gd_menu_reset), s);
>>> +    g_signal_connect(s->powerdown_item, "activate",
>>> +                     G_CALLBACK(gd_menu_powerdown), s);
>>>      g_signal_connect(s->quit_item, "activate",
>>>                       G_CALLBACK(gd_menu_quit), s);
>>>      g_signal_connect(s->full_screen_item, "activate",
>>> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>>>      int i;
>>>  
>>>      accel_group = gtk_accel_group_new();
>>> -    s->file_menu = gtk_menu_new();
>>> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
>>> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
>>> +    s->machine_menu = gtk_menu_new();
>>> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
>>> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
>>> +
>>> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
>>> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
>>> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
>>> +                                 "<QEMU>/Machine/Stop");
>>> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
>>> +
>>> +    separator = gtk_separator_menu_item_new();
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
>>> +
>>> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
>>> +
>>> +    s->powerdown_item =
>>> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
>> 
>> 'Power Down' please.  I can make this change if you don't object.
>
> You also mentioned picking up the label string for Stop. Can do both and
> send v3.

How do you feel about changing Stop to Pause?  I think Pause makes more
sense to me.  I think it will translate better too.

Stop is a synonym with terminate.  I don't think it's a given that you
can always restart once you stop.

However, pause and unpause are much more clearly related.

Please do send a v3.

Regards,

Anthony Liguori

>
> Jan
>
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
Jan Kiszka - Feb. 22, 2013, 5:59 p.m.
On 2013-02-22 18:53, Anthony Liguori wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 2013-02-22 18:16, Anthony Liguori wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>
>>>> This adds basic guest control commands to the "Machine" menu - a nice
>>>> added-value for the GTK UI. We pick up the stock stop accelerators (if
>>>> any) but not its image as we add a check item.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>>  - File -> Machine menu
>>>>  - stock accels for stop
>>>>
>>>>  ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>>>  1 files changed, 71 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>>> index bc4ec67..5e8a158 100644
>>>> --- a/ui/gtk.c
>>>> +++ b/ui/gtk.c
>>>> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>>>>  
>>>>      GtkAccelGroup *accel_group;
>>>>  
>>>> -    GtkWidget *file_menu_item;
>>>> -    GtkWidget *file_menu;
>>>> +    GtkWidget *machine_menu_item;
>>>> +    GtkWidget *machine_menu;
>>>> +    GtkWidget *stop_item;
>>>> +    GtkWidget *reset_item;
>>>> +    GtkWidget *powerdown_item;
>>>>      GtkWidget *quit_item;
>>>>  
>>>>      GtkWidget *view_menu_item;
>>>> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>>>>      GdkCursor *null_cursor;
>>>>      Notifier mouse_mode_notifier;
>>>>      gboolean free_scale;
>>>> +
>>>> +    bool external_stop_update;
>>>>  } GtkDisplayState;
>>>>  
>>>>  static GtkDisplayState *global_state;
>>>> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>>>>      const char *status = "";
>>>>      gchar *title;
>>>>      const char *grab = "";
>>>> +    bool is_stopped = !runstate_is_running();
>>>>  
>>>>      if (gd_is_grab_active(s)) {
>>>>          grab = " - Press Ctrl+Alt+G to release grab";
>>>>      }
>>>>  
>>>> -    if (!runstate_is_running()) {
>>>> +    if (is_stopped) {
>>>>          status = " [Stopped]";
>>>>      }
>>>> +    s->external_stop_update = true;
>>>> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
>>>> +                                   is_stopped);
>>>> +    s->external_stop_update = false;
>>>>  
>>>>      if (qemu_name) {
>>>>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
>>>> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>>>>  
>>>>  /** Window Menu Actions **/
>>>>  
>>>> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
>>>> +{
>>>> +    GtkDisplayState *s = opaque;
>>>> +
>>>> +    if (s->external_stop_update) {
>>>> +        return;
>>>> +    }
>>>> +    if (runstate_is_running()) {
>>>> +        qmp_stop(NULL);
>>>> +    } else {
>>>> +        qmp_cont(NULL);
>>>> +    }
>>>> +}
>>>> +
>>>> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
>>>> +{
>>>> +    qmp_system_reset(NULL);
>>>> +}
>>>> +
>>>> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
>>>> +{
>>>> +    qmp_system_powerdown(NULL);
>>>> +}
>>>> +
>>>>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>>>>  {
>>>>      qmp_quit(NULL);
>>>> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>>>>      g_signal_connect(s->drawing_area, "key-release-event",
>>>>                       G_CALLBACK(gd_key_event), s);
>>>>  
>>>> +    g_signal_connect(s->stop_item, "activate",
>>>> +                     G_CALLBACK(gd_menu_stop), s);
>>>> +    g_signal_connect(s->reset_item, "activate",
>>>> +                     G_CALLBACK(gd_menu_reset), s);
>>>> +    g_signal_connect(s->powerdown_item, "activate",
>>>> +                     G_CALLBACK(gd_menu_powerdown), s);
>>>>      g_signal_connect(s->quit_item, "activate",
>>>>                       G_CALLBACK(gd_menu_quit), s);
>>>>      g_signal_connect(s->full_screen_item, "activate",
>>>> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>>>>      int i;
>>>>  
>>>>      accel_group = gtk_accel_group_new();
>>>> -    s->file_menu = gtk_menu_new();
>>>> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
>>>> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
>>>> +    s->machine_menu = gtk_menu_new();
>>>> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
>>>> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
>>>> +
>>>> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
>>>> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
>>>> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
>>>> +                                 "<QEMU>/Machine/Stop");
>>>> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
>>>> +
>>>> +    separator = gtk_separator_menu_item_new();
>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
>>>> +
>>>> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
>>>> +
>>>> +    s->powerdown_item =
>>>> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
>>>
>>> 'Power Down' please.  I can make this change if you don't object.
>>
>> You also mentioned picking up the label string for Stop. Can do both and
>> send v3.
> 
> How do you feel about changing Stop to Pause?  I think Pause makes more
> sense to me.  I think it will translate better too.
> 
> Stop is a synonym with terminate.  I don't think it's a given that you
> can always restart once you stop.
> 
> However, pause and unpause are much more clearly related.

No concerns. I had the monitor command stop and the tap in the windows
caption in mind. Change the latter as well?

> 
> Please do send a v3.

Too late, will need a v4.

Jan
Anthony Liguori - Feb. 22, 2013, 7:09 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2013-02-22 18:53, Anthony Liguori wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> On 2013-02-22 18:16, Anthony Liguori wrote:
>>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>>
>>>>> This adds basic guest control commands to the "Machine" menu - a nice
>>>>> added-value for the GTK UI. We pick up the stock stop accelerators (if
>>>>> any) but not its image as we add a check item.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>>  - File -> Machine menu
>>>>>  - stock accels for stop
>>>>>
>>>>>  ui/gtk.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>>>>  1 files changed, 71 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>>>> index bc4ec67..5e8a158 100644
>>>>> --- a/ui/gtk.c
>>>>> +++ b/ui/gtk.c
>>>>> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>>>>>  
>>>>>      GtkAccelGroup *accel_group;
>>>>>  
>>>>> -    GtkWidget *file_menu_item;
>>>>> -    GtkWidget *file_menu;
>>>>> +    GtkWidget *machine_menu_item;
>>>>> +    GtkWidget *machine_menu;
>>>>> +    GtkWidget *stop_item;
>>>>> +    GtkWidget *reset_item;
>>>>> +    GtkWidget *powerdown_item;
>>>>>      GtkWidget *quit_item;
>>>>>  
>>>>>      GtkWidget *view_menu_item;
>>>>> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>>>>>      GdkCursor *null_cursor;
>>>>>      Notifier mouse_mode_notifier;
>>>>>      gboolean free_scale;
>>>>> +
>>>>> +    bool external_stop_update;
>>>>>  } GtkDisplayState;
>>>>>  
>>>>>  static GtkDisplayState *global_state;
>>>>> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>>>>>      const char *status = "";
>>>>>      gchar *title;
>>>>>      const char *grab = "";
>>>>> +    bool is_stopped = !runstate_is_running();
>>>>>  
>>>>>      if (gd_is_grab_active(s)) {
>>>>>          grab = " - Press Ctrl+Alt+G to release grab";
>>>>>      }
>>>>>  
>>>>> -    if (!runstate_is_running()) {
>>>>> +    if (is_stopped) {
>>>>>          status = " [Stopped]";
>>>>>      }
>>>>> +    s->external_stop_update = true;
>>>>> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
>>>>> +                                   is_stopped);
>>>>> +    s->external_stop_update = false;
>>>>>  
>>>>>      if (qemu_name) {
>>>>>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
>>>>> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>>>>>  
>>>>>  /** Window Menu Actions **/
>>>>>  
>>>>> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
>>>>> +{
>>>>> +    GtkDisplayState *s = opaque;
>>>>> +
>>>>> +    if (s->external_stop_update) {
>>>>> +        return;
>>>>> +    }
>>>>> +    if (runstate_is_running()) {
>>>>> +        qmp_stop(NULL);
>>>>> +    } else {
>>>>> +        qmp_cont(NULL);
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
>>>>> +{
>>>>> +    qmp_system_reset(NULL);
>>>>> +}
>>>>> +
>>>>> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
>>>>> +{
>>>>> +    qmp_system_powerdown(NULL);
>>>>> +}
>>>>> +
>>>>>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>>>>>  {
>>>>>      qmp_quit(NULL);
>>>>> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>>>>>      g_signal_connect(s->drawing_area, "key-release-event",
>>>>>                       G_CALLBACK(gd_key_event), s);
>>>>>  
>>>>> +    g_signal_connect(s->stop_item, "activate",
>>>>> +                     G_CALLBACK(gd_menu_stop), s);
>>>>> +    g_signal_connect(s->reset_item, "activate",
>>>>> +                     G_CALLBACK(gd_menu_reset), s);
>>>>> +    g_signal_connect(s->powerdown_item, "activate",
>>>>> +                     G_CALLBACK(gd_menu_powerdown), s);
>>>>>      g_signal_connect(s->quit_item, "activate",
>>>>>                       G_CALLBACK(gd_menu_quit), s);
>>>>>      g_signal_connect(s->full_screen_item, "activate",
>>>>> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>>>>>      int i;
>>>>>  
>>>>>      accel_group = gtk_accel_group_new();
>>>>> -    s->file_menu = gtk_menu_new();
>>>>> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
>>>>> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
>>>>> +    s->machine_menu = gtk_menu_new();
>>>>> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
>>>>> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
>>>>> +
>>>>> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
>>>>> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
>>>>> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
>>>>> +                                 "<QEMU>/Machine/Stop");
>>>>> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
>>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
>>>>> +
>>>>> +    separator = gtk_separator_menu_item_new();
>>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
>>>>> +
>>>>> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
>>>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
>>>>> +
>>>>> +    s->powerdown_item =
>>>>> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
>>>>
>>>> 'Power Down' please.  I can make this change if you don't object.
>>>
>>> You also mentioned picking up the label string for Stop. Can do both and
>>> send v3.
>> 
>> How do you feel about changing Stop to Pause?  I think Pause makes more
>> sense to me.  I think it will translate better too.
>> 
>> Stop is a synonym with terminate.  I don't think it's a given that you
>> can always restart once you stop.
>> 
>> However, pause and unpause are much more clearly related.
>
> No concerns. I had the monitor command stop and the tap in the windows
> caption in mind. Change the latter as well?

Yes.  Thanks.

Regards,

Anthony Liguori

>
>> 
>> Please do send a v3.
>
> Too late, will need a v4.
>
> Jan
>
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux

Patch

diff --git a/ui/gtk.c b/ui/gtk.c
index bc4ec67..5e8a158 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -80,8 +80,11 @@  typedef struct GtkDisplayState
 
     GtkAccelGroup *accel_group;
 
-    GtkWidget *file_menu_item;
-    GtkWidget *file_menu;
+    GtkWidget *machine_menu_item;
+    GtkWidget *machine_menu;
+    GtkWidget *stop_item;
+    GtkWidget *reset_item;
+    GtkWidget *powerdown_item;
     GtkWidget *quit_item;
 
     GtkWidget *view_menu_item;
@@ -117,6 +120,8 @@  typedef struct GtkDisplayState
     GdkCursor *null_cursor;
     Notifier mouse_mode_notifier;
     gboolean free_scale;
+
+    bool external_stop_update;
 } GtkDisplayState;
 
 static GtkDisplayState *global_state;
@@ -160,14 +165,19 @@  static void gd_update_caption(GtkDisplayState *s)
     const char *status = "";
     gchar *title;
     const char *grab = "";
+    bool is_stopped = !runstate_is_running();
 
     if (gd_is_grab_active(s)) {
         grab = " - Press Ctrl+Alt+G to release grab";
     }
 
-    if (!runstate_is_running()) {
+    if (is_stopped) {
         status = " [Stopped]";
     }
+    s->external_stop_update = true;
+    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
+                                   is_stopped);
+    s->external_stop_update = false;
 
     if (qemu_name) {
         title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
@@ -584,6 +594,30 @@  static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
 
 /** Window Menu Actions **/
 
+static void gd_menu_stop(GtkMenuItem *item, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+
+    if (s->external_stop_update) {
+        return;
+    }
+    if (runstate_is_running()) {
+        qmp_stop(NULL);
+    } else {
+        qmp_cont(NULL);
+    }
+}
+
+static void gd_menu_reset(GtkMenuItem *item, void *opaque)
+{
+    qmp_system_reset(NULL);
+}
+
+static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
+{
+    qmp_system_powerdown(NULL);
+}
+
 static void gd_menu_quit(GtkMenuItem *item, void *opaque)
 {
     qmp_quit(NULL);
@@ -952,6 +986,12 @@  static void gd_connect_signals(GtkDisplayState *s)
     g_signal_connect(s->drawing_area, "key-release-event",
                      G_CALLBACK(gd_key_event), s);
 
+    g_signal_connect(s->stop_item, "activate",
+                     G_CALLBACK(gd_menu_stop), s);
+    g_signal_connect(s->reset_item, "activate",
+                     G_CALLBACK(gd_menu_reset), s);
+    g_signal_connect(s->powerdown_item, "activate",
+                     G_CALLBACK(gd_menu_powerdown), s);
     g_signal_connect(s->quit_item, "activate",
                      G_CALLBACK(gd_menu_quit), s);
     g_signal_connect(s->full_screen_item, "activate",
@@ -985,15 +1025,35 @@  static void gd_create_menus(GtkDisplayState *s)
     int i;
 
     accel_group = gtk_accel_group_new();
-    s->file_menu = gtk_menu_new();
-    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
-    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
+    s->machine_menu = gtk_menu_new();
+    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
+    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
+
+    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
+    gtk_stock_lookup(GTK_STOCK_STOP, &item);
+    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
+                                 "<QEMU>/Machine/Stop");
+    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, item.modifier);
+    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
+
+    separator = gtk_separator_menu_item_new();
+    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
+
+    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
+    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
+
+    s->powerdown_item = gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
+    gtk_menu_append(GTK_MENU(s->machine_menu), s->powerdown_item);
+
+    separator = gtk_separator_menu_item_new();
+    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
 
     s->quit_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_QUIT, NULL);
     gtk_stock_lookup(GTK_STOCK_QUIT, &item);
     gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->quit_item),
-                                 "<QEMU>/File/Quit");
-    gtk_accel_map_add_entry("<QEMU>/File/Quit", item.keyval, item.modifier);
+                                 "<QEMU>/Machine/Quit");
+    gtk_accel_map_add_entry("<QEMU>/Machine/Quit", item.keyval, item.modifier);
+    gtk_menu_append(GTK_MENU(s->machine_menu), s->quit_item);
 
     s->view_menu = gtk_menu_new();
     gtk_menu_set_accel_group(GTK_MENU(s->view_menu), accel_group);
@@ -1068,9 +1128,9 @@  static void gd_create_menus(GtkDisplayState *s)
     gtk_window_add_accel_group(GTK_WINDOW(s->window), accel_group);
     s->accel_group = accel_group;
 
-    gtk_menu_append(GTK_MENU(s->file_menu), s->quit_item);
-    gtk_menu_item_set_submenu(GTK_MENU_ITEM(s->file_menu_item), s->file_menu);
-    gtk_menu_shell_append(GTK_MENU_SHELL(s->menu_bar), s->file_menu_item);
+    gtk_menu_item_set_submenu(GTK_MENU_ITEM(s->machine_menu_item),
+                              s->machine_menu);
+    gtk_menu_shell_append(GTK_MENU_SHELL(s->menu_bar), s->machine_menu_item);
 
     gtk_menu_item_set_submenu(GTK_MENU_ITEM(s->view_menu_item), s->view_menu);
     gtk_menu_shell_append(GTK_MENU_SHELL(s->menu_bar), s->view_menu_item);