Re: [PATCH 3/4] libxl: add version_info function [and 1 more messages]

[Andre Przywara <andre.przywara@amd.com>]

Vincent Hanquez wrote:
> On 19/04/10 21:43, Andre Przywara wrote:
>> I am not fully convinced of this. There is quite a lot of information
>> copied (up to 4KB), and some fields (like pagesize or the version
>> numbers) are just plain integers. And this approach just propagates the
>> underlying design.
>> In the xl info implementation I used this feature to just get the
>> pagesize. One could think of just querying the version number, too. If
>> you don't like the additional parameter, what about a wrapping macro, 
>> then?
>> I don't see the problem with the complexity, though, as it is hidden
>> inside the library.
> 
> The query mask is exposed to the user, and the structure is partially 
> filled. I can imagine that it's going to create those kind of issues, 
> where the wrong mask is passed, or change later, and then the field the 
> user was expecting would be left null.
> 
> The macro would help in the previous case, but I still feel it's 
> unnecessary. As a user, I would just not bother, and call the function
> always with full mask, until I reach the optimisation phase, and its 
> appears on the graph.
> 
> A couple of K of data copied shouldn't make a difference, and would be 
> completely lost in noise, however if you really think it makes a 
> difference, you could have special dedicated info call to get specific 
> things like version number.
I just wanted to avoid introducing a lot of functions which could be 
actually one. And  if a user passes a wrong mask, then well, he is just 
doing the wrong thing and cannot expect it to work. If Xen provides a 
possibility to query single parts of the information, why should we hide 
this capability from the upper layers?
Anyway, my plan was now to make the current mask function static and 
create three exported functions, one for getting all the information, 
one for getting the pagesize and one for getting the Xen version number 
only. Together with a comment one could later expose the mask function 
again if the need arises.
But when I was looking at the libxl_sprintf thing, I realized that all 
the information in the info structure is static and will never change 
during runtime. Am I right? If so, we can solve this whole thing by not 
only storing the strings in libxl_ctx, but the whole structure. The 
first call to the function would query all the members, subsequent calls 
would just return the pointer. This avoids duplication should the 
function be called multiple times. It would be automatically freed when 
destroying the xl context.

What do you think of this?

Regards,
Andre.


-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12