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@xxxxxxxxxx> --- 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. Second, by using a pointer to bool AND not requiring opaque to be passed, we essentially have a tri-state bool. I'd suggest (ab)using the void pointer to store the value directly, we do have the bool var = opaque != NULL; pattern elsewhere in the code.
+bool +virCPUx86FeatureIsMSR(const char *name, + void *opaque)
Can you name this virCPUx86FeatureFilterMSR, leaving virCPUx86FeatureIsMSR for a static helper function that will perform the actual lookup and this function would (possibly) invert the result? All the ternary operators make the function harder to follow.
+{
+ virCPUx86FeaturePtr feature;
+ virCPUx86DataIterator iter;
+ virCPUx86DataItemPtr item;
+ virCPUx86MapPtr map;
+ bool inverted = false;
+
+ if (opaque)
+ inverted = !*(bool *)opaque;
+
+ if ((!(map = virCPUx86GetMap()) ||
This condition would be more readable if it were split in two.
+ !(feature = x86FeatureFind(map, name))) &&
+ !(feature = x86FeatureFindInternal(name)))
+ return inverted ? true : false;
+
+ virCPUx86DataIteratorInit(&iter, &feature->data);
+ while ((item = virCPUx86DataNext(&iter))) {
+ if (item->type == VIR_CPU_X86_DATA_MSR)
+ return inverted ? false : true;
+ }
+
+ return inverted ? true : false;
+}
+
+
struct cpuArchDriver cpuDriverX86 = {
.name = "x86",
.arch = archs,
Jano
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
