Re: [libvirt] [PATCH v2 1/8] hellolibvirt: Update hellolibvirt example
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.
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.
Thanks,
John