[Kimchi-devel] [PATCH 1/2] Choose disk image type in vm template - backend

[Daniel H Barboza]

On 10/30/2014 05:13 PM, Aline Manera wrote:
>
>
> On 10/30/2014 10:05 AM, Daniel Henrique Barboza wrote:
>> These changes implements the backend support for setting the disk
>> image type in an existing VM template, by using a new parameter called
>> "type" in the 'disk' object of the template_update schema.
>>
>> Supported types: 'bochs', 'cloop', 'cow', 'dmg', 'qcow', 'qcow2',
>> 'qed', 'raw', 'vmdk', 'vpc'.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>> ---
>>   docs/API.md              | 1 +
>>   src/kimchi/API.json      | 8 +++++++-
>>   src/kimchi/i18n.py       | 1 +
>>   src/kimchi/vmtemplate.py | 5 ++++-
>>   4 files changed, 13 insertions(+), 2 deletions(-)
>
> Mockmodel is missing

No changes were made in any /src/kimchi/model class, thus I do not 
believe any
change in mockmodel.py is needed.
>
>>
>> diff --git a/docs/API.md b/docs/API.md
>> index 6984649..8e2962d 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -209,6 +209,7 @@ Represents a snapshot of the Virtual Machine's 
>> primary monitor.
>>           * index: The device index
>>           * size: The device size in GB
>>           * base: Base image of this disk
>> +        * type: Type of the image. Valid types: bochs, cloop, cow, 
>> dmg, qcow, qcow2, qed, raw, vmdk, vpc.
>
> "format" is more meaningful than "type"

I'll change it

>
> You need to update the GET and PUT methods too.
>
> GET /templates/<name> should return the disks dict with the "format" 
> information as well

The methods already returned the dict with the 'type' information. In 
the case it doesn't after
the rename to 'format', I'll update them.

>
>>
>>       * 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/API.json b/src/kimchi/API.json
>> index a1156d5..21491d1 100644
>> --- a/src/kimchi/API.json
>> +++ b/src/kimchi/API.json
>> @@ -208,7 +208,7 @@
>>                   "format": {
>>                       "description": "The format of the volume",
>>                       "type": "string",
>> -                    "pattern": "^qcow2|raw$",
>> +                    "pattern": 
>> "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$",
>>                       "error": "KCHVOL0015E"
>>                   },
>>                   "url": {
>> @@ -619,6 +619,12 @@
>>                                   "type": "integer",
>>                                   "minimum": 1,
>>                                   "error": "KCHTMPL0022E"
>> +                            },
>> +                            "type": {
>> +                                "description": "Type of the image of 
>> the disk",
>> +                                "type": "string",
>> +                                "pattern": 
>> "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$",
>> +                                "error": "KCHTMPL0027E"
>>                               }
>>                           }
>>                       },
>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
>> index 74ea98e..8ca5cc6 100644
>> --- a/src/kimchi/i18n.py
>> +++ b/src/kimchi/i18n.py
>> @@ -145,6 +145,7 @@ messages = {
>>       "KCHTMPL0024E": _("Cannot identify base image %(path)s format"),
>>       "KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be 
>> a product of sockets, cores, and threads."),
>>       "KCHTMPL0026E": _("When specifying CPU topology, each element 
>> must be an integer greater than zero."),
>> +    "KCHTMPL0027E": _("Invalid disk image type."),
>
> I think it is good to point to user what are the valid types.
Fair enough.
>
>>
>>       "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>>       "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
>> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
>> index b48cdbd..9635d67 100644
>> --- a/src/kimchi/vmtemplate.py
>> +++ b/src/kimchi/vmtemplate.py
>> @@ -195,7 +195,10 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>>               src = os.path.join(storage_path, volume)
>>               dev = "%s%s" % (self._bus_to_dev[self.info['disk_bus']],
>>                               string.lowercase[index])
>> -            fmt = 'raw' if self._get_storage_type() in ['logical'] 
>> else 'qcow2'
>> +            if self._get_storage_type() in ['logical']:
>> +                fmt = 'raw'
>> +            else:
>> +                fmt = disk.get('type', 'qcow2')
>>               params = {'src': src, 'dev': dev, 'bus': 
>> self.info['disk_bus'],
>>                         'type': fmt}
>>               ret += """
>