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

Re: [libvirt] [PATCH] lib: implement new API to retrieve the list of CPU models



Eric Blake <eblake redhat com> writes:

> I'm not sure whether returning XML or a straight-up list makes more
> sense.  If you used char ***models, then the user would get an array of
> directly-usable strings, "486", "pentium", ..., instead of a document
> that has to be parsed.  On the other hand, your idea of returning XML
> lets us return information for multiple arches simultaneously. But do we
> need that flexibility, since arch is also an input parameter?  Is the
> idea that you pass arch=NULL to get the full list, or arch="x86" to get
> the x86 subset of the xml?  Why not just make arch mandatory and return
> char ***; but then you have the question of which arches are supported.

I've thought a bit about XML vs Array and whether specifying the arch in
the returned XML snippet or not, and these are the reasons behind my
choice:

1) leave room for VIR_CONNECT_GET_CPU_MODEL_NAMES_EXPAND_FEATURES, in
the same way as it is done by virConnectBaselineCPU.  It might turn
useful in future to get all the data in one shot, without requiring
additional round-trips for each model trough virConnectBaselineCPU.

2) as you said, support arch=NULL to get the full list (and now that
I've thought more about it, I should change the error to
VIR_ERR_NO_SUPPORT when arch==NULL instead of raising an internal
error).

So if these two features will be implemented at some point, it will
still be possible trough this new function to get the same
functionalities of the "virConnectGetCPUMapDesc" function that I have
proposed in my RFC.


> So, let's get agreement on the best design before worrying about
> implementation (I'm still 50/50 on whether xml vs. char*** makes more
> sense, without more discussion to sway me one way or the other).

Right.


> This is a rather big patch.  When adding new API, it's best to do it in
> pieces:
>
> 1. use of the API itself (include, tools, src/driver, src/libvirt*)
> 2. RPC support (daemon, src/remote)
> 3. qemu support (src/qemu)
> 4. test support (src/test)
> 5. python bindings (python)
>
> as long as each part builds and passes 'make check'.

I'll split the patch in a series before resubmitting it.

Giuseppe


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