[et-mgmt-tools] [PATCH] virt-manager: Make virt-manager remember last image directory
Michal Novotny
minovotn at redhat.com
Thu Jun 18 14:06:15 UTC 2009
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/et-mgmt-tools/attachments/20090618/ba28c01e/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/ba28c01e/attachment.ksh>
More information about the et-mgmt-tools
mailing list