On Tue, Feb 19, 2013 at 11:26:34AM +0100, Michal Privoznik wrote:
> On 19.02.2013 03:14, John Ferlan wrote:
> > Add a list of active domains, list of active/inactive networks, and
> > list of active/inactive storage pools
> > ---
> > examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++-----
> > 1 file changed, 173 insertions(+), 28 deletions(-)
> >
> > diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c
> > index 234637e..f191782 100644
> > --- a/examples/hellolibvirt/hellolibvirt.c
> > +++ b/examples/hellolibvirt/hellolibvirt.c
> > @@ -85,65 +85,200 @@ out:
> > return ret;
> > }
> >
> > -
> > +typedef int (*virFunction)(virConnectPtr conn,
> > + char **nameList,
> > + int maxnames);
> > static int
> > -showDomains(virConnectPtr conn)
> > +listObject(virConnectPtr conn, int maxnames, const char *objNameStr,
> > + virFunction fcn)
> > {
> > - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains;
> > + int ret = 0, i, numNames;
> > char **nameList = NULL;
> >
> > - numActiveDomains = virConnectNumOfDomains(conn);
> > - if (-1 == numActiveDomains) {
> > + nameList = malloc(sizeof(*nameList) * maxnames);
> > +
> > + if (NULL == nameList) {
>
> If blue is the sky .... These Yoda conditions should really be made
> inverted. But there are some even outside of the hellolibvirt.c.
Yeah, this was my coding style from before libvirt, but it's clearly
not libvirt standard and they should be removed.
Dave
> > ret = 1;
> > - printf("Failed to get number of active domains\n");
> > + printf("Could not allocate memory for list of %s\n", objNameStr);
> > + goto out;
> > + }
> > +
> > + numNames = (*fcn)(conn, nameList, maxnames);
> > +
> > + if (-1 == numNames) {
> > + ret = 1;
> > + printf("Could not get list of %s from hypervisor\n", objNameStr);
> > showError(conn);
> > goto out;
> > }
> >
> > - numInactiveDomains = virConnectNumOfDefinedDomains(conn);
> > - if (-1 == numInactiveDomains) {
> > + printf(" %s: \n", objNameStr);
> > + for (i = 0; i < numNames; i++) {
> > + printf("\t%s\n", *(nameList + i));
> > + /* The API documentation doesn't say so, but the names
> > + * returned by are strdup'd and must be freed here.
> > + */
>
> The docs should be fixed then.
>
> > + free(*(nameList + i));
> > + }
> > +
> > +out:
> > + free(nameList);
> > + return ret;
> > +}
>
> I wonder if we should advise users to use the other list APIs that are
> around for a while (virConnectListAll. I guess it all boils down to
> question if the hellolibvirt binary is supposed to work with ancient
> libvirts or is just an example shipped within a release.
> In case it is supposed to work with prehistoric versions, we must add a
> fallback code. If we are satisfied with the example working with current
> libvirt, there's no need for fallbacking code then.
>
> Michal
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list