[Kimchi-devel] [PATCH V1] Can not create a VM from a template with disks['volume'] parameters. #181

[Sheldon]

Reviewed-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>

I agree to remove disks["volume"].

If template allow disks["volume"], it should not be writable.

So what the disks["volume"] will be used?

only, one scenario, all vms base on this disks["volume"] image, it is a 
shared image, read-only.

Now kimchi do no support this one.

Anyway, the doc and the code should be consistence.



On 02/28/2014 05:50 PM, Shu Ming wrote:
> The latest code have removed disks["volume"] from API.json, but API.md
> was not updated accordingly. The in-consistence between API.json and
> API.md should be corrected first. Then, the invalid code in
> VMTemplate.validate_integrity() should also be cleaned.
> ---
>   docs/API.md              | 1 -
>   src/kimchi/vmtemplate.py | 9 ---------
>   tests/test_model.py      | 4 +---
>   tests/test_rest.py       | 4 +---
>   4 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index e97eace..fc39c65 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -161,7 +161,6 @@ Represents a snapshot of the Virtual Machine's primary monitor.
>         (either *size* or *volume* must be specified):
>           * index: The device index
>           * size: The device size in GB
> -        * volume: A volume name that contains the initial disk contents
>
>       * graphics *(optional)*: The graphics paramenters of this template
>           * type: The type of graphics. It can be VNC or spice or None.
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index f786fa5..3e52be5 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -361,15 +361,6 @@ class VMTemplate(object):
>           except Exception:
>               invalid['cdrom'] = [iso]
>
> -        # validate disks integrity
> -        volumes = []
> -        for disk in self.info['disks']:
> -            volume = disk.get("volume")
> -            if volume is not None and not os.path.exists(volume):
> -                volumes.append(volume)
> -        if volumes:
> -            invalid['disks'] = volumes
> -
>           self.info['invalid'] = invalid
>
>           return self.info
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 8bb1ccc..a08b3d9 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -468,8 +468,7 @@ class ModelTests(unittest.TestCase):
>               iso_gen.construct_fake_iso(iso, True, '12.04', 'ubuntu')
>
>               params = {'name': 'test', 'memory': 1024, 'cpus': 1,
> -                      'networks': ['test-network'], 'cdrom': iso,
> -                      'disks': [{'volume': iso}]}
> +                      'networks': ['test-network'], 'cdrom': iso}
>               inst.templates_create(params)
>               rollback.prependDefer(inst.template_delete, 'test')
>
> @@ -479,7 +478,6 @@ class ModelTests(unittest.TestCase):
>               info = inst.template_lookup('test')
>               self.assertEquals(info['invalid']['cdrom'], [iso])
>               self.assertEquals(info['invalid']['networks'], [net_name])
> -            self.assertEquals(info['invalid']['disks'], [iso])
>
>       @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 d59882b..54530f3 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -1088,8 +1088,7 @@ class RestTests(unittest.TestCase):
>           self.assertEquals(201, resp.status)
>
>           t = {'name': 'test', 'memory': 1024, 'cpus': 1,
> -             'networks': ['test-network'], 'cdrom': iso,
> -             'disks': [{'volume': iso}]}
> +             'networks': ['test-network'], 'cdrom': iso}
>
>           req = json.dumps(t)
>           resp = self.request('/templates', req, 'POST')
> @@ -1104,7 +1103,6 @@ class RestTests(unittest.TestCase):
>           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])
>
>           # Delete the template
>           resp = request(host, port, '/templates/test', '{}', 'DELETE')


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center