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

[Michal Novotny]

Hi,
I've been working with virt-manager and I found out that it doesn't
remember the last image directory at all so I created a patch that
writes a configuration to file after the storagebrowse dialog is closed
but only if user chooses file storage (no device storage). Really useful
when you use one folder as a domain image storage path and you don't
want to use default path for KVM/Xen...

If anything, just ask me,
Michal

# HG changeset patch
# User Michal Novotny <minovotn@...>
# 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.

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
 
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)
 
@@ -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) )
+
         self.emit("storage-browse-finish", path)
         self.close()
 

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

[Cole Robinson]

On 06/16/2009 10:26 AM, Michal Novotny wrote:

> # HG changeset patch
> # User Michal Novotny <minovotn@...>
> # 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.

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.

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.

Some comments in line

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

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

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

>          self.emit("storage-browse-finish", path)
>          self.close()

Thanks,
Cole

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

[Michal Novotny]

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@...
# 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Cole Robinson]

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

Nope, your patch will need to deal with that somehow (or move the gconf
setting out of the browser and into the caller, but then you'll want to
differentiate between a local directory and a storage directory, so it's
kind of hairy).

>> 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?

My patch doesn't alter the storage browser at all, it just turns it on
for local install media and cdrom changing. I'll push it today, it will
conflict with your patch a bit though, so you'll have to rebase.

>> 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) ?

Yes, domain installation requires an active libvirt connection.

>>    
>>> 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 ?

Yes, you can't even launch the VM wizard without an active connection.

>>> @@ -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?

We really only want to store directories returned from the regular GTK
file chooser, not from the libvirt storage browser. So if the path comes
from current_vol(), we don't want to store it. The gconf entries should
only be for when they manually select an unmanaged path.

Thanks,
Cole

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

[Michal Novotny]

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@...
# 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

# HG changeset patch
# User Michal Novotny <minovotn@...>
# 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Michal Novotny]

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@...
# 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

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

# HG changeset patch
# User Michal Novotny <minovotn@...>
# 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Cole Robinson]

On 06/18/2009 12:22 PM, Michal Novotny wrote:

> # HG changeset patch
> # User Michal Novotny <minovotn@...>
> # 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@...
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Michal Novotny]

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

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

[Michal Novotny]

Hi,
I've modified my patch to match those requirements and hopefully they're already met now so please review this patch. In addition to saving those 5 directories (for image/media/save/restore and screenshot files) I found a bug that prevented showing existing PNG files in folders when saving screenshots and that has been fixed too (simple fix by changing "*.png" mask to "png" only because of asterisk and dot appended by util.browse_local() function itself).

Thanks,
Michal

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

    

...


# HG changeset patch
# User Michal Novotny <minovotn@...>
# Date 1245665443 -7200
# Node ID 51fc9462316334d9360908c5cb713ccfc74de81b
# 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 51fc94623163 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 Mon Jun 22 12:10:43 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 51fc94623163 src/virtManager/addhardware.py
--- a/src/virtManager/addhardware.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/addhardware.py Mon Jun 22 12:10:43 2009 +0200
@@ -678,7 +678,8 @@
 
     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(),
+                                                   self.config.CONFIG_DIR_IMAGE)
         filename = self._browse_file(_("Locate or Create New Storage File"),
                                      textent, folder=folder,
                                      confirm_overwrite=True)
diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/choosecd.py
--- a/src/virtManager/choosecd.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/choosecd.py Mon Jun 22 12:10:43 2009 +0200
@@ -20,6 +20,7 @@
 import gtk.glade
 import gobject
 import logging
+import os.path
 
 import virtinst
 
@@ -133,7 +134,9 @@
         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,
+                                                   self.config.CONFIG_DIR_MEDIA)
+        self._browse_file(_("Locate ISO Image"), folder=folder)
 
     def populate_opt_media(self):
         try:
@@ -146,14 +149,17 @@
 
     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, self.config.CONFIG_DIR_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 = vmmStorageBrowser(self.config, self.conn,
+                                                     True)
             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 51fc94623163 src/virtManager/config.py
--- a/src/virtManager/config.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/config.py Mon Jun 22 12:10:43 2009 +0200
@@ -45,6 +45,13 @@
 
 class vmmConfig:
 
+    # GConf directory names for saving last used paths
+    CONFIG_DIR_IMAGE = "image"
+    CONFIG_DIR_MEDIA = "media"
+    CONFIG_DIR_SAVE = "save"
+    CONFIG_DIR_RESTORE = "restore"
+    CONFIG_DIR_SCREENSHOT = "screenshot"
+
     CONSOLE_SCALE_NEVER = 0
     CONSOLE_SCALE_FULLSCREEN = 1
     CONSOLE_SCALE_ALWAYS = 2
@@ -261,6 +268,36 @@
         self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
 
 
+    def get_default_directory(self, conn, _type):
+        if len(_type) == 0:
+            logging.error("Unknown type for get_default_directory")
+            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 == vmmConfig.CONFIG_DIR_IMAGE or
+                _type == vmmConfig.CONFIG_DIR_MEDIA):
+                path = self.get_default_image_dir(conn)
+            if (_type == vmmConfig.CONFIG_DIR_SAVE or
+                _type == vmmConfig.CONFIG_DIR_RESTORE):
+                path = self.get_default_save_dir(conn)
+
+        logging.debug("get_default_directory(%s): returning %s" % (_type, path))
+        return path
+
+    def set_default_directory(self, folder, _type):
+        if len(_type) == 0:
+            logging.error("Unknown type for set_default_directory")
+            return
+
+        logging.debug("set_default_directory(%s): saving %s" % (_type, folder))
+        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 51fc94623163 src/virtManager/create.py
--- a/src/virtManager/create.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/create.py Mon Jun 22 12:10:43 2009 +0200
@@ -995,16 +995,22 @@
             nodetect_label.show()
 
     def browse_iso(self, ignore1=None, ignore2=None):
+        folder = self.config.get_default_directory(self.conn,
+                                                   self.config.CONFIG_DIR_MEDIA)
         self._browse_file(_("Locate ISO Image"),
-                          self.set_iso_storage_path)
+                          self.set_iso_storage_path,
+                          folder, True)
         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,
+                                                   self.config.CONFIG_DIR_IMAGE)
         self._browse_file(_("Locate existing storage"),
-                          self.set_disk_storage_path)
+                          self.set_disk_storage_path,
+                          folder, False)
 
     def toggle_storage_select(self, src):
         act = src.get_active()
@@ -1014,9 +1020,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, self.config.CONFIG_DIR_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, self.config.CONFIG_DIR_IMAGE)
         self.window.get_widget("config-storage-entry").set_text(path)
 
     # Navigation methods
@@ -1648,9 +1658,10 @@
             logging.exception("Error detecting distro.")
             self.detectedDistro = (None, None)
 
