On 06/16/2009 10:26 AM, Michal Novotny wrote:
> # HG changeset patch
> # User Michal Novotny <minovotn@xxxxxxxxxx>
> # Date 1245162093 -7200
> # Node ID 73427fcfc222006d7f74ce79277f8432d74d7c52
> # Parent 74f1f8309b139af9d84edfb1f0186c2306c0f332
> Make virt-manager remember last chosen image directory
>
> Virt-manager never remembers last image directory after choosing file an image
> file to be appended to any domain. Since I am having all the domain image files
> in the same directory that is not default, this patch saves the last image path
> to gconf configuration file and offers it as startfolder next time.
>
I like the idea in principle, but I think it needs to be a bit smarter.
I have a fix pending that will enable the storage browser for install media on
a local connection. We won't want the storage browser squashing the
default_image_path in that case, since there's a good chance that their media
directory and image directory are not the same.
We should have two gconf entries: /paths/last_image_path and
/paths/last_media_path, and give the storage_browser a flag or something to
allow the caller to opt in on which gconf entry we want to store. There's
probably a smarter way to do it but I can't think right now.
A general comment: the gconf entries should change the virt-manager.schema.in.
You can probably just use a default value of "", that way gconf won't throw an
exception the first time you try to access the key.
Some comments in line
> diff -r 74f1f8309b13 -r 73427fcfc222 src/virtManager/addhardware.py
> --- a/src/virtManager/addhardware.py Mon Jun 15 15:54:56 2009 +0200
> +++ b/src/virtManager/addhardware.py Tue Jun 16 16:21:33 2009 +0200
> @@ -674,7 +674,7 @@
>
> def browse_storage_file_address(self, src, ignore=None):
> textent = self.window.get_widget("storage-file-address")
> - folder = self.config.get_default_image_dir(self.vm.get_connection())
> + folder = self.config.get_user_default_image_dir(self.vm.get_connection())
> filename = self._browse_file(_("Locate or Create New Storage File"),
> textent, folder=folder,
> confirm_overwrite=True)
> diff -r 74f1f8309b13 -r 73427fcfc222 src/virtManager/config.py
> --- a/src/virtManager/config.py Mon Jun 15 15:54:56 2009 +0200
> +++ b/src/virtManager/config.py Tue Jun 16 16:21:33 2009 +0200
> @@ -237,7 +237,15 @@
> def is_vmlist_network_traffic_visible(self):
> return self.conf.get_bool(self.conf_dir + "/vmlist-fields/network_traffic")
>
> + def get_user_default_image_dir(self, conn):
> + try:
> + return self.conf.get_value(self.conf_dir + "/paths/default_image_path")
> + except ValueError:
> + return self.get_default_image_dir(conn)
>
> + def set_user_default_image_dir(self, dir):
> + self.conf.set_value(self.conf_dir + "/paths/default_image_path", dir)
> + logging.debug("Setting path to %s", dir)
>
> def set_vmlist_domain_id_visible(self, state):
> self.conf.set_bool(self.conf_dir + "/vmlist-fields/domain_id", state)
> @@ -556,6 +564,14 @@
>
>
> def get_default_image_dir(self, connection):
> + # Exception for case connection argument is str (happens when
> + # creating new domains because we don't have connection object)
> + if str(connection) == connection:
> + if connection == "Xen":
> + return DEFAULT_XEN_IMAGE_DIR
> + else:
> + return DEFAULT_VIRT_IMAGE_DIR
> +
> if connection.get_type() == "Xen":
> return DEFAULT_XEN_IMAGE_DIR
>
The hack shouldn't be necessary (see below).
> diff -r 74f1f8309b13 -r 73427fcfc222 src/virtManager/create.py
> --- a/src/virtManager/create.py Mon Jun 15 15:54:56 2009 +0200
> +++ b/src/virtManager/create.py Tue Jun 16 16:21:33 2009 +0200
> @@ -1004,7 +1004,8 @@
> self.window.get_widget("config-storage-box").set_sensitive(src.get_active())
>
> def browse_storage(self, ignore1):
> - f = self._browse_file(_("Locate existing storage"))
> + folder = self.config.get_user_default_image_dir( self.capsdomain.hypervisor_type )
> + f = self._browse_file(_("Locate existing storage"), folder=folder)
> if f != None:
> self.window.get_widget("config-storage-entry").set_text(f)
>
'self.conn' contains the active connection, so you should use that rather than
self.capsdomain...
> @@ -1653,7 +1654,6 @@
> self.detectedDistro = (None, None)
>
> def _browse_file(self, dialog_name, folder=None, is_media=False):
> -
> if self.conn.is_remote() or not is_media:
> if self.storage_browser == None:
> self.storage_browser = vmmStorageBrowser(self.config,
> diff -r 74f1f8309b13 -r 73427fcfc222 src/virtManager/storagebrowse.py
> --- a/src/virtManager/storagebrowse.py Mon Jun 15 15:54:56 2009 +0200
> +++ b/src/virtManager/storagebrowse.py Tue Jun 16 16:21:33 2009 +0200
> @@ -20,6 +20,7 @@
>
> import gobject
> import gtk.glade
> +import os.path
>
> import traceback
> import logging
> @@ -242,6 +243,11 @@
> def _do_finish(self, path=None):
> if not path:
> path = self.current_vol().get_path()
> +
> + # Save path to config if we use file as storage
> + if "/dev" not in path:
> + self.config.set_user_default_image_dir( os.path.dirname(path) )
> +
A couple things here: This will match any path which contains "/dev", you
probably want path.startswith(). Also, this will store the directory of a
chosen storage volume, which isn't what we want. If you make this chunk the
first thing that happens in the function, it should be fine.
> self.emit("storage-browse-finish", path)
> self.close()
Thanks,
Cole
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
