[Kimchi-devel] [PATCH] [Kimchi 1/2] Enhancement to /plugins/kimchi/interfaces

[Archana Singh]

Hi Aline,

Please find my understanding inline.

Thanks,

Archana Singh


On 08/30/2016 10:26 PM, Aline Manera wrote:
>
>
> On 08/30/2016 07:20 AM, sureshab at linux.vnet.ibm.com wrote:
>> From: Suresh Babu Angadi <sureshab at in.ibm.com>
>>
>> Per RFC: [Kimchi-devel] [RFC] changes to /plugins/kimchi/interfaces
>>
>> * this patch includes backend changes to add '_inuse' filter
>> * Updated API.md to include '_inuse' filter for /interfaces Collection
>> * modified api call to plugins/kimchi/interfaces?_inuse=false in
>>    kimchi.api.js, since unused interfaces are used to create virtual 
>> network
>>
>> Signed-off-by: Suresh Babu Angadi <sureshab at in.ibm.com>
>> ---
>>   docs/API.md             |  3 +++
>>   i18n.py                 |  1 +
>>   model/interfaces.py     | 20 ++++++++++++++++----
>>   ui/js/src/kimchi.api.js |  2 +-
>>   4 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/API.md b/docs/API.md
>> index 7bd677f..357c2b8 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -656,6 +656,9 @@ A interface represents available network 
>> interface on VM.
>>   **Methods:**
>>
>>   * **GET**: Retrieve a summarized list of current Interfaces
>> +    * Parameters: _inuse:  Filter interfaces list with availability. 
>> Currently supports true | false.
>> +                'true' option lists interfaces used by virtual 
>> networks,'false' option lists all
>> +                interfaces which are not currently in use by virtual 
>> networks.
>>
>>   ### Resource: Interface
>>
>> diff --git a/i18n.py b/i18n.py
>> index e8d9c05..aee6ead 100644
>> --- a/i18n.py
>> +++ b/i18n.py
>> @@ -257,6 +257,7 @@ messages = {
>>       "KCHVOL0029E": _("Unable to upload chunk data to storage 
>> volume. Details: %(err)s."),
>>
>>       "KCHIFACE0001E": _("Interface %(name)s does not exist"),
>> +    "KCHIFACE0002E": _("Failed to list interfaces. Invalid _inuse 
>> parameter. Supported options for _inuse are: %(supported_inuse)s"),
>>
>>       "KCHNET0001E": _("Network %(name)s already exists"),
>>       "KCHNET0002E": _("Network %(name)s does not exist"),
>> diff --git a/model/interfaces.py b/model/interfaces.py
>> index 2d74fd4..6be4356 100644
>> --- a/model/interfaces.py
>> +++ b/model/interfaces.py
>> @@ -17,10 +17,11 @@
>>   # License along with this library; if not, write to the Free Software
>>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> 02110-1301 USA
>>
>> -from wok.exception import NotFoundError
>> +from wok.exception import InvalidParameter, NotFoundError
>>
>>   from wok.plugins.gingerbase import netinfo
>>   from wok.plugins.kimchi.model.networks import NetworksModel
>> +from wok.utils import wok_log
>>
>>
>>   class InterfacesModel(object):
>> @@ -28,9 +29,20 @@ class InterfacesModel(object):
>>           self.conn = kargs['conn']
>>           self.networks = NetworksModel(**kargs)
>>
>> -    def get_list(self):
>> -        return list(set(netinfo.all_favored_interfaces()) -
>> - set(self.networks.get_all_networks_interfaces()))
>> +    def get_list(self, _inuse=None):
>> +        if _inuse == "true":
>> +            return list(set(netinfo.all_favored_interfaces()) &
>> + set(self.networks.get_all_networks_interfaces()))
>> +        elif _inuse == "false":
>> +            return list(set(netinfo.all_favored_interfaces()) -
>> + set(self.networks.get_all_networks_interfaces()))
>> +        elif _inuse is None:
>> +            return list(set(netinfo.all_favored_interfaces()))
>> +        else:
>
> Should't the case of "_inuse == true" be the same of "_unse == None" ?
>
> So we could simplify the code as below:
>
> if _inuse == false:
>    return list(set(netinfo.all_favored_interfaces())
> set(self.networks.get_all_networks_interfaces()))
>
> return list(set(netinfo.all_favored_interfaces()))
>
> Tip: You don't need the 'else' statement when you have a 'return' in 
> the 'if' statement.
If I understood correctly, in case of _inuse filter is not passed(i.e 
None), it should list all favored interfaces (i.e both in use and not  
in use)
in case of _inuse=true, it should list interfaces which are already used 
by virtual networks, i.e only those favored interfaces which are present 
in set(self.networks.get_all_networks_interfaces()))
in case of _inuse=false, it should list interfaces which are not used by 
virtual networks, i.e only those favored interfaces which are not 
present in set(self.networks.get_all_networks_interfaces()).

>
>> +            wok_log.error("Invalid filter _inuse. _inuse: %s. 
>> Supported"
>> +                          " options are %s" % (_inuse, 'true/false'))
>> +            raise InvalidParameter("KCHIFACE0002E",
>> +                                   {'supported_inuse': ['true', 
>> 'false']}) 
Also it ensures that if _inuse is passed it should only be true or false 
or it should not be passed, other value if passed in _inuse filter then 
it should throw error.

>>
>>
>>   class InterfaceModel(object):
>> diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js
>> index 740d922..fd3e5d7 100644
>> --- a/ui/js/src/kimchi.api.js
>> +++ b/ui/js/src/kimchi.api.js
>> @@ -626,7 +626,7 @@ var kimchi = {
>>
>>       getInterfaces : function(suc, err) {
>>           wok.requestJSON({
>> -            url : 'plugins/kimchi/interfaces',
>> +            url : 'plugins/kimchi/interfaces?_inuse=false',
>>               type : 'GET',
>>               contentType : 'application/json',
>>               dataType : 'json',
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>