Re: [PATCH 1/8] hellolibvirt: Update hellolibvirt example

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

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]