[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [virt-manager PATCH] Make deleting storage files configurable



On 11/23/2012 10:33 AM, Martin Kletzander wrote:
> This patch adds checkbox for making the default state of "Delete
> associated storage files" checkbox configurable in Edit|Preferences
> dialog.  This changes nothing by default, but adds the possibility to
> personalize the manager a bit.
> ---
> As this is my first patch for virt-manager, it most probably needs
> some polishing, but I'm open for any suggestions.

Thanks for the patch Martin, it looks quite fine :) However I'd like to
approach this in another way.

I think we should change the Delete dialog to default to deleting storage, but
give an 'Are you sure?' type checkbox when the user finally hits the 'delete'
button if the operation will be destructive. That dialog can have a 'don't
warn about this again' checkbox, which will have a matching checkbox in the
prefs dialog.

This is how we do it for things like VM destroy which is a potentially
dangerous operation.

Check out _do_destroy_domain in engine.py for an example of using the checkbox
dialog.

Thanks,
Cole

> ---
>  src/virt-manager.schemas.in    | 13 ++++++++++++
>  src/virtManager/config.py      | 11 +++++++++-
>  src/virtManager/delete.py      |  6 +++---
>  src/virtManager/details.py     |  2 +-
>  src/virtManager/preferences.py | 13 +++++++++++-
>  src/vmm-preferences.ui         | 48 +++++++++++++++++++++++++++++++-----------
>  6 files changed, 75 insertions(+), 18 deletions(-)
> 
> diff --git a/src/virt-manager.schemas.in b/src/virt-manager.schemas.in
> index 8154534..c25baa9 100644
> --- a/src/virt-manager.schemas.in
> +++ b/src/virt-manager.schemas.in
> @@ -235,6 +235,19 @@
>      </schema>
> 
>      <schema>
> +      <key>/schemas/apps/::PACKAGE::/check-delete-storage</key>
> +      <applyto>/apps/::PACKAGE::/check-delete-storage</applyto>
> +      <owner>::PACKAGE::</owner>
> +      <type>bool</type>
> +      <default>0</default>
> +
> +      <locale name="C">
> +        <short>Whether the "Delete all associated storage" checkbox should be checked by default</short>
> +        <long>Whether to offer deleting all related storage files by default while deleting a virtual machine</long>
> +      </locale>
> +    </schema>
> +
> +    <schema>
>        <key>/schemas/apps/::PACKAGE::/paths/default-image-path</key>
>        <applyto>/apps/::PACKAGE::/paths/default-image-path</applyto>
>        <owner>::PACKAGE::</owner>
> diff --git a/src/virtManager/config.py b/src/virtManager/config.py
> index 508fea0..5315423 100644
> --- a/src/virtManager/config.py
> +++ b/src/virtManager/config.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2006 Red Hat, Inc.
> +# Copyright (C) 2006, 2012 Red Hat, Inc.
>  # Copyright (C) 2006 Daniel P. Berrange <berrange redhat com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -412,6 +412,15 @@ class vmmConfig(object):
>      def set_view_system_tray(self, val):
>          self.conf.set_bool(self.conf_dir + "/system-tray", val)
> 
> +    # Delete storage by default
> +    def on_check_delete_storage_changed(self, callback):
> +        return self.conf.notify_add(self.conf_dir + "/check-delete-storage", callback)
> +    def get_check_delete_storage(self):
> +        return self.conf.get_bool(self.conf_dir + "/check-delete-storage")
> +    def set_check_delete_storage(self, val):
> +        self.conf.set_bool(self.conf_dir + "/check-delete-storage", val)
> +
> +
> 
>      # Stats history and interval length
>      def get_stats_update_interval(self):
> diff --git a/src/virtManager/delete.py b/src/virtManager/delete.py
> index 9b7d08a..3c97cf1 100644
> --- a/src/virtManager/delete.py
> +++ b/src/virtManager/delete.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2009 Red Hat, Inc.
> +# Copyright (C) 2009, 2012 Red Hat, Inc.
>  # Copyright (C) 2009 Cole Robinson <crobinso redhat com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -95,8 +95,8 @@ class vmmDeleteDialog(vmmGObjectUI):
> 
>          self.widget("delete-cancel").grab_focus()
> 
> -        # Disable storage removal by default
> -        self.widget("delete-remove-storage").set_active(False)
> +        # Set storage removal to configured value
> +        self.widget("delete-remove-storage").set_active(self.config.get_check_delete_storage())
>          self.widget("delete-remove-storage").toggled()
> 
>          populate_storage_list(self.widget("delete-storage-list"),
> diff --git a/src/virtManager/details.py b/src/virtManager/details.py
> index d20e748..274d1b9 100644
> --- a/src/virtManager/details.py
> +++ b/src/virtManager/details.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2006-2008 Red Hat, Inc.
> +# Copyright (C) 2006-2008, 2012 Red Hat, Inc.
>  # Copyright (C) 2006 Daniel P. Berrange <berrange redhat com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> diff --git a/src/virtManager/preferences.py b/src/virtManager/preferences.py
> index 0eaf20b..bea1716 100644
> --- a/src/virtManager/preferences.py
> +++ b/src/virtManager/preferences.py
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (C) 2006 Red Hat, Inc.
> +# Copyright (C) 2006, 2012 Red Hat, Inc.
>  # Copyright (C) 2006 Daniel P. Berrange <berrange redhat com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -32,6 +32,7 @@ class vmmPreferences(vmmGObjectUI):
>          vmmGObjectUI.__init__(self, "vmm-preferences.ui", "vmm-preferences")
> 
>          self.add_gconf_handle(self.config.on_view_system_tray_changed(self.refresh_view_system_tray))
> +        self.add_gconf_handle(self.config.on_check_delete_storage_changed(self.refresh_check_delete_storage))
>          self.add_gconf_handle(self.config.on_console_accels_changed(self.refresh_console_accels))
>          self.add_gconf_handle(self.config.on_console_scaling_changed(self.refresh_console_scaling))
>          self.add_gconf_handle(self.config.on_stats_update_interval_changed(self.refresh_update_interval))
> @@ -51,6 +52,7 @@ class vmmPreferences(vmmGObjectUI):
>          self.add_gconf_handle(self.config.on_confirm_unapplied_changed(self.refresh_confirm_unapplied))
> 
>          self.refresh_view_system_tray()
> +        self.refresh_check_delete_storage()
>          self.refresh_update_interval()
>          self.refresh_history_length()
>          self.refresh_console_accels()
> @@ -71,6 +73,7 @@ class vmmPreferences(vmmGObjectUI):
> 
>          self.window.connect_signals({
>              "on_prefs_system_tray_toggled" : self.change_view_system_tray,
> +            "on_prefs_check_delete_storage_toggled" : self.change_check_delete_storage,
>              "on_prefs_stats_update_interval_changed": self.change_update_interval,
>              "on_prefs_stats_history_length_changed": self.change_history_length,
>              "on_prefs_console_accels_toggled": self.change_console_accels,
> @@ -119,6 +122,11 @@ class vmmPreferences(vmmGObjectUI):
>          val = self.config.get_view_system_tray()
>          self.widget("prefs-system-tray").set_active(bool(val))
> 
> +    def refresh_check_delete_storage(self, ignore1=None, ignore2=None,
> +                                     ignore3=None, ignore4=None):
> +        val = self.config.get_check_delete_storage()
> +        self.widget("prefs-check-delete-storage").set_active(bool(val))
> +
>      def refresh_update_interval(self, ignore1=None, ignore2=None,
>                                  ignore3=None, ignore4=None):
>          self.widget("prefs-stats-update-interval").set_value(
> @@ -292,6 +300,9 @@ class vmmPreferences(vmmGObjectUI):
>      def change_view_system_tray(self, src):
>          self.config.set_view_system_tray(src.get_active())
> 
> +    def change_check_delete_storage(self, src):
> +        self.config.set_check_delete_storage(src.get_active())
> +
>      def change_update_interval(self, src):
>          self.config.set_stats_update_interval(src.get_value_as_int())
>      def change_history_length(self, src):
> diff --git a/src/vmm-preferences.ui b/src/vmm-preferences.ui
> index 6cc3916..5746474 100644
> --- a/src/vmm-preferences.ui
> +++ b/src/vmm-preferences.ui
> @@ -97,20 +97,44 @@
>                              <property name="column_spacing">6</property>
>                              <property name="row_spacing">6</property>
>                              <child>
> -                              <object class="GtkCheckButton" id="prefs-system-tray">
> -                                <property name="label" translatable="yes">Enable _system tray icon</property>
> +                              <object class="GtkVBox" id="vbox7">
>                                  <property name="visible">True</property>
> -                                <property name="can_focus">True</property>
> -                                <property name="receives_default">False</property>
> -                                <property name="use_action_appearance">False</property>
> -                                <property name="use_underline">True</property>
> -                                <property name="draw_indicator">True</property>
> -                                <signal name="toggled" handler="on_prefs_system_tray_toggled" swapped="no"/>
> +                                <property name="can_focus">False</property>
> +                                <child>
> +                                  <object class="GtkCheckButton" id="prefs-system-tray">
> +                                    <property name="label" translatable="yes">Enable _system tray icon</property>
> +                                    <property name="visible">True</property>
> +                                    <property name="can_focus">True</property>
> +                                    <property name="receives_default">False</property>
> +                                    <property name="use_action_appearance">False</property>
> +                                    <property name="use_underline">True</property>
> +                                    <property name="draw_indicator">True</property>
> +                                    <signal name="toggled" handler="on_prefs_system_tray_toggled" swapped="no"/>
> +                                  </object>
> +                                  <packing>
> +                                    <property name="expand">False</property>
> +                                    <property name="fill">True</property>
> +                                    <property name="position">0</property>
> +                                  </packing>
> +                                </child>
> +                                <child>
> +                                  <object class="GtkCheckButton" id="prefs-check-delete-storage">
> +                                    <property name="label" translatable="yes">_Delete storage by default</property>
> +                                    <property name="visible">True</property>
> +                                    <property name="can_focus">True</property>
> +                                    <property name="receives_default">False</property>
> +                                    <property name="use_action_appearance">False</property>
> +                                    <property name="use_underline">True</property>
> +                                    <property name="draw_indicator">True</property>
> +                                    <signal name="toggled" handler="on_prefs_check_delete_storage_toggled" swapped="no"/>
> +                                  </object>
> +                                  <packing>
> +                                    <property name="expand">False</property>
> +                                    <property name="fill">True</property>
> +                                    <property name="position">1</property>
> +                                  </packing>
> +                                </child>
>                                </object>
> -                              <packing>
> -                                <property name="x_options">GTK_FILL</property>
> -                                <property name="y_options">GTK_FILL</property>
> -                              </packing>
>                              </child>
>                            </object>
>                          </child>
> --
> 1.8.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list redhat com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]