From 140a1f3b15d47ea5d2d56bdf56d0484419023280 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Wed, 29 Jan 2020 16:14:55 -0500 Subject: [PATCH] cloner: Drop ability to clone a paused VM Generally this doesn't work with qemu metadata locking nowadays, and it was never a safe idea to begin with, because disk contents could be in an inconsistent state. https://bugzilla.redhat.com/show_bug.cgi?id=1725330 Signed-off-by: Cole Robinson --- man/virt-clone.pod | 3 +-- tests/test_cli.py | 18 +++++++++--------- tests/test_cloner.py | 2 +- virtManager/object/domain.py | 4 +--- virtinst/cloner.py | 11 +++-------- virtinst/virtclone.py | 3 +-- 6 files changed, 16 insertions(+), 25 deletions(-) diff --git a/man/virt-clone.pod b/man/virt-clone.pod index a4c518d12..01c414e2a 100644 --- a/man/virt-clone.pod +++ b/man/virt-clone.pod @@ -38,8 +38,7 @@ Connect to a non-default hypervisor. See L for details =item B<--original> ORIGINAL_GUEST -Name of the original guest to be cloned. This guest must be shut off or paused -since it is not possible to safely clone active guests at this time. +Name of the original guest to be cloned. This guest must be shut off. =item B<--original-xml> ORIGINAL_XML diff --git a/tests/test_cli.py b/tests/test_cli.py index a62247937..0392fac43 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1297,7 +1297,7 @@ _CLONE_NVRAM = "%s/clone-nvram-auto.xml" % XMLDIR vclon = App("virt-clone") c = vclon.add_category("remote", "--connect %(URI-TEST-REMOTE)s") -c.add_valid("-o test --auto-clone") # Auto flag, no storage +c.add_valid("-o test --clone-running --auto-clone") # Auto flag, no storage c.add_valid("--original-xml " + _CLONE_MANAGED + " --auto-clone") # Auto flag w/ managed storage c.add_invalid("--original-xml " + _CLONE_UNMANAGED + " --auto-clone") # Auto flag w/ local storage, which is invalid for remote connection c.add_invalid("--original-xml " + _CLONE_UNMANAGED + " --auto-clone") # Auto flag w/ local storage, which is invalid for remote connection @@ -1307,24 +1307,24 @@ c = vclon.add_category("misc", "") c.add_compare("--connect %(URI-KVM)s -o test-clone --auto-clone --clone-running", "clone-auto1") c.add_compare("--connect %(URI-TEST-FULL)s -o test-clone-simple --name newvm --auto-clone --clone-running", "clone-auto2") c.add_valid("--connect %(URI-KVM)s --original-xml " + _CLONE_NVRAM + " --auto-clone --clone-running") # hits a particular nvram code path -c.add_valid("-o test --auto-clone --uuid 12345678-12F4-1234-1234-123456789AFA --reflink --mac 12:34:56:1A:B2:C3") # Auto flag, no storage +c.add_valid("-o test --clone-running --auto-clone --uuid 12345678-12F4-1234-1234-123456789AFA --reflink --mac 12:34:56:1A:B2:C3") # Auto flag, no storage c.add_valid("--original-xml " + _CLONE_MANAGED + " --auto-clone") # Auto flag w/ managed storage c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --auto-clone") # Auto flag w/ local storage c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone --auto-clone --clone-running --nonsparse") # Auto flag, actual VM, skip state check c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone-simple -n newvm --preserve-data --file %(EXISTIMG1)s") # Preserve data shouldn't complain about existing volume c.add_valid("-n clonetest --original-xml " + _CLONE_UNMANAGED + " --file %(EXISTIMG3)s --file %(EXISTIMG4)s --check path_exists=off") # Skip existing file check c.add_invalid("--auto-clone") # Just the auto flag -c.add_invalid("-o test --file foo") # Didn't specify new name -c.add_invalid("-o test --auto-clone -n test") # new name raises error +c.add_invalid("-o test --clone-running --file foo") # Didn't specify new name +c.add_invalid("-o test --clone-running --auto-clone -n test") # new name raises error c.add_invalid("--connect %(URI-TEST-FULL)s -o test-many-devices --auto-clone") # VM is running, but --clone-running isn't passed c.add_invalid("--connect %(URI-TEST-FULL)s -o test-clone-simple -n newvm --file %(EXISTIMG1)s --clone-running") # Should complain about overwriting existing file c.add_invalid("--connect %(URI-TEST-REMOTE)s -o test-clone-simple --auto-clone --file /dev/default-pool/testvol9.img --check all=off", grep="Clone onto existing storage volume") # hit a specific error message c = vclon.add_category("general", "-n clonetest") -c.add_valid("-o test --auto-clone --replace") # Auto flag, no storage, --replace is redundant -c.add_valid("-o test --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s") # Nodisk, but with spurious files passed -c.add_valid("-o test --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s --prompt") # Working scenario w/ prompt shouldn't ask anything +c.add_valid("-o test --clone-running --auto-clone --replace") # Auto flag, no storage, --replace is redundant +c.add_valid("-o test --clone-running --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s") # Nodisk, but with spurious files passed +c.add_valid("-o test --clone-running --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s --prompt") # Working scenario w/ prompt shouldn't ask anything c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s") # XML File with 2 disks c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --file %(NEWCLONEIMG1)s --skip-copy=hda") # XML w/ disks, skipping one disk target c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --file virt-install --file %(EXISTIMG1)s --preserve") # XML w/ disks, overwriting existing files with --preserve @@ -1334,10 +1334,10 @@ c.add_valid("--original-xml " + _CLONE_MANAGED + " --file %(NEWIMG1)s") # XML w c.add_valid("--original-xml " + _CLONE_MANAGED + " --file %(NEWIMG1)s --reflink") # XML w/ managed storage, specify managed path c.add_valid("--original-xml " + _CLONE_NOEXIST + " --file %(EXISTIMG1)s --preserve") # XML w/ managed storage, specify managed path across pools# Libvirt test driver doesn't support cloning across pools# XML w/ non-existent storage, with --preserve c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone -n test --auto-clone --replace") # Overwriting existing running VM -c.add_invalid("-o test foobar") # Positional arguments error +c.add_invalid("-o test --clone-running foobar") # Positional arguments error c.add_invalid("-o idontexist") # Non-existent vm name c.add_invalid("-o idontexist --auto-clone") # Non-existent vm name with auto flag, -c.add_invalid("-o test -n test") # Colliding new name +c.add_invalid("-o test --clone-running -n test") # Colliding new name c.add_invalid("--original-xml " + _CLONE_UNMANAGED + "") # XML file with several disks, but non specified c.add_invalid("--original-xml " + _CLONE_UNMANAGED + " --file virt-install --file %(EXISTIMG1)s") # XML w/ disks, overwriting existing files with no --preserve c.add_invalid("--original-xml " + _CLONE_UNMANAGED + " --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s --force-copy=hdc") # XML w/ disks, force copy but not enough disks passed diff --git a/tests/test_cloner.py b/tests/test_cloner.py index cad852a18..c710fd4d0 100644 --- a/tests/test_cloner.py +++ b/tests/test_cloner.py @@ -197,7 +197,7 @@ class TestClone(unittest.TestCase): cloner = Cloner(conn) cloner.original_guest = "test-snapshots" cloner.setup_original() - self.assertTrue("paused or shutoff" in str(err.exception)) + self.assertTrue("must be shutoff" in str(err.exception)) with self.assertRaises(ValueError) as err: cloner = Cloner(conn) diff --git a/virtManager/object/domain.py b/virtManager/object/domain.py index 9349073a4..76d51dab4 100644 --- a/virtManager/object/domain.py +++ b/virtManager/object/domain.py @@ -1512,9 +1512,7 @@ class vmmDomain(vmmLibvirtObject): def is_paused(self): return self.status() in [libvirt.VIR_DOMAIN_PAUSED] def is_clonable(self): - return self.status() in [libvirt.VIR_DOMAIN_SHUTOFF, - libvirt.VIR_DOMAIN_PAUSED, - libvirt.VIR_DOMAIN_PMSUSPENDED] + return self.status() in [libvirt.VIR_DOMAIN_SHUTOFF] def run_status(self): return LibvirtEnumMap.pretty_run_status( diff --git a/virtinst/cloner.py b/virtinst/cloner.py index ce9a2e410..d830df3aa 100644 --- a/virtinst/cloner.py +++ b/virtinst/cloner.py @@ -297,15 +297,10 @@ class Cloner(object): log.debug("Original sizes: %s", [d.get_size() for d in self.original_disks]) - # If domain has devices to clone, it must be 'off' or 'paused' - if (not self.clone_running and - (self.original_dom and len(self.original_disks) != 0)): + if not self.clone_running and self.original_dom: status = self.original_dom.info()[0] - - if status not in [libvirt.VIR_DOMAIN_SHUTOFF, - libvirt.VIR_DOMAIN_PAUSED]: - raise RuntimeError(_("Domain with devices to clone must be " - "paused or shutoff.")) + if status not in [libvirt.VIR_DOMAIN_SHUTOFF]: + raise RuntimeError(_("Domain to clone must be shutoff.")) def _setup_disk_clone_destination(self, orig_disk, clone_disk): """ diff --git a/virtinst/virtclone.py b/virtinst/virtclone.py index 59a70d7d6..5f49d14f5 100644 --- a/virtinst/virtclone.py +++ b/virtinst/virtclone.py @@ -92,8 +92,7 @@ def parse_args(): geng = parser.add_argument_group(_("General Options")) geng.add_argument("-o", "--original", dest="original_guest", - help=_("Name of the original guest; " - "The status must be shut off or paused.")) + help=_("Name of the original guest to clone.")) geng.add_argument("--original-xml", help=_("XML file to use as the original guest.")) geng.add_argument("--auto-clone", action="store_true",