From 16ebab2230c5a8e9a0c7b08ecced61f54c6955bd Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 4 Sep 2020 15:34:13 -0400 Subject: [PATCH] clone: Rework the UI * Drop the network editing, users can use the details window * Drop the combo box approach in favor of a regular treeview * Drop a lot validation checks which are redundant with modern virtinst. We probably lose some checks but I don't think it's too important * Use the cloner API * Add uitest coverage Signed-off-by: Cole Robinson --- tests/uitests/test_clonevm.py | 267 ++++++-- ui/clone.ui | 620 ++++-------------- virtManager/clone.py | 1117 +++++++++++++-------------------- virtinst/cloner.py | 61 +- virtinst/virtclone.py | 2 + 5 files changed, 811 insertions(+), 1256 deletions(-) diff --git a/tests/uitests/test_clonevm.py b/tests/uitests/test_clonevm.py index 4377ac42..c9d5eac3 100644 --- a/tests/uitests/test_clonevm.py +++ b/tests/uitests/test_clonevm.py @@ -1,9 +1,31 @@ # This work is licensed under the GNU GPLv2 or later. # See the COPYING file in the top-level directory. +import os + +from tests import utils from tests.uitests import utils as uiutils +class _CloneRow: + """ + Helper class for interacting with the clone row + """ + def __init__(self, *args): + self.chkcell = args[2] + self.txtcell = args[5] + + self.is_cloneable = self.chkcell.showing + self.is_share_requested = ( + not self.is_cloneable or not self.chkcell.checked) + self.is_clone_requested = not self.is_share_requested + + def check_in_text(self, substr): + uiutils.check(lambda: substr in self.txtcell.text) + + def select(self): + self.txtcell.click() + class CloneVM(uiutils.UITestCase): """ @@ -21,63 +43,216 @@ class CloneVM(uiutils.UITestCase): self.app.root.find("Clone...", "menu item").click() return self.app.root.find("Clone Virtual Machine", "frame") + def _get_all_rows(self, win): + slist = win.find("storage-list") + def pred(node): + return node.roleName == "table cell" + cells = slist.findChildren(pred, isLambda=True) + + idx = 0 + rows = [] + cellcount = 6 + while idx < len(cells): + rows.append(_CloneRow(*cells[idx:idx + cellcount])) + idx += cellcount + # Skip the next row which is always a separator + idx += cellcount + return rows + ############## # Test cases # ############## - def testClone(self): - """ - Clone test-clone, which is meant to hit many clone code paths - """ - win = self._open_window("test-clone") - win.find("Clone", "push button").click() - - # Verify the new VM popped up - self.app.root.find("test-clone1", "table cell") - def testCloneSimple(self): - """ - Clone test-clone-simple - """ + # Disable predictable so UUID generation doesn't collide + uri = utils.URIs.test_full.replace(",predictable", "") + self.app.uri = uri + + # Clone 'test-clone-simple' which is the most basic case + # Cancel, and reopen win = self._open_window("test-clone-simple") - win.find("Clone", "push button").click() + win.find("Cancel", "push button").click() + uiutils.check(lambda: not win.showing) - # Verify the new VM popped up - self.app.root.find("test-clone-simple-clone", "table cell") - - def testFullClone(self): - """ - Clone test-full-clone, which should error due to lack of space - """ - win = self._open_window("test-clone-full") - win.find("Clone", "push button").click() - - # Verify error dialog popped up - self.app.root.find( - ".*There is not enough free space.*", "label") - - def testCloneTweaks(self): - """ - Clone test-clone-simple, but tweak bits in the clone UI - """ + # Do default clone win = self._open_window("test-clone-simple") - win.find_fuzzy(None, - "text", "Name").set_text("test-new-vm") + rows = self._get_all_rows(win) + assert len(rows) == 1 + assert rows[0].is_clone_requested + rows[0].check_in_text("test-clone-simple.img") - win.find("Details...", "push button").click() - macwin = self.app.root.find("Change MAC address", "dialog") - macwin.find(None, - "text", "New MAC:").set_text("00:16:3e:cc:cf:05") - macwin.find("OK", "push button").click() + win.find("Clone", "push button").click() + uiutils.check(lambda: not win.showing) - win.combo_select("Clone this disk.*", "Details...") + # Check path was generated correctly + win = self._open_window("test-clone-simple-clone") + rows = self._get_all_rows(win) + assert len(rows) == 1 + assert rows[0].is_clone_requested + rows[0].check_in_text("test-clone-simple-clone.img") + + # Share storage and deal with warnings + rows[0].chkcell.click() + rows[0].check_in_text("Share disk with") + # Do 'cancel' first + win.find("Clone", "push button").click() + self._click_alert_button("cause data to be overwritten", "Cancel") + uiutils.check(lambda: win.active) + win.find("Clone", "push button").click() + self._click_alert_button("cause data to be overwritten", "OK") + uiutils.check(lambda: not win.active) + + # Verify the new VM shared storage + win = self._open_window("test-clone-simple-clone1") + rows = self._get_all_rows(win) + assert len(rows) == 1 + rows[0].check_in_text("test-clone-simple-clone.img") + + def testCloneMulti(self): + # Clone 'test-clone', check some results, make sure clone works + win = self._open_window("test-clone\n") + win.find("Clone", "push button").click() + uiutils.check(lambda: not win.showing) + self.app.topwin.find("test-clone1", "table cell") + + # Check test-many-devices which will not work, but confirm + # it errors gracefully + self.app.topwin.find("test-many-devices").click() + sbutton = self.app.topwin.find("Shut Down", "push button") + sbutton.click() + uiutils.check(lambda: not sbutton.sensitive) + self.sleep(.5) + win = self._open_window("test-many-devices") + win.find("Clone", "push button").click() + self._click_alert_button("No such file or", "Close") + win.keyCombo("F4") + uiutils.check(lambda: not win.showing) + + def testCloneStorageChange(self): + # Disable predictable so UUID generation doesn't collide + uri = utils.URIs.test_full.replace(",predictable", "") + self.app.uri = uri + + # Trigger some error handling scenarios + win = self._open_window("test-clone-simple") + newname = "test-aaabbb" + win.find("Name:", "text").set_text(newname) + win.find("Clone", "push button").click() + uiutils.check(lambda: not win.showing) + + win = self._open_window(newname) + row = self._get_all_rows(win)[0] + row.check_in_text(newname) + oldnewname = newname + newname = "test-aaazzzzbbb" + win.find("Name:", "text").set_text(newname) + row.select() + + win.find("Details", "push button").click() stgwin = self.app.root.find("Change storage path", "dialog") - stgwin.find(None, "text", - "New Path:").set_text("/dev/default-pool/my-new-path") - stgwin.find("OK", "push button").click() - + pathtxt = stgwin.find(None, "text", "New Path:") + uiutils.check(lambda: newname in pathtxt.text) + stgwin.find("Browse", "push button").click() + self._select_storagebrowser_volume("default-pool", "iso-vol") + uiutils.check(lambda: "iso-vol" in pathtxt.text) + stgwin.find("OK").click() + self._click_alert_button("overwrite the existing", "No") + uiutils.check(lambda: stgwin.showing) + stgwin.find("OK").click() + self._click_alert_button("overwrite the existing", "Yes") + uiutils.check(lambda: not stgwin.showing) + # Can't clone onto existing storage volume win.find("Clone", "push button").click() + self._click_alert_button(".*Clone onto existing.*", "Close") - # Verify the new VM popped up - self.app.root.find("test-new-vm", "table cell") + # Reopen dialog and request to share it + win.find("Details", "push button").click() + stgwin = self.app.root.find("Change storage path", "dialog") + chkbox = stgwin.find("Create a new", "check") + uiutils.check(lambda: chkbox.checked) + chkbox.click() + + # Cancel and reopen, confirm changes didn't stick + stgwin.find("Cancel").click() + uiutils.check(lambda: not stgwin.showing) + win.find("Details", "push button").click() + stgwin = self.app.root.find("Change storage path", "dialog") + chkbox = stgwin.find("Create a new", "check") + uiutils.check(lambda: chkbox.checked) + # Requesting sharing again and exit + chkbox.click() + stgwin.find("OK").click() + uiutils.check(lambda: not stgwin.active) + + # Finish install, verify storage was shared + win.find("Clone", "push button").click() + self._click_alert_button("cause data to be overwritten", "OK") + uiutils.check(lambda: not win.active) + win = self._open_window(newname) + row = self._get_all_rows(win)[0].check_in_text(oldnewname) + + + def testCloneError(self): + # Trigger some error handling scenarios + win = self._open_window("test-clone-full\n") + win.find("Clone", "push button").click() + self._click_alert_button("not enough free space", "Close") + uiutils.check(lambda: win.showing) + win.keyCombo("F4") + + win = self._open_window("test-clone-simple") + badname = "test/foo" + win.find("Name:", "text").set_text(badname) + rows = self._get_all_rows(win) + rows[0].chkcell.click() + rows[0].check_in_text("Share disk with") + win.find("Clone", "push button").click() + win.find("Clone", "push button").click() + self._click_alert_button("cause data to be overwritten", "OK") + self._click_alert_button(badname, "Close") + uiutils.check(lambda: win.active) + + + def testCloneNonmanaged(self): + # Verify unmanaged clone actual works + import tempfile + tmpsrc = tempfile.NamedTemporaryFile() + tmpdst = tempfile.NamedTemporaryFile() + + open(tmpsrc.name, "w").write(__file__) + + self.app.open(xmleditor_enabled=True) + manager = self.app.topwin + + win = self._open_details_window("test-clone-simple") + win.find("IDE Disk 1", "table cell").click() + win.find("XML", "page tab").click() + xmleditor = win.find("XML editor") + origpath = "/dev/default-pool/test-clone-simple.img" + newpath = tmpsrc.name + xmleditor.set_text(xmleditor.text.replace(origpath, newpath)) + win.find("config-apply").click() + win.find("Details", "page tab").click() + disksrc = win.find("disk-source-path") + uiutils.check(lambda: disksrc.text == newpath) + win.keyCombo("F4") + uiutils.check(lambda: not win.active) + + uiutils.check(lambda: manager.active) + win = self._open_window("test-clone-simple") + row = self._get_all_rows(win)[0] + row.check_in_text(tmpsrc.name) + row.select() + + win.find("Details", "push button").click() + stgwin = self.app.root.find("Change storage path", "dialog") + pathtxt = stgwin.find(None, "text", "New Path:") + os.unlink(tmpdst.name) + pathtxt.set_text(tmpdst.name) + stgwin.find("OK").click() + win.find("Clone", "push button").click() + uiutils.check(lambda: not win.active) + uiutils.check(lambda: os.path.exists(tmpdst.name)) + + assert open(tmpsrc.name).read() == open(tmpdst.name).read() diff --git a/ui/clone.ui b/ui/clone.ui index cddf09f9..f671c5a2 100644 --- a/ui/clone.ui +++ b/ui/clone.ui @@ -1,5 +1,5 @@ - + @@ -12,9 +12,6 @@ Clone Virtual Machine dialog - - - True @@ -96,7 +93,7 @@ True False vertical - 18 + 24 True @@ -127,7 +124,7 @@ True False start - Create clone based on: + Original VM: 0 @@ -138,8 +135,8 @@ True False - end - Destination host: + start + Connection: 0 @@ -162,217 +159,28 @@ - True + False True 0 - + True False - 12 + 12 + 6 - + True False - 10 - 12 - - - True - False - True - vertical - - - True - False - start - No networking devices - - - True - True - 0 - - - - - True - False - vertical - 6 - - - - - - True - True - 1 - - - - - 1 - 1 - - - - - True - False - end - start - Networking: - True - - - - 0 - 1 - - - - - True - False - True - True - vertical - - - True - False - start - No storage to clone - - - False - True - 0 - - - - - True - True - never - - - True - False - - - True - False - vertical - - - True - False - - - True - False - 6 - - - True - False - vertical - 12 - - - - - - - - False - False - 0 - - - - - True - False - - - - - - False - False - 1 - - - - - False - False - 0 - - - - - True - False - - - - - - False - False - 1 - - - - - - - - - True - True - 1 - - - - - 1 - 2 - - - - - True - False - end - start - True - Storage: - True - - - - 0 - 2 - - + 6 True False - end - center + start + False _Name: True True @@ -382,25 +190,123 @@ - 0 - 0 + False + True + 0 True True - center + start + start 3 3 + True ● + 30 - 1 + True + True + 1 + + + + + 0 + 0 + + + + + True + False + True + 6 + 6 + + + 200 + True + True + True + True + in + + + True + True + + + + + + + + storage-list + + + + + + + 0 + 1 + + + + + True + False + 6 + + + True + False + start + Storage: + True + True + storage-list + + + + False + True + 0 + + + + + _Details... + True + True + True + True + + + + False + True + end + 1 + + + + + 0 0 + + 0 + 1 + @@ -422,47 +328,6 @@ False vertical 12 - - - True - False - 3 - - - True - False - start - gtk-info - - - False - True - 0 - - - - - True - False - start - False - <span size='small'>Cloning creates a new, independent copy of the original disk. Sharing -uses the existing disk image for both the original and the new machine.</span> - True - - - False - True - 1 - - - - - True - True - 0 - - True @@ -500,12 +365,12 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True True - 1 + 0 - True + False True 1 @@ -572,240 +437,9 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp - - - False - 5 - Change MAC address - center-on-parent - True - dialog - - + - - - True - False - vertical - 2 - - - True - False - end - - - gtk-cancel - True - True - True - True - - - - False - False - 0 - - - - - gtk-ok - True - True - True - True - - - - False - False - 1 - - - - - False - True - end - 0 - - - - - True - False - 6 - 6 - - - True - False - vertical - 12 - - - True - False - 12 - - - True - False - 6 - 6 - - - True - False - 2 - - - True - False - start - type string - - - - - 2 - 0 - - - - - True - False - True - 2 - - - True - False - start - orig-mac - 20 - - - - - 2 - 1 - - - - - True - True - True - - - 2 - 2 - - - - - True - False - end - New _MAC: - True - change-mac-new - - - 0 - 2 - 2 - - - - - True - False - 6 - 7 - - - True - False - network-idle - 6 - - - - - 0 - 0 - 2 - - - - - True - False - end - center - Type: - True - - - - 1 - 0 - - - - - True - False - end - center - MAC: - True - - - - 1 - 1 - - - - - True - True - 0 - - - - - False - True - 0 - - - - - - - False - True - 1 - - - - - - change-mac-cancel - change-mac-ok - False @@ -815,9 +449,6 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True dialog - - - True @@ -936,10 +567,9 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True False - start orig-path + True start - 25 1 @@ -1033,7 +663,6 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True False - 22 True @@ -1057,6 +686,7 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True True False + start True True @@ -1071,8 +701,13 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp True True - center + True 25 + + + new-path + + 1 @@ -1113,5 +748,8 @@ like change passwords or static IPs, please see the virt-sysprep(1) tool.</sp change-storage-cancel change-storage-ok + + + diff --git a/virtManager/clone.py b/virtManager/clone.py index 71ea67a9..dffbf5e7 100644 --- a/virtManager/clone.py +++ b/virtManager/clone.py @@ -4,111 +4,165 @@ # This work is licensed under the GNU GPLv2 or later. # See the COPYING file in the top-level directory. -import os +import collections from gi.repository import Gtk +from gi.repository import Pango import virtinst from virtinst import Cloner -from virtinst import DeviceInterface from virtinst import log +from virtinst import xmlutil from .lib import uiutil from .baseclass import vmmGObjectUI from .asyncjob import vmmAsyncJob from .storagebrowse import vmmStorageBrowser -from .object.storagepool import vmmStoragePool - -STORAGE_COMBO_CLONE = 0 -STORAGE_COMBO_SHARE = 1 -STORAGE_COMBO_SEP = 2 -STORAGE_COMBO_DETAILS = 3 - -STORAGE_INFO_ORIG_PATH = 0 -STORAGE_INFO_NEW_PATH = 1 -STORAGE_INFO_TARGET = 2 -STORAGE_INFO_SIZE = 3 -STORAGE_INFO_DEVTYPE = 4 -STORAGE_INFO_DO_CLONE = 5 -STORAGE_INFO_CAN_CLONE = 6 -STORAGE_INFO_CAN_SHARE = 7 -STORAGE_INFO_DO_DEFAULT = 8 -STORAGE_INFO_DEFINFO = 9 -STORAGE_INFO_FAILINFO = 10 -STORAGE_INFO_COMBO = 11 -STORAGE_INFO_MANUAL_PATH = 12 - -NETWORK_INFO_LABEL = 0 -NETWORK_INFO_ORIG_MAC = 1 -NETWORK_INFO_NEW_MAC = 2 -def can_we_clone(conn, vol, path): +def _get_cloneable_msg(diskinfo): """Is the passed path even clone-able""" - ret = True - msg = None - - if not path: - msg = _("No storage to clone.") - - elif not vol: - is_dev = path.startswith("/dev") - if conn.is_remote(): - msg = _("Cannot clone unmanaged remote storage.") - elif not os.access(path, os.R_OK): - if is_dev: - msg = _("Block devices to clone must be libvirt\n" - "managed storage volumes.") - else: - msg = _("No write access to parent directory.") - elif not os.path.exists(path): - msg = _("Path does not exist.") - - else: - pool = vol.get_parent_pool() - if not vmmStoragePool.supports_volume_creation( - pool.get_type(), clone=True): - msg = _("Cannot clone %s storage pool.") % pool.get_type() - - if msg: - ret = False - - return (ret, msg) + if diskinfo.get_cloneable_msg(): + return diskinfo.get_cloneable_msg() + if not diskinfo.disk.path: + return _("No storage to clone.") -def do_we_default(conn, vol, path, ro, shared, devtype): - """ Returns (do we clone by default?, info string if not)""" - ignore = conn - info = "" - can_default = True +class _StorageInfo: + """ + Our wrapper class that is close to Cloner _DiskInfo content. + We track all user choices explicitly in this class, and then + serialize them into the actual cloner diskinfo once the + user clicks 'Clone' to complete the process. + """ + def __init__(self, vm, cloner, diskinfo): + self._diskinfo = diskinfo + self._orig_disk = diskinfo.disk + self._orig_vm_name = vm.get_name() + self._new_vm_name = cloner.new_guest.name - def append_str(str1, str2, delim=", "): - if not str2: - return str1 - if str1: - str1 += delim - str1 += str2 - return str1 + self.share_msg = diskinfo.get_share_msg() + self.cloneable_msg = _get_cloneable_msg(diskinfo) - if (devtype == virtinst.DeviceDisk.DEVICE_CDROM or - devtype == virtinst.DeviceDisk.DEVICE_FLOPPY): - info = append_str(info, _("Removable")) + self._manual_path = None + self._generated_path = diskinfo.new_disk and diskinfo.new_disk.path + if not self._generated_path: + self._reset_generated_path() - if ro: - info = append_str(info, _("Read Only")) - elif not vol and path and not os.access(path, os.W_OK): - info = append_str(info, _("No write access")) + self._is_clone_requested = diskinfo.is_clone_requested() - if vol: - pool_type = vol.get_parent_pool().get_type() - if pool_type == virtinst.StoragePool.TYPE_DISK: - info = append_str(info, _("Disk device")) - can_default = False + if self.share_msg or not self.is_cloneable(): + self._is_clone_requested = False - if shared: - info = append_str(info, _("Shareable")) + def is_cloneable(self): + return not bool(self.cloneable_msg) + def is_clone_requested(self): + return self._is_clone_requested + def is_share_requested(self): + return not self._is_clone_requested + def warn_about_sharing(self): + return not self.share_msg - return (not info, info, can_default) + def get_target(self): + return self._orig_disk.target + def get_orig_disk_path(self): + return self._orig_disk.path + def get_new_disk_path(self): + if self._manual_path: + return self._manual_path + return self._generated_path + + def set_clone_requested(self, val): + self._is_clone_requested = bool(val) + def set_manual_path(self, val): + self._manual_path = val + def set_new_vm_name(self, val): + if not val or val == self._new_vm_name: + return + self._new_vm_name = val + self._reset_generated_path() + + def _reset_generated_path(self): + self._generated_path = Cloner.generate_clone_disk_path( + self._orig_disk.conn, + self._orig_vm_name, + self._new_vm_name, + self._orig_disk.path) + + def set_values_on_diskinfo(self, diskinfo): + if not self._is_clone_requested: + diskinfo.set_share_requested() + diskinfo.new_disk = None + return + + diskinfo.set_clone_requested() + sparse = True + newpath = self.get_new_disk_path() + diskinfo.set_new_path(newpath, sparse) + + + ################### + # UI info helpers # + ################### + + def get_tooltip(self): + lines = [] + lines.append(_("Disk target: %s") % self.get_target()) + lines.append(_("Original path: %s") % self.get_orig_disk_path()) + if self.get_new_disk_path(): + lines.append(_("New path: %s") % self.get_new_disk_path()) + lines.append("\n") + + if self.share_msg: + lines.append(_("Storage is safe to share: %(reason)s") % { + "reason": self.share_msg}) + else: + lines.append( + _("Sharing this storage is potentially dangerous.")) + + if self.cloneable_msg: + lines.append(_("Storage is not cloneable: %(reason)s") % { + "reason": self.cloneable_msg}) + return "\n".join(lines) + + def get_markup(self, vm): + lines = [] + + line = "" + path = self._orig_disk.path + if path: + line += path + else: + line += _("No storage.") + lines.append(line) + + line = "" + if self.is_share_requested(): + line += _("Share disk with %s") % vm.get_name() + if self.is_clone_requested(): + line += _("Clone this disk") + sizelabel = self.get_size_label() + if sizelabel: + line += " (%s)" % sizelabel + if line: + lines.append(line) + + label = "\n".join(lines) + markup = "%s" % xmlutil.xml_escape(label) + return markup + + def get_icon_name(self): + if self._orig_disk.is_floppy(): + return "media-floppy" + if self._orig_disk.is_cdrom(): + return "media-optical" + return "drive-harddisk" + + def get_size_label(self): + size = self._orig_disk.get_size() + if not size: + return "" # pragma: no cover + return "%.1f GiB" % float(size) class vmmCloneVM(vmmGObjectUI): @@ -129,72 +183,60 @@ class vmmCloneVM(vmmGObjectUI): def __init__(self): vmmGObjectUI.__init__(self, "clone.ui", "vmm-clone") self.vm = None - self.clone_design = None - self.storage_list = {} - self.target_list = [] + self._storage_list = None + self._storage_browser = None - self.net_list = {} - self.mac_list = [] - - self.storage_browser = None - - self.change_mac = self.widget("vmm-change-mac") - self.change_mac.set_transient_for(self.topwin) - - self.change_storage = self.widget("vmm-change-storage") - self.change_storage.set_transient_for(self.topwin) + self._storage_dialog = self.widget("vmm-change-storage") + self._storage_dialog.set_transient_for(self.topwin) self.builder.connect_signals({ - "on_clone_delete_event": self.close, - "on_clone_cancel_clicked": self.close, - "on_clone_ok_clicked": self.finish, + "on_clone_delete_event": self._close_cb, + "on_clone_cancel_clicked": self._close_cb, + "on_clone_ok_clicked": self._finish_clicked_cb, + "on_storage_selection_changed": self._storage_selection_changed_cb, + "on_storage_details_clicked": self._storage_details_clicked_cb, - # Change mac dialog - "on_vmm_change_mac_delete_event": self.change_mac_close, - "on_change_mac_cancel_clicked": self.change_mac_close, - "on_change_mac_ok_clicked": self.change_mac_finish, - - # Change storage dialog - "on_vmm_change_storage_delete_event": self.change_storage_close, - "on_change_storage_cancel_clicked": self.change_storage_close, - "on_change_storage_ok_clicked": self.change_storage_finish, - "on_change_storage_doclone_toggled": self.change_storage_doclone_toggled, - - "on_change_storage_browse_clicked": self.change_storage_browse, + # Storage subdialog signals + "on_vmm_change_storage_delete_event": self._storage_dialog_close_cb, + "on_change_storage_cancel_clicked": self._storage_dialog_close_cb, + "on_change_storage_ok_clicked": self._storage_dialog_finish_cb, + "on_change_storage_doclone_toggled": self._storage_dialog_doclone_toggled_cb, + "on_change_storage_browse_clicked": self._storage_dialog_browse_cb, }) self.bind_escape_key_close() self._cleanup_on_app_close() self._init_ui() + + ####################### + # Standard UI methods # + ####################### + @property def conn(self): - if self.vm: - return self.vm.conn - return None + return self.vm and self.vm.conn or None def show(self, parent, vm): log.debug("Showing clone wizard") self._set_vm(vm) - self.reset_state() + self._reset_state() self.topwin.set_transient_for(parent) self.topwin.resize(1, 1) self.topwin.present() + def _storage_dialog_close(self): + self._storage_dialog.hide() + return 1 + def close(self, ignore1=None, ignore2=None): log.debug("Closing clone wizard") - self.change_mac_close() - self.change_storage_close() + self._storage_dialog_close() self.topwin.hide() self._set_vm(None) - self.clone_design = None - self.storage_list = {} - self.target_list = [] - self.net_list = {} - self.mac_list = [] - + self._storage_list = None return 1 def _vm_removed_cb(self, _conn, vm): @@ -210,479 +252,177 @@ class vmmCloneVM(vmmGObjectUI): self.vm = newvm def _cleanup(self): - self.change_mac.destroy() - self.change_mac = None + self._storage_dialog.destroy() + self._storage_dialog = None - self.change_storage.destroy() - self.change_storage = None - - if self.storage_browser: - self.storage_browser.cleanup() - self.storage_browser = None - - def change_mac_close(self, ignore1=None, ignore2=None): - self.change_mac.hide() - return 1 - - def change_storage_close(self, ignore1=None, ignore2=None): - self.change_storage.hide() - return 1 + if self._storage_browser: + self._storage_browser.cleanup() + self._storage_browser = None - # First time setup + ########### + # UI init # + ########### def _init_ui(self): - context = self.topwin.get_style_context() - defcolor = context.get_background_color(Gtk.StateType.NORMAL) - self.widget("storage-viewport").override_background_color( - Gtk.StateType.NORMAL, - defcolor) + storage_list = self.widget("storage-list") - # Populate state - def reset_state(self): + # [disk target, tooltip] + model = Gtk.ListStore(str, str) + storage_list.set_model(model) + storage_list.set_tooltip_column(1) + + cloneCol = Gtk.TreeViewColumn(_("Clone")) + pathCol = Gtk.TreeViewColumn(_("Storage")) + pathCol.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE) + pathCol.set_expand(True) + + storage_list.append_column(cloneCol) + storage_list.append_column(pathCol) + + def separator_cb(_model, _iter): + return _model[_iter][0] == "separator" + storage_list.set_row_separator_func(separator_cb) + + chkbox = Gtk.CellRendererToggle() + chkbox.connect('toggled', self._storage_clone_toggled_cb) + chkimg = Gtk.CellRendererPixbuf() + chkimg.set_property('stock-size', Gtk.IconSize.MENU) + cloneCol.pack_start(chkimg, False) + cloneCol.pack_start(chkbox, False) + + def chk_cell_data_cb(column, cell, model, _iter, data): + target = model[_iter][0] + if target == "separator": + return + sinfo = self._storage_list[target] + visible = sinfo.is_cloneable() + active = sinfo.is_clone_requested() + _chkimg = column.get_cells()[0] + _chkbox = column.get_cells()[1] + _chkbox.set_property('active', active) + _chkbox.set_property('visible', visible) + _chkimg.set_property('visible', not visible) + icon = Gtk.STOCK_INFO + if sinfo.warn_about_sharing(): + icon = Gtk.STOCK_DIALOG_WARNING + _chkimg.set_property('stock-id', icon) + tooltip = sinfo.get_tooltip() + if tooltip != model[_iter][1]: + model[_iter][1] = tooltip + + cloneCol.set_cell_data_func(chkbox, chk_cell_data_cb) + cloneCol.set_cell_data_func(chkimg, chk_cell_data_cb) + + pathtxt = Gtk.CellRendererText() + pathCol.set_sort_column_id(0) + pathtxt.set_property("width-chars", 30) + pathtxt.set_property("ellipsize", Pango.EllipsizeMode.MIDDLE) + pathimg = Gtk.CellRendererPixbuf() + pathimg.set_property('stock-size', Gtk.IconSize.MENU) + pathimg.set_padding(3, 0) + pathCol.pack_start(pathimg, False) + pathCol.pack_start(pathtxt, True) + + def path_cb(column, cell, model, _iter, data): + target = model[_iter][0] + if target == "separator": + return + sinfo = self._storage_list[target] + _pathimg = column.get_cells()[0] + _pathtxt = column.get_cells()[1] + markup = sinfo.get_markup(self.vm) + _pathtxt.set_property('markup', markup) + _pathimg.set_property('icon-name', sinfo.get_icon_name()) + + pathCol.set_cell_data_func(pathtxt, path_cb) + pathCol.set_cell_data_func(pathimg, path_cb) + + + def _reset_state(self): self.widget("clone-cancel").grab_focus() + self.widget("clone-new-name").set_text("") # Populate default clone values - self.setup_clone_info() - - cd = self.clone_design - self.widget("clone-orig-name").set_text(cd.original_guest) - self.widget("clone-new-name").set_text(cd.clone_name) + cloner = self._build_cloner() + cloner.prepare() + self.widget("clone-orig-name").set_text(cloner.src_name) + self.widget("clone-new-name").set_text(cloner.new_guest.name) uiutil.set_grid_row_visible( self.widget("clone-dest-host"), self.conn.is_remote()) self.widget("clone-dest-host").set_text(self.conn.get_pretty_desc()) - # We need to determine which disks fail (and why). - self.storage_list, self.target_list = self.check_all_storage() + # Collect info about all the VMs disks + self._storage_list = collections.OrderedDict() - self.populate_storage_lists() - self.populate_network_list() + for diskinfo in cloner.get_diskinfos(): + sinfo = _StorageInfo(self.vm, cloner, diskinfo) + self._storage_list[sinfo.get_target()] = sinfo - return + self._populate_storage_ui() - def setup_clone_info(self): - self.clone_design = self.build_new_clone_design() - def build_new_clone_design(self, new_name=None): - design = Cloner(self.conn.get_backend()) - design.original_guest = self.vm.get_name() - if not new_name: - new_name = design.generate_clone_name() - design.clone_name = new_name - - # Erase any clone_policy from the original design, so that we - # get the entire device list. - design.clone_policy = [] - return design - - def populate_network_list(self): - net_box = self.widget("clone-network-box") - for c in net_box.get_children(): - net_box.remove(c) - c.destroy() - - self.net_list = {} - self.mac_list = [] - - def build_net_row(labelstr, origmac, newmac): - - label = Gtk.Label(label=labelstr) - label.set_alignment(0, .5) - button = Gtk.Button(_("Details...")) - button.connect("clicked", self.net_change_mac, origmac) - - hbox = Gtk.HBox() - hbox.set_spacing(12) - hbox.pack_start(label, True, True, 0) - hbox.pack_end(button, False, False, False) - hbox.show_all() - net_box.pack_start(hbox, False, False, False) - - net_row = [] - net_row.insert(NETWORK_INFO_LABEL, labelstr) - net_row.insert(NETWORK_INFO_ORIG_MAC, origmac) - net_row.insert(NETWORK_INFO_NEW_MAC, newmac) - self.net_list[origmac] = net_row - self.mac_list.append(origmac) - - for net in self.vm.xmlobj.devices.interface: - mac = net.macaddr - net_dev = net.source - net_type = net.type - - # Generate a new MAC - newmac = DeviceInterface.generate_mac( - self.conn.get_backend()) - - # [ interface type, device name, origmac, newmac, label ] - if net_type == DeviceInterface.TYPE_USER: - label = _("Usermode (%(mac)s)") % {"mac": mac} - - elif net_type == DeviceInterface.TYPE_VIRTUAL: - net = None - for netobj in self.vm.conn.list_nets(): - if netobj.get_name() == net_dev: - net = netobj - break - - if net: - label = _("%(netmode)s (%(mac)s)") % { - "netmode": net.pretty_forward_mode(), - "mac": mac, - } - elif net_dev: - label = _("Virtual Network %(netdevice)s (%(mac)s)") % { - "netdevice": net_dev, - "mac": mac, - } - else: - label = _("Virtual Network (%(mac)s)") % {"mac": mac} - - else: - # 'bridge' or anything else - pretty_net_type = net_type.capitalize() - if net_dev: - label = _("%(nettype)s %(netdevice)s (%(mac)s)") % { - "nettype": pretty_net_type, - "netdevice": net_dev, - "mac": mac, - } - else: - label = _("%(nettype)s (%(mac)s)") % { - "nettype": pretty_net_type, - "mac": mac, - } - - build_net_row(label, mac, newmac) - - no_net = (not list(self.net_list.keys())) - self.widget("clone-network-box").set_visible(not no_net) - self.widget("clone-no-net").set_visible(no_net) - - def check_all_storage(self): - """ - Determine which storage is clonable, and which isn't - """ - diskinfos = self.vm.xmlobj.devices.disk - cd = self.clone_design - - storage_list = {} - - # We need to determine which disks fail (and why). - all_targets = [d.target for d in diskinfos] - - for disk in diskinfos: - force_target = disk.target - path = disk.path - ro = disk.read_only - shared = disk.shareable - devtype = disk.device - - size = None - clone_path = None - failinfo = "" - definfo = "" - - storage_row = [] - storage_row.insert(STORAGE_INFO_ORIG_PATH, path or "-") - storage_row.insert(STORAGE_INFO_NEW_PATH, clone_path) - storage_row.insert(STORAGE_INFO_TARGET, force_target) - storage_row.insert(STORAGE_INFO_SIZE, size) - storage_row.insert(STORAGE_INFO_DEVTYPE, devtype) - storage_row.insert(STORAGE_INFO_DO_CLONE, False) - storage_row.insert(STORAGE_INFO_CAN_CLONE, False) - storage_row.insert(STORAGE_INFO_CAN_SHARE, False) - storage_row.insert(STORAGE_INFO_DO_DEFAULT, False) - storage_row.insert(STORAGE_INFO_DEFINFO, definfo) - storage_row.insert(STORAGE_INFO_FAILINFO, failinfo) - storage_row.insert(STORAGE_INFO_COMBO, None) - storage_row.insert(STORAGE_INFO_MANUAL_PATH, False) - - skip_targets = all_targets[:] - skip_targets.remove(force_target) - - vol = self.conn.get_vol_by_path(path) - default, definfo, can_default = do_we_default(self.conn, vol, path, - ro, shared, devtype) - - def storage_add(failinfo=None): - # pylint: disable=cell-var-from-loop - storage_row[STORAGE_INFO_DEFINFO] = definfo - storage_row[STORAGE_INFO_DO_DEFAULT] = default - storage_row[STORAGE_INFO_CAN_SHARE] = bool(definfo) - if failinfo: - storage_row[STORAGE_INFO_FAILINFO] = failinfo - storage_row[STORAGE_INFO_DO_CLONE] = False - - storage_list[force_target] = storage_row - - # If origdisk is empty, deliberately make it fail - if not path: - storage_add(_("Nothing to clone.")) - continue - - try: - cd.skip_target = skip_targets - cd.setup_original() - except Exception as e: - log.exception("Disk target '%s' caused clone error", - force_target) - storage_add(str(e)) - continue - - can_clone, cloneinfo = can_we_clone(self.conn, vol, path) - if not can_clone: - storage_add(cloneinfo) - continue - - storage_row[STORAGE_INFO_CAN_CLONE] = True - - # If we cannot create default clone_path don't even try to do that - if not can_default: - storage_add() - continue - - try: - # Generate disk path, make sure that works - clone_path = self.generate_clone_path_name(path) - - log.debug("Original path: %s\nGenerated clone path: %s", - path, clone_path) - - cd.clone_paths = clone_path - size = cd.original_disks[0].get_size() - except Exception as e: - log.exception("Error setting generated path '%s'", - clone_path) - storage_add(str(e)) - - storage_row[STORAGE_INFO_NEW_PATH] = clone_path - storage_row[STORAGE_INFO_SIZE] = self.pretty_storage(size) - storage_add() - - return storage_list, all_targets - - def generate_clone_path_name(self, origpath, newname=None): - cd = self.clone_design - if not newname: - newname = cd.clone_name - clone_path = cd.generate_clone_disk_path(origpath, - newname=newname) - return clone_path - - def set_paths_from_clone_name(self): - cd = self.clone_design - newname = self.widget("clone-new-name").get_text() - - if not newname: - return - if cd.clone_name == newname: - return - - for row in list(self.storage_list.values()): - origpath = row[STORAGE_INFO_ORIG_PATH] - if row[STORAGE_INFO_MANUAL_PATH]: - continue - if not row[STORAGE_INFO_DO_CLONE]: - return - try: - newpath = self.generate_clone_path_name(origpath, newname) - row[STORAGE_INFO_NEW_PATH] = newpath - except Exception as e: - log.debug("Generating new path from clone name failed: %s", - str(e)) - - def build_storage_entry(self, disk, storage_box): - origpath = disk[STORAGE_INFO_ORIG_PATH] - devtype = disk[STORAGE_INFO_DEVTYPE] - size = disk[STORAGE_INFO_SIZE] - can_clone = disk[STORAGE_INFO_CAN_CLONE] - do_clone = disk[STORAGE_INFO_DO_CLONE] - can_share = disk[STORAGE_INFO_CAN_SHARE] - is_default = disk[STORAGE_INFO_DO_DEFAULT] - definfo = disk[STORAGE_INFO_DEFINFO] - failinfo = disk[STORAGE_INFO_FAILINFO] - target = disk[STORAGE_INFO_TARGET] + ####################### + # Functional routines # + ####################### + def _build_cloner(self): + conn = self.conn.get_backend() orig_name = self.vm.get_name() + new_name = self.widget("clone-new-name").get_text() + cloner = Cloner(conn, src_name=orig_name) + if new_name: + cloner.set_clone_name(new_name) + return cloner - disk_label = os.path.basename(origpath) - info_label = None - if not can_clone: - info_label = Gtk.Label() - info_label.set_alignment(0, .5) - info_label.set_markup("%s" % failinfo) - info_label.set_line_wrap(True) - if not is_default: - disk_label += (definfo and " (%s)" % definfo or "") - # Build icon - icon = Gtk.Image() - if devtype == virtinst.DeviceDisk.DEVICE_FLOPPY: - iconname = "media-floppy" - elif devtype == virtinst.DeviceDisk.DEVICE_CDROM: - iconname = "media-optical" - else: - iconname = "drive-harddisk" - icon.set_from_icon_name(iconname, Gtk.IconSize.MENU) - disk_name_label = Gtk.Label(label=disk_label) - disk_name_label.set_alignment(0, .5) - disk_name_box = Gtk.HBox(spacing=9) - disk_name_box.pack_start(icon, False, False, 0) - disk_name_box.pack_start(disk_name_label, True, True, 0) + ####################### + # Storage UI building # + ####################### - def sep_func(model, it, combo): - ignore = combo - return model[it][2] + def _set_paths_from_clone_name(self): + newname = self.widget("clone-new-name").get_text() + for sinfo in list(self._storage_list.values()): + sinfo.set_new_vm_name(newname) - # [String, sensitive, is sep] - model = Gtk.ListStore(str, bool, bool) - option_combo = Gtk.ComboBox() - option_combo.set_model(model) - text = Gtk.CellRendererText() - option_combo.pack_start(text, True) - option_combo.add_attribute(text, "text", 0) - option_combo.add_attribute(text, "sensitive", 1) - option_combo.set_row_separator_func(sep_func, option_combo) - option_combo.connect("changed", self.storage_combo_changed, target) + def _populate_storage_ui(self): + """ + Fill in the storage UI from the collected StorageInfo classes + """ + model = self.widget("storage-list").get_model() + model.clear() + for sinfo in self._storage_list.values(): + model.append([sinfo.get_target(), sinfo.get_tooltip()]) + model.append(["separator", None]) - vbox = Gtk.VBox(spacing=1) - if can_clone or can_share: - model.insert(STORAGE_COMBO_CLONE, - [(_("Clone this disk") + - (size and " (%s)" % size or "")), - can_clone, False]) - model.insert(STORAGE_COMBO_SHARE, - [_("Share disk with %s") % orig_name, can_share, - False]) - model.insert(STORAGE_COMBO_SEP, ["", False, True]) - model.insert(STORAGE_COMBO_DETAILS, - [_("Details..."), True, False]) + def _storage_clone_toggled_cb(self, src, treepath): + model = self.widget("storage-list").get_model() + row = model[treepath] + target = row[0] + sinfo = self._storage_list[target] + active = not src.get_property("active") + sinfo.set_clone_requested(active) + model.emit("row-changed", row.path, row.iter) - if (can_clone and is_default) or do_clone: - option_combo.set_active(STORAGE_COMBO_CLONE) - else: - option_combo.set_active(STORAGE_COMBO_SHARE) - else: - model.insert(STORAGE_COMBO_CLONE, - [_("Storage cannot be shared or cloned."), - False, False]) - option_combo.set_active(STORAGE_COMBO_CLONE) - vbox.pack_start(disk_name_box, False, False, 0) - vbox.pack_start(option_combo, False, False, 0) - if info_label: - vbox.pack_start(info_label, False, False, 0) - storage_box.pack_start(vbox, False, False, 0) + ########################### + # Storage window handling # + ########################### - disk[STORAGE_INFO_COMBO] = option_combo + def _show_storage_window(self): + tgt = uiutil.get_list_selection(self.widget("storage-list")) + sinfo = self._storage_list[tgt] - def populate_storage_lists(self): - storage_box = self.widget("clone-storage-box") - for c in storage_box.get_children(): - storage_box.remove(c) - c.destroy() - - for target in self.target_list: - disk = self.storage_list[target] - self.build_storage_entry(disk, storage_box) - - num_c = min(len(self.target_list), 3) - if num_c: - scroll = self.widget("clone-storage-scroll") - scroll.set_size_request(-1, 80 * num_c) - storage_box.show_all() - - no_storage = not bool(len(self.target_list)) - self.widget("clone-storage-box").set_visible(not no_storage) - self.widget("clone-no-storage-pass").set_visible(no_storage) - - skip_targets = [] - new_disks = [] - for target in self.target_list: - do_clone = self.storage_list[target][STORAGE_INFO_DO_CLONE] - new_path = self.storage_list[target][STORAGE_INFO_NEW_PATH] - - if do_clone: - new_disks.append(new_path) - else: - skip_targets.append(target) - - self.clone_design.skip_target = skip_targets - try: - self.clone_design.clone_paths = new_disks - except Exception as e: - # Just log the error and go on. The UI will fail later if needed - log.debug("Error setting clone_paths: %s", str(e)) - - # If any storage cannot be cloned or shared, don't allow cloning - clone = True - tooltip = "" - for row in list(self.storage_list.values()): - can_clone = row[STORAGE_INFO_CAN_CLONE] - can_share = row[STORAGE_INFO_CAN_SHARE] - if not (can_clone or can_share): - clone = False - tooltip = _("One or more disks cannot be cloned or shared.") - break - - ok_button = self.widget("clone-ok") - ok_button.set_sensitive(clone) - ok_button.set_tooltip_text(tooltip) - - def net_change_mac(self, ignore, origmac): - row = self.net_list[origmac] - orig_mac = row[NETWORK_INFO_ORIG_MAC] - new_mac = row[NETWORK_INFO_NEW_MAC] - typ = row[NETWORK_INFO_LABEL] - - self.widget("change-mac-orig").set_text(orig_mac) - self.widget("change-mac-type").set_text(typ) - self.widget("change-mac-new").set_text(new_mac) - - self.change_mac.show_all() - - def storage_combo_changed(self, src, target): - idx = src.get_active() - row = self.storage_list[target] - - if idx == STORAGE_COMBO_CLONE: - row[STORAGE_INFO_DO_CLONE] = True - return - elif idx == STORAGE_COMBO_SHARE: - row[STORAGE_INFO_DO_CLONE] = False - return - elif idx != STORAGE_COMBO_DETAILS: - return - - do_clone = row[STORAGE_INFO_DO_CLONE] - if do_clone: - src.set_active(STORAGE_COMBO_CLONE) - else: - src.set_active(STORAGE_COMBO_SHARE) - - # Show storage - self.storage_change_path(row) - - def change_storage_doclone_toggled(self, src): - do_clone = src.get_active() - - self.widget("change-storage-new").set_sensitive(do_clone) - self.widget("change-storage-browse").set_sensitive(do_clone) - - def storage_change_path(self, row): # If storage paths are dependent on manually entered clone name, # make sure they are up to date - self.set_paths_from_clone_name() + self._set_paths_from_clone_name() - orig = row[STORAGE_INFO_ORIG_PATH] - new = row[STORAGE_INFO_NEW_PATH] - tgt = row[STORAGE_INFO_TARGET] - size = row[STORAGE_INFO_SIZE] - can_clone = row[STORAGE_INFO_CAN_CLONE] - can_share = row[STORAGE_INFO_CAN_SHARE] - do_clone = row[STORAGE_INFO_DO_CLONE] + orig = sinfo.get_orig_disk_path() + new = sinfo.get_new_disk_path() + size = sinfo.get_size_label() + can_clone = sinfo.is_cloneable() + do_clone = sinfo.is_clone_requested() self.widget("change-storage-doclone").set_active(True) self.widget("change-storage-doclone").toggled() @@ -691,137 +431,77 @@ class vmmCloneVM(vmmGObjectUI): self.widget("change-storage-size").set_text(size or "-") self.widget("change-storage-doclone").set_active(do_clone) - if can_clone: - self.widget("change-storage-new").set_text(new or "") - else: - self.widget("change-storage-new").set_text("") - self.widget("change-storage-doclone").set_sensitive(can_clone and - can_share) + self.widget("change-storage-new").set_text(new or "") + self.widget("change-storage-doclone").set_sensitive(can_clone) self.widget("vmm-change-storage").show_all() - def change_mac_finish(self, ignore): - orig = self.widget("change-mac-orig").get_text() - new = self.widget("change-mac-new").get_text() - row = self.net_list[orig] - try: - DeviceInterface.check_mac_in_use(self.conn.get_backend(), new) - row[NETWORK_INFO_NEW_MAC] = new - except Exception as e: - self.err.show_err(_("Error changing MAC address: %s") % str(e)) - return - - self.change_mac_close() - - def change_storage_finish(self, ignore): + def _storage_dialog_finish(self): target = self.widget("change-storage-target").get_text() - row = self.storage_list[target] + sinfo = self._storage_list[target] # Sync 'do clone' checkbox, and main dialog combo - combo = row[STORAGE_INFO_COMBO] do_clone = self.widget("change-storage-doclone").get_active() - if do_clone: - combo.set_active(STORAGE_COMBO_CLONE) - else: - combo.set_active(STORAGE_COMBO_SHARE) + sinfo.set_clone_requested(do_clone) - row[STORAGE_INFO_DO_CLONE] = do_clone if not do_clone: - self.change_storage_close() + self._storage_dialog_close() return new_path = self.widget("change-storage-new").get_text() - if virtinst.DeviceDisk.path_definitely_exists(self.clone_design.conn, - new_path): - res = self.err.yes_no(_("Cloning will overwrite the existing " - "file"), - _("Using an existing image will overwrite " - "the path during the clone process. Are " - "you sure you want to use this path?")) + if virtinst.DeviceDisk.path_definitely_exists( + self.vm.conn.get_backend(), new_path): + text1 = _("Cloning will overwrite the existing file") + text2 = _("Using an existing image will overwrite " + "the path during the clone process. Are " + "you sure you want to use this path?") + res = self.err.yes_no(text1, text2) if not res: return - try: - self.clone_design.clone_paths = new_path - row[STORAGE_INFO_NEW_PATH] = new_path - row[STORAGE_INFO_MANUAL_PATH] = True - self.populate_storage_lists() - except Exception as e: - self.err.show_err(_("Error changing storage path: %s") % str(e)) - return + sinfo.set_manual_path(new_path) + self._storage_dialog_close() - self.change_storage_close() - def pretty_storage(self, size): - if not size: - return "" - return "%.1f GiB" % float(size) + ################### + # Finish routines # + ################### - # Listeners - def validate(self): - self.set_paths_from_clone_name() - name = self.widget("clone-new-name").get_text() - - # Make another clone_design - cd = self.build_new_clone_design(name) - - # Set MAC addresses - clonemacs = [] - for mac in self.mac_list: - row = self.net_list[mac] - clonemacs.append(row[NETWORK_INFO_NEW_MAC]) - cd.clone_macs = clonemacs - - skip_targets = [] + def _validate(self, cloner): new_paths = [] warn_str = "" - for target in self.target_list: - path = self.storage_list[target][STORAGE_INFO_ORIG_PATH] - new_path = self.storage_list[target][STORAGE_INFO_NEW_PATH] - do_clone = self.storage_list[target][STORAGE_INFO_DO_CLONE] - do_default = self.storage_list[target][STORAGE_INFO_DO_DEFAULT] - - if do_clone: - new_paths.append(new_path) - else: - skip_targets.append(target) - if not path or path == '-': - continue - - if not do_default: - continue - - warn_str += "%s: %s\n" % (target, path) - - cd.skip_target = skip_targets - cd.setup_original() - cd.clone_paths = new_paths + for sinfo in self._storage_list.values(): + if sinfo.is_clone_requested(): + new_paths.append(sinfo.get_new_disk_path()) + continue + if not sinfo.warn_about_sharing(): + continue + warn_str += "%s: %s\n" % ( + sinfo.get_target(), sinfo.get_orig_disk_path()) if warn_str: res = self.err.ok_cancel( - _("Skipping disks may cause data to be overwritten."), - _("The following disk devices will not be cloned:\n\n%s\n" + _("Sharing storage may cause data to be overwritten."), + _("The following disk devices will be shared with %(vmname)s:" + "\n\n%(pathlist)s\n" "Running the new guest could overwrite data in these " - "disk images.") - % warn_str) - + "disk images.") % { + "vmname": self.vm.get_name(), "pathlist": warn_str}) if not res: return False - cd.setup_clone() + for diskinfo in cloner.get_diskinfos(): + diskinfo.raise_error() - self.clone_design = cd - return True - - def _finish_cb(self, error, details, conn): + def _finish_cb(self, error, details, conn, cloner): self.reset_finish_cursor() if error is not None: error = (_("Error creating virtual machine clone '%(vm)s': " "%(error)s") % { - "vm": self.clone_design.clone_name, + "vm": cloner.new_guest.name, "error": error, }) self.err.show_err(error, details=details) @@ -830,37 +510,12 @@ class vmmCloneVM(vmmGObjectUI): conn.schedule_priority_tick(pollvm=True) self.close() - def finish(self, src_ignore): - try: - if not self.validate(): - return - except Exception as e: - self.err.show_err(_("Uncaught error validating input: %s") % str(e)) - return - - self.set_finish_cursor() - - title = (_("Creating virtual machine clone '%s'") % - self.clone_design.clone_name) - text = title - if self.clone_design.clone_disks: - text = (_("Creating virtual machine clone '%s' and selected " - "storage (this may take a while)") % - self.clone_design.clone_name) - - progWin = vmmAsyncJob(self._async_clone, [], - self._finish_cb, [self.conn], - title, text, self.topwin) - progWin.run() - - def _async_clone(self, asyncjob): + def _async_clone(self, asyncjob, cloner): meter = asyncjob.get_meter() refresh_pools = [] - for disk in self.clone_design.clone_disks: - if not disk.wants_storage_creation(): - continue - + for diskinfo in cloner.get_nonshare_diskinfos(): + disk = diskinfo.new_disk pool = disk.get_parent_pool() if not pool: continue @@ -869,25 +524,91 @@ class vmmCloneVM(vmmGObjectUI): if poolname not in refresh_pools: refresh_pools.append(poolname) - self.clone_design.start_duplicate(meter) + cloner.start_duplicate(meter) for poolname in refresh_pools: try: pool = self.conn.get_pool_by_name(poolname) self.idle_add(pool.refresh) - except Exception: + except Exception: # pragma: no cover log.debug("Error looking up pool=%s for refresh after " "VM clone.", poolname, exc_info=True) - def change_storage_browse(self, ignore): + def _build_final_cloner(self): + self._set_paths_from_clone_name() + + cloner = self._build_cloner() + for diskinfo in cloner.get_diskinfos(): + target = diskinfo.disk.target + sinfo = self._storage_list[target] + sinfo.set_values_on_diskinfo(diskinfo) + + cloner.prepare() + for diskinfo in cloner.get_diskinfos(): + diskinfo.raise_error() + + if self._validate(cloner) is False: + return + return cloner + + def _finish(self): + try: + cloner = self._build_final_cloner() + if not cloner: + return + except Exception as e: + msg = _("Error with clone settings: %s") % str(e) + return self.err.show_err(msg) + + self.set_finish_cursor() + + title = (_("Creating virtual machine clone '%s'") % + cloner.new_guest.name) + + text = title + if cloner.get_nonshare_diskinfos(): + text = (_("Creating virtual machine clone '%s' and selected " + "storage (this may take a while)") % + cloner.new_guest.name) + + progWin = vmmAsyncJob(self._async_clone, [cloner], + self._finish_cb, [self.conn, cloner], + title, text, self.topwin) + progWin.run() + + + ################ + # UI listeners # + ################ + + def _close_cb(self, src, event=None): + return self.close() + def _finish_clicked_cb(self, src): + return self._finish() + + def _storage_selection_changed_cb(self, src): + row = uiutil.get_list_selected_row(self.widget("storage-list")) + self.widget("storage-details-button").set_sensitive(bool(row)) + + def _storage_details_clicked_cb(self, src): + self._show_storage_window() + + def _storage_dialog_doclone_toggled_cb(self, src): + do_clone = src.get_active() + self.widget("change-storage-new").set_sensitive(do_clone) + self.widget("change-storage-browse").set_sensitive(do_clone) + + def _storage_dialog_close_cb(self, src, event=None): + return self._storage_dialog_close() + def _storage_dialog_finish_cb(self, src): + self._storage_dialog_finish() + + def _storage_dialog_browse_cb(self, ignore): def callback(src_ignore, txt): self.widget("change-storage-new").set_text(txt) - if self.storage_browser and self.storage_browser.conn != self.conn: - self.storage_browser.cleanup() - self.storage_browser = None - if self.storage_browser is None: - self.storage_browser = vmmStorageBrowser(self.conn) - self.storage_browser.set_finish_cb(callback) + if self._storage_browser is None: + self._storage_browser = vmmStorageBrowser(self.conn) + self._storage_browser.set_finish_cb(callback) - self.storage_browser.show(self.topwin) + self._storage_browser.show(self.topwin) diff --git a/virtinst/cloner.py b/virtinst/cloner.py index 6d160e61..f103df54 100644 --- a/virtinst/cloner.py +++ b/virtinst/cloner.py @@ -208,11 +208,13 @@ class _CloneDiskInfo: self.disk.set_backend_for_existing_path() self.new_disk = None - self._do_share_reason = _get_shareable_msg(self.disk) + self._share_msg = _get_shareable_msg(self.disk) + self._cloneable_msg = -1 + self._newpath_msg = None self._action = None self.set_clone_requested() - if self._do_share_reason: + if self.get_share_msg(): self.set_share_requested() def is_clone_requested(self): @@ -222,38 +224,51 @@ class _CloneDiskInfo: def is_preserve_requested(self): return self._action in [self._ACTION_PRESERVE] + def _set_action(self, action): + if action != self._action: + self._action = action def set_clone_requested(self): - self._action = self._ACTION_CLONE + self._set_action(self._ACTION_CLONE) def set_share_requested(self): - self._action = self._ACTION_SHARE + self._set_action(self._ACTION_SHARE) def set_preserve_requested(self): - self._action = self._ACTION_PRESERVE - - def check_cloneable(self): - try: - msg = _get_cloneable_msg(self.disk) - if msg: - raise ValueError(msg) - except Exception as e: - log.debug("Exception processing clone original path", exc_info=True) - err = _("Could not determine original disk information: %s" % str(e)) - raise ValueError(err) from None + self._set_action(self._ACTION_PRESERVE) def set_new_path(self, path, sparse): allow_create = not self.is_preserve_requested() if allow_create: - self.check_cloneable() + msg = self.get_cloneable_msg() + if msg: + return try: self.new_disk = Cloner.build_clone_disk( self.disk, path, allow_create, sparse) except Exception as e: log.debug("Error setting clone path.", exc_info=True) - raise ValueError( - _("Could not use path '%(path)s' for cloning: %(error)s") % { - "path": path, - "error": str(e), - }) + err = (_("Could not use path '%(path)s' for cloning: %(error)s") % + {"path": path, "error": str(e)}) + self._newpath_msg = err + + def get_share_msg(self): + return self._share_msg + def get_cloneable_msg(self): + if self._cloneable_msg == -1: + self._cloneable_msg = _get_cloneable_msg(self.disk) + return self._cloneable_msg + def get_newpath_msg(self): + return self._newpath_msg + + def raise_error(self): + if self.is_clone_requested() and self.get_cloneable_msg(): + msg = self.get_cloneable_msg() + err = _("Could not determine original disk information: %s" % msg) + raise ValueError(err) + if self.is_share_requested(): + return + if self.get_newpath_msg(): + msg = self.get_newpath_msg() + raise ValueError(msg) class Cloner(object): @@ -452,6 +467,7 @@ class Cloner(object): # can copy. It's valid for nvram to not exist at VM define # time, libvirt will create it for us diskinfo.set_new_path(new_nvram_path, self._sparse) + diskinfo.raise_error() diskinfo.new_disk.get_vol_install().reflink = self._reflink else: # There's no action to perform for this case, so drop it @@ -481,6 +497,9 @@ class Cloner(object): self.new_guest.name, orig_disk.path) diskinfo.set_new_path(newpath, self._sparse) + if not diskinfo.new_disk: + # We hit an error, clients will raise it later + continue new_disk = diskinfo.new_disk assert new_disk diff --git a/virtinst/virtclone.py b/virtinst/virtclone.py index ef26003c..a888d55d 100644 --- a/virtinst/virtclone.py +++ b/virtinst/virtclone.py @@ -52,11 +52,13 @@ def _process_disks(options, cloner): if origpath is None: newpath = None diskinfo.set_new_path(newpath, options.sparse) + diskinfo.raise_error() def _validate_disks(cloner): # Extra CLI validation for specified disks for diskinfo in cloner.get_diskinfos(): + diskinfo.raise_error() if not diskinfo.new_disk: continue warn_overwrite = not diskinfo.is_preserve_requested()