On 04/05/2016 01:43 PM, Ramon Medeiros wrote:


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         """

This function should not exist! All the data validation MUST be done if scan is True or False.
The only difference when scan=False is we will not try to identify the OS and OS version on source_media (cdrom ou disk[base])

 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@br.ibm.com 


-- 

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