[lttng-dev] [Patch LTTng-ust 1/7] Add or update data structures to support CTF named metadata

[Geneviève Bastien]

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.

Only thing where answer is needed inline below.

On 14-01-30 06:06 PM, Mathieu Desnoyers wrote:
> ----- Original Message -----
>> From: "Geneviève Bastien" <gbastien+lttng at versatic.net>
>> To: lttng-dev at lists.lttng.org
>> 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 <gbastien+lttng at versatic.net>
>> ---
>>   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?

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
>> lttng-dev at lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>