Re: [RFC] Simple tool for VMEnters/VMExits matching and trace validation

[Stefano De Venuto <stefano.devenuto99@gmail.com>]

Ciao Yordan,

Thanks so much for the comments!
I've just sent the version 2 addressing them.

On 4/20/21 10:12 AM, Yordan Karadzhov wrote:
> Ciao Stefano,
> 
> First of all, I am very happy to see you progressing so fast in the
> development of your VMEnters/VMExits matching analysis. I have several
> comments concerning the code (see below).
> 
> On 16.04.21 г. 19:46, Stefano De Venuto wrote:
>> Add a tool in examples/ that scans a merged host + guest trace and
>> search for host events that are inside a vmentry/vmexit block (and
>> vice-versa for guest events ouside the block) and report the found
>> ones.
>>
>> It can be useful as a starting point for identifying issues of for
>> checking the effectiveness of host/guest traces synchronization, or
>> even for improving the placing of the tracepoints in the kernel.
>>
>> Signed-off-by: Stefano De Venuto <stefano.devenuto99@gmail.com>
>> ---
>>   examples/checker.c | 202 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 202 insertions(+)
>>   create mode 100644 examples/checker.c
>>
>> diff --git a/examples/checker.c b/examples/checker.c
>> new file mode 100644
>> index 0000000..0b04343
>> --- /dev/null
>> +++ b/examples/checker.c
>> @@ -0,0 +1,202 @@
> 
> The first problem is that this patch fails to apply. My guess is that
> you hand-edited the patch and removed some lines starting with "+". Note
> that the total number of additions and removals in the file is indicated
> in the line above. If this number does not match the number of lines
> starting with "+", the patch will fail to apply.
> 
> As a general advise - try to avoid hand-editing patches unless you are
> certain you know what you are doing.
> 

Ok, thanks for the advice!

>> +
>> +        custom_streams[i-1] = custom_stream;
>> +    }
>> +
>> +    host_guest_mapping = NULL;
>> +    n_guest =
>> kshark_tracecmd_get_hostguest_mapping(&host_guest_mapping);
>> +    if (n_guest < 0) {
>> +      printf("Failed mapping: %d\n", n_guest);
>> +      return 1;
>> +    }
>> +
>> +    entries = NULL;
>> +    n_entries = kshark_load_all_entries(kshark_ctx, &entries);
>> +
>> +    for (int i = 0; i < n_entries; ++i) {
>> +        current = entries[i];
>> +
>> +        stream = kshark_get_stream_from_entry(current);
>> +        event_name = kshark_get_event_name(current);
>> +
>> +        custom_stream = custom_streams[stream->stream_id];
>> +
>> +        if (!strcmp(event_name, KVM_ENTRY) || !strcmp(event_name,
>> KVM_EXIT)) {
>> +            if (!strcmp(event_name, KVM_ENTRY)) {
>> +
>> +                /*
>> +                 * The recovering process of the vCPU field of the
>> kvm_entry event
>> +                 * is done by splitting the info field.
>> +                 */
>> +                info = kshark_get_info(current);
>> +
>> +                token = strtok(info, " ");
>> +                token = strtok(NULL, " ");
>> +
>> +                /* Removing the last comma */
>> +                token[strlen(token) - 1] = '\0';
>> +
>> +                custom_stream->cpus[current->cpu] = atoi(token);
> 
> It will be better if you make the recovering of the vCPU above a static
> helper function.
> 

I totally replaced that method since with
kshark_read_event_field_int(entry, "vcpu_id", &vcpu) I can get this
information in such a cleaner way!
Also more robust, since plugins can remove the vcpu field from the info.

Regards,
Stefano