On 02/22/2013 07:46 AM, John Ferlan wrote:
On 02/20/2013 08:05 PM, Dave Allan 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
On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote: 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.