On Thu, 29 Oct 2020 13:18:10 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> Added 32bit flags for the time synchronization protocols. The first added
> flag is TRACECMD_TSYNC_FLAG_INTERPOLATE, used to specify how the
> timestamps must be corrected.
> - If the flag is set, an interpolation is performed:
> Find the (min, max) interval from the offsets array and calculate
> offset specific to the given timestamp using interpolation in that
> interval.
> - If the flag is not set, do not interpolate:
> Find the (min, max) interval from the offsets array and use the
> min offset for all timespamps within the interval.
"timestamps"
>
> These flags are set by the timestamp synchronization protocols at the
> protocol initialization time.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx>
> ---
> --- a/tracecmd/trace-tsync.c
> +++ b/tracecmd/trace-tsync.c
> @@ -132,7 +132,8 @@ out:
> static void write_guest_time_shift(struct buffer_instance *instance)
> {
> struct tracecmd_output *handle;
> - struct iovec vector[5];
> + struct iovec vector[6];
> + unsigned int flags;
> long long *scalings = NULL;
> long long *offsets = NULL;
> long long *ts = NULL;
> @@ -145,6 +146,9 @@ static void write_guest_time_shift(struct buffer_instance *instance)
> &ts, &offsets, &scalings);
> if (ret < 0 || !count || !ts || !offsets || !scalings)
> return;
> + ret = tracecmd_tsync_get_proto_flags(&instance->tsync, &flags);
> + if (ret < 0)
> + return;
>
> file = instance->output_file;
> fd = open(file, O_RDWR);
> @@ -154,14 +158,16 @@ static void write_guest_time_shift(struct buffer_instance *instance)
> vector[0].iov_len = 8;
> vector[0].iov_base = &top_instance.trace_id;
> vector[1].iov_len = 4;
> - vector[1].iov_base = &count;
> - vector[2].iov_len = 8 * count;
> - vector[2].iov_base = ts;
> + vector[1].iov_base = &flags;
> + vector[2].iov_len = 4;
> + vector[2].iov_base = &count;
> vector[3].iov_len = 8 * count;
> - vector[3].iov_base = offsets;
> + vector[3].iov_base = ts;
> vector[4].iov_len = 8 * count;
> - vector[4].iov_base = scalings;
> - tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 5);
> + vector[4].iov_base = offsets;
> + vector[5].iov_len = 8 * count;
> + vector[5].iov_base = scalings;
> + tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 6);
> tracecmd_append_options(handle);
> tracecmd_output_close(handle);
> #ifdef TSYNC_DEBUG
To make the above cleaner, I would use an enum to define the vector
indexes:
enum {
VECTOR_TRACE_ID,
VECTOR_FLAGS,
VECTOR_COUNT,
VECTOR_TIMES,
VECTOR_OFFSETS,
VECTOR_SCALINGS,
}
And then you can make it:
vector[VECTOR_TRACE_ID].iov_len = 8;
vector[VECTOR_TRACE_ID].iov_base = &top_instance.trace_id;
vector[VECTOR_FLAGS].iov_len = 4;
vector[VECTOR_FLAGS].iov_base = &flags;
vector[VECTOR_COUNT].iov_len = 4;
vector[VECTOR_COUNT].iov_base = &count;
vector[VECTOR_TIMES].iov_len = 8 * count;
vector[VECTOR_TIMES].iov_base = ts;
vector[VECTOR_OFFSETS].iov_len = 8 * count;
vector[VECTOR_OFFSETS].iov_base = offsets;
vector[VECTOR_SCALINGS].iov_len = 8 * count;
vector[VECTOR_SCALINGS].iov_base = scalings;
It makes it obvious what each vector is used for.
This is just an opinion. You don't need to implement it. I just hate
hard coded numbers ;-)
-- Steve
|