[et-mgmt-tools] [PATCH] virt-manager: Make virt-manager remember last image directory
Michal Novotny
minovotn at redhat.com
Thu Jun 18 16:22:32 UTC 2009
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 at redhat.com>
>>>> # 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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/et-mgmt-tools
> ------------------------------------------------------------------------
>
> _______________________________________________
> et-mgmt-tools mailing list
> et-mgmt-tools at redhat.com
> https://www.redhat.com/mailman/listinfo/et-mgmt-tools
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/et-mgmt-tools/attachments/20090618/5d27e43b/attachment.htm>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: remember-last-directories.patch
URL: <http://listman.redhat.com/archives/et-mgmt-tools/attachments/20090618/5d27e43b/attachment.ksh>
More information about the et-mgmt-tools
mailing list