[Kimchi-devel] [PATCH v4][Kimchi 0/4] Use a single field to create a template

[Ramon Medeiros]

On 04/05/2016 01:28 PM, Aline Manera wrote:
>
>
> On 04/05/2016 09:52 AM, Ramon Medeiros wrote:
>>
>>
>> On 04/04/2016 03:52 PM, Aline Manera wrote:
>>>
>>> Hi Ramon,
>>>
>>> I just want to send you some tips on how to do that feature.
>>>
>>> 1. You are changing the way the template is created. Now 
>>> source_media is a required parameter and need to be properly added 
>>> on API.json
>> Cdrom can be removed, but disks need to stay. For example: if you 
>> want to create a template without disks?
>>
>
> You can remove the 'base' value available for disks.
But, what about scsi ? See the following test :

test_scsi_fc_storage (i ran it with pdb and saw the "disks" list. I 
can't understand how scsi templates can use source_media.
>
>> Or, for scsi installation, you need disks to specify the lun device.
>>>
>>> 2. source_media will replace cdrom and disk[base], so, please update 
>>> API.json to reflect it.
>>>
>>> 3. Following the Template creation flow, the request will first 
>>> touch on model/templates.py in create() function.
>>>
>>> The code below needs to be moved around as at this point you will 
>>> not know if source_media is an ISO or an Image file.
>>>
>>>         ...
>>>         iso = params.get('cdrom')
>>>         # check search permission
>>>         if iso and iso.startswith('/') and os.path.exists(iso):
>>>             st_mode = os.stat(iso).st_mode
>>>             if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode):
>>>                 user = UserTests().probe_user()
>>>                 run_setfacl_set_attr(iso, user=user)
>>>                 ret, excp = probe_file_permission_as_user(iso, user)
>>>                 if ret is False:
>>>                     raise InvalidParameter('KCHISO0008E',
>>>                                            {'filename': iso, 'user': 
>>> user,
>>>                                             'err': excp})
>> done
>>>
>>> 4. The second step is vmtemplate.py.
>>>      As you can see you will need to change the function 
>>> _get_os_info() to something like:
>>> *
>>> **        source_media = args.get('source_media)**
>>> **        if source_media is not None:**
>>> **            if source_media.startswith('http|https|ftp|...') or 
>>> <use python magic to identify it is an ISO> :**
>>> **                args[cdrom] = source_media**
>>> **            else:**
>>> **                # assume it is an image**
>>> **                args[disks] = {base: source_media}*
>> Nope, when this function run, source_media is also identified on 
>> cdrom or disks, so the code works like in the past.
>>
>
> Not sure I got your point here.
> When you receive a source_media you need to identify if it is a cdrom 
> or a disk, right?
> So this part of code is needed, correct?
>
Not it.

You suggest to identify the media inside the method, but, when this 
function is called, the media is already identified:

93     def _create_template(self, args, scan=False):
  94         """
  95         Creates a new template
  96         """
  97         # no source_media argument: raise error
  98         if args.get("source_media") is None:
  99             raise MissingParameter('KCHTMPL0016E')
100
101         # identify source media
102 *self._identify_installation_media(args)*
103
104         # Fetch defaults based on the os distro and version
105         try:
106             distro, version = self._*self._get_os_info(args, scan)*
107         except ImageFormatError as e:
108             raise OperationFailed('KCHTMPL0020E', {'err': e.message})
109         os_distro = args.get('os_distro', distro)
110         os_version = args.get('os_version', version)
111         entry = osinfo.lookup(os_distro, os_version)

>>>
>>>         # the code below keeps the same
>>>
>>> Does all that make sense for you?
>>>
>>> Regards,
>>> Aline Manera
>>>
>>> On 03/31/2016 10:47 AM, Ramon Medeiros wrote:
>>>> Instead of specify if the media is cdrom or disk, use source_media to create a template
>>>>
>>>> Changes:
>>>>
>>>> v2:
>>>> Remove libvirt connection from VMtemplate
>>>> Fix incorrect changes on tests
>>>> Fix return status when source_media not passed
>>>>
>>>> v3:
>>>> Fix pep8 issues
>>>> Remove/add some constants
>>>> Rewrite API message
>>>>
>>>>
>>>> Ramon Medeiros (4):
>>>>    Create a single field to pass the installation media
>>>>    Fix checking duplicate template before creating it
>>>>    Identify installation media while creating template
>>>>    Update tests
>>>>
>>>>   API.json                    |   5 ++
>>>>   i18n.py                     |   2 +-
>>>>   model/templates.py          |  15 ++++--
>>>>   tests/test_authorization.py |   4 +-
>>>>   tests/test_livemigration.py |   7 +--
>>>>   tests/test_mockmodel.py     |  13 +++---
>>>>   tests/test_model.py         |  94 ++++++++++++++++++++-----------------
>>>>   tests/test_rest.py          |  36 +++++++--------
>>>>   tests/test_template.py      |  47 +++++++++----------
>>>>   tests/test_vmtemplate.py    |  40 ++++++++--------
>>>>   vmtemplate.py               | 110 +++++++++++++++++++++++++++++++++++++-------
>>>>   11 files changed, 235 insertions(+), 138 deletions(-)
>>>>
>>>
>>
>> -- 
>>
>> Ramon Nunes Medeiros
>> Kimchi Developer
>> Linux Technology Center Brazil
>> IBM Systems & Technology Group
>> Phone : +55 19 2132 7878
>> ramonn at br.ibm.com  
>

-- 

Ramon Nunes Medeiros
Kimchi Developer
Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ramonn at br.ibm.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20160405/6c34910a/attachment.html>