[libvirt] [PATCH 4/4] test_driver: implement testDomainGetInterfaceParameters

[Ilias Stamatis]

On Tue, Jul 2, 2019 at 2:50 PM Erik Skultety <eskultet at redhat.com> wrote:
>
> On Tue, Jul 02, 2019 at 02:29:02PM +0200, Ilias Stamatis wrote:
> > Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> > ---
> >  src/test/test_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 8c3a41b34b..59937ea968 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -2935,6 +2935,69 @@ testDomainGetNumaParameters(virDomainPtr dom,
> >  }
> >
> >
> > +static int
> > +testDomainGetInterfaceParameters(virDomainPtr dom,
> > +                                 const char *device,
> > +                                 virTypedParameterPtr params,
> > +                                 int *nparams,
> > +                                 unsigned int flags)
> > +{
> > +    unsigned int in_average = 0, in_peak = 0, in_burst = 0, in_floor = 0;
> > +    unsigned int out_average = 0, out_peak = 0, out_burst = 0;
>
> ^This is against the coding style, keep it 1 variable definition per line.
>
> > +    virDomainObjPtr vm = NULL;
> > +    virDomainDefPtr def = NULL;
> > +    virDomainNetDefPtr net = NULL;
> > +    int ret = -1;
> > +
> > +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > +                  VIR_DOMAIN_AFFECT_CONFIG |
> > +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> > +
> > +    if ((*nparams) == 0) {
> > +        *nparams = 7;
> > +        return 0;
> > +    }
> > +
> > +    if (!(vm = testDomObjFromDomain(dom)))
> > +        return -1;
> > +
> > +    if (!(def = virDomainObjGetOneDef(vm, flags)))
> > +        goto cleanup;
> > +
> > +    if (!(net = virDomainNetFind(def, device)))
> > +        goto cleanup;
> > +
> > +    if (net->bandwidth && net->bandwidth->in) {
> > +        in_average = net->bandwidth->in->average;
> > +        in_peak = net->bandwidth->in->peak;
> > +        in_burst = net->bandwidth->in->burst;
> > +        in_floor = net->bandwidth->in->floor;
> > +    }
> > +
> > +    if (net->bandwidth && net->bandwidth->out) {
> > +        out_average = net->bandwidth->out->average;
> > +        out_peak = net->bandwidth->out->peak;
> > +        out_burst = net->bandwidth->out->burst;
> > +    }
>
> We could also save 1 comparison with
> 'if (net->bandwidth) {
>     if (in){}
>     if (out){}
> }'
>
> ...alternatively, you could introduce a structure with all the attributes
> packed and introduce a helper like testDomainFillInterfaceParameters and then
> you'd do
>
> if (net->bandwidth)
>     testDomainFillInterfaceParameters(net->bandwidth, &params);

Now that you're saying that, I can actually use virNetDevBandwidthRate
which is already defined.
And if net->bandwidth->in and net->bandwidth->out exist I can just
copy the whole structs as they are.

I think that's what's simplest so I'll go with it.

Thanks,
Ilias

>
> Otherwise looking good.
>
> Erik
>
>
> > +
> > +    TEST_SET_PARAM(0, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, VIR_TYPED_PARAM_UINT, in_average);
> > +    TEST_SET_PARAM(1, VIR_DOMAIN_BANDWIDTH_IN_PEAK, VIR_TYPED_PARAM_UINT, in_peak);
> > +    TEST_SET_PARAM(2, VIR_DOMAIN_BANDWIDTH_IN_BURST, VIR_TYPED_PARAM_UINT, in_burst);
> > +    TEST_SET_PARAM(3, VIR_DOMAIN_BANDWIDTH_IN_FLOOR, VIR_TYPED_PARAM_UINT, in_floor);
> > +    TEST_SET_PARAM(4, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, VIR_TYPED_PARAM_UINT, out_average);
> > +    TEST_SET_PARAM(5, VIR_DOMAIN_BANDWIDTH_OUT_PEAK, VIR_TYPED_PARAM_UINT, out_peak);
> > +    TEST_SET_PARAM(6, VIR_DOMAIN_BANDWIDTH_OUT_BURST, VIR_TYPED_PARAM_UINT, out_burst);
> > +
> > +    if (*nparams > 7)
> > +        *nparams = 7;
> > +
> > +    ret = 0;
> > + cleanup:
> > +    virDomainObjEndAPI(&vm);
> > +    return ret;
> > +}
> > +
> > +
> >  static int testConnectNumOfDefinedDomains(virConnectPtr conn)
> >  {
> >      testDriverPtr privconn = conn->privateData;
> > @@ -7369,6 +7432,7 @@ static virHypervisorDriver testHypervisorDriver = {
> >      .domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
> >      .domainGetMemoryParameters = testDomainGetMemoryParameters, /* 5.6.0 */
> >      .domainGetNumaParameters = testDomainGetNumaParameters, /* 5.6.0 */
> > +    .domainGetInterfaceParameters = testDomainGetInterfaceParameters, /* 5.6.0 */
> >      .connectListDefinedDomains = testConnectListDefinedDomains, /* 0.1.11 */
> >      .connectNumOfDefinedDomains = testConnectNumOfDefinedDomains, /* 0.1.11 */
> >      .domainCreate = testDomainCreate, /* 0.1.11 */
> > --
> > 2.22.0
> >
> > --
> > libvir-list mailing list
> > libvir-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list