[Kimchi-devel] [PATCH 1/5 - v4] Fix Template backend create/update for multiple disks
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Nov 30 14:14:02 UTC 2015
On 30/11/2015 11:15, Rodrigo Trujillo wrote:
> 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 ?
>
It is not a good idea to change the API according to which HTTP method
to use.
By REST API, we understand the POST method will create a resource with a
given set of information (which we return on lookup()) and by PUT we
change the set of information already available.
So the JSON must be the same in all cases.
>>
>>> }
>>> -
>>> }
>>> },
>>> "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
>>
>
> _______________________________________________
> 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