-    def _browse_file(self, dialog_name, callback, folder=None):
+    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
+            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
+                                                                     is_media)
             self.storage_browser.connect("storage-browse-finish",
                                          callback)
         self.storage_browser.local_args = { "dialog_name": dialog_name,
diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/details.py
--- a/src/virtManager/details.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/details.py Mon Jun 22 12:10:43 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
@@ -1408,13 +1409,22 @@
     def control_vm_screenshot(self, src):
         # If someone feels kind they could extend this code to allow
         # user to choose what image format they'd like to save in....
+        # Also _type was changed to "png" from "*.png" to show existing files
         path = util.browse_local(self.window.get_widget("vmm-details"),
                                  _("Save Virtual Machine Screenshot"),
-                                 _type = ("*.png", "PNG files"),
+                                 self.config.get_default_directory(
+                                             self.vm.get_connection(),
+                                             self.config.CONFIG_DIR_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,
+                                          self.config.CONFIG_DIR_SCREENSHOT)
+
         filename = path
         if not(filename.endswith(".png")):
             filename += ".png"
diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/engine.py
--- a/src/virtManager/engine.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/engine.py Mon Jun 22 12:10:43 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,17 @@
 
         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,
+                                                  self.config.CONFIG_DIR_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, self.config.CONFIG_DIR_SAVE)
+
         progWin = vmmAsyncJob(self.config, self._save_callback, [vm, path],
                               _("Saving Virtual Machine"))
         progWin.run()
diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/manager.py
--- a/src/virtManager/manager.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/manager.py Mon Jun 22 12:10:43 2009 +0200
@@ -23,6 +23,7 @@
 import gtk.glade
 import logging
 import traceback
+import os.path
 
 import sparkline
 import libvirt
@@ -397,11 +398,16 @@
 
         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,
+                                             self.config.CONFIG_DIR_RESTORE))
 
         if not path:
             return
 
