[virt-tools-list] [PATCH virt-manager 1/2] virtManager: add QoS information for a network

[Cole Robinson]

On 07/03/2014 08:13 AM, Giuseppe Scrivano wrote:
> Add UI elements to display and modify QoS settings in the network
> details.
> 
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1089117
> 

Some UI bits:

- I'd use KiB/sec to save some text, rather than spelling it out
- Consistent spacing please: 6 pixels between columns, 3 between rows IIRC
- If the checkboxes are unselected, I'd hide the entire child table
- If a have a shutoff network, unselect

Functional bits:

- Trying to set 'floor' gives me: floor attribute isn't supported for
network's bandwidth yet. Not sure how it's expected to be used, maybe we want
to conditionally hide it.
- The libvirt test driver doesn't support the 'define new config on top of
running network, shutdown network, new config takes effect'. That's a libvirt
bug, the test driver isn't handling newDef or INACTIVE_XML, etc. Not blocking
this but something to think on, the sooner that's in the test driver the
easier our lives will be going forward.

> diff --git a/virtManager/host.py b/virtManager/host.py
> index 4166929..65dec66 100644
> --- a/virtManager/host.py
> +++ b/virtManager/host.py
> @@ -43,10 +43,11 @@ INTERFACE_PAGE_ERROR = 1
>  
>  (EDIT_NET_NAME,
>  EDIT_NET_AUTOSTART,
> +EDIT_NET_QOS,
>  
>  EDIT_POOL_NAME,
>  EDIT_POOL_AUTOSTART,
> -) = range(4)
> +) = range(5)
>  
>  
>  class vmmHost(vmmGObjectUI):
> @@ -127,6 +128,24 @@ class vmmHost(vmmGObjectUI):
>              "on_interface_list_changed": self.interface_selected,
>  
>              "on_config_autoconnect_toggled": self.toggle_autoconnect,
> +
> +            "on_qos_inbound_average_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_inbound_peak_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_inbound_burst_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_inbound_floor_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_outbound_average_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_outbound_peak_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +            "on_qos_outbound_burst_changed":  (lambda *x:
> +                self.enable_net_apply(x, EDIT_NET_QOS)),
> +
> +            "on_net_qos_inbound_enable_toggled": self.change_qos_in_enable,
> +            "on_net_qos_outbound_enable_toggled": self.change_qos_out_enable,
>          })
>  
>          self.repopulate_networks()
> @@ -481,6 +500,34 @@ class vmmHost(vmmGObjectUI):
>              if EDIT_NET_NAME in self.active_edits:
>                  net.define_name(self.widget("net-name").get_text())
>                  self.repopulate_networks()
> +            if EDIT_NET_QOS in self.active_edits:
> +                in_qos = self.widget("net-qos-inbound-enable").get_active()
> +                out_qos = self.widget("net-qos-outbound-enable").get_active()
> +
> +                def get_value(name, enabled):
> +                    if not enabled:
> +                        return None
> +                    return self.widget(name).get_text() or None
> +
> +                args = {}
> +                args['inbound_average'] = get_value("qos-inbound-average", in_qos)
> +                args['inbound_peak'] = get_value("qos-inbound-peak", in_qos)
> +                args['inbound_burst'] = get_value("qos-inbound-burst", in_qos)
> +                args['inbound_floor'] = get_value("qos-inbound-floor", in_qos)
> +
> +                args['outbound_average'] = get_value("qos-outbound-average", out_qos)
> +                args['outbound_peak'] = get_value("qos-outbound-peak", out_qos)
> +                args['outbound_burst'] = get_value("qos-outbound-burst", out_qos)
> +
> +                if net.set_qos(**args):
> +                    self.err.show_err(
> +                        _("Network could not be updated"),
> +                        text2=_("This change will take effect when the "
> +                                "network is restarted"),
> +                        buttons=Gtk.ButtonsType.OK,
> +                        dialog_type=Gtk.MessageType.INFO)
> +
> +

I'd break this out into something like def _apply_network_qos() so all the
logic doesn't need to be so nested.

Also doing the redefine right here means that any future live editing
additions to the network UI will do cascading redefines rather than make all
the XML changes up front and redefine only once. It doesn't need to be solved
now, but the next time we add bits here we can't continue that pattern.

Thanks,
Cole