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 <crobinso@redhat.com>
This commit is contained in:
Cole Robinson 2020-01-29 16:14:55 -05:00
parent 1e1cd4c564
commit 140a1f3b15
6 changed files with 16 additions and 25 deletions

View File

@ -38,8 +38,7 @@ Connect to a non-default hypervisor. See L<virt-install(1)> for details
=item B<--original> ORIGINAL_GUEST =item B<--original> ORIGINAL_GUEST
Name of the original guest to be cloned. This guest must be shut off or paused Name of the original guest to be cloned. This guest must be shut off.
since it is not possible to safely clone active guests at this time.
=item B<--original-xml> ORIGINAL_XML =item B<--original-xml> ORIGINAL_XML

View File

@ -1297,7 +1297,7 @@ _CLONE_NVRAM = "%s/clone-nvram-auto.xml" % XMLDIR
vclon = App("virt-clone") vclon = App("virt-clone")
c = vclon.add_category("remote", "--connect %(URI-TEST-REMOTE)s") 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_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
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-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_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("--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_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("--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 --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("--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_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("--auto-clone") # Just the auto flag
c.add_invalid("-o test --file foo") # Didn't specify new name c.add_invalid("-o test --clone-running --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 --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-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-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.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 = 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 --clone-running --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 --clone-running --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 --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 --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 %(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 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_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("--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_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") # Non-existent vm name
c.add_invalid("-o idontexist --auto-clone") # Non-existent vm name with auto flag, 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 + "") # 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 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 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

View File

@ -197,7 +197,7 @@ class TestClone(unittest.TestCase):
cloner = Cloner(conn) cloner = Cloner(conn)
cloner.original_guest = "test-snapshots" cloner.original_guest = "test-snapshots"
cloner.setup_original() 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: with self.assertRaises(ValueError) as err:
cloner = Cloner(conn) cloner = Cloner(conn)

View File

@ -1512,9 +1512,7 @@ class vmmDomain(vmmLibvirtObject):
def is_paused(self): def is_paused(self):
return self.status() in [libvirt.VIR_DOMAIN_PAUSED] return self.status() in [libvirt.VIR_DOMAIN_PAUSED]
def is_clonable(self): def is_clonable(self):
return self.status() in [libvirt.VIR_DOMAIN_SHUTOFF, return self.status() in [libvirt.VIR_DOMAIN_SHUTOFF]
libvirt.VIR_DOMAIN_PAUSED,
libvirt.VIR_DOMAIN_PMSUSPENDED]
def run_status(self): def run_status(self):
return LibvirtEnumMap.pretty_run_status( return LibvirtEnumMap.pretty_run_status(

View File

@ -297,15 +297,10 @@ class Cloner(object):
log.debug("Original sizes: %s", log.debug("Original sizes: %s",
[d.get_size() for d in self.original_disks]) [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:
if (not self.clone_running and
(self.original_dom and len(self.original_disks) != 0)):
status = self.original_dom.info()[0] status = self.original_dom.info()[0]
if status not in [libvirt.VIR_DOMAIN_SHUTOFF]:
if status not in [libvirt.VIR_DOMAIN_SHUTOFF, raise RuntimeError(_("Domain to clone must be shutoff."))
libvirt.VIR_DOMAIN_PAUSED]:
raise RuntimeError(_("Domain with devices to clone must be "
"paused or shutoff."))
def _setup_disk_clone_destination(self, orig_disk, clone_disk): def _setup_disk_clone_destination(self, orig_disk, clone_disk):
""" """

View File

@ -92,8 +92,7 @@ def parse_args():
geng = parser.add_argument_group(_("General Options")) geng = parser.add_argument_group(_("General Options"))
geng.add_argument("-o", "--original", dest="original_guest", geng.add_argument("-o", "--original", dest="original_guest",
help=_("Name of the original guest; " help=_("Name of the original guest to clone."))
"The status must be shut off or paused."))
geng.add_argument("--original-xml", geng.add_argument("--original-xml",
help=_("XML file to use as the original guest.")) help=_("XML file to use as the original guest."))
geng.add_argument("--auto-clone", action="store_true", geng.add_argument("--auto-clone", action="store_true",