+        # Save last restore path
+        folder = os.path.dirname(path)
+        self.config.set_default_directory(folder, self.config.CONFIG_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 a996e00b3bb6 -r 51fc94623163 src/virtManager/storagebrowse.py
--- a/src/virtManager/storagebrowse.py Thu Jun 18 10:54:21 2009 -0400
+++ b/src/virtManager/storagebrowse.py Mon Jun 22 12:10:43 2009 +0200
@@ -23,6 +23,7 @@
 
 import traceback
 import logging
+import os.path
 
 import virtinst
 
@@ -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")
@@ -240,6 +242,13 @@
         self._do_finish()
 
     def _do_finish(self, path=None):
+        if (not self.is_media and path is not None and
+            not path.startswith("/dev")):
+            # Save new default image directory
+            folder = os.path.dirname(path)
+            self.config.set_default_directory(folder,
+                                              self.config.CONFIG_DIR_IMAGE)
+
         if not path:
             path = self.current_vol().get_path()
         self.emit("storage-browse-finish", path)

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

[Cole Robinson]

Michal Novotny wrote:

<snip>

>>>
>>> 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?

Sure, here's an untested patch:

> diff -r 2e0e047d21f0 src/virtManager/util.py
> --- a/src/virtManager/util.py Mon Jun 22 14:24:09 2009 -0400
> +++ b/src/virtManager/util.py Mon Jun 22 14:43:11 2009 -0400
> @@ -90,25 +90,33 @@
>      return ret
>  
>  
> -def browse_local(parent, dialog_name, start_folder=None, _type=None,
> -                 dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN, confirm_func=None):
> +def browse_local(parent, dialog_name, config, conn, start_folder=None,
> +                 _type=None, dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN,
> +                 confirm_func=None, browse_reason=None):
>      """
>      Helper function for launching a filechooser
>  
>      @param parent: Parent window for the filechooser
> +    @param config: vmmConfig used by the calling class
> +    @param conn: vmmConnection used by the calling class
>      @param dialog_name: String to use in the title bar of the filechooser.
>      @param start_folder: Folder the filechooser is viewing at startup
>      @param _type: File extension to filter by (e.g. "iso", "png")
>      @param dialog_type: Maps to FileChooserDialog 'action'
>      @param confirm_func: Optional callback function if file is chosen.
> +    @param browse_reason: The vmmConfig.CONFIG_DIR* reason we are browsing.
> +        If set, this will override the 'folder' parameter with the gconf
> +        value, and store the user chosen path.
>      """
> -
>      overwrite_confirm = False
>      choose_button = gtk.STOCK_OPEN
>      if dialog_type == gtk.FILE_CHOOSER_ACTION_SAVE:
>          choose_button = gtk.STOCK_SAVE
>          overwrite_confirm = True
>  
> +    if browse_reason:
> +        start_folder = config.get_default_directory(conn, browse_reason)
> +
>      fcdialog = gtk.FileChooserDialog(dialog_name, parent,
>                                       dialog_type,
>                                       (gtk.STOCK_CANCEL, gtk.RESPONSE_CANCEL,
> @@ -142,10 +150,15 @@
>      if(response == gtk.RESPONSE_ACCEPT):
>          filename = fcdialog.get_filename()
>          fcdialog.destroy()
> -        return filename
> +        ret = filename
>      else:
>          fcdialog.destroy()
> -        return None
> +        ret = None
> +
> +    if ret and browse_reason:
> +        config.set_default_directory(os.path.dirname(ret), browse_reason)
> +
> +    return ret
>  
>  def dup_conn(config, conn, libconn=None, return_conn_class=False):
>  

This way, any caller of 'util.browse_local' doesn't need to store the
dirname in gconf: browse_local will do it for us. It sucks that we have
to pass conn and config to the helper, but that info is typically easy
to access from the callers, so shouldn't be a big change.

Thanks,
Cole

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

[Cole Robinson]

Michal Novotny wrote:
> Hi,
> I've modified my patch to match those requirements and hopefully they're
> already met now so please review this patch. In addition to saving those
> 5 directories (for image/media/save/restore and screenshot files) I
> found a bug that prevented showing existing PNG files in folders when
> saving screenshots and that has been fixed too (simple fix by changing
> "*.png" mask to "png" only because of asterisk and dot appended by
> util.browse_local() function itself).
>

Besides the change to browse_local() I mentioned in the other mail, just
a few nit picks inline.

> # HG changeset patch
> # User Michal Novotny <minovotn@...>
> # Date 1245665443 -7200
> # Node ID 51fc9462316334d9360908c5cb713ccfc74de81b
> # 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 51fc94623163 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 Mon Jun 22 12:10:43 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 51fc94623163 src/virtManager/addhardware.py
> --- a/src/virtManager/addhardware.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/addhardware.py Mon Jun 22 12:10:43 2009 +0200
> @@ -678,7 +678,8 @@
>  
>      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(),
> +                                                   self.config.CONFIG_DIR_IMAGE)
>          filename = self._browse_file(_("Locate or Create New Storage File"),
>                                       textent, folder=folder,
>                                       confirm_overwrite=True)
> diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/choosecd.py
> --- a/src/virtManager/choosecd.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/choosecd.py Mon Jun 22 12:10:43 2009 +0200
> @@ -20,6 +20,7 @@
>  import gtk.glade
>  import gobject
>  import logging
> +import os.path
>  
>  import virtinst
>  
> @@ -133,7 +134,9 @@
>          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,
> +                                                   self.config.CONFIG_DIR_MEDIA)
> +        self._browse_file(_("Locate ISO Image"), folder=folder)
>  
>      def populate_opt_media(self):
>          try:
> @@ -146,14 +149,17 @@
>  
>      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, self.config.CONFIG_DIR_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 = vmmStorageBrowser(self.config, self.conn,
> +                                                     True)
>              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 51fc94623163 src/virtManager/config.py
> --- a/src/virtManager/config.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/config.py Mon Jun 22 12:10:43 2009 +0200
> @@ -45,6 +45,13 @@
>  
>  class vmmConfig:
>  
> +    # GConf directory names for saving last used paths
> +    CONFIG_DIR_IMAGE = "image"
> +    CONFIG_DIR_MEDIA = "media"
> +    CONFIG_DIR_SAVE = "save"
> +    CONFIG_DIR_RESTORE = "restore"
> +    CONFIG_DIR_SCREENSHOT = "screenshot"
> +
>      CONSOLE_SCALE_NEVER = 0
>      CONSOLE_SCALE_FULLSCREEN = 1
>      CONSOLE_SCALE_ALWAYS = 2
> @@ -261,6 +268,36 @@
>          self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
>  
>  
> +    def get_default_directory(self, conn, _type):
> +        if len(_type) == 0:

If somehow a non string/list value was passed in here (say, None), this
would backtrace. You can do something like

    config_dirs = [CONFIG_DIR_IMAGE, CONFIG_DIR_MEDIA, ...]
    ...
    if _type not in self.config_dirs:
        logging.error(blah)

> +            logging.error("Unknown type for get_default_directory")
> +            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:

The check for len(path) == 0 is redundant here: 'not path' will match
the zero length string.

> +            if (_type == vmmConfig.CONFIG_DIR_IMAGE or
> +                _type == vmmConfig.CONFIG_DIR_MEDIA):
> +                path = self.get_default_image_dir(conn)
> +            if (_type == vmmConfig.CONFIG_DIR_SAVE or
> +                _type == vmmConfig.CONFIG_DIR_RESTORE):
> +                path = self.get_default_save_dir(conn)
> +
> +        logging.debug("get_default_directory(%s): returning %s" % (_type, path))
> +        return path
> +
> +    def set_default_directory(self, folder, _type):
> +        if len(_type) == 0:
> +            logging.error("Unknown type for set_default_directory")
> +            return
> +
> +        logging.debug("set_default_directory(%s): saving %s" % (_type, folder))
> +        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 51fc94623163 src/virtManager/create.py
> --- a/src/virtManager/create.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/create.py Mon Jun 22 12:10:43 2009 +0200
> @@ -995,16 +995,22 @@
>              nodetect_label.show()
>  
>      def browse_iso(self, ignore1=None, ignore2=None):
> +        folder = self.config.get_default_directory(self.conn,
> +                                                   self.config.CONFIG_DIR_MEDIA)
>          self._browse_file(_("Locate ISO Image"),
> -                          self.set_iso_storage_path)
> +                          self.set_iso_storage_path,
> +                          folder, True)
>          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,
> +                                                   self.config.CONFIG_DIR_IMAGE)
>          self._browse_file(_("Locate existing storage"),
> -                          self.set_disk_storage_path)
> +                          self.set_disk_storage_path,
> +                          folder, False)
>  
>      def toggle_storage_select(self, src):
>          act = src.get_active()
> @@ -1014,9 +1020,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, self.config.CONFIG_DIR_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, self.config.CONFIG_DIR_IMAGE)
>          self.window.get_widget("config-storage-entry").set_text(path)
>  
>      # Navigation methods
> @@ -1648,9 +1658,10 @@
>              logging.exception("Error detecting distro.")
>              self.detectedDistro = (None, None)
>  
> -    def _browse_file(self, dialog_name, callback, folder=None):
> +    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
>          if self.storage_browser == None:
> -            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
> +            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
> +                                                                     is_media)
>              self.storage_browser.connect("storage-browse-finish",
>                                           callback)
>          self.storage_browser.local_args = { "dialog_name": dialog_name,
> diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/details.py
> --- a/src/virtManager/details.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/details.py Mon Jun 22 12:10:43 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
> @@ -1408,13 +1409,22 @@
>      def control_vm_screenshot(self, src):
>          # If someone feels kind they could extend this code to allow
>          # user to choose what image format they'd like to save in....
> +        # Also _type was changed to "png" from "*.png" to show existing files

The change is correct, but no need to comment it in the code.

>          path = util.browse_local(self.window.get_widget("vmm-details"),
>                                   _("Save Virtual Machine Screenshot"),
> -                                 _type = ("*.png", "PNG files"),
> +                                 self.config.get_default_directory(
> +                                             self.vm.get_connection(),
> +                                             self.config.CONFIG_DIR_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,
> +                                          self.config.CONFIG_DIR_SCREENSHOT)
> +
>          filename = path
>          if not(filename.endswith(".png")):
>              filename += ".png"
> diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/engine.py
> --- a/src/virtManager/engine.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/engine.py Mon Jun 22 12:10:43 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,17 @@
>  
>          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,
> +                                                  self.config.CONFIG_DIR_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, self.config.CONFIG_DIR_SAVE)
> +
>          progWin = vmmAsyncJob(self.config, self._save_callback, [vm, path],
>                                _("Saving Virtual Machine"))
>          progWin.run()
> diff -r a996e00b3bb6 -r 51fc94623163 src/virtManager/manager.py
> --- a/src/virtManager/manager.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/manager.py Mon Jun 22 12:10:43 2009 +0200
> @@ -23,6 +23,7 @@
>  import gtk.glade
>  import logging
>  import traceback
> +import os.path
>  
>  import sparkline
>  import libvirt
> @@ -397,11 +398,16 @@
>  
>          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,
> +                                             self.config.CONFIG_DIR_RESTORE))
>  
>          if not path:
>              return
>  
> +        # Save last restore path
> +        folder = os.path.dirname(path)
> +        self.config.set_default_directory(folder, self.config.CONFIG_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 a996e00b3bb6 -r 51fc94623163 src/virtManager/storagebrowse.py
> --- a/src/virtManager/storagebrowse.py Thu Jun 18 10:54:21 2009 -0400
> +++ b/src/virtManager/storagebrowse.py Mon Jun 22 12:10:43 2009 +0200
> @@ -23,6 +23,7 @@
>  
>  import traceback
>  import logging
> +import os.path
>  
>  import virtinst
>  
> @@ -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")
> @@ -240,6 +242,13 @@
>          self._do_finish()
>  
>      def _do_finish(self, path=None):
> +        if (not self.is_media and path is not None and
> +            not path.startswith("/dev")):
> +            # Save new default image directory
> +            folder = os.path.dirname(path)
> +            self.config.set_default_directory(folder,
> +                                              self.config.CONFIG_DIR_IMAGE)
> +
>          if not path:
>              path = self.current_vol().get_path()
>          self.emit("storage-browse-finish", path)

If we move all the gconf getting/setting into browse_local, the above
changes to the storage browser aren't necessary: the storage browser
will just need to be sure to specify 'conn', 'config', and
'browse_reason' when launching the local browser.

Thanks,
Cole

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

[Michal Novotny]

Hi,
this is new version of my patch so please check.

Thanks,
Michal

On 06/22/2009 12:15 PM, Michal Novotny wrote:

Hi,
I've modified my patch to match those requirements and hopefully they're already met now so please review this patch. In addition to saving those 5 directories (for image/media/save/restore and screenshot files) I found a bug that prevented showing existing PNG files in folders when saving screenshots and that has been fixed too (simple fix by changing "*.png" mask to "png" only because of asterisk and dot appended by util.browse_local() function itself).

Thanks,
Michal

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

    

...

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

# HG changeset patch
# User Michal Novotny <minovotn@...>
# Date 1245737179 -7200
# Node ID 7800fad8b767b020cbddaa34da11bed6c25cb446
# Parent  2e0e047d21f0c5d936b157cbb20e221ca8d114de
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 2e0e047d21f0 -r 7800fad8b767 src/virt-manager.schemas.in
--- a/src/virt-manager.schemas.in Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virt-manager.schemas.in Tue Jun 23 08:06:19 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 2e0e047d21f0 -r 7800fad8b767 src/virtManager/addhardware.py
--- a/src/virtManager/addhardware.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/addhardware.py Tue Jun 23 08:06:19 2009 +0200
@@ -701,14 +701,15 @@
 
         conn = self.vm.get_connection()
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, conn)
+            self.storage_browser = vmmStorageBrowser(self.config, conn, False)
         if self._browse_cb_id:
             self.storage_browser.disconnect(self._browse_cb_id)
 
         self._browse_cb_id = self.storage_browser.connect("storage-browse-finish", set_storage_cb)
         self.storage_browser.local_args = { "dialog_name": dialog_name,
-                                            "start_folder": folder,
-                                            "confirm_func": confirm_func, }
+                                            "confirm_func": confirm_func,
+                                            "browse_reason":
+                                                  self.config.CONFIG_DIR_IMAGE }
         self.storage_browser.show(conn)
         return None
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/choosecd.py
--- a/src/virtManager/choosecd.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/choosecd.py Tue Jun 23 08:06:19 2009 +0200
@@ -149,11 +149,13 @@
 
     def _browse_file(self, dialog_name):
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
-                                                     #self.topwin)
+            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
+                                                     True)
             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,
