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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list