Re: [Kimchi-devel] [PATCH 1/5 - v4] Fix Template backend create/update for multiple disks

See comments below. Rodrigo Trujillo On 11/30/2015 09:02 AM, Aline Manera wrote: > > > On 30/11/2015 01:29, Rodrigo Trujillo wrote: >> This patch fixes the backend code in order to allow create templates >> with multiple disks and update them correctly. >> Backend supports adding multiple disks with a json like: >> >> "disks": [{"size": 3, >> "format": "qcow"}, >> {"size": 5, >> "pool":"/plugins/kimchi/storagepools/poolX"}, >> {"size": 7, >> "format": "raw", >> "pool": "/plugins/kimchi/storagepools/poolY"} >> ] >> >> If any information is missing, Kimchi will use values from >> template.conf. >> >> Kimchi will expose template disk data like: >> "disks": [{"index" : ... , >> "size" : ... , >> "format" : ... , >> "pool" : { >> "name" : ... , >> "type" : ... } , >> }, ... >> ] >> >> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; >> --- >> src/wok/plugins/kimchi/API.json | 19 ++++++++++++++++++- >> src/wok/plugins/kimchi/docs/API.md | 6 ++++++ >> src/wok/plugins/kimchi/model/templates.py | 9 +++++---- >> src/wok/plugins/kimchi/osinfo.py | 1 + >> src/wok/plugins/kimchi/vmtemplate.py | 26 >> +++++++++++++++++++++----- >> 5 files changed, 51 insertions(+), 10 deletions(-) >> >> diff --git a/src/wok/plugins/kimchi/API.json >> b/src/wok/plugins/kimchi/API.json >> index 7addfc7..6c6a5f7 100644 >> --- a/src/wok/plugins/kimchi/API.json >> +++ b/src/wok/plugins/kimchi/API.json >> @@ -477,6 +477,12 @@ >> "type": "integer", >> "minimum": 0 >> }, >> + "format": { >> + "description": "Type of the image >> of the disk", >> + "type": "string", >> + "pattern": >> "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$", >> + "error": "KCHTMPL0027E" >> + }, >> "size": { >> "description": "Size (GB) of the >> disk", >> "type": "number", >> @@ -488,8 +494,13 @@ >> "type": "string", >> "pattern": "^/.+$", >> "error": "KCHTMPL0023E" >> + }, > >> + "pool": { >> + "description": "Location (URI) of >> the storage pool", >> + "type": "string", >> + "pattern": >> "^/plugins/kimchi/storagepools/[^/]+/?$", >> + "error": "KCHTMPL0015E" > > 'pool' will be an object with 2 elements: 'name' and 'type'. Aline, I did the following decision: - To CREATE or UPDATE a template, you just need the pool URI, so I left "pool" as a single string parameter. - When you list a template (LOOKUP), it will return "pool: { name: xxx, type: yyy }" What do you think ?

> >> } >> - >> } >> }, >> "minItems": 1, >> @@ -660,6 +671,12 @@ >> "type": "string", >> "pattern": >> "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$", >> "error": "KCHTMPL0027E" >> + }, > >> + "pool": { >> + "description": "Location (URI) of >> the storage pool", >> + "type": "string", >> + "pattern": >> "^/plugins/kimchi/storagepools/[^/]+/?$", >> + "error": "KCHTMPL0015E" > > 'pool' will be an object with 2 elements: 'name' and 'type'. Explained above. > >> } >> } >> }, >> diff --git a/src/wok/plugins/kimchi/docs/API.md >> b/src/wok/plugins/kimchi/docs/API.md >> index 1e7d8ca..a4dd084 100644 >> --- a/src/wok/plugins/kimchi/docs/API.md >> +++ b/src/wok/plugins/kimchi/docs/API.md >> @@ -289,6 +289,8 @@ 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 >> + * format: Format of the image. Valid formats: bochs, cloop, >> cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc >> + * pool: URI of the storagepool where disk will be created > > 'pool' will be an object with 2 elements: 'name' and 'type'. Explained above. > >> * base: Base image of this disk >> >> * graphics *(optional)*: The graphics paramenters of this >> template >> @@ -388,6 +390,9 @@ A interface represents available network >> interface on VM. >> * size: The device size in GB >> * volume: A volume name that contains the initial disk >> contents >> * format: Format of the image. Valid formats: bochs, >> cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc. >> + * pool: Information about the pool where disk or volume >> will be created >> + * name: URI of the storagepool >> + * type: Type of the storagepool (dir, nfs, scsci, >> iscsi, etc) This is the information related to the GET >> * graphics: A dict of graphics paramenters of this template >> * type: The type of graphics. It can be VNC or spice or None. >> * vnc: Graphical display using the Virtual Network >> @@ -422,6 +427,7 @@ A interface represents available network >> interface on VM. >> * size: The device size in GB >> * volume: A volume name that contains the initial disk >> contents >> * format: Format of the image. Valid formats: bochs, >> cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc. > >> + * pool: URI of the storagepool where template allocates vm >> disk. > > 'pool' will be an object with 2 elements: 'name' and 'type'. Explained above. > >> * graphics *(optional)*: A dict of graphics paramenters of >> this template >> * type: The type of graphics. It can be VNC or spice or None. >> * vnc: Graphical display using the Virtual Network >> diff --git a/src/wok/plugins/kimchi/model/templates.py >> b/src/wok/plugins/kimchi/model/templates.py >> index 47b2c9e..bac6e47 100644 >> --- a/src/wok/plugins/kimchi/model/templates.py >> +++ b/src/wok/plugins/kimchi/model/templates.py >> @@ -234,8 +234,9 @@ class LibvirtVMTemplate(VMTemplate): >> self.conn = conn >> VMTemplate.__init__(self, args, scan) >> >> - def _storage_validate(self): >> - pool_uri = self.info['storagepool'] >> + def _storage_validate(self, pool_uri=None): >> + if pool_uri is None: >> + pool_uri = self.info['storagepool'] >> pool_name = pool_name_from_uri(pool_uri) >> try: >> conn = self.conn.get() >> @@ -278,8 +279,8 @@ class LibvirtVMTemplate(VMTemplate): >> xml = pool.XMLDesc(0) >> return xpath_get_text(xml, "/pool/target/path")[0] >> >> - def _get_storage_type(self): >> - pool = self._storage_validate() >> + def _get_storage_type(self, pool_uri=None): >> + pool = self._storage_validate(pool_uri) >> xml = pool.XMLDesc(0) >> return xpath_get_text(xml, "/pool/@type")[0] >> >> diff --git a/src/wok/plugins/kimchi/osinfo.py >> b/src/wok/plugins/kimchi/osinfo.py >> index 30ecd4f..5b0b5bf 100644 >> --- a/src/wok/plugins/kimchi/osinfo.py >> +++ b/src/wok/plugins/kimchi/osinfo.py >> @@ -158,6 +158,7 @@ def _get_tmpl_defaults(): >> for disk in storage_section.keys(): >> data = storage_section[disk] >> data['index'] = int(disk.split('.')[1]) >> + data['pool'] = defaults['storagepool'] >> defaults['disks'].append(data) >> >> # Parse processor section to get cpus and cpu_topology values >> diff --git a/src/wok/plugins/kimchi/vmtemplate.py >> b/src/wok/plugins/kimchi/vmtemplate.py >> index 3097b66..c7635b0 100644 >> --- a/src/wok/plugins/kimchi/vmtemplate.py >> +++ b/src/wok/plugins/kimchi/vmtemplate.py >> @@ -76,14 +76,30 @@ class VMTemplate(object): >> graphics.update(graph_args) >> args['graphics'] = graphics >> >> - # Merge disks dict >> + # Provide compatibility with old template version and sets >> data to >> + # correct output format if necessary >> + def _fix_disk_compatibilities(disks): >> + for i, disk in enumerate(disks): >> + if 'pool' not in disk: >> + disk['pool'] = {'name': self.info['storagepool']} >> + if (isinstance(disk['pool'], str) or >> + isinstance(disk['pool'], unicode)): >> + disk['pool'] = {'name': disk['pool']} >> + disk['pool']['type'] = \ >> + self._get_storage_type(disk['pool']['name']) >> + > >> + if self.info.get('disks') is not None: >> + _fix_disk_compatibilities(self.info['disks']) >> + if args.get('disks') is not None: >> + _fix_disk_compatibilities(args['disks']) >> + > > Minor comment: you can merge those 2 if's I double checked the code and I cannot see how I can merge the if's. Because, self.info['disks'] and arg['disks'] will not have the same information at this point of the code. > >> + # Merge disks dict with disks provided by user >> default_disk = self.info['disks'][0] >> for i, d in enumerate(args.get('disks', [])): >> disk = dict(default_disk) >> + disk['index'] = i >> disk.update(d) >> - >> - # Assign right disk format to logical and [i]scsi >> storagepools >> - if self._get_storage_type() in ['logical', 'iscsi', >> 'scsi']: >> + if disk['pool']['type'] in ['logical', 'iscsi', 'scsi']: >> disk['format'] = 'raw' >> args['disks'][i] = disk >> >> @@ -401,7 +417,7 @@ class VMTemplate(object): >> def _get_storage_path(self): >> return '' >> >> - def _get_storage_type(self): >> + def _get_storage_type(self, pool=None): >> return '' >> >> def _get_volume_path(self): > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel