[Kimchi-devel] [PATCH 1/5 - v4] Fix Template backend create/update for multiple disks
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Mon Nov 30 13:15:30 UTC 2015
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 at linux.vnet.ibm.com>
>> ---
>> 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 at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list