On Thu, Jul 25, 2019 at 5:47 PM Erik Skultety <eskultet(a)redhat.com> wrote:
On Tue, Jul 23, 2019 at 12:17:55PM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)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