On Thu, Jul 25, 2019 at 5:47 PM Erik Skultety <eskultet@xxxxxxxxxx> wrote:
>
> On Tue, Jul 23, 2019 at 12:17:55PM +0200, Ilias Stamatis wrote:
> > Signed-off-by: Ilias Stamatis <stamatis.iliass@xxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
