[Kimchi-devel] [PATCH] [Kimchi] Issue #791: New UI for adding network bridges

Lucio Correia luciojhc at linux.vnet.ibm.com
Mon Feb 8 17:49:38 UTC 2016


Hi  Peter,

Please use V2, V3 and so on when sending updated versions of patches 
already sent, including the list of changes from the prior version.

I have some comments below.

On 08-02-2016 13:03, peterpnns at gmail.com wrote:
> From: peterpennings <peterpnns at gmail.com>
>
> This commit adds the new UI changes for network bridges and enables OVS bridges that where temporary hidden
>
> Signed-off-by: peterpennings <peterpnns at gmail.com>
> ---
>   netinfo.py                           |   3 +-
>   ui/js/src/kimchi.network.js          |   2 +
>   ui/js/src/kimchi.network_add_main.js | 159 +++++++++++++++++++++--------------
>   ui/pages/network-add.html.tmpl       |   5 +-
>   4 files changed, 103 insertions(+), 66 deletions(-)
>
> diff --git a/netinfo.py b/netinfo.py
> index 2d83466..c00f603 100644
> --- a/netinfo.py
> +++ b/netinfo.py
> @@ -221,8 +221,7 @@ def is_bare_nic(iface):
>   #  a slave of bond.
>   #  The bridge will not be exposed when all it's port are tap.
>   def all_favored_interfaces():
> -    return list(set(aggregated_bridges() + bare_nics() + bondings()) -
> -                set(ovs_bridges()))
> +    return aggregated_bridges() + bare_nics() + bondings()
>
>
>   def get_interface_type(iface):
> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
> index 923aeb9..96c3fea 100644
> --- a/ui/js/src/kimchi.network.js
> +++ b/ui/js/src/kimchi.network.js
> @@ -17,6 +17,8 @@
>    */
>
>   kimchi.NETWORK_TYPE_MACVTAP = "macvtap";
> +kimchi.NETWORK_TYPE_BRIDGED = "bridge";
> +
>
>   kimchi.initNetwork = function() {
>       if(wok.tabMode['network'] === 'admin') {
> diff --git a/ui/js/src/kimchi.network_add_main.js b/ui/js/src/kimchi.network_add_main.js
> index 76d436f..33b4946 100644
> --- a/ui/js/src/kimchi.network_add_main.js
> +++ b/ui/js/src/kimchi.network_add_main.js
> @@ -31,20 +31,20 @@ kimchi.startNetworkCreation = function() {
>       var network = kimchi.getNetworkDialogValues();
>       var data = {
>           name : network.name,
> -        connection: network.type
> +        connection: network.type,
> +        interface: network.interface,
> +        vlan_id: network.vlan_id
>       };
> -    if (network.type === kimchi.NETWORK_TYPE_MACVTAP) {
> -        data.connection = "macvtap";
> -        data.interface = network.interface;
> -        if ($("#enableVlan").prop("checked")) {
> -            data.vlan_id = network.vlan_id;
> -            if (!(data.vlan_id >=1 && data.vlan_id <= 4094)) {
> -                wok.message.error.code('KCHNET6001E');
> -                errorCallback();
> -                return;
> -            }
> +
> +    if (network.type === kimchi.NETWORK_TYPE_BRIDGED && $("#enableVlan").prop("checked")) {
> +        data.vlan_id = network.vlan_id;
This line is not necessary, it is repetitive.


> +        if (!(data.vlan_id >=1 && data.vlan_id <= 4094)) {
consequently this could be an && instead of another if



> +            wok.message.error(i18n['KCHNET6001E'],'#alert-modal-container');
> +            errorCallback();
> +            return;
>           }
>       }
> +
>       kimchi.createNetwork(data, function(result) {
>           network.state = result.state === "active" ? "up" : "down";
>           network.interface = result.interface ? result.interface : i18n["KCHNET6001M"];
> @@ -59,30 +59,8 @@ kimchi.startNetworkCreation = function() {
>   };
>
>   kimchi.openNetworkDialog = function(okCallback) {
> -        kimchi.getInterfaces(function(result) {
> -        var options = [];
> -        $selectDestination = $('#networkDestinationID');
> -        var nics = {};
> -        var selectDestinationOptionHTML = '';
> -        for (var i = 0; i < result.length; i++) {
> -            options.push({label:result[i].name,value:result[i].name});
> -            nics[result[i].name] = result[i];
> -            selectDestinationOptionHTML += '<option value="'+ result[i].name + '">' + result[i].name + '</option>';
> -        }
> -        $selectDestination.append(selectDestinationOptionHTML);
> -        $selectDestination.selectpicker();
> -        onChange = function() {
> -            if (result.length>0 && $selectDestination.val() !== "") {
> -                $("#enableVlan").prop("disabled",false);
> -                $("#networkVlanID").val("");
> -            } else {
> -                $("#enableVlan").prop("disabled",true);
> -            }
> -        };
> -        $("#networkDestinationID").on("change", onChange);
> -        kimchi.setDefaultNetworkType(result.length!==0);
> -        onChange();
> -    });
> +    kimchi.loadInterfaces();
> +
>       $("#networkFormOk").on("click", function() {
>           $("#networkFormOk").button("disable");
>           $("#networkName").prop("readonly", "readonly");
> @@ -98,33 +76,12 @@ kimchi.openNetworkDialog = function(okCallback) {
>       });
>   };
>
> -kimchi.enableBridgeOptions = function(enable) {
> -    $("#enableVlan").prop("checked", false);
> -    $("#networkVlanID").val("");
> -    if (enable) {
> -        $('#bridgedContent').slideDown(300);
> -        $('#enableVlan').prop("disabled", false);
> -        $('#networkVlanID').prop("disabled", true);
> -    } else {
> -        $('#bridgedContent').slideUp(300);
> -        $('#enableVlan').prop("disabled", true);
> -        $('#networkVlanID').prop("disabled", true);
> -    }
> -};
> -
> -
>   kimchi.setDefaultNetworkType = function(isInterfaceAvail) {
> -    $("#networkType").val('macvtap', isInterfaceAvail);
> -    $("#networkType option:contains('bridged')").prop("disabled", !isInterfaceAvail);
> -    $("#networkType").val('nat', !isInterfaceAvail);
>       $("#networkType").selectpicker();
>       if (!isInterfaceAvail) {
>           kimchi.enableBridgeOptions(false);
>           $("#networkBriDisabledLabel").removeClass('hidden');
>       } else {
> -        if (kimchi.capabilities && kimchi.capabilities.nm_running) {
> -            wok.message.warn(i18n['KCHNET6001W'],'#alert-modal-container');
> -        }
>           $("#networkBriDisabledLabel").remove();
>       }
>   };
> @@ -135,24 +92,44 @@ kimchi.getNetworkDialogValues = function() {
>           type : $("#networkType").val()
>       };
>       if (network.type === kimchi.NETWORK_TYPE_MACVTAP) {
> -        network.interface = $("#networkDestinationID").val();
> -        network.vlan_id = parseInt($("#networkVlanID").val());
> +        network.interface = $("#networkDestinationID").find(':selected').data('type');
> +    }
> +    if (network.type === kimchi.NETWORK_TYPE_BRIDGED) {
> +        network.interface = $("#networkDestinationID").find(':selected').data('type');

I believe network.interface should be the name of interface, not type.

I got this error testing your patch:
KCHNET0020E: Failed to activate interface bridge: Cannot find device 
"bridge" .

> +        if ($("#enableVlan").prop("checked") && (network.interface === 'nic' || network.interface === 'bonding')) {
> +            network.vlan_id = parseInt($("#networkVlanID").val());
> +        }
>       }
>       return network;
>   };
>
>   kimchi.setupNetworkFormEvent = function() {
> +    if (kimchi.capabilities && kimchi.capabilities.nm_running) {
> +            wok.message.warn(i18n['KCHNET6001W'],'#alert-modal-container');
> +    }
>       $('#bridgedContent').hide();
>       $("#networkName").on("keyup", function(event) {
>           $("#networkName").toggleClass("invalid-field", !$("#networkName").val().match(/^[^\"\/]+$/));
>           kimchi.updateNetworkFormButton();
>       });
> -    $('#networkType').on('change', function() {
> -        var selectedType = $(this).val();
> +
> +    $('#networkType, #networkDestinationID').on('change', function() {
> +        var selectedType = $("#networkType").val();
> +        var selectedDestination = $("#networkDestinationID").find(':selected').data('type');
>           if(selectedType ==  'isolated' ||  selectedType ==  'nat') {
>               kimchi.enableBridgeOptions(false);
> -        } else if (selectedType ==  'macvtap') {
> -            kimchi.enableBridgeOptions(true);
> +        } else {
> +            kimchi.enableBridgeOptions(true, selectedType, selectedDestination);
> +        }
> +
> +    });
> +
> +    $('#networkType').on('change', function() {
> +        var selectedType = $("#networkType").val();
> +        if(selectedType === kimchi.NETWORK_TYPE_MACVTAP) {
> +            kimchi.loadInterfaces(new Array("nic", "bonding"));
> +        } else {
> +            kimchi.loadInterfaces();
>           }
>       });
>   };
> @@ -164,3 +141,61 @@ kimchi.updateNetworkFormButton = function() {
>           $("#networkFormOk").button("enable");
>       }
>   };
> +
> +kimchi.enableBridgeOptions = function(enable, networkType, networkDestination) {
> +    $("#enableVlan").prop("checked", false);
> +    $("#networkVlanID").val("");
> +    $('#vlan').hide();
> +    if (enable) {
> +        $('#bridgedContent').slideDown(300);
> +        $('#enableVlan').prop("disabled", false);
> +        $('#networkVlanID').prop("disabled", true);
> +        if (networkType === kimchi.NETWORK_TYPE_BRIDGED && (networkDestination === 'nic' || networkDestination === 'bonding')) {
> +            $('#vlan').show();
> +        }
> +    } else {
> +        $('#bridgedContent').slideUp(300);
> +        $('#enableVlan').prop("disabled", true);
> +        $('#networkVlanID').prop("disabled", true);
> +    };
> +
> +};
> +
> +kimchi.loadInterfaces = function(interfaceFilterArray) {
> +   kimchi.getInterfaces(function(result) {
> +
> +        var options = [];
> +        $selectDestination = $('#networkDestinationID');
> +        var nics = {};
> +        $('#networkDestinationID').find('option').remove();
> +        var selectDestinationOptionHTML = '';
> +        for (var i = 0; i < result.length; i++) {
> +            if (typeof interfaceFilterArray === 'undefined') {
> +                options.push({label:result[i].name,value:result[i].name});
> +                nics[result[i].name] = result[i];
> +                selectDestinationOptionHTML += '<option data-type="'+ result[i].type +'" value="'+ result[i].name + '">' + result[i].name + '</option>';
> +            } else {
> +                for (var k = 0; k < interfaceFilterArray.length; k++) {
> +                    if (result[i].type == interfaceFilterArray[k]) {
> +                        options.push({label:result[i].name,value:result[i].name});
> +                        nics[result[i].name] = result[i];
> +                        selectDestinationOptionHTML += '<option data-type="'+ result[i].type +'" value="'+ result[i].name + '">' + result[i].name + '</option>';
> +                    }
> +                }
> +            }
> +
> +        }
> +        $selectDestination.append(selectDestinationOptionHTML);
> +        $('#networkDestinationID').selectpicker('refresh');
> +        onChange = function() {
> +            if (result.length>0 && $selectDestination.val() !== "") {
> +                $("#enableVlan").prop("disabled",false);
> +                $("#networkVlanID").val("");
> +            } else {
> +                $("#enableVlan").prop("disabled",true);
> +            }
> +        };
> +        kimchi.setDefaultNetworkType(result.length!==0);
> +        onChange();
> +    });
> +};
> \ No newline at end of file
> diff --git a/ui/pages/network-add.html.tmpl b/ui/pages/network-add.html.tmpl
> index 7158b75..6bd2acf 100644
> --- a/ui/pages/network-add.html.tmpl
> +++ b/ui/pages/network-add.html.tmpl
> @@ -40,7 +40,8 @@
>               <select id="networkType" class="selectpicker col-md-12 col-lg-12">
>                   <option value="isolated">$_("Isolated: no external network connection")</option>
>                   <option value="nat">$_("NAT: outbound physical network connection only")</option>
> -                <option value="macvtap">$_("Bridged: Virtual machines are connected to physical network directly")</option>
> +                <option value="macvtap">$_("Macvtap: Virtual machines are connected to physical network directly")</option>
> +                <option value="bridge">$_("Bridged: Virtual machines are connected through a network bridge")</option>
>               </select>
>           </div>
>           <div id="networkBriDisabledLabel" class="form-group hidden">
> @@ -52,7 +53,7 @@
>                   <select id="networkDestinationID"  class="selectpicker col-md-12 col-lg-12">
>                   </select>
>               </div>
> -            <div class="form-group">
> +            <div class="form-group" id="vlan">
>                   <input id="enableVlan" class="wok-checkbox" type="checkbox" value="" />
>                   <label for="enableVlan" id="labelEnableVlan">$_("Enable VLAN") </label>
>                   <div id="vlan-enabled">
>


-- 
Lucio Correia
Software Engineer
IBM LTC Brazil




More information about the Kimchi-devel mailing list