[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/7] libvirt.h: consolidate typed parameter handling



2011/5/18 Hu Tao <hutao cn fujitsu com>:
> On Tue, May 17, 2011 at 04:42:09PM -0600, Eric Blake wrote:
>> * include/libvirt/libvirt.h.in (virTypedParameterType)
>> (VIR_TYPED_PARAM_FIELD_LENGTH, _virTypedParameter): New enum,
>> macro, and type.
>> (virSchedParameter, virBlkioParameter, virMemoryParameter):
>> Rewrite in terms of a common type, while keeping all old public
>> names for backwards compatibility.
>> (struct _virSchedParameter, struct _virBlkioParameter)
>> (struct _virMemoryParameter): Delete - these are private names.
>> * python/generator.py (enum): Cope with the refactoring.
>> ---
>>  include/libvirt/libvirt.h.in |  144 +++++++++++++++++++++++------------------
>>  python/generator.py          |   12 ++++
>>  2 files changed, 93 insertions(+), 63 deletions(-)
>>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index a174201..7cd6e13 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -281,18 +281,70 @@ struct _virNodeInfo {
>>  };
>>
>>
>> +/* Common data types shared among interfaces with name/type/value lists.  */
>> +
>> +/**
>> + * virTypedParameterType:
>> + *
>> + * Express the type of a virTypedParameter
>> + */
>> +typedef enum {
>> +    VIR_TYPED_PARAM_INT     = 1, /* integer case */
>> +    VIR_TYPED_PARAM_UINT    = 2, /* unsigned integer case */
>> +    VIR_TYPED_PARAM_LLONG   = 3, /* long long case */
>> +    VIR_TYPED_PARAM_ULLONG  = 4, /* unsigned long long case */
>> +    VIR_TYPED_PARAM_DOUBLE  = 5, /* double case */
>> +    VIR_TYPED_PARAM_BOOLEAN = 6  /* boolean(character) case */
>> +} virTypedParameterType;
>> +
>> +/**
>> + * VIR_TYPED_PARAM_FIELD_LENGTH:
>> + *
>> + * Macro providing the field length of virTypedParameter name
>> + */
>> +#define VIR_TYPED_PARAM_FIELD_LENGTH 80
>> +
>> +/**
>> + * virTypedParameter:
>> + *
>> + * A named parameter, including a type and value.
>> + */
>> +typedef struct _virTypedParameter virTypedParameter;
>> +
>> +struct _virTypedParameter {
>> +    char field[VIR_TYPED_PARAM_FIELD_LENGTH];  /* parameter name */
>> +    int type;   /* parameter type, virTypedParameterType */
>
> virTypedParameterType type; ?

We typically use int even for enum values. There is probably a reason
for that but I'm not aware of it right now.

In this case we might need to stick to int for ABI compatibility,
because the old structs used int. Assuming that enum is not guaranteed
to be the same size as int by the C standard, but I don't know.

>> +    union {
>> +        int i;                      /* type is INT */
>> +        unsigned int ui;            /* type is UINT */
>> +        long long int l;            /* type is LLONG */
>> +        unsigned long long int ul;  /* type is ULLONG */
>> +        double d;                   /* type is DOUBLE */
>> +        char b;                     /* type is BOOLEAN */
>> +    } value; /* parameter value */
>> +};
>> +
>> +/**
>> + * virTypedParameterPtr:
>> + *
>> + * a pointer to a virTypedParameter structure.
>> + */
>> +typedef virTypedParameter *virTypedParameterPtr;
>> +
>> +/* Management of scheduler parameters */
>> +
>>  /**
>>   * virDomainSchedParameterType:
>>   *
>>   * A scheduler parameter field type
>>   */
>>  typedef enum {
>> -    VIR_DOMAIN_SCHED_FIELD_INT     = 1, /* integer case */
>> -    VIR_DOMAIN_SCHED_FIELD_UINT    = 2, /* unsigned integer case */
>> -    VIR_DOMAIN_SCHED_FIELD_LLONG   = 3, /* long long case */
>> -    VIR_DOMAIN_SCHED_FIELD_ULLONG  = 4, /* unsigned long long case */
>> -    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = 5, /* double case */
>> -    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6  /* boolean(character) case */
>> +    VIR_DOMAIN_SCHED_FIELD_INT     = VIR_TYPED_PARAM_INT,
>> +    VIR_DOMAIN_SCHED_FIELD_UINT    = VIR_TYPED_PARAM_UINT,
>> +    VIR_DOMAIN_SCHED_FIELD_LLONG   = VIR_TYPED_PARAM_LLONG,
>> +    VIR_DOMAIN_SCHED_FIELD_ULLONG  = VIR_TYPED_PARAM_ULLONG,
>> +    VIR_DOMAIN_SCHED_FIELD_DOUBLE  = VIR_TYPED_PARAM_DOUBLE,
>> +    VIR_DOMAIN_SCHED_FIELD_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN,
>>  } virSchedParameterType;
>
> Can we remove VIR_DOMAIN_SCHED_FIELD_XXX and use VIR_TYPED_PARAM_XXX
> directly since parameter types are basically types like int, long, ...
> and don't depend on what parameters are?
>
> Likewise for other PARAMs in this patch.

We cannot, because the old symbols are part of the released API so we
cannot remove them without breaking backwards compatibility.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]