----- Original Message ----- > From: "Geneviève Bastien" <[email protected]> > To: "Mathieu Desnoyers" <[email protected]>, "Geneviève Bastien" > <[email protected]> > Cc: [email protected] > Sent: Friday, January 31, 2014 3:27:50 PM > Subject: Re: [lttng-dev] [Patch LTTng-ust 1/7] Add or update data structures > to support CTF named metadata > > Thanks for reviewing all those patches! I'll make the changes soon when > the naming of this concept is settled (see Stefan's email). "Global > types" sounds good.
OK. Replied in a separate email for this one. > > Only thing where answer is needed inline below. > > On 14-01-30 06:06 PM, Mathieu Desnoyers wrote: > > ----- Original Message ----- > >> From: "Geneviève Bastien" <[email protected]> > >> To: [email protected] > >> Sent: Friday, January 24, 2014 3:04:39 PM > >> Subject: [lttng-dev] [Patch LTTng-ust 1/7] Add or update data structures > >> to support CTF named metadata > >> > >> The structures match those in LTTng-tools to support named metadata in > >> general > >> and CTF enumerations in particular. > >> > >> Signed-off-by: Geneviève Bastien <[email protected]> > >> --- > >> include/lttng/ust-ctl.h | 36 +++++++++++++++++++++++++++++++++++- > >> include/lttng/ust-events.h | 20 +++++++++++++++++++- > >> 2 files changed, 54 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h > >> index 40e831e..29c89fe 100644 > >> --- a/include/lttng/ust-ctl.h > >> +++ b/include/lttng/ust-ctl.h > >> @@ -295,13 +295,23 @@ struct ustctl_float_type { > >> char padding[USTCTL_UST_FLOAT_TYPE_PADDING]; > >> } LTTNG_PACKED; > >> > >> -#define USTCTL_UST_BASIC_TYPE_PADDING 296 > >> +#define USTCTL_UST_ENUM_ENTRY_PADDING 296 > >> +struct ustctl_enum_entry { > >> + unsigned long long start, end; /* start and end are inclusive */ > >> + char string[LTTNG_UST_SYM_NAME_LEN]; > >> + char padding[USTCTL_UST_ENUM_ENTRY_PADDING]; > >> +}; > >> + > >> +#define USTCTL_UST_BASIC_TYPE_PADDING 40 > >> union _ustctl_basic_type { > >> struct ustctl_integer_type integer; > >> struct { > >> enum ustctl_string_encodings encoding; > >> } string; > >> struct ustctl_float_type _float; > >> + struct { > >> + char name[LTTNG_UST_SYM_NAME_LEN]; > >> + } enumeration; > >> char padding[USTCTL_UST_BASIC_TYPE_PADDING]; > >> } LTTNG_PACKED; > >> > >> @@ -329,6 +339,30 @@ struct ustctl_type { > >> } u; > >> } LTTNG_PACKED; > >> > >> +#define USTCTL_UST_ENUM_TYPE_PADDING 296 > >> +struct ustctl_enum { > >> + char name[LTTNG_UST_SYM_NAME_LEN]; > >> + struct ustctl_integer_type container_type; > >> + struct ustctl_enum_entry *entries; > >> + unsigned int len; > >> + char padding[USTCTL_UST_ENUM_TYPE_PADDING]; > >> +} LTTNG_PACKED; > >> + > >> +/* CTF Metadata types for named metadata described outside event > >> descriptions */ > >> +enum ustctl_metadata_types { > >> + ustctl_mtype_enum, > >> + NR_USTCTL_METADATA_TYPES, > >> +}; > >> + > >> +#define USTCTL_UST_NAMED_METADATA_PADDING 640 > >> +struct ustctl_named_metadata { > >> + enum ustctl_metadata_types mtype; > > OK to use the enumeration, but define this mtype field as a fixed-size > > field (e.g. int32_t). (yes, I did the mistake to use enums in the ABI > > in the past). Currently, compilers use 32-bit int typically, but they > > have some freedom in the choice of the backing type. > > > >> + union { > >> + struct ustctl_enum ctf_enum; > >> + char padding[USTCTL_UST_NAMED_METADATA_PADDING]; > >> + } u; > >> +}; > >> + > >> #define USTCTL_UST_FIELD_PADDING 28 > >> struct ustctl_field { > >> char name[LTTNG_UST_SYM_NAME_LEN]; > >> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h > >> index d93c912..260b221 100644 > >> --- a/include/lttng/ust-events.h > >> +++ b/include/lttng/ust-events.h > >> @@ -89,6 +89,12 @@ enum lttng_string_encodings { > >> NR_STRING_ENCODINGS, > >> }; > >> > >> +/* CTF Metadata types for named metadata described outside event > >> descriptions */ > >> +enum lttng_metadata_types { > >> + mtype_enum, > >> + NR_METADATA_TYPES, > >> +}; > >> + > >> #define LTTNG_UST_ENUM_ENTRY_PADDING 16 > >> struct lttng_enum_entry { > >> unsigned long long start, end; /* start and end are inclusive */ > >> @@ -204,12 +210,22 @@ struct lttng_type { > >> #define LTTNG_UST_ENUM_TYPE_PADDING 24 > >> struct lttng_enum { > >> const char *name; > >> - struct lttng_type container_type; > >> + struct lttng_integer_type container_type; > > doing this change assumes that lttng_enum was never used in lttng-ust, is > > that true ? > Are you asking me or the list? I haven't seen any reference to it and > you told me on IRC that [you thought] it hadn't been used anywhere ever. > But I don't know about the past of UST... For more security, maybe I > should just add a new field at the end of this struct? I think it should be safe to assume that nobody used it, since it's not supported neither at the instrumentation API level, nor used by lttng-ust, nor written into the CTF metadata by lttng-sessiond. You might want to document this assumption in the patch changelog though. Thanks, Mathieu > > Thanks, > > Geneviève > > > > Thanks, > > > > Mathieu > > > >> const struct lttng_enum_entry *entries; > >> unsigned int len; > >> char padding[LTTNG_UST_ENUM_TYPE_PADDING]; > >> }; > >> > >> +#define LTTNG_UST_NAMED_METADATA_PADDING 60 > >> +struct lttng_named_metadata { > >> + enum lttng_metadata_types mtype; > >> + unsigned int nowrite; /* inherited from the field using it */ > >> + union { > >> + const struct lttng_enum *ctf_enum; > >> + char padding[LTTNG_UST_NAMED_METADATA_PADDING]; > >> + } u; > >> +}; > >> + > >> /* > >> * Event field description > >> * > >> @@ -267,6 +283,8 @@ struct lttng_event_desc { > >> union { > >> struct { > >> const char **model_emf_uri; > >> + unsigned int nr_metadata; > >> + const struct lttng_named_metadata *named_metadata; > >> } ext; > >> char padding[LTTNG_UST_EVENT_DESC_PADDING]; > >> } u; > >> -- > >> 1.8.5.3 > >> > >> > >> _______________________________________________ > >> lttng-dev mailing list > >> [email protected] > >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > >> > > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