+                                            "browse_reason":
+                                                 self.config.CONFIG_DIR_MEDIA }
         self.storage_browser.show(self.conn)
         return None
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/config.py
--- a/src/virtManager/config.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/config.py Tue Jun 23 08:06:19 2009 +0200
@@ -45,6 +45,13 @@
 
 class vmmConfig:
 
+    # GConf directory names for saving last used paths
+    CONFIG_DIR_IMAGE = "image"
+    CONFIG_DIR_MEDIA = "media"
+    CONFIG_DIR_SAVE = "save"
+    CONFIG_DIR_RESTORE = "restore"
+    CONFIG_DIR_SCREENSHOT = "screenshot"
+
     CONSOLE_SCALE_NEVER = 0
     CONSOLE_SCALE_FULLSCREEN = 1
     CONSOLE_SCALE_ALWAYS = 2
@@ -261,6 +268,36 @@
         self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
 
 
+    def get_default_directory(self, conn, _type):
+        if not _type:
+            logging.error("Unknown type for get_default_directory")
+            return
+
+        try:
+            path = self.conf.get_value(self.conf_dir + "/paths/default-%s-path"
+                                                                       % _type)
+        except:
+            path = None
+
+        if not path:
+            if (_type == self.CONFIG_DIR_IMAGE or
+                _type == self.CONFIG_DIR_MEDIA):
+                path = self.get_default_image_dir(conn)
+            if (_type == self.CONFIG_DIR_SAVE or
+                _type == self.CONFIG_DIR_RESTORE):
+                path = self.get_default_save_dir(conn)
+
+        logging.debug("get_default_directory(%s): returning %s" % (_type, path))
+        return path
+
+    def set_default_directory(self, folder, _type):
+        if not _type:
+            logging.error("Unknown type for set_default_directory")
+            return
+
+        logging.debug("set_default_directory(%s): saving %s" % (_type, folder))
+        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 2e0e047d21f0 -r 7800fad8b767 src/virtManager/create.py
--- a/src/virtManager/create.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/create.py Tue Jun 23 08:06:19 2009 +0200
@@ -996,7 +996,8 @@
 
     def browse_iso(self, ignore1=None, ignore2=None):
         self._browse_file(_("Locate ISO Image"),
-                          self.set_iso_storage_path)
+                          self.set_iso_storage_path,
+                          is_media=True)
         self.window.get_widget("install-local-box").activate()
 
     def toggle_enable_storage(self, src):
@@ -1004,7 +1005,8 @@
 
     def browse_storage(self, ignore1):
         self._browse_file(_("Locate existing storage"),
-                          self.set_disk_storage_path)
+                          self.set_disk_storage_path,
+                          is_media=False)
 
     def toggle_storage_select(self, src):
         act = src.get_active()
@@ -1648,13 +1650,20 @@
             logging.exception("Error detecting distro.")
             self.detectedDistro = (None, None)
 
-    def _browse_file(self, dialog_name, callback, folder=None):
+    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
+            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
+                                                     is_media)
             self.storage_browser.connect("storage-browse-finish",
                                          callback)
+        if is_media:
+            reason = self.config.CONFIG_DIR_MEDIA
+        else:
+            reason = self.config.CONFIG_DIR_IMAGE
+
         self.storage_browser.local_args = { "dialog_name": dialog_name,
-                                            "start_folder": folder}
+                                            "start_folder": folder,
+                                            "browse_reason": reason}
         self.storage_browser.show(self.conn)
 
     def show_help(self, ignore):
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/details.py
--- a/src/virtManager/details.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/details.py Tue Jun 23 08:06:19 2009 +0200
@@ -1410,8 +1410,10 @@
         # 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"),
-                                 _type = ("*.png", "PNG files"),
-                                 dialog_type = gtk.FILE_CHOOSER_ACTION_SAVE)
+                                 self.config, self.vm.get_connection(),
+                                 _type = ("png", "PNG files"),
+                                 dialog_type = gtk.FILE_CHOOSER_ACTION_SAVE,
+                                 browse_reason=self.config.CONFIG_DIR_SCREENSHOT)
         if not path:
             return
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/engine.py
--- a/src/virtManager/engine.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/engine.py Tue Jun 23 08:06:19 2009 +0200
@@ -406,8 +406,9 @@
 
         path = util.browse_local(src.window.get_widget("vmm-details"),
                                  _("Save Virtual Machine"),
-                                 self.config.get_default_save_dir(con),
-                                 dialog_type=gtk.FILE_CHOOSER_ACTION_SAVE)
+                                 self.config, self.get_connection(uri),
+                                 dialog_type=gtk.FILE_CHOOSER_ACTION_SAVE,
+                                 browse_reason=self.config.CONFIG_DIR_SAVE)
 
         if not path:
             return
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/manager.py
--- a/src/virtManager/manager.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/manager.py Tue Jun 23 08:06:19 2009 +0200
@@ -397,7 +397,8 @@
 
         path = util.browse_local(self.window.get_widget("vmm-manager"),
                                  _("Restore Virtual Machine"),
-                                 self.config.get_default_save_dir(conn))
+                                 self.config, conn,
+                                 browse_reason=self.config.CONFIG_DIR_RESTORE)
 
         if not path:
             return
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/storagebrowse.py
--- a/src/virtManager/storagebrowse.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/storagebrowse.py Tue Jun 23 08:06:19 2009 +0200
@@ -38,7 +38,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",
@@ -58,8 +58,14 @@
         # Add Volume wizard
         self.addvol = None
 
