Re: [PATCH] virt-manager: Make virt-manager remember last image directory

[Michal Novotny <minovotn@xxxxxxxxxx>]

Ok, I had to rebase it to current codebase because there were some changesets that were not applied to my hg repository.

Here's the new version, checked using `make check-pylint`,
Michal

On 06/18/2009 04:06 PM, Michal Novotny wrote:

Hi,
this is new and updated version of my patch. It remembers 5 types of last used paths now - for last image, media, save and restore and screenshot files.

Schemas were also updated but it wasn't working right so there is a still an exception try in get_default_directory() function in config.py.

Thanks,
Michal

On 06/18/2009 11:15 AM, Michal Novotny wrote:

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

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

# 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.

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
@@ -272,5 +272,70 @@
         <long>Whether to install a sound device for remote VMs or not</long>
       </locale>
     </schema>
+
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/paths/default-image-path</key>
+      <applyto>/apps/::PACKAGE::/paths/default-image-path</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>string</type>
+      <default></default>
+
+      <locale name="C">
+        <short>Default image path</short>
+        <long>Default path for choosing VM images</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/paths/default-media-path</key>
+      <applyto>/apps/::PACKAGE::/paths/default-media-path</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>string</type>
+      <default></default>
+
+      <locale name="C">
+        <short>Default media path</short>
+        <long>Default path for choosing media</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/paths/default-save-path</key>
+      <applyto>/apps/::PACKAGE::/paths/default-save-path</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>string</type>
+      <default></default>
+
+      <locale name="C">
+        <short>Default save domain path</short>
+        <long>Default path for saving VM snaphots</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/paths/default-restore-path</key>
+      <applyto>/apps/::PACKAGE::/paths/default-restore-path</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>string</type>
+      <default></default>
+
+      <locale name="C">
+        <short>Default restore path</short>
+        <long>Default path for stored VM snapshots</long>
+      </locale>
+    </schema>
+
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/paths/default-screenshot-path</key>
+      <applyto>/apps/::PACKAGE::/paths/default-screenshot-path</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>string</type>
+      <default></default>
+
+      <locale name="C">
+        <short>Default screenshot path</short>
+        <long>Default path for saving screenshots from VMs</long>
+      </locale>
+    </schema>
   </schemalist>
 </gconfschemafile>
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" )
 
-    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)
 
     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)

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools