Bug 1656068

*** Bug 1606610 has been marked as a duplicate of this bug. ***

Test plan:

This is the same RFE as bz 1468004, so the test plan from that (if it exists) can be reused. In addition, automated test are currently proposed here: [1].

The automated tests cover 3 scenarios (and assume a `dedicated` CPU policy and `vcpu_pin_set` configured):

1. With emulator threads policy set to `share` and `cpu_share_set` set, emulator threads should be pinned to `cpu_share_set`.
2. With emulator threads policy set to `share` and `cpu_share_set` unset, emulator threads should float over (aka be pinned to) the instance's pCPUs.
3. With emulator threads policy set to `isolate`, `cpu_shared_set` is ignored, and emulator threads should be pinned to a pCPU distinct from the instance's pCPUs.

If we want to be more thorough, here's a truth table of emulator threads policy, cpu_share_set, and expected behaviour:

                           +--------------------------------+---------------------------------------+                          
                           |       cpu_share_set set        |           cpu_share_set unset         |
+--------------------------+--------------------------------+---------------------------------------+
| share (implicit default) | Pinned to cpu_share_set        | Pinned to all of the instance's pCPUs |
+--------------------------+--------------------------------+---------------------------------------+
| share (explicitly set)   | Pinned to cpu_share_set        | Pinned to all of the instance's pCPUs |
+--------------------------+--------------------------------+---------------------------------------+
|                  isolate | Pinned to a single pCPU        | Pinned to a single pCPU               |
|                          | distinct from instance's pCPUs | distinct from instance's pCPUs        |
+--------------------------+--------------------------------+---------------------------------------+

[1] https://review.rdoproject.org/r/#/c/18070/

Correction to the table above - the default policy isn't an implicit share, but just unset. In those cases, regardless of the value of cpu_share_set, the emulator threads float over (aka are pinned to) all of the instance's pCPUs.

                         +--------------------------------+---------------------------------------+
                         |       cpu_share_set set        |           cpu_share_set unset         |
+------------------------+--------------------------------+---------------------------------------+
|        Unset (default) | Pinned to all of the           | Pinned to all of the instance's pCPUs |
|                        | instance's pCPUs               |                                       |
+------------------------+--------------------------------+---------------------------------------+
| share (explicitly set) | Pinned to cpu_share_set        | Pinned to all of the instance's pCPUs |
+------------------------+--------------------------------+---------------------------------------+
|                isolate | Pinned to a single pCPU        | Pinned to a single pCPU               |
|                        | distinct from instance's pCPUs | distinct from instance's pCPUs        |
+------------------------+--------------------------------+---------------------------------------+

Per cfields I'd like to take a table like this and include it into documentation. I've reworded some of the fields for clarity, thoughts on accuracy?

                          +--------------------------------------+----------------------------------------+
                          |                                      |                                        |
                          |  cpu_share_set: [CPU list defined]   |  cpu_share_set: [CPU list not defined] |
|---------------------------------------------------------------------------------------------------------+
|                         |                                      |                                        |
|  share: unset (default) |  Floats across vcpus in vcpu_pin_set |  Floats across vcpus in vcpu_pin_set   |
+---------------------------------------------------------------------------------------------------------+
|                         |                                      |                                        |
|  share: set             |  Pinned to cpu_share_set list        |  Floats across vcpus in vcpu_pin_set   |
+----------------------------------------------------------------+----------------------------------------+
|                         |                                                                               |
|  isolate                |  Isolated to one CPU in NovaVcpuPinSet. Requires an extra CPU per instance.   |
+-------------------------+-------------------------------------------------------------------------------+

Not quite.

For the first two cells in the left-most columns, it's not 'share: unset/set', it's the flavor extra spec that's either unset or set to 'share'. 

Also, 'Floats across vcpus in vcpu_pin_set' isn't totally correct. It's really the *instance's* CPUs, which are a subset of vcpu_pin_set.

Thank Artom, that clarifies vcpu_pin_set for me. I've cleaned up the muddied wording on the left hand column as well. 

                            +--------------------------------------+----------------------------------------+
                            |                                      |                                        |
                            |  cpu_share_set: [CPU list defined]   |  cpu_share_set: [CPU list not defined] |
|---------------------------------------------------------------------------------------------------------+
| hw:emulator_thread_policy |                                      |                                        |
|        [unset]            |  Floats across instances vcpus       |  Floats across instances vcpus         |
+-----------------------------------------------------------------------------------------------------------+
| hw:emulator_thread_policy |                                      |                                        |
|         share             |  Pinned to cpu_share_set list        |  Floats across instances vcpus         |
+------------------------------------------------------------------+----------------------------------------+
| hw:emulator_thread_policy |                                                                               |
|        isolate            |  Isolated to one CPU in NovaVcpuPinSet. Requires an extra CPU per instance.   |
+---------------------------+-------------------------------------------------------------------------------+

the use of floats and pinned is inconsistent

you shoudl say that it si "pinned to the instance vcpus" or it "floats across the cpu_share_set"

in both cases we restict the enulator thread to a set of allowed cpus it just a different set.

so either we should use the term floats in all cases or pinned.

also i would remove the term list for this entirly.

the cpu_share_set and vcpu_pin_set are mathmatical sets expresed in a sting encoded form
 [CPU list defined] makes this less clear

i would use "cpu_share_set: [defined]" and cpu_share_set: [undefined]
if more clarity is needed.


the other thing to be aware of is the semantic above are implemention defined and are only vlaid in the context
of the libvirt virt driver. they may change in the future and have changed in backwards incompatible ways in the past.
as such i think this is good to add to the docs but it needs to be revauated depending on the osp version.


+-----------------------------------------------------------------------------------------------------------+
| hw:emulator_thread_policy |                                      |                                        |
|         share             |  Pinned to cpu_share_set list        |  Floats across instances vcpus         |
+------------------------------------------------------------------+----------------------------------------+

is an example of one incompatible change we did when this feature was intoduced.

the meaning of hw:emulator_thread_policy=share was changed by this feature. so on older version of osp it where
cpu_share_set is not a thing hw:emulator_thread_policy=share always means float across instance vcpus.

This is good feedback, though some of the wording chosen was deliberate. As this is becoming more of a documentation discussion, I'll move this conversation to email. But feel free to copy anyone on it who might be interested.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2019:0924