Inactive domain config may contain definitions that are not supported by the current qemu process, since the VM might be started from a different config or the config was edited.

Step 5 as above is a valid use case in such situation.

I think it is still a bug, some other scenarios have been tried as followed:

1.command "virsh iothreadadd" with flag "--config"

1)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-3
 2               0-3
 3               0-3
 4               0-3

2)[root@localhost ~]# virsh iothreadadd rhel7.2 5 --config

3)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-3
 2               0-3
 3               0-3
 4               0-3
 5               0-3

4)[root@localhost ~]# virsh reboot rhel7.2

5)[root@localhost ~]# virsh dumpxml rhel7.2
 ***<iothreads>5</iothreads>***

Actual results:
In step 2, the new IOThread is added successfully.
In step 5,the xml of the guest is modified also.



2.command "virsh iothreadpin" with flag "--config"

1)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0-3
 2               0-3
 3               0-3
 4               0-3
 5               0-3

2)[root@localhost ~]# virsh iothreadpin rhel7.2 1 0 --config

3)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0
 2               0-3
 3               0-3
 4               0-3
 5               0-3

4)[root@localhost ~]# virsh reboot rhel7.2

5)[root@localhost ~]# virsh dumpxml rhel7.2
 ***<iothreadpin iothread='1' cpuset='0'/>***

Actual results:
In step 2, the IOThreadpin of ID 1 is modified successfully
In step 5,the xml of the guest is modified also.

3.command "virsh iothreaddel" with flag "--config"

1)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0
 2               0-3
 3               0-3
 4               0-3
 5               0-3

2)[root@localhost ~]# virsh iothreaddel rhel7.2  5 --config

3)[root@localhost ~]# virsh iothreadinfo rhel7.2 --config
 IOThread ID     CPU Affinity   
---------------------------------------------------
 1               0
 2               0-3
 3               0-3
 4               0-3


4)[root@localhost ~]# virsh reboot rhel7.2

5)[root@localhost ~]# virsh dumpxml rhel7.2
 ***<iothreads>4</iothreads>***


Actual results:
In step 2, the IOThread of ID 5 is deleted successfully
In step 5,the xml of the guest is modified also.


Summary:
Because the host with qemu-kvm do not support the virsh command about "iothread" ,the host should not have the permission to modify the attribute of IOThread.
But in the above scenarios,the host can modify the attribute succefully. After rebooting the guest,the xml of the guest is also modified. I think it is not right.

Hmm, yeah, I noticed that libvirt ignores iothread config if the emulator doesn't support it, but certain APIs then complain/fail. Assigning to John, since he designed this approach originally.

To a degree IOThreads are a "victim" of differences qemu-kvm and qemu-kvm-rhev at least with respect to the thought process I had implementing them, what gets installed on RHEL 7.2, and how easy they are to configure. I cannot easily rattle off any virsh command(s) that could add some difference where --config would allow something to work, while --live wouldn't allow the command to work w/r/t the -rhev difference. However, someone who knew how to configure some difference and wanted to create a configuration using something they knew worked in the future certainly could do so by modifying the inactive/--config domain.

The design of using "--config" makes an assumption that the user for some reason knows something more than the currently executing system about the environment they are about to change in which a subsequent guest or host reboot/change would allow some change you are currently making to "work" once the guest or host reboots. In doing so, it allows certain "runtime" checks to not be made. It's one of those 'how it works' things.

Consider one can "virsh attach-disk $domain $source $target --config" some $source that doesn't exist which succeeds. If you then destroy and start the guest it fails to start noting that the disk source isn't accessible. So, is that a bug or is that someone trying to configure something they shouldn't configure?  I would say IOThreads are in a similar situation.

Using my upstream code, if using just virsh reboot, I don't see the issue - I have to destroy and start - although that could be another environmental difference... My guest domain has "<on_reboot>restart</on_reboot>" - is yours different *and* is your guest down when you issue the dumpxml command? I feel like I'm missing something in your steps.  Even in the original configuration - there's no way a start would have worked if <iothreads> were defined in the XML.  You must have started, then added via --config in order to get that iothreadinfo output, but you didn't show that step.  I have to assume at this point that your "reboot" causes the domain to be destroyed. Then you issue the dumpxml command on the down domain, which would logically show the "next" configuration.

Using the same running domain with no iothreads configured, then addiothreads --config and virsh reboot... The iothreads still don't show up in the rebooted system, although they do show up in the inactive config. So that's why I'm a bit curious or confused about your steps 4 & 5 for iothread{pin|add}. 

FWIW: There was an issue with iothreadpin, but that's handled in bug 1249981.

Back to the original problem, I'd say things are working as expected.

Use of 'iothreadadd ... --config' or 'iothreadpin... --config' would seem to follow the same logic that it's working correctly at least with respect to what should happen when --config is supplied.

Still not convinced this is a bug.

(In reply to John Ferlan from comment #4)
> Still not convinced this is a bug.

I'll reiterate that I think the bug here is that we start a VM with iothreads even if the qemu version used does not support them. They might be unused, but all the other APIs then start to behave strangely. The user configured them, but we did not provide them, thus we are misleading the user.

Hmm...  well saying it that way makes a bit more sense (I certainly didn't see that from just comment 3 - you may have been thinking it...).

The original design didn't include adding, pinning, sched, and deleting IOThreads. It only had the ability to define some number of threads allowed for a domain and then the ability to "add" (either inactive or hotplug) an IOThread to a disk. Once a disk attempted to make use, then the check would be made that qemu being used supported IOThreads. I'm not sure at the time of development that the whole concept of the differences between qemu-kvm and qemu-kvm-rhev was fully "established". I certainly wasn't fully aware of those differences, too.

Anyway - moving back to assigned in order to add a run time check for IOThreads being "possible" and failing to start if defined (even though not used).

Looks like this was fixed as part of bz 1249981 with git commit 'cc2d49f9b'

In particular the change in src/qemu/qemu_command.c:

-    if (def->niothreadids > 0 &&
-        virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+    if (def->niothreadids) {
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("IOThreads not supported for this QEMU"));
+            goto error;
+        }
+

will check for niothreads being defined in the domain xml and if there, but the capability doesn't exist in qemu will cause a failure to start.

In 7.2, the virQEMUCapsGet would fail and the iothread objects wouldn't be created.

I'm going to duplicate this bz.

*** This bug has been marked as a duplicate of bug 1249981 ***