+        if is_media:
+            reason = self.config.CONFIG_DIR_MEDIA
+        else:
+            reason = self.config.CONFIG_DIR_IMAGE
+
         # Arguments to pass to util.browse_local for local storage
-        self.local_args = {"dialog_name": _("Choose local storage")}
+        self.local_args = {"dialog_name": _("Choose local storage"),
+                           "browse_reason": reason, }
 
         self.window.signal_autoconnect({
             "on_vmm_storage_browse_delete_event" : self.close,
@@ -232,7 +238,8 @@
                           "".join(traceback.format_exc()))
 
     def browse_local(self, src):
-        filename = util.browse_local(parent=self.topwin, **self.local_args)
+        filename = util.browse_local(parent=self.topwin, config=self.config,
+                                     conn=self.conn, **self.local_args)
         if filename:
             self._do_finish(path=filename)
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/util.py
--- a/src/virtManager/util.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/util.py Tue Jun 23 08:06:19 2009 +0200
@@ -21,6 +21,7 @@
 import logging
 import gtk
 import libxml2
+import os.path
 
 import libvirt
 
@@ -90,17 +91,24 @@
     return ret
 
 
-def browse_local(parent, dialog_name, start_folder=None, _type=None,
-                 dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN, confirm_func=None):
+def browse_local(parent, dialog_name, config, conn, start_folder=None,
+                 _type=None, dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN,
+                 confirm_func=None, browse_reason=None):
     """
     Helper function for launching a filechooser
 
     @param parent: Parent window for the filechooser
     @param dialog_name: String to use in the title bar of the filechooser.
+    @param config: vmmConfig used by calling class
+    @param conn: vmmConnection used by calling class
     @param start_folder: Folder the filechooser is viewing at startup
     @param _type: File extension to filter by (e.g. "iso", "png")
     @param dialog_type: Maps to FileChooserDialog 'action'
     @param confirm_func: Optional callback function if file is chosen.
+    @param browse_reason: The vmmConfig.CONFIG_DIR* reason we are browsing.
+        If set, this will override the 'folder' parameter with the gconf
+        value, and store the user chosen path.
+
     """
 
     overwrite_confirm = False
@@ -109,6 +117,9 @@
         choose_button = gtk.STOCK_SAVE
         overwrite_confirm = True
 
+    if browse_reason:
+        start_folder = config.get_default_directory(conn, browse_reason)
+
     fcdialog = gtk.FileChooserDialog(dialog_name, parent,
                                      dialog_type,
                                      (gtk.STOCK_CANCEL, gtk.RESPONSE_CANCEL,
@@ -142,10 +153,15 @@
     if(response == gtk.RESPONSE_ACCEPT):
         filename = fcdialog.get_filename()
         fcdialog.destroy()
-        return filename
+        ret = filename
     else:
         fcdialog.destroy()
-        return None
+        ret = None
+
+    if ret and browse_reason and not path.startwith("/dev"):
+        config.set_default_directory(os.path.dirname(ret), browse_reason)
+
+    return ret
 
 def dup_conn(config, conn, libconn=None, return_conn_class=False):
 

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

[Michal Novotny]

Sorry, I attached wrong version, this is the right one...

Michal

On 06/23/2009 08:07 AM, Michal Novotny wrote:

Hi,
this is new version of my patch so please check.

Thanks,
Michal

...
# HG changeset patch
# User Michal Novotny <minovotn@...>
# Date 1245737179 -7200
# Node ID 7800fad8b767b020cbddaa34da11bed6c25cb446
# Parent  2e0e047d21f0c5d936b157cbb20e221ca8d114de
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 2e0e047d21f0 -r 7800fad8b767 src/virt-manager.schemas.in
--- a/src/virt-manager.schemas.in Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virt-manager.schemas.in Tue Jun 23 08:06:19 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 2e0e047d21f0 -r 7800fad8b767 src/virtManager/addhardware.py
--- a/src/virtManager/addhardware.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/addhardware.py Tue Jun 23 08:06:19 2009 +0200
@@ -701,14 +701,15 @@
 
         conn = self.vm.get_connection()
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, conn)
+            self.storage_browser = vmmStorageBrowser(self.config, conn, False)
         if self._browse_cb_id:
             self.storage_browser.disconnect(self._browse_cb_id)
 
         self._browse_cb_id = self.storage_browser.connect("storage-browse-finish", set_storage_cb)
         self.storage_browser.local_args = { "dialog_name": dialog_name,
-                                            "start_folder": folder,
-                                            "confirm_func": confirm_func, }
+                                            "confirm_func": confirm_func,
+                                            "browse_reason":
+                                                  self.config.CONFIG_DIR_IMAGE }
         self.storage_browser.show(conn)
         return None
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/choosecd.py
--- a/src/virtManager/choosecd.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/choosecd.py Tue Jun 23 08:06:19 2009 +0200
@@ -149,11 +149,13 @@
 
     def _browse_file(self, dialog_name):
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
-                                                     #self.topwin)
+            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
+                                                     True)
             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,
+                                            "browse_reason":
+                                                 self.config.CONFIG_DIR_MEDIA }
         self.storage_browser.show(self.conn)
         return None
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/config.py
--- a/src/virtManager/config.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/config.py Tue Jun 23 08:06:19 2009 +0200
@@ -45,6 +45,13 @@
 
 class vmmConfig:
 
+    # GConf directory names for saving last used paths
+    CONFIG_DIR_IMAGE = "image"
+    CONFIG_DIR_MEDIA = "media"
+    CONFIG_DIR_SAVE = "save"
+    CONFIG_DIR_RESTORE = "restore"
+    CONFIG_DIR_SCREENSHOT = "screenshot"
+
     CONSOLE_SCALE_NEVER = 0
     CONSOLE_SCALE_FULLSCREEN = 1
     CONSOLE_SCALE_ALWAYS = 2
@@ -261,6 +268,36 @@
         self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
 
 
+    def get_default_directory(self, conn, _type):
+        if not _type:
+            logging.error("Unknown type for get_default_directory")
+            return
+
+        try:
+            path = self.conf.get_value(self.conf_dir + "/paths/default-%s-path"
+                                                                       % _type)
+        except:
+            path = None
+
+        if not path:
+            if (_type == self.CONFIG_DIR_IMAGE or
+                _type == self.CONFIG_DIR_MEDIA):
+                path = self.get_default_image_dir(conn)
+            if (_type == self.CONFIG_DIR_SAVE or
+                _type == self.CONFIG_DIR_RESTORE):
+                path = self.get_default_save_dir(conn)
+
+        logging.debug("get_default_directory(%s): returning %s" % (_type, path))
+        return path
+
+    def set_default_directory(self, folder, _type):
+        if not _type:
+            logging.error("Unknown type for set_default_directory")
+            return
+
+        logging.debug("set_default_directory(%s): saving %s" % (_type, folder))
+        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 2e0e047d21f0 -r 7800fad8b767 src/virtManager/create.py
--- a/src/virtManager/create.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/create.py Tue Jun 23 08:06:19 2009 +0200
@@ -996,7 +996,8 @@
 
     def browse_iso(self, ignore1=None, ignore2=None):
         self._browse_file(_("Locate ISO Image"),
-                          self.set_iso_storage_path)
+                          self.set_iso_storage_path,
+                          is_media=True)
         self.window.get_widget("install-local-box").activate()
 
     def toggle_enable_storage(self, src):
