Re: [virt-tools-list] [virt-manager PATCH] clone: keep the same image format on a cross-pool clone
Cole Robinson writes: > On 04/01/2015 09:13 AM, Giuseppe Scrivano wrote: >> Cole Robinson writes: >> >>> ACK, but I'm surprised the test suite doesn't need tweaking. maybe we should >>> extend a clone test to use the fake qemu URI so we can validate format >>> copying >>> in the output disk XML >> >> as follow-up, I've added a test for the cloned disks. OK to push it? >> >> Thanks, >> Giuseppe >> >> From 66c1ace004d4f2dccf48cd00ab32c7aeee503691 Mon Sep 17 00:00:00 2001 >> From: Giuseppe Scrivano >> Date: Wed, 1 Apr 2015 15:07:51 +0200 >> Subject: [PATCH] tests: check that clone keeps the same image format for >> disks >> >> Signed-off-by: Giuseppe Scrivano >> --- >> tests/clone-xml/cross-pool-disks-out.xml | 22 ++ >> tests/clone-xml/cross-pool-out.xml | 2 +- >> tests/clonetest.py | 21 - >> 3 files changed, 39 insertions(+), 6 deletions(-) >> create mode 100644 tests/clone-xml/cross-pool-disks-out.xml >> >> diff --git a/tests/clone-xml/cross-pool-disks-out.xml >> b/tests/clone-xml/cross-pool-disks-out.xml >> new file mode 100644 >> index 000..8eb3acb >> --- /dev/null >> +++ b/tests/clone-xml/cross-pool-disks-out.xml >> @@ -0,0 +1,22 @@ >> + >> + new1.img >> + 1073 >> + 1073 >> + >> + >> + >> + >> + >> + >> + >> + >> + new2.img >> + 100 >> + 5 >> + >> + >> + >> + >> + >> + >> + >> diff --git a/tests/clone-xml/cross-pool-out.xml >> b/tests/clone-xml/cross-pool-out.xml >> index 4200dce..5541adf 100644 >> --- a/tests/clone-xml/cross-pool-out.xml >> +++ b/tests/clone-xml/cross-pool-out.xml >> @@ -22,7 +22,7 @@ >> >> >> >> - >> + >> >> >> >> diff --git a/tests/clonetest.py b/tests/clonetest.py >> index 0119201..5789d41 100644 >> --- a/tests/clonetest.py >> +++ b/tests/clonetest.py >> @@ -1,4 +1,4 @@ >> -# Copyright (C) 2013 Red Hat, Inc. >> +# Copyright (C) 2013, 2015 Red Hat, Inc. >> # >> # This program is free software; you can redistribute it and/or modify >> # it under the terms of the GNU General Public License as published by >> @@ -55,7 +55,8 @@ class TestClone(unittest.TestCase): >> os.unlink(f) >> >> def _clone_helper(self, filebase, disks=None, force_list=None, >> - skip_list=None, compare=True, useconn=None): >> + skip_list=None, compare=True, useconn=None, >> + clone_disks_file=None): >> """Helper for comparing clone input/output from 2 xml files""" >> infile = os.path.join(clonexml_dir, filebase + "-in.xml") >> in_content = utils.read_file(infile) >> @@ -70,7 +71,8 @@ class TestClone(unittest.TestCase): >> cloneobj = self._default_clone_values(cloneobj, disks) >> >> if compare: >> -self._clone_compare(cloneobj, filebase) >> +self._clone_compare(cloneobj, filebase, >> +clone_disks_file=clone_disks_file) >> self._clone_define(filebase) >> else: >> cloneobj.setup() >> @@ -90,13 +92,18 @@ class TestClone(unittest.TestCase): >> cloneobj.clone_paths = disks >> return cloneobj >> >> -def _clone_compare(self, cloneobj, outbase): >> +def _clone_compare(self, cloneobj, outbase, clone_disks_file=None): >> """Helps compare output from passed clone instance with an xml >> file""" >> outfile = os.path.join(clonexml_dir, outbase + "-out.xml") >> >> cloneobj.setup() >> >> utils.diff_compare(cloneobj.clone_xml, outfile) >> +if clone_disks_file: >> +xml_clone_disks = "" >> +for i in cloneobj.get_clone_disks(): >> +xml_clone_disks += i.get_vol_install().get_xml_config() >> +utils.diff_compare(xml_clone_disks, clone_disks_file) >> >> def _clone_define(self, filebase): >> """Take the valid output xml and attempt to define it on the >> @@ -138,8 +145,12 @@ class TestClone(unittest.TestCase): >> >> def testCloneStorageCrossPool(self): >> base = "cross-pool" >> +useconn = utils.open_test_remote() >> +clone_disks_file = os.path.join(clonexml_dir, base + >> "-disks-out.xml") >> self._clone_helper(base, ["%s/new1.img" % POOL2, >> - "%s/new2.img" % POOL2]) >> + "%s/new2.img" % POOL1], >> + clone_disks_file=clone_disks_file, >> + useconn=useconn) >> >> def testCloneStorageForce(self): >> base = "force" >> > > ACK pushed. Thanks, Giuseppe ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] clone: keep the same image format on a cross-pool clone
On 04/01/2015 09:13 AM, Giuseppe Scrivano wrote: > Cole Robinson writes: > >> ACK, but I'm surprised the test suite doesn't need tweaking. maybe we should >> extend a clone test to use the fake qemu URI so we can validate format >> copying >> in the output disk XML > > as follow-up, I've added a test for the cloned disks. OK to push it? > > Thanks, > Giuseppe > > From 66c1ace004d4f2dccf48cd00ab32c7aeee503691 Mon Sep 17 00:00:00 2001 > From: Giuseppe Scrivano > Date: Wed, 1 Apr 2015 15:07:51 +0200 > Subject: [PATCH] tests: check that clone keeps the same image format for disks > > Signed-off-by: Giuseppe Scrivano > --- > tests/clone-xml/cross-pool-disks-out.xml | 22 ++ > tests/clone-xml/cross-pool-out.xml | 2 +- > tests/clonetest.py | 21 - > 3 files changed, 39 insertions(+), 6 deletions(-) > create mode 100644 tests/clone-xml/cross-pool-disks-out.xml > > diff --git a/tests/clone-xml/cross-pool-disks-out.xml > b/tests/clone-xml/cross-pool-disks-out.xml > new file mode 100644 > index 000..8eb3acb > --- /dev/null > +++ b/tests/clone-xml/cross-pool-disks-out.xml > @@ -0,0 +1,22 @@ > + > + new1.img > + 1073 > + 1073 > + > + > + > + > + > + > + > + > + new2.img > + 100 > + 5 > + > + > + > + > + > + > + > diff --git a/tests/clone-xml/cross-pool-out.xml > b/tests/clone-xml/cross-pool-out.xml > index 4200dce..5541adf 100644 > --- a/tests/clone-xml/cross-pool-out.xml > +++ b/tests/clone-xml/cross-pool-out.xml > @@ -22,7 +22,7 @@ > > > > - > + > > > > diff --git a/tests/clonetest.py b/tests/clonetest.py > index 0119201..5789d41 100644 > --- a/tests/clonetest.py > +++ b/tests/clonetest.py > @@ -1,4 +1,4 @@ > -# Copyright (C) 2013 Red Hat, Inc. > +# Copyright (C) 2013, 2015 Red Hat, Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -55,7 +55,8 @@ class TestClone(unittest.TestCase): > os.unlink(f) > > def _clone_helper(self, filebase, disks=None, force_list=None, > - skip_list=None, compare=True, useconn=None): > + skip_list=None, compare=True, useconn=None, > + clone_disks_file=None): > """Helper for comparing clone input/output from 2 xml files""" > infile = os.path.join(clonexml_dir, filebase + "-in.xml") > in_content = utils.read_file(infile) > @@ -70,7 +71,8 @@ class TestClone(unittest.TestCase): > cloneobj = self._default_clone_values(cloneobj, disks) > > if compare: > -self._clone_compare(cloneobj, filebase) > +self._clone_compare(cloneobj, filebase, > +clone_disks_file=clone_disks_file) > self._clone_define(filebase) > else: > cloneobj.setup() > @@ -90,13 +92,18 @@ class TestClone(unittest.TestCase): > cloneobj.clone_paths = disks > return cloneobj > > -def _clone_compare(self, cloneobj, outbase): > +def _clone_compare(self, cloneobj, outbase, clone_disks_file=None): > """Helps compare output from passed clone instance with an xml > file""" > outfile = os.path.join(clonexml_dir, outbase + "-out.xml") > > cloneobj.setup() > > utils.diff_compare(cloneobj.clone_xml, outfile) > +if clone_disks_file: > +xml_clone_disks = "" > +for i in cloneobj.get_clone_disks(): > +xml_clone_disks += i.get_vol_install().get_xml_config() > +utils.diff_compare(xml_clone_disks, clone_disks_file) > > def _clone_define(self, filebase): > """Take the valid output xml and attempt to define it on the > @@ -138,8 +145,12 @@ class TestClone(unittest.TestCase): > > def testCloneStorageCrossPool(self): > base = "cross-pool" > +useconn = utils.open_test_remote() > +clone_disks_file = os.path.join(clonexml_dir, base + > "-disks-out.xml") > self._clone_helper(base, ["%s/new1.img" % POOL2, > - "%s/new2.img" % POOL2]) > + "%s/new2.img" % POOL1], > + clone_disks_file=clone_disks_file, > + useconn=useconn) > > def testCloneStorageForce(self): > base = "force" > ACK - Cole ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] clone: keep the same image format on a cross-pool clone
Cole Robinson writes: > ACK, but I'm surprised the test suite doesn't need tweaking. maybe we should > extend a clone test to use the fake qemu URI so we can validate format copying > in the output disk XML as follow-up, I've added a test for the cloned disks. OK to push it? Thanks, Giuseppe >From 66c1ace004d4f2dccf48cd00ab32c7aeee503691 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 1 Apr 2015 15:07:51 +0200 Subject: [PATCH] tests: check that clone keeps the same image format for disks Signed-off-by: Giuseppe Scrivano --- tests/clone-xml/cross-pool-disks-out.xml | 22 ++ tests/clone-xml/cross-pool-out.xml | 2 +- tests/clonetest.py | 21 - 3 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 tests/clone-xml/cross-pool-disks-out.xml diff --git a/tests/clone-xml/cross-pool-disks-out.xml b/tests/clone-xml/cross-pool-disks-out.xml new file mode 100644 index 000..8eb3acb --- /dev/null +++ b/tests/clone-xml/cross-pool-disks-out.xml @@ -0,0 +1,22 @@ + + new1.img + 1073 + 1073 + + + + + + + + + new2.img + 100 + 5 + + + + + + + diff --git a/tests/clone-xml/cross-pool-out.xml b/tests/clone-xml/cross-pool-out.xml index 4200dce..5541adf 100644 --- a/tests/clone-xml/cross-pool-out.xml +++ b/tests/clone-xml/cross-pool-out.xml @@ -22,7 +22,7 @@ - + diff --git a/tests/clonetest.py b/tests/clonetest.py index 0119201..5789d41 100644 --- a/tests/clonetest.py +++ b/tests/clonetest.py @@ -1,4 +1,4 @@ -# Copyright (C) 2013 Red Hat, Inc. +# Copyright (C) 2013, 2015 Red Hat, Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -55,7 +55,8 @@ class TestClone(unittest.TestCase): os.unlink(f) def _clone_helper(self, filebase, disks=None, force_list=None, - skip_list=None, compare=True, useconn=None): + skip_list=None, compare=True, useconn=None, + clone_disks_file=None): """Helper for comparing clone input/output from 2 xml files""" infile = os.path.join(clonexml_dir, filebase + "-in.xml") in_content = utils.read_file(infile) @@ -70,7 +71,8 @@ class TestClone(unittest.TestCase): cloneobj = self._default_clone_values(cloneobj, disks) if compare: -self._clone_compare(cloneobj, filebase) +self._clone_compare(cloneobj, filebase, +clone_disks_file=clone_disks_file) self._clone_define(filebase) else: cloneobj.setup() @@ -90,13 +92,18 @@ class TestClone(unittest.TestCase): cloneobj.clone_paths = disks return cloneobj -def _clone_compare(self, cloneobj, outbase): +def _clone_compare(self, cloneobj, outbase, clone_disks_file=None): """Helps compare output from passed clone instance with an xml file""" outfile = os.path.join(clonexml_dir, outbase + "-out.xml") cloneobj.setup() utils.diff_compare(cloneobj.clone_xml, outfile) +if clone_disks_file: +xml_clone_disks = "" +for i in cloneobj.get_clone_disks(): +xml_clone_disks += i.get_vol_install().get_xml_config() +utils.diff_compare(xml_clone_disks, clone_disks_file) def _clone_define(self, filebase): """Take the valid output xml and attempt to define it on the @@ -138,8 +145,12 @@ class TestClone(unittest.TestCase): def testCloneStorageCrossPool(self): base = "cross-pool" +useconn = utils.open_test_remote() +clone_disks_file = os.path.join(clonexml_dir, base + "-disks-out.xml") self._clone_helper(base, ["%s/new1.img" % POOL2, - "%s/new2.img" % POOL2]) + "%s/new2.img" % POOL1], + clone_disks_file=clone_disks_file, + useconn=useconn) def testCloneStorageForce(self): base = "force" -- 2.1.0 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] clone: keep the same image format on a cross-pool clone
Cole Robinson writes: > ACK, but I'm surprised the test suite doesn't need tweaking. maybe we should > extend a clone test to use the fake qemu URI so we can validate format copying > in the output disk XML thanks for the review. I've pushed this for now, I'll take a look at a possible test case soon. Giuseppe ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] clone: keep the same image format on a cross-pool clone
On 03/31/2015 12:00 PM, Giuseppe Scrivano wrote: > Signed-off-by: Giuseppe Scrivano > --- > virtinst/cloner.py | 6 -- > virtinst/storage.py | 8 +--- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/virtinst/cloner.py b/virtinst/cloner.py > index 68a5786..c533e7e 100644 > --- a/virtinst/cloner.py > +++ b/virtinst/cloner.py > @@ -1,5 +1,5 @@ > # > -# Copyright 2013 Red Hat, Inc. > +# Copyright 2013, 2015 Red Hat, Inc. > # Copyright(c) FUJITSU Limited 2007. > # > # Cloning a virtual machine module. > @@ -347,9 +347,11 @@ class Cloner(object): > vol_install.name = clone_vol_install.name > else: > # Cross pool cloning > -# Deliberately don't sync input_vol params here > +# Sync only the format of the image. > clone_vol_install.input_vol = orig_disk.get_vol_object() > vol_install = clone_vol_install > +vol_install.input_vol = orig_disk.get_vol_object() > +vol_install.sync_input_vol(only_format=True) > > vol_install.reflink = self.reflink > clone_disk.set_vol_install(vol_install) > diff --git a/virtinst/storage.py b/virtinst/storage.py > index c07f558..8438896 100644 > --- a/virtinst/storage.py > +++ b/virtinst/storage.py > @@ -1,5 +1,5 @@ > # > -# Copyright 2008, 2013 Red Hat, Inc. > +# Copyright 2008, 2013, 2015 Red Hat, Inc. > # Cole Robinson > # > # This program is free software; you can redistribute it and/or modify > @@ -610,15 +610,17 @@ class StorageVolume(_StorageObject): > reflink = property(_get_reflink, _set_reflink, > doc="flags for VIR_STORAGE_VOL_CREATE_REFLINK") > > -def sync_input_vol(self): > +def sync_input_vol(self, only_format=False): > # Pull parameters from input vol into this class > parsevol = StorageVolume(self.conn, > parsexml=self._input_vol.XMLDesc(0)) > > +self.format = parsevol.format > +if only_format: > +return > self.pool = self._input_vol.storagePoolLookupByVolume() > self.capacity = parsevol.capacity > self.allocation = parsevol.allocation > -self.format = parsevol.format > > > ## > ACK, but I'm surprised the test suite doesn't need tweaking. maybe we should extend a clone test to use the fake qemu URI so we can validate format copying in the output disk XML - Cole ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
