[libvirt] [PATCH 2/4] test_driver: implement virDomainDelIOThread

[Ilias Stamatis]

On Thu, Jul 25, 2019 at 5:47 PM Erik Skultety <eskultet at redhat.com> wrote:
>
> On Tue, Jul 23, 2019 at 12:17:55PM +0200, Ilias Stamatis wrote:
> > Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> > ---
> >  src/test/test_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 313cf5e7ef..29262e4d34 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -2653,6 +2653,77 @@ testDomainAddIOThread(virDomainPtr dom,
> >  }
> >
> >
> > +static int
> > +testDomainDelIOThread(virDomainPtr dom,
> > +                      unsigned int iothread_id,
> > +                      unsigned int flags)
> > +{
> > +    virDomainObjPtr vm = NULL;
> > +    virDomainDefPtr def = NULL;
> > +    size_t i, j;
> > +    int ret = -1;
> > +
> > +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> > +
> > +    if (iothread_id == 0) {
> > +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> > +                       _("invalid value of 0 for iothread_id"));
> > +        return -1;
> > +    }
> > +
> > +    if (!(vm = testDomObjFromDomain(dom)))
> > +        goto cleanup;
> > +
> > +    if (!(def = virDomainObjGetOneDef(vm, flags)))
> > +        goto cleanup;
> > +
> > +    if (!virDomainIOThreadIDFind(def, iothread_id)) {
> > +        virReportError(VIR_ERR_INVALID_ARG,
> > +                       _("cannot find IOThread '%u' in iothreadids list"),
> > +                       iothread_id);
> > +        goto cleanup;
> > +    }
> > +
> > +    for (i = 0; i < def->ndisks; i++) {
> > +        if (def->disks[i]->iothread == iothread_id) {
> > +            virReportError(VIR_ERR_INVALID_ARG,
> > +                           _("cannot remove IOThread %u since it "
> > +                             "is being used by disk '%s'"),
> > +                           iothread_id, def->disks[i]->dst);
> > +            goto cleanup;
> > +        }
> > +    }
> > +
> > +    for (i = 0; i < def->ncontrollers; i++) {
> > +        if (def->controllers[i]->iothread == iothread_id) {
> > +            virReportError(VIR_ERR_INVALID_ARG,
> > +                           _("cannot remove IOThread '%u' since it "
> > +                             "is being used by controller"),
> > +                           iothread_id);
> > +            goto cleanup;
> > +        }
> > +    }
> > +
> > +    for (i = 0; i < def->niothreadids; i++) {
> > +        if (def->iothreadids[i]->iothread_id == iothread_id) {
> > +            for (j = i + 1; j < def->niothreadids; j++)
> > +                def->iothreadids[j]->autofill = false;
>
> So, I read both the commit and the commentary in the QEMU driver which added
> ^this autofill clearing hunk. I haven't tried with QEMU, but just from reading
> those, I'm still not clear why it's actually needed. Even more so in test
> driver, I tried to remove the nested loop and everything seemed to be working,
> I had half of the thread defined, half of them autofilled, removed from the
> beginning, middle of the list, basically from anywhere and the data that
> libvirt reported were intact, both in the XML and the dedicated API. Right now,
> it's magic to me.
>
> Erik

Tbh, I also didn't understand it when I read it but I thought since
it's there let's just copy it to make sure. Maybe somebody that
touched that code could explain? If you checked that there are no
side-effects when removing it then sure let's remove it. From the test
driver at least.

Ilias