On 06/18/2009 04:19 AM, Cole Robinson wrote:
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.
Ok, basically I agree. Is there a way to get to know whether we're
choosing media or an image in StorageBrowser (SB, for short) class ?
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.
Yeah, that's exactly why I ask. So, if there is no flag, there is a
need to add some for SB. The SB itself stores the data in my code...
Anyway you said you have one patch pending, right? What exactly about?
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.
Ok, I didn't know that. This needs further study but now I need to
solve things about Xen that I am primarily working on.
Some comments in line
Ok, I'll comment it too.
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).
I see, so there's a valid connection object in self.conn that provides
"vmmConnection" (or some) object even when we are creating the domain
(ie. it doesn't exist yet) ?
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...
Ok, even in domain creation we have this object ?
@@ -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.
Oh, right. This is right. I didn't realized that function would be
better, thanks for your input. This isn't what we won't ? Why not? I
thought we want to strip eg. /disk2/vms/vm1/image.img to get
/disk2/vms/vm1/ and this is what we want, right ? Or could you give me
an example why not this approach?
self.emit("storage-browse-finish", path)
self.close()
Thanks,
Cole
Thanks,
Michal
|
