On Wed, May 18, 2011 at 07:52:43AM +0200, Matthias Bolte wrote:
> 2011/5/18 Hu Tao <hutao@xxxxxxxxxxxxxx>:
> > 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(-)
> >>
> >> +/**
> >> + * 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.
enum don't have a fixed size defined by the C standard. So we avoid
using then for any API either in function signature or within public
structure because we can't guarantee that the resulting ABI will be
stable from one compiler to another.
> 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.
yes we must keep int.
> >> + 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.
Agreed.
Patch looks fine to me, and we should do that cleanup before the next
release !
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list