@@ -1004,7 +1005,8 @@
 
     def browse_storage(self, ignore1):
         self._browse_file(_("Locate existing storage"),
-                          self.set_disk_storage_path)
+                          self.set_disk_storage_path,
+                          is_media=False)
 
     def toggle_storage_select(self, src):
         act = src.get_active()
@@ -1648,13 +1650,20 @@
             logging.exception("Error detecting distro.")
             self.detectedDistro = (None, None)
 
-    def _browse_file(self, dialog_name, callback, folder=None):
+    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, self.conn)
+            self.storage_browser = vmmStorageBrowser(self.config, self.conn,
+                                                     is_media)
             self.storage_browser.connect("storage-browse-finish",
                                          callback)
+        if is_media:
+            reason = self.config.CONFIG_DIR_MEDIA
+        else:
+            reason = self.config.CONFIG_DIR_IMAGE
+
         self.storage_browser.local_args = { "dialog_name": dialog_name,
-                                            "start_folder": folder}
+                                            "start_folder": folder,
+                                            "browse_reason": reason}
         self.storage_browser.show(self.conn)
 
     def show_help(self, ignore):
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/details.py
--- a/src/virtManager/details.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/details.py Tue Jun 23 08:06:19 2009 +0200
@@ -1410,8 +1410,10 @@
         # 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"),
-                                 _type = ("*.png", "PNG files"),
-                                 dialog_type = gtk.FILE_CHOOSER_ACTION_SAVE)
+                                 self.config, self.vm.get_connection(),
+                                 _type = ("png", "PNG files"),
+                                 dialog_type = gtk.FILE_CHOOSER_ACTION_SAVE,
+                                 browse_reason=self.config.CONFIG_DIR_SCREENSHOT)
         if not path:
             return
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/engine.py
--- a/src/virtManager/engine.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/engine.py Tue Jun 23 08:06:19 2009 +0200
@@ -406,8 +406,9 @@
 
         path = util.browse_local(src.window.get_widget("vmm-details"),
                                  _("Save Virtual Machine"),
-                                 self.config.get_default_save_dir(con),
-                                 dialog_type=gtk.FILE_CHOOSER_ACTION_SAVE)
+                                 self.config, self.get_connection(uri),
+                                 dialog_type=gtk.FILE_CHOOSER_ACTION_SAVE,
+                                 browse_reason=self.config.CONFIG_DIR_SAVE)
 
         if not path:
             return
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/manager.py
--- a/src/virtManager/manager.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/manager.py Tue Jun 23 08:06:19 2009 +0200
@@ -397,7 +397,8 @@
 
         path = util.browse_local(self.window.get_widget("vmm-manager"),
                                  _("Restore Virtual Machine"),
-                                 self.config.get_default_save_dir(conn))
+                                 self.config, conn,
+                                 browse_reason=self.config.CONFIG_DIR_RESTORE)
 
         if not path:
             return
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/storagebrowse.py
--- a/src/virtManager/storagebrowse.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/storagebrowse.py Tue Jun 23 08:06:19 2009 +0200
@@ -38,7 +38,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",
@@ -58,8 +58,14 @@
         # Add Volume wizard
         self.addvol = None
 
+        if is_media:
+            reason = self.config.CONFIG_DIR_MEDIA
+        else:
+            reason = self.config.CONFIG_DIR_IMAGE
+
         # Arguments to pass to util.browse_local for local storage
