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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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
# HG changeset patch
# User Michal Novotny <minovotn@xxxxxxxxxx>
# Date 1245333774 -7200
# Node ID 41b1f4a1f1efba4e333f761e7ed7417beaa4d9a1
# Parent  6daa550d93f7434e43b04dc0ce305dd3b0d8803e
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 6daa550d93f7 -r 41b1f4a1f1ef src/virt-manager.schemas.in
--- a/src/virt-manager.schemas.in	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virt-manager.schemas.in	Thu Jun 18 16:02:54 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 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/addhardware.py
--- a/src/virtManager/addhardware.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/addhardware.py	Thu Jun 18 16:02:54 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_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 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/choosecd.py
--- a/src/virtManager/choosecd.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/choosecd.py	Thu Jun 18 16:02:54 2009 +0200
@@ -20,6 +20,7 @@
 import gtk.glade
 import gobject
 import logging
+import os.path
 
 import virtinst
 
@@ -130,8 +131,11 @@
         pass
 
     def browse_fv_iso_location(self, ignore1=None, ignore2=None):
-        filename = self._browse_file(_("Locate ISO Image"))
+        folder = self.config.get_default_directory( self.conn, "media" )
+        filename = self._browse_file(_("Locate ISO Image"), folder=folder)
         if filename != None:
+            dir = os.path.dirname( filename )
+            self.config.set_default_directory( dir, "media")
             self.window.get_widget("iso-path").set_text(filename)
 
     def populate_opt_media(self):
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/config.py
--- a/src/virtManager/config.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/config.py	Thu Jun 18 16:02:54 2009 +0200
@@ -238,6 +238,33 @@
         return self.conf.get_bool(self.conf_dir + "/vmlist-fields/network_traffic")
 
 
+    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, dir, type):
+        logging.debug("SD1")
+        if type not in ["image", "media", "save", "restore", "screenshot"]:
+            logging.error("Unknown type for set_default_directory: %s" % type)
+            return
+
+        logging.debug("SDD")
+        self.conf.set_value( self.conf_dir + "/paths/default-%s-path" % type, dir)
+        logging.debug("setting /paths/default-%s-path = %s" % (type, dir))
 
     def set_vmlist_domain_id_visible(self, state):
         self.conf.set_bool(self.conf_dir + "/vmlist-fields/domain_id", state)
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/create.py
--- a/src/virtManager/create.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/create.py	Thu Jun 18 16:02:54 2009 +0200
@@ -995,7 +995,8 @@
             nodetect_label.show()
 
     def browse_iso(self, ignore1=None, ignore2=None):
-        f = self._browse_file(_("Locate ISO Image"), is_media=True)
+        folder = self.config.get_default_directory( self.conn, "media")
+        f = self._browse_file(_("Locate ISO Image"), is_media=True, folder=folder)
         if f != None:
             self.window.get_widget("install-local-box").child.set_text(f)
         self.window.get_widget("install-local-box").activate()
@@ -1004,7 +1005,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_default_directory( self.conn, "image" )
+        f = self._browse_file(_("Locate existing storage"), folder=folder )
         if f != None:
             self.window.get_widget("config-storage-entry").set_text(f)
 
@@ -1657,7 +1659,8 @@
         if self.conn.is_remote() or not is_media:
             if self.storage_browser == None:
                 self.storage_browser = vmmStorageBrowser(self.config,
-                                                         self.conn)
+                                                         self.conn,
+                                                         is_media)
                 self.storage_browser.connect("storage-browse-finish",
                                              self.set_storage_path)
             self.storage_browser.local_args = { "dialog_name": dialog_name,
@@ -1665,7 +1668,7 @@
             self.storage_browser.show(self.conn)
             return None
 
-        return util.browse_local(self.topwin, dialog_name, folder)
+        return util.browse_local(self.topwin,dialog_name,folder)
 
     def show_help(self, ignore):
         # No help available yet.
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/details.py
--- a/src/virtManager/details.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/details.py	Thu Jun 18 16:02:54 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
+        dir = os.path.dirname( path )
+        self.config.set_default_directory(dir, "screenshot")
+
         filename = path
         if not(filename.endswith(".png")):
             filename += ".png"
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/engine.py
--- a/src/virtManager/engine.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/engine.py	Thu Jun 18 16:02:54 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
+        dir = os.path.dirname( path )
+        self.config.set_default_directory( dir, "save")
+
         progWin = vmmAsyncJob(self.config, self._save_callback, [vm, path],
                               _("Saving Virtual Machine"))
         progWin.run()
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/manager.py
--- a/src/virtManager/manager.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/manager.py	Thu Jun 18 16:02:54 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
+        dir = os.path.dirname( path )
+        self.config.set_default_directory(dir, "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)
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/storagebrowse.py
--- a/src/virtManager/storagebrowse.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/storagebrowse.py	Thu Jun 18 16:02:54 2009 +0200
@@ -20,6 +20,7 @@
 
 import gobject
 import gtk.glade
+import os.path
 
 import traceback
 import logging
@@ -38,7 +39,7 @@
                                   gobject.TYPE_NONE, [str]),
     }
 
-    def __init__(self, config, conn):
+    def __init__(self, config, conn, is_media = False):
         self.__gobject_init__()
         self.window = gtk.glade.XML(config.get_glade_dir() + \
                                     "/vmm-storage-browse.glade",
@@ -46,6 +47,7 @@
                                     domain="virt-manager")
         self.config = config
         self.conn = conn
+        self.is_media = is_media
         self.conn_signal_ids = []
 
         self.topwin = self.window.get_widget("vmm-storage-browse")
@@ -242,6 +244,16 @@
     def _do_finish(self, path=None):
         if not path:
             path = self.current_vol().get_path()
+
+        # Save path to config if we use file storage
+        if not path.startswith("/dev"):
+            if self.is_media:
+                type = "media"
+            else:
+                type = "image"
+            directory = os.path.dirname( path )
+            self.config.set_default_directory( directory, type )
+
         self.emit("storage-browse-finish", path)
         self.close()
 
diff -r 6daa550d93f7 -r 41b1f4a1f1ef src/virtManager/util.py
--- a/src/virtManager/util.py	Tue Jun 16 21:22:58 2009 -0400
+++ b/src/virtManager/util.py	Thu Jun 18 16:02:54 2009 +0200
@@ -20,6 +20,7 @@
 
 import logging
 import gtk
+import os.path
 
 import libvirt
 

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
[Index of Archives]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux