On 06/18/2009 12:22 PM, Michal Novotny wrote:
> # HG changeset patch
> # User Michal Novotny <minovotn@xxxxxxxxxx>
> # Date 1245341781 -7200
> # Node ID b599a65ca0615704a47e38eabcd2ff0947bcbaec
> # Parent a996e00b3bb6a99fdf9505c053bc1f6bee532d3b
> Make virt-manager remember last used paths
>
> This patch makes virt-manager remember last used paths for disk images, saved
> snapshots, restored snapshots, media paths and also screenshot paths not to
> bother users with changing paths from the default location per HV technology.
> Useful when installing multiple domains and having all the media/image files
> in non-default locations.
>
Comments inline:
> diff -r a996e00b3bb6 -r b599a65ca061 src/virt-manager.schemas.in
> --- a/src/virt-manager.schemas.in Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virt-manager.schemas.in Thu Jun 18 18:16:21 2009 +0200
<snip>
This all looks fine.
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/addhardware.py
> --- a/src/virtManager/addhardware.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/addhardware.py Thu Jun 18 18:16:21 2009 +0200
> @@ -678,7 +678,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_default_directory(self.vm.get_connection(), "image")
> filename = self._browse_file(_("Locate or Create New Storage File"),
> textent, folder=folder,
> confirm_overwrite=True)
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/choosecd.py
> --- a/src/virtManager/choosecd.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/choosecd.py Thu Jun 18 18:16:21 2009 +0200
> @@ -20,6 +20,7 @@
> import gtk.glade
> import gobject
> import logging
> +import os.path
>
> import virtinst
>
> @@ -133,7 +134,8 @@
> pass
>
> def browse_fv_iso_location(self, ignore1=None, ignore2=None):
> - self._browse_file(_("Locate ISO Image"))
> + folder = self.config.get_default_directory( self.conn, "media" )
> + self._browse_file(_("Locate ISO Image"), folder=folder)
>
> def populate_opt_media(self):
> try:
> @@ -146,14 +148,16 @@
>
> def set_storage_path(self, src, path):
> self.window.get_widget("iso-path").set_text(path)
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "media" )
>
I'd prefer if function calls look like:
func(arg1, arg2)
rather than
func( arg1, arg2 )
The former is more inline with existing code.
Also, I like all new code to try and stay under the 80 column length. If it's
too ugly to make it fit, don't worry, but that isn't the common case.
> - def _browse_file(self, dialog_name):
> + def _browse_file(self, dialog_name, folder):
> if self.storage_browser == None:
> self.storage_browser = vmmStorageBrowser(self.config, self.conn)
> #self.topwin)
> self.storage_browser.connect("storage-browse-finish",
> self.set_storage_path)
> - self.storage_browser.local_args = { "dialog_name": dialog_name }
> + self.storage_browser.local_args = { "dialog_name": dialog_name, "start_folder": folder }
> self.storage_browser.show(self.conn)
> return None
>
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/config.py
> --- a/src/virtManager/config.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/config.py Thu Jun 18 18:16:21 2009 +0200
> @@ -261,6 +261,30 @@
> self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
>
>
> + def get_default_directory(self, conn, _type):
> + if _type not in ["image", "media", "save", "restore", "screenshot"]:
> + logging.error("Unknown type for get_default_directory: %s" % _type)
> + return
> +
> + try:
> + path = self.conf.get_value( self.conf_dir + "/paths/default-%s-path" % _type )
> + except:
> + path = None
> +
> + if not path or len(path) == 0:
> + if _type == "image" or _type == "media":
> + path = self.get_default_image_dir(conn)
> + if _type == "save":
> + path = self.get_default_save_dir(conn)
> +
> + return path
> +
> + def set_default_directory(self, folder, _type):
> + if _type not in ["image", "media", "save", "restore", "screenshot"]:
> + logging.error("Unknown type for set_default_directory: %s" % _type)
> + return
> +
> + self.conf.set_value( self.conf_dir + "/paths/default-%s-path" % _type, folder)
>
Rather than have the user pass a string in like "image", "media", ..., how
about defining some constants in the config class:
CONFIG_DIR_IMAGE = 1
CONFIG_DIR_SAVE = 2
...
(They can even be the string values you use above anyways). Then have a
separate function which converts that to the requested gconf path. This way
you don't have to duplicate the ["image", "media", ...] whitelist in two
places, and I think will make the calling code more readable.
Also, to simplify a lot of the code, you could push down the gconf fetching/
storing into util.browse_local: add an argument which takes one of the above
CONFIG_DIR values, sets the start folder, and then records the directory that
the user chooses. That way, all callers would need to pass one value, and
wouldn't need to play with the result.
> def on_vmlist_domain_id_visible_changed(self, callback):
> self.conf.notify_add(self.conf_dir + "/vmlist-fields/domain_id", callback)
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/create.py
> --- a/src/virtManager/create.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/create.py Thu Jun 18 18:16:21 2009 +0200
> @@ -995,16 +995,20 @@
> nodetect_label.show()
>
> def browse_iso(self, ignore1=None, ignore2=None):
> + folder = self.config.get_default_directory( self.conn, "media")
> self._browse_file(_("Locate ISO Image"),
> - self.set_iso_storage_path)
> + self.set_iso_storage_path,
> + folder)
> self.window.get_widget("install-local-box").activate()
>
> def toggle_enable_storage(self, src):
> self.window.get_widget("config-storage-box").set_sensitive(src.get_active())
>
> def browse_storage(self, ignore1):
> + folder = self.config.get_default_directory( self.conn, "image")
> self._browse_file(_("Locate existing storage"),
> - self.set_disk_storage_path)
> + self.set_disk_storage_path,
> + folder)
>
> def toggle_storage_select(self, src):
> act = src.get_active()
> @@ -1014,9 +1018,13 @@
> self.window.get_widget("config-macaddr").set_sensitive(src.get_active())
>
> def set_iso_storage_path(self, ignore, path):
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "media")
> self.window.get_widget("install-local-box").child.set_text(path)
>
> def set_disk_storage_path(self, ignore, path):
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "image")
> self.window.get_widget("config-storage-entry").set_text(path)
>
> # Navigation methods
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/details.py
> --- a/src/virtManager/details.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/details.py Thu Jun 18 18:16:21 2009 +0200
> @@ -31,6 +31,7 @@
> import os
> import socket
> import cairo
> +import os.path
>
> from virtManager.error import vmmErrorDialog
> from virtManager.addhardware import vmmAddHardware
> @@ -1410,11 +1411,17 @@
> # user to choose what image format they'd like to save in....
> path = util.browse_local(self.window.get_widget("vmm-details"),
> _("Save Virtual Machine Screenshot"),
> + self.config.get_default_directory(self.vm.get_connection(),
> + "screenshot"),
> _type = ("*.png", "PNG files"),
> dialog_type = gtk.FILE_CHOOSER_ACTION_SAVE)
> if not path:
> return
>
> + # Save default screenshot directory
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "screenshot" )
> +
> filename = path
> if not(filename.endswith(".png")):
> filename += ".png"
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/engine.py
> --- a/src/virtManager/engine.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/engine.py Thu Jun 18 18:16:21 2009 +0200
> @@ -24,6 +24,7 @@
> import logging
> import gnome
> import traceback
> +import os.path
>
> from virtManager.about import vmmAbout
> from virtManager.connect import vmmConnect
> @@ -406,12 +407,16 @@
>
> path = util.browse_local(src.window.get_widget("vmm-details"),
> _("Save Virtual Machine"),
> - self.config.get_default_save_dir(con),
> + self.config.get_default_directory(con, "save"),
> dialog_type=gtk.FILE_CHOOSER_ACTION_SAVE)
>
> if not path:
> return
>
> + # Save default directory for saving VMs
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "save")
> +
> progWin = vmmAsyncJob(self.config, self._save_callback, [vm, path],
> _("Saving Virtual Machine"))
> progWin.run()
> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/manager.py
> --- a/src/virtManager/manager.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/manager.py Thu Jun 18 18:16:21 2009 +0200
> @@ -23,6 +23,7 @@
> import gtk.glade
> import logging
> import traceback
> +import os.path
>
> import sparkline
> import libvirt
> @@ -397,11 +398,15 @@
>
> path = util.browse_local(self.window.get_widget("vmm-manager"),
> _("Restore Virtual Machine"),
> - self.config.get_default_save_dir(conn))
> + self.config.get_default_directory(conn, "restore"))
>
> if not path:
> return
>
> + # Save last restore path
> + folder = os.path.dirname( path )
> + self.config.set_default_directory( folder, "restore")
> +
> if not conn.is_valid_saved_image(path):
> self.err.val_err(_("The file '%s' does not appear to be a "
> "valid saved machine image") % path)
Thanks! This will be useful to have.
Cole
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
