Re: [libvirt] [PATCH 2/4] test_driver: implement virDomainDelIOThread
On Thu, Jul 25, 2019 at 5:47 PM Erik Skultety wrote: > > On Tue, Jul 23, 2019 at 12:17:55PM +0200, Ilias Stamatis wrote: > > Signed-off-by: Ilias Stamatis > > --- > > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] test_driver: implement virDomainDelIOThread
On Tue, Jul 23, 2019 at 12:17:55PM +0200, Ilias Stamatis wrote: > Signed-off-by: Ilias Stamatis > --- > 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] test_driver: implement virDomainDelIOThread
Signed-off-by: Ilias Stamatis --- 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; + +virDomainIOThreadIDDefFree(def->iothreadids[i]); +VIR_DELETE_ELEMENT(def->iothreadids, i, def->niothreadids); + +break; +} +} + +ret = 0; + cleanup: +virDomainObjEndAPI(); +return ret; +} + + static int testDomainSetUserPassword(virDomainPtr dom, const char *user ATTRIBUTE_UNUSED, @@ -7793,6 +7864,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainCoreDump = testDomainCoreDump, /* 0.3.2 */ .domainCoreDumpWithFormat = testDomainCoreDumpWithFormat, /* 1.2.3 */ .domainAddIOThread = testDomainAddIOThread, /* 5.6.0 */ +.domainDelIOThread = testDomainDelIOThread, /* 5.6.0 */ .domainSetUserPassword = testDomainSetUserPassword, /* 5.6.0 */ .domainSetVcpus = testDomainSetVcpus, /* 0.1.4 */ .domainSetVcpusFlags = testDomainSetVcpusFlags, /* 0.8.5 */ -- 2.22.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list