[libvirt] [PATCH v3 2/2] network: Add code for setting network bandwidth for ethernet interfaces

[Anirban Chakraborty]

On 10/8/14, 8:39 PM, "Eric Blake" <eblake at redhat.com> wrote:

>On 10/08/2014 04:10 PM, Anirban Chakraborty wrote:
>
>>> Needs rework after my comments on 1/2.  I also wonder if this should
>>> just be folded in to that patch, and/or made into a switch statement
>>> where the compiler forces us to think about any future
>>> VIR_DOMAIN_NET_TYPE_* additions on whether they should return true or
>>> false.
>
>>> bool virNetDevSupportBandwidth(virDomainNetType type)
>>> {
>>>    switch (type) {
>>>    case VIR_DOMAIN_NET_TYPE_BRIDGE:
>>>    case VIR_DOMAIN_NET_TYPE_NETWORK:
>>>    case VIR_DOMAIN_NET_TYPE_DIRECT:
>>>    case VIR_DOMAIN_NET_TYPE_ETHERNET:
>>>        return true;
>>>    case VIR_DOMAIN_NET_TYPE_USER:
>>>    case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
>>>    case VIR_DOMAIN_NET_TYPE_SERVER:
>>>    case VIR_DOMAIN_NET_TYPE_CLIENT:
>>>    case VIR_DOMAIN_NET_TYPE_MCAST:
>>>    case VIR_DOMAIN_NET_TYPE_INTERNAL:
>>>    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
>>>    case VIR_DOMAIN_NET_TYPE_LAST:
>>>        /* cover all enums to appease the compiler */ ;
>>>    }
>>>    return false;
>>> }
>> 
>> We could have the function defined with a switch, however, instead of
>> listing all the types, I could return false from the default case.
>> 
>> Something like:
>>     switch (type) {
>>     case VIR_DOMAIN_NET_TYPE_BRIDGE:
>>     case VIR_DOMAIN_NET_TYPE_NETWORK:
>>     case VIR_DOMAIN_NET_TYPE_DIRECT:
>>     case VIR_DOMAIN_NET_TYPE_ETHERNET:
>>         return true;
>> 
>>     default;
>>         return false;
>
>Alas, using a default case means the compiler can no longer tell you
>about missed cases.  I wrote my example to intentionally spell out ALL
>cases without a default, in order to ensure the compiler will loudly
>warn if we add another enum in the future.

I understood the intention. However, I was aiming for a compact code. If
we add additional enums for network type, we should be adding those types
in the above switch provided the new interface type supports bandwidth. I
do not know if we should prevent future coding error by elaborating the
current code. Forgive me, as I do not know the coding convention for
libvirt. If this is the standard practice, I¹ll change the above switch as
per your wish.

Anirban