[virt-tools-list] [virt-manager PATCH] addstorage: remove _check_ideal_path

[Cole Robinson]

On 07/23/2015 08:31 AM, Pavel Hrdina wrote:
> This feature has been added few years ago.  I don't think, that it's a
> good feature, as it can ask a user to use different storage than he
> actually wants to use.  One thing is automatically create a new storage
> for user, if he let as do that, but we shouldn't annoy a user with this
> question as he probably don't want to use the proposed storage.  For
> example he would like to use different storage pool or while importing
> existing storage.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1232599
> 

I agree this feature is subpar, I've pushed this patch. This was a workaround
to try and reuse stranded disk images that usually happen in one of two ways:

1) User deletes the VM but not the storage. Used to be easier to screw this up
with virt-manager since we made you opt in to deleting storage but not so much
nowadays

2) A VM install fails to even launch a VM, and the newly created disk image
isn't cleaned up. We still don't handle this case, and VM launch issues are
common over time which happens due to libvirt and qemu regressions. So we
should fix that too

Thanks,
Cole

> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  virtManager/addstorage.py | 31 -------------------------------
>  1 file changed, 31 deletions(-)
> 
> diff --git a/virtManager/addstorage.py b/virtManager/addstorage.py
> index a40ceed..275356b 100644
> --- a/virtManager/addstorage.py
> +++ b/virtManager/addstorage.py
> @@ -252,34 +252,6 @@ class vmmAddStorage(vmmGObjectUI):
>      def is_default_storage(self):
>          return self.widget("config-storage-create").get_active()
>  
> -    def _check_ideal_path(self, path, vmname, collidelist):
> -        # See if the ideal disk path (/default/pool/vmname.img)
> -        # exists, and if unused, prompt the use for using it
> -        conn = self.conn.get_backend()
> -        ideal = self._get_ideal_path(vmname)
> -        if ideal in collidelist:
> -            return path
> -
> -        do_exist = False
> -        ret = True
> -        try:
> -            do_exist = virtinst.VirtualDisk.path_definitely_exists(conn, ideal)
> -            ret = virtinst.VirtualDisk.path_in_use_by(conn, ideal)
> -        except:
> -            logging.exception("Error checking default path usage")
> -
> -        if not do_exist or ret:
> -            return path
> -
> -        do_use = self.err.yes_no(
> -            _("The following storage already exists, but is not\n"
> -              "in use by any virtual machine:\n\n%s\n\n"
> -              "Would you like to reuse this storage?") % ideal)
> -
> -        if do_use:
> -            return ideal
> -        return path
> -
>      def validate_storage(self, vmname,
>                           path=None, size=None, sparse=None,
>                           device="disk", fmt=None, collidelist=None):
> @@ -314,9 +286,6 @@ class vmmAddStorage(vmmGObjectUI):
>                  else:
>                      path = self.widget("config-storage-entry").get_text().strip()
>  
> -            if is_default:
> -                path = self._check_ideal_path(path, vmname, collidelist)
> -
>              if not path and device in ["disk", "lun"]:
>                  return self.err.val_err(_("A storage path must be specified."))
>  
>