[PATCH 0/3] template integrity verification: verify storagepool

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template. By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry. ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+) -- 1.8.4.2

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- docs/API.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/API.md b/docs/API.md index e97eace..af9162b 100644 --- a/docs/API.md +++ b/docs/API.md @@ -253,6 +253,7 @@ A interface represents available network interface on VM. * networks *(optional)*: An array of invalid network names. * cdrom *(optional)*: An array of invalid cdrom names. * disks *(optional)*: An array of invalid volume names. + * storagepools *(optional)*: An array of invalid storagepool names. * **DELETE**: Remove the Template * **POST**: *See Template Actions* -- 1.8.4.2

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> update mockmodel and model Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index b23a024..762f839 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -859,6 +859,9 @@ class MockVMTemplate(VMTemplate): def _get_all_networks_name(self): return self.model.networks_get_list() + def _get_all_storagepools_name(self): + return self.model.storagepools_get_list() + def _storage_validate(self): pool_uri = self.info['storagepool'] pool_name = pool_name_from_uri(pool_uri) diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index eac1a2a..9177e46 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -163,6 +163,11 @@ class LibvirtVMTemplate(VMTemplate): conn = self.conn.get() return sorted(conn.listNetworks() + conn.listDefinedNetworks()) + def _get_all_storagepools_name(self): + conn = self.conn.get() + names = conn.listStoragePools() + conn.listDefinedStoragePools() + return sorted(map(lambda x: x.decode('utf-8'), names)) + def _network_validate(self): names = self.info['networks'] for name in names: diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index af07ee3..b8689d0 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -31,6 +31,7 @@ from kimchi import isoinfo from kimchi import osinfo from kimchi.exception import InvalidParameter, IsoFormatError from kimchi.isoinfo import IsoImage +from kimchi.utils import pool_name_from_uri QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'" @@ -348,6 +349,9 @@ class VMTemplate(object): def _get_all_networks_name(self): return [] + def _get_all_storagepools_name(self): + return [] + def validate_integrity(self): invalid = {} # validate networks integrity @@ -356,6 +360,12 @@ class VMTemplate(object): if invalid_networks: invalid['networks'] = invalid_networks + # validate storagepools integrity + pool_uri = self.info['storagepool'] + pool_name = pool_name_from_uri(pool_uri) + if pool_name not in self._get_all_storagepools_name(): + invalid['storagepools'] = [pool_name] + # validate iso integrity # FIXME when we support multiples cdrom devices iso = self.info['cdrom'] -- 1.8.4.2

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> update test_model.py and test_rest.py Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index 74e2424..0646e2c 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -459,6 +459,12 @@ class ModelTests(unittest.TestCase): 'subnet': '127.0.100.0/24'} inst.networks_create(net_args) + pool_name = 'kimchi-test-pool' + pool_args = {'name': pool_name, + 'path': '/tmp/kimchi-images', + 'type': 'dir'} + inst.storagepools_create(pool_args ) + path = '/tmp/kimchi-iso/' if not os.path.exists(path): os.makedirs(path) @@ -467,17 +473,20 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'memory': 1024, 'cpus': 1, 'networks': ['test-network'], 'cdrom': iso, + 'storagepool': '/storagepools/%s' % pool_name, 'disks': [{'volume': iso}]} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') inst.network_delete(net_name) + inst.storagepool_delete(pool_name) shutil.rmtree(path) info = inst.template_lookup('test') self.assertEquals(info['invalid']['cdrom'], [iso]) self.assertEquals(info['invalid']['networks'], [net_name]) self.assertEquals(info['invalid']['disks'], [iso]) + self.assertEquals(info['invalid']['storagepools'], [pool_name]) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_clone(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index ca96dc0..528056f 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1089,8 +1089,15 @@ class RestTests(unittest.TestCase): resp = request(host, port, '/networks', req, 'POST') self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-storagepool', + 'path': '/tmp/kimchi-images', + 'type': 'dir'}) + resp = request(host, port, '/storagepools', req, 'POST') + self.assertEquals(201, resp.status) + t = {'name': 'test', 'memory': 1024, 'cpus': 1, 'networks': ['test-network'], 'cdrom': iso, + 'storagepool': '/storagepools/test-storagepool', 'disks': [{'volume': iso}]} req = json.dumps(t) @@ -1102,11 +1109,17 @@ class RestTests(unittest.TestCase): resp = request(host, port, '/networks/test-network', '{}', 'DELETE') self.assertEquals(204, resp.status) + # Delete the storagepool + resp = request(host, port, '/storagepools/test-storagepool', + '{}', 'DELETE') + self.assertEquals(204, resp.status) + # Verify the template res = json.loads(self.request('/templates/test').read()) self.assertEquals(res['invalid']['cdrom'], [iso]) self.assertEquals(res['invalid']['networks'], ['test-network']) self.assertEquals(res['invalid']['disks'], [iso]) + self.assertEquals(res['invalid']['storagepools'], ['test-storagepool']) # Delete the template resp = request(host, port, '/templates/test', '{}', 'DELETE') -- 1.8.4.2

2014/2/26 11:08, shaohef@linux.vnet.ibm.com:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry. It seems the two lines above confused me.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)

On 02/26/2014 03:53 PM, Shu Ming wrote:
2014/2/26 11:08, shaohef@linux.vnet.ibm.com:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry. It seems the two lines above confused me. This is cover letter message.
For template integrity, we need to verify storagepool exist, we do not need to check the all volume ref_cnt. (royce's patch) But for delete storagepool, we need to check the all volume ref_cnt is zero. This will be another patch for storagepool delete verification later.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

On 02/26/2014 04:22 PM, Sheldon wrote:
On 02/26/2014 03:53 PM, Shu Ming wrote:
2014/2/26 11:08, shaohef@linux.vnet.ibm.com:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry. It seems the two lines above confused me. This is cover letter message.
For template integrity, we need to verify storagepool exist, we do not need to check the all volume ref_cnt. (royce's patch)
But for delete storagepool, we need to check the all volume ref_cnt is zero. This will be another patch for storagepool delete verification later.
I thought template integrity and delete storagepool are both depends on royce's patch(volume ref_cnt) at first. And my thought is wrong.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/26/2014 12:08 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)

Please, rebase and send again. On 02/26/2014 12:08 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)

On 03/04/2014 10:17 PM, Aline Manera wrote:
Please, rebase and send again.
done. see V2 version. Thanks.
On 02/26/2014 12:08 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
Sometimes, user create a template, storagepool will change later. So users can not create a vm from this template successfully. It is necessary to check storagepool of a template.
By the way storagepool, this verification does not depend on volume ref_cnt Storagepool delete depends on volume ref_cnt I make a mistake. So sorry.
ShaoHe Feng (3): template integrity verification: verify storagepool, update API.md template integrity verification: verify storagepool in backend template integrity verification: update test case to verify storagepool
docs/API.md | 1 + src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/templates.py | 5 +++++ src/kimchi/vmtemplate.py | 10 ++++++++++ tests/test_model.py | 9 +++++++++ tests/test_rest.py | 13 +++++++++++++ 6 files changed, 41 insertions(+)
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
participants (4)
-
Aline Manera
-
shaohef@linux.vnet.ibm.com
-
Sheldon
-
Shu Ming