Re: [libvirt] [PATCH v2 3/7] cpu_x86: Introduce virCPUx86FeatureIsMSR

On Thu, Jun 20, 2019 at 12:53:38AM +0200, Jiri Denemark wrote: >This function may be used as a virCPUDefFeatureFilter callback for >virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to >filter or pick out features reported via MSR. > >Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; >--- > src/cpu/cpu_x86.c | 40 ++++++++++++++++++++++++++++++++++++++++ > src/cpu/cpu_x86.h | 3 +++ > src/libvirt_private.syms | 1 + > 3 files changed, 44 insertions(+) > >diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c >index 6eef5cef00..a7ec0f7095 100644 >--- a/src/cpu/cpu_x86.c >+++ b/src/cpu/cpu_x86.c >@@ -3362,6 +3362,46 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData, > } > > >+/** >+ * virCPUx86FeatureIsMSR: >+ * @name CPU feature name >+ * @opaque NULL or a pointer to bool >+ * >+ * This is a callback for functions filtering features in virCPUDef. >+ * >+ * Checks whether a given CPU feature is reported via MSR. When @opaque is NULL >+ * or a pointer to true, the function will pick out (return true for) MSR >+ * features. If @opaque is a pointer to false, the logic will be inverted and >+ * the function will filter out (return false for) MSR features. >+ */ Uhm. What? Do I understand it correctly? @opaque | *opaque | IsMSR | return --------+---------+-------+-------- NULL | SEGV | true | true 0xBEEF | true | true | true 0xBEEF | false | true | false --------+---------+-------+-------- First, it feels odd that 'false' is the value resulting in changed behavior. I'd rather see it act differently when the bool pointer is present and points to true.