Re: [Xen-devel] [PATCH 1/4] libxl: extend physinfo structure [and 1 more messages]

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Andre Przywara writes ("[Xen-devel] [PATCH 1/4] libxl: extend physinfo 
structure"):
> The libxl version of the physinfo sysctl does not contain some
> fields like nr_nodes or capabilities needed for xl info output.
> Add them to the structure and the retrieving function.

Rather than this:

  --- a/tools/libxl/libxl.h     Thu Apr 15 19:11:16 2010 +0100
  +++ b/tools/libxl/libxl.h     Sun Apr 18 14:34:32 2010 +0200
  +#define PHYS_CAP_HVM          1
  +#define PHYS_CAP_HVM_DIRECTIO 2

I think it would be better to expect libxl callers to include the
public Xen header file sysctl.h.  While libxl callers should not need
to know about information about the underlying libraris, the Xen
interface is a key public interface and is fine to expose - at least
as far as these kind of bitmasks.

That also avoids the possibility of getting these #defines out of step
with the underlying code (or even the possibility of having to write
conversion code!)

Certainly if you _don't_ convert, you shouldn't redefine the bitfield
constants.

Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 1/4] libxl: extend physinfo 
structure"):
> I think, It would be much nicer to provide the phys_caps already decoded 
> instead of copying the xc value as is.

Is it really worth reprocessing a bitfield like this which is after
all not actually going to be used much by calling code ?

> so what about something like having phys_cap_hvm and 
> phys_cap_hvm_directio field directly in the structure ? If things 
> extends, we should add new field in libxl anyway.

That's a possibility which I wouldn't object to but it seems
unnecessary.  In that case it should be a series of 1-bit unsigned
C bitfields rather than a set of flags in a uint32.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel