Hi Cole,
comments inline...
On 06/19/2009 03:39 PM, Cole Robinson wrote:
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 )
Ok, I'll change this. This is just about synchronizing my coding style
with existing virt-manager codebase.
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.
Ok, fine. I'll rewrite it to fit on 80 columns per line...
- 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.
Well, the idea is good. I know what you mean and I'll use string values
there to preserve most of those codes.
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.
Well, I don't know what do you mean by that. Do you mean to import
gconf directly into util.py and directly call gconf in browse_local?
You wrote something about altering util.browse_local() ... could you
provide me the prototype you mean?
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
Ok, no problem.
Michal
|
