[libvirt] [PATCH v2 1/8] hellolibvirt: Update hellolibvirt example
Laine Stump
laine at laine.org
Sun Feb 24 13:29:57 UTC 2013
On 02/22/2013 07:46 AM, John Ferlan wrote:
> On 02/20/2013 08:05 PM, Dave Allan wrote:
>> On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote:
>>> Update the code to be more in line with how code looks elsewhere in
>>> libvirt. Allow listing of domains, networks, storage pools, and
>>> network interfaces.
>> I like the changes to make the style more in line with the rest of the
>> codebase, however, I really think that this example code should be
>> about a minimal use of the API to list a few domains and that's all,
>> so I'm not in favor of extending it to list other kinds of objects. I
>> think people can find the details of how to do that kind of thing in
>> the API docs.
>>
>> Dave
>>
>>> Update the function prototypes in libvirt.c to include a message about
>>> the client needing to free() returned name fields. Fix the all domains
>>> example flags values.
>>> ---
>>> examples/hellolibvirt/hellolibvirt.c | 131 ++++++++++++++++++++++-------------
>>> src/libvirt.c | 21 +++---
>>> 2 files changed, 91 insertions(+), 61 deletions(-)
> Any other (strong) opinions one way or another? Should I remove the
> hellolibvirt.c for now?
>
> Adding network, volume groups, and interfaces was (mostly) trivial once
> I got past the set up the structure to handle multiple types. It
> certainly shows how similar the API's are for various objects and how
> trivial it is to gather generic information for each.
That's a neat idea, but I think putting the actual functions being
called into a table and calling them indirectly obscures what's trying
to be exhibited.
Aside from that, the virInterface API *still* isn't available on all
platforms, so I don't think that's a good thing to have in a "basic"
example program - we would get too many noobs asking why the example
program is "broken".
I think it would be better to just provide simple examples for domain,
with a comment somewhere that says "storage pool, network, and interface
APIs work in a similar fashion" or something like that. (And maybe you
could have this more complicated example called hellolibvirt2.c or
something)
>
> Any thoughts on the libvirt.c changes which are primarily documentation
> of the need to free the 'names' returned. That was a result of a review
> comment which noted that the comment in hellolibvirt.c if true should be
> rectified. I forgot to put the v2->v1 differences marker when generating
> the patch to call this one out specifically.
Those changes seem fine to me, although maybe they should be in a
separate patch, since they aren't really part of updating hellolibvirt.
More information about the libvir-list
mailing list