-        self.local_args = {"dialog_name": _("Choose local storage")}
+        self.local_args = {"dialog_name": _("Choose local storage"),
+                           "browse_reason": reason, }
 
         self.window.signal_autoconnect({
             "on_vmm_storage_browse_delete_event" : self.close,
@@ -232,7 +238,8 @@
                           "".join(traceback.format_exc()))
 
     def browse_local(self, src):
-        filename = util.browse_local(parent=self.topwin, **self.local_args)
+        filename = util.browse_local(parent=self.topwin, config=self.config,
+                                     conn=self.conn, **self.local_args)
         if filename:
             self._do_finish(path=filename)
 
diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/util.py
--- a/src/virtManager/util.py Mon Jun 22 14:24:09 2009 -0400
+++ b/src/virtManager/util.py Tue Jun 23 08:06:19 2009 +0200
@@ -21,6 +21,7 @@
 import logging
 import gtk
 import libxml2
+import os.path
 
 import libvirt
 
@@ -90,17 +91,24 @@
     return ret
 
 
-def browse_local(parent, dialog_name, start_folder=None, _type=None,
-                 dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN, confirm_func=None):
+def browse_local(parent, dialog_name, config, conn, start_folder=None,
+                 _type=None, dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN,
+                 confirm_func=None, browse_reason=None):
     """
     Helper function for launching a filechooser
 
     @param parent: Parent window for the filechooser
     @param dialog_name: String to use in the title bar of the filechooser.
+    @param config: vmmConfig used by calling class
+    @param conn: vmmConnection used by calling class
     @param start_folder: Folder the filechooser is viewing at startup
     @param _type: File extension to filter by (e.g. "iso", "png")
     @param dialog_type: Maps to FileChooserDialog 'action'
     @param confirm_func: Optional callback function if file is chosen.
+    @param browse_reason: The vmmConfig.CONFIG_DIR* reason we are browsing.
+        If set, this will override the 'folder' parameter with the gconf
+        value, and store the user chosen path.
+
     """
 
     overwrite_confirm = False
@@ -109,6 +117,9 @@
         choose_button = gtk.STOCK_SAVE
         overwrite_confirm = True
 
+    if browse_reason:
+        start_folder = config.get_default_directory(conn, browse_reason)
+
     fcdialog = gtk.FileChooserDialog(dialog_name, parent,
                                      dialog_type,
                                      (gtk.STOCK_CANCEL, gtk.RESPONSE_CANCEL,
@@ -142,10 +153,15 @@
     if(response == gtk.RESPONSE_ACCEPT):
         filename = fcdialog.get_filename()
         fcdialog.destroy()
-        return filename
+        ret = filename
     else:
         fcdialog.destroy()
-        return None
+        ret = None
+
+    if ret and browse_reason and not ret.startwith("/dev"):
+        config.set_default_directory(os.path.dirname(ret), browse_reason)
+
+    return ret
 
 def dup_conn(config, conn, libconn=None, return_conn_class=False):
 

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

[Cole Robinson]

Michal Novotny wrote:

> Sorry, I attached wrong version, this is the right one...
>
> Michal
>
> On 06/23/2009 08:07 AM, Michal Novotny wrote:
>> Hi,
>> this is new version of my patch so please check.
>>
>> Thanks,
>> Michal

Thanks! I've applied this now:

http://hg.et.redhat.com/cgi-bin/hg-virt.cgi/applications/virt-manager--devel/rev/aa4f30fce78b

There is still one issue I would like to have cleaned up, but it can be
addressed by an iterative patch, rather than the repeated back and
forth. See comments below.

<snip>

> diff -r 2e0e047d21f0 -r 7800fad8b767 src/virtManager/storagebrowse.py
> --- a/src/virtManager/storagebrowse.py Mon Jun 22 14:24:09 2009 -0400
> +++ b/src/virtManager/storagebrowse.py Tue Jun 23 08:06:19 2009 +0200
> @@ -38,7 +38,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",
> @@ -58,8 +58,14 @@
>          # Add Volume wizard
>          self.addvol = None
>  
> +        if is_media:
> +            reason = self.config.CONFIG_DIR_MEDIA
> +        else:
> +            reason = self.config.CONFIG_DIR_IMAGE
> +
>          # Arguments to pass to util.browse_local for local storage
> -        self.local_args = {"dialog_name": _("Choose local storage")}
> +        self.local_args = {"dialog_name": _("Choose local storage"),
> +                           "browse_reason": reason, }
>  
>          self.window.signal_autoconnect({
>              "on_vmm_storage_browse_delete_event" : self.close,

The 'is_media' piece here is no longer necessary. Just have the
StorageBrowser caller always specify "browse_reason" in the browse_local
args dictionary. It should help simplify all the calling code.

Thanks,
Cole

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

[Michal Novotny]

Hi,
I found a typo in my patch which prevented remembering last used directories being
remembered correctly. Also, 'is_media' variable have been changed to 'browse_reason'
which is the argument passed directly to util.browse_local() function so please review.

Thanks,
Michal

On 06/23/2009 08:09 AM, Michal Novotny wrote:

Sorry, I attached wrong version, this is the right one...

Michal

On 06/23/2009 08:07 AM, Michal Novotny wrote:

Hi,
this is new version of my patch so please check.

Thanks,
Michal

...

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

[remember-last-directories-fix.patch]

---

# HG changeset patch
# User Michal Novotny <minovotn@...>
# Date 1245935008 -7200
# Node ID 39b81ca60edc70066cc44cf760865c2d8f03ac8f
# Parent  aa4f30fce78b6805c94c70729139bb326b023bcb
Fix typo in remember-paths and change 'is_media' to 'browse_reason'

This is the patch that fixes a bug introduced by my previous patch for
remembering last used paths in rev. aa4f30fce78b. Also, 'is_media'
variable is no longer used there and is replaced by 'browse_reason'
wherever applicable.

diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/addhardware.py
--- a/src/virtManager/addhardware.py Tue Jun 23 19:30:12 2009 -0400
+++ b/src/virtManager/addhardware.py Thu Jun 25 15:03:28 2009 +0200
@@ -678,9 +678,8 @@
 
     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())
         filename = self._browse_file(_("Locate or Create New Storage File"),
-                                     textent, folder=folder,
+                                     textent, folder=None,
                                      confirm_overwrite=True)
         if filename != None:
             textent.set_text(filename)
@@ -699,17 +698,23 @@
             if path:
                 textent.set_text(path)
 
+        if folder == None:
+            reason = self.config.CONFIG_DIR_IMAGE
+        else:
+            reason = None
+
         conn = self.vm.get_connection()
         if self.storage_browser == None:
-            self.storage_browser = vmmStorageBrowser(self.config, conn, False)
+            self.storage_browser = vmmStorageBrowser(self.config, conn,
+                                                     reason)
         if self._browse_cb_id:
             self.storage_browser.disconnect(self._browse_cb_id)
 
         self._browse_cb_id = self.storage_browser.connect("storage-browse-finish", set_storage_cb)
         self.storage_browser.local_args = { "dialog_name": dialog_name,
                                             "confirm_func": confirm_func,
-                                            "browse_reason":
-                                                  self.config.CONFIG_DIR_IMAGE }
+                                            "browse_reason": reason,
+                                            "start_folder": folder }
         self.storage_browser.show(conn)
         return None
 
diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/choosecd.py
--- a/src/virtManager/choosecd.py Tue Jun 23 19:30:12 2009 -0400
+++ b/src/virtManager/choosecd.py Thu Jun 25 15:03:28 2009 +0200
@@ -150,7 +150,7 @@
     def _browse_file(self, dialog_name):
         if self.storage_browser == None:
             self.storage_browser = vmmStorageBrowser(self.config, self.conn,
-                                                     True)
+                                                   self.config.CONFIG_DIR_MEDIA)
             self.storage_browser.connect("storage-browse-finish",
                                          self.set_storage_path)
         self.storage_browser.local_args = { "dialog_name": dialog_name,
diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/create.py
--- a/src/virtManager/create.py Tue Jun 23 19:30:12 2009 -0400
+++ b/src/virtManager/create.py Thu Jun 25 15:03:28 2009 +0200
@@ -997,7 +997,7 @@
     def browse_iso(self, ignore1=None, ignore2=None):
         self._browse_file(_("Locate ISO Image"),
                           self.set_iso_storage_path,
-                          is_media=True)
+                          browse_reason=self.config.CONFIG_DIR_MEDIA)
         self.window.get_widget("install-local-box").activate()
 
     def toggle_enable_storage(self, src):
@@ -1006,7 +1006,7 @@
     def browse_storage(self, ignore1):
         self._browse_file(_("Locate existing storage"),
                           self.set_disk_storage_path,
-                          is_media=False)
+                          browse_reason=self.config.CONFIG_DIR_IMAGE)
 
     def toggle_storage_select(self, src):
         act = src.get_active()
@@ -1650,20 +1650,16 @@
             logging.exception("Error detecting distro.")
             self.detectedDistro = (None, None)
 
-    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
+    def _browse_file(self, dialog_name, callback, folder=None, browse_reason=False):
         if self.storage_browser == None:
             self.storage_browser = vmmStorageBrowser(self.config, self.conn,
-                                                     is_media)
+                                                     browse_reason)
             self.storage_browser.connect("storage-browse-finish",
                                          callback)
-        if is_media:
-            reason = self.config.CONFIG_DIR_MEDIA
-        else:
-            reason = self.config.CONFIG_DIR_IMAGE
 
         self.storage_browser.local_args = { "dialog_name": dialog_name,
                                             "start_folder": folder,
-                                            "browse_reason": reason}
+                                            "browse_reason": browse_reason}
         self.storage_browser.show(self.conn)
 
     def show_help(self, ignore):
diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/storagebrowse.py
--- a/src/virtManager/storagebrowse.py Tue Jun 23 19:30:12 2009 -0400
+++ b/src/virtManager/storagebrowse.py Thu Jun 25 15:03:28 2009 +0200
@@ -38,7 +38,7 @@
                                   gobject.TYPE_NONE, [str]),
     }
 
-    def __init__(self, config, conn, is_media=False):
+    def __init__(self, config, conn, browse_reason=False):
         self.__gobject_init__()
         self.window = gtk.glade.XML(config.get_glade_dir() + \
                                     "/vmm-storage-browse.glade",
@@ -58,14 +58,9 @@
         # Add Volume wizard
         self.addvol = None
 
-        if is_media:
-            reason = self.config.CONFIG_DIR_MEDIA
-        else:
-            reason = self.config.CONFIG_DIR_IMAGE
-
         # Arguments to pass to util.browse_local for local storage
         self.local_args = {"dialog_name": _("Choose local storage"),
-                           "browse_reason": reason, }
+                           "browse_reason": browse_reason }
 
         self.window.signal_autoconnect({
             "on_vmm_storage_browse_delete_event" : self.close,
diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/util.py
--- a/src/virtManager/util.py Tue Jun 23 19:30:12 2009 -0400
+++ b/src/virtManager/util.py Thu Jun 25 15:03:28 2009 +0200
@@ -158,7 +158,7 @@
         fcdialog.destroy()
         ret = None
 
-    if ret and browse_reason and not ret.startwith("/dev"):
+    if ret and browse_reason and not ret.startswith("/dev"):
         config.set_default_directory(os.path.dirname(ret), browse_reason)
 
     return ret

---

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

[Cole Robinson]

Michal Novotny wrote:
> Hi,
> I found a typo in my patch which prevented remembering last used
> directories being
> remembered correctly. Also, 'is_media' variable have been changed to
> 'browse_reason'
> which is the argument passed directly to util.browse_local() function so
> please review.
>

Comments below.

> # HG changeset patch
> # User Michal Novotny <minovotn@...>
> # Date 1245935008 -7200
> # Node ID 39b81ca60edc70066cc44cf760865c2d8f03ac8f
> # Parent  aa4f30fce78b6805c94c70729139bb326b023bcb
> Fix typo in remember-paths and change 'is_media' to 'browse_reason'
>
> This is the patch that fixes a bug introduced by my previous patch for
> remembering last used paths in rev. aa4f30fce78b. Also, 'is_media'
> variable is no longer used there and is replaced by 'browse_reason'
> wherever applicable.
>
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/addhardware.py
> --- a/src/virtManager/addhardware.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/addhardware.py Thu Jun 25 15:03:28 2009 +0200
> @@ -678,9 +678,8 @@
>  
>      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())
>          filename = self._browse_file(_("Locate or Create New Storage File"),
> -                                     textent, folder=folder,
> +                                     textent, folder=None,
>                                       confirm_overwrite=True)
>          if filename != None:
>              textent.set_text(filename)
> @@ -699,17 +698,23 @@
>              if path:
>                  textent.set_text(path)
>  
> +        if folder == None:
> +            reason = self.config.CONFIG_DIR_IMAGE
> +        else:
> +            reason = None
> +
>          conn = self.vm.get_connection()
>          if self.storage_browser == None:
> -            self.storage_browser = vmmStorageBrowser(self.config, conn, False)
> +            self.storage_browser = vmmStorageBrowser(self.config, conn,
> +                                                     reason)

There doesn't need to be any reason argument here, because (see below)

>          if self._browse_cb_id:
>              self.storage_browser.disconnect(self._browse_cb_id)
>  
>          self._browse_cb_id = self.storage_browser.connect("storage-browse-finish", set_storage_cb)
>          self.storage_browser.local_args = { "dialog_name": dialog_name,
>                                              "confirm_func": confirm_func,
> -                                            "browse_reason":
> -                                                  self.config.CONFIG_DIR_IMAGE }
> +                                            "browse_reason": reason,
> +                                            "start_folder": folder }

The user specifies it in local_args here. These local_args are what is
passed straight through to the local browser. The reason/is_media arg
can be completely dropped from StorageBrowser: just have the caller
always pass the appropriate browse_reason in local_args.

>          self.storage_browser.show(conn)
>          return None
>  
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/choosecd.py
> --- a/src/virtManager/choosecd.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/choosecd.py Thu Jun 25 15:03:28 2009 +0200
> @@ -150,7 +150,7 @@
>      def _browse_file(self, dialog_name):
>          if self.storage_browser == None:
>              self.storage_browser = vmmStorageBrowser(self.config, self.conn,
> -                                                     True)
> +                                                   self.config.CONFIG_DIR_MEDIA)
>              self.storage_browser.connect("storage-browse-finish",
>                                           self.set_storage_path)
>          self.storage_browser.local_args = { "dialog_name": dialog_name,
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/create.py
> --- a/src/virtManager/create.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/create.py Thu Jun 25 15:03:28 2009 +0200
> @@ -997,7 +997,7 @@
>      def browse_iso(self, ignore1=None, ignore2=None):
>          self._browse_file(_("Locate ISO Image"),
>                            self.set_iso_storage_path,
> -                          is_media=True)
> +                          browse_reason=self.config.CONFIG_DIR_MEDIA)
>          self.window.get_widget("install-local-box").activate()
>  
>      def toggle_enable_storage(self, src):
> @@ -1006,7 +1006,7 @@
>      def browse_storage(self, ignore1):
>          self._browse_file(_("Locate existing storage"),
>                            self.set_disk_storage_path,
> -                          is_media=False)
> +                          browse_reason=self.config.CONFIG_DIR_IMAGE)
>  
>      def toggle_storage_select(self, src):
>          act = src.get_active()
> @@ -1650,20 +1650,16 @@
>              logging.exception("Error detecting distro.")
>              self.detectedDistro = (None, None)
>  
> -    def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
> +    def _browse_file(self, dialog_name, callback, folder=None, browse_reason=False):
>          if self.storage_browser == None:
>              self.storage_browser = vmmStorageBrowser(self.config, self.conn,
> -                                                     is_media)
> +                                                     browse_reason)
>              self.storage_browser.connect("storage-browse-finish",
>                                           callback)
> -        if is_media:
> -            reason = self.config.CONFIG_DIR_MEDIA
> -        else:
> -            reason = self.config.CONFIG_DIR_IMAGE
>  
>          self.storage_browser.local_args = { "dialog_name": dialog_name,
>                                              "start_folder": folder,
> -                                            "browse_reason": reason}
> +                                            "browse_reason": browse_reason}
>          self.storage_browser.show(self.conn)
>  
>      def show_help(self, ignore):
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/storagebrowse.py
> --- a/src/virtManager/storagebrowse.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/storagebrowse.py Thu Jun 25 15:03:28 2009 +0200
> @@ -38,7 +38,7 @@
>                                    gobject.TYPE_NONE, [str]),
>      }
>  
> -    def __init__(self, config, conn, is_media=False):
> +    def __init__(self, config, conn, browse_reason=False):
>          self.__gobject_init__()
>          self.window = gtk.glade.XML(config.get_glade_dir() + \
>                                      "/vmm-storage-browse.glade",
> @@ -58,14 +58,9 @@
>          # Add Volume wizard
>          self.addvol = None
>  
> -        if is_media:
> -            reason = self.config.CONFIG_DIR_MEDIA
> -        else:
> -            reason = self.config.CONFIG_DIR_IMAGE
> -
>          # Arguments to pass to util.browse_local for local storage
>          self.local_args = {"dialog_name": _("Choose local storage"),
> -                           "browse_reason": reason, }
> +                           "browse_reason": browse_reason }
>

All of this browse_reason handling can go away as a result

The rest looks fine.

Thanks,
Cole

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

[Cole Robinson]

On 06/25/2009 09:20 AM, Cole Robinson wrote:

> Michal Novotny wrote:
>> Hi,
>> I found a typo in my patch which prevented remembering last used
>> directories being
>> remembered correctly. Also, 'is_media' variable have been changed to
>> 'browse_reason'
>> which is the argument passed directly to util.browse_local() function so
>> please review.
>>
>

In the meantime, I committed the bug fix portion of your patch:

http://hg.et.redhat.com/cgi-bin/hg-virt.cgi/applications/virt-manager--devel/rev/f80c479680cf

Thanks,
Cole

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