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(-)
>
> diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c
> index 234637e..85e5ae6 100644
> --- a/examples/hellolibvirt/hellolibvirt.c
> +++ b/examples/hellolibvirt/hellolibvirt.c
> @@ -15,7 +15,7 @@ showError(virConnectPtr conn)
> virErrorPtr err;
>
> err = malloc(sizeof(*err));
> - if (NULL == err) {
> + if (!err) {
> printf("Could not allocate memory for error data\n");
> goto out;
> }
> @@ -56,14 +56,14 @@ showHypervisorInfo(virConnectPtr conn)
> * to fail if, for example, there is no connection to a
> * hypervisor, so check what it returns. */
> hvType = virConnectGetType(conn);
> - if (NULL == hvType) {
> + if (!hvType) {
> ret = 1;
> printf("Failed to get hypervisor type\n");
> showError(conn);
> goto out;
> }
>
> - if (0 != virConnectGetVersion(conn, &hvVer)) {
> + if (virConnectGetVersion(conn, &hvVer) < 0) {
> ret = 1;
> printf("Failed to get hypervisor version\n");
> showError(conn);
> @@ -85,73 +85,101 @@ out:
> return ret;
> }
>
> +/* Turn off the message in order to compile here
> + *
> + * The virConnectListAll*(), vir*IsActive(), vir*GetName(), and vir*Free()
> + * functions require specific types which aren't possible to provide in
> + * this tabular manner.
> + */
> +#pragma GCC diagnostic ignored "-Wstrict-prototypes"
> +typedef struct _objectTable objectTable;
> +struct _objectTable {
> + const char *objName;
> + int (*activeFcn)(virConnectPtr);
> + int (*inactiveFcn)(virConnectPtr);
> + int (*allFcn)();
> + int (*isActiveFcn)();
> + const char *(*nameFcn)();
> + int (*freeFcn)();
> + unsigned int flags;
> +};
> +
> +objectTable fcnTable[] = {
> + {"domains",
> + virConnectNumOfDomains, virConnectNumOfDefinedDomains,
> + virConnectListAllDomains, virDomainIsActive,
> + virDomainGetName, virDomainFree,
> + VIR_CONNECT_LIST_DOMAINS_ACTIVE |
> + VIR_CONNECT_LIST_DOMAINS_INACTIVE},
> + {"networks",
> + virConnectNumOfNetworks, virConnectNumOfDefinedNetworks,
> + virConnectListAllNetworks, virNetworkIsActive,
> + virNetworkGetName, virNetworkFree,
> + VIR_CONNECT_LIST_NETWORKS_ACTIVE |
> + VIR_CONNECT_LIST_NETWORKS_INACTIVE},
> + {"storage pools",
> + virConnectNumOfStoragePools, virConnectNumOfDefinedStoragePools,
> + virConnectListAllStoragePools, virStoragePoolIsActive,
> + virStoragePoolGetName, virStoragePoolFree,
> + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE |
> + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE},
> + {"interfaces",
> + virConnectNumOfInterfaces, virConnectNumOfDefinedInterfaces,
> + virConnectListAllInterfaces, virInterfaceIsActive,
> + virInterfaceGetName, virInterfaceFree,
> + VIR_CONNECT_LIST_INTERFACES_ACTIVE |
> + VIR_CONNECT_LIST_INTERFACES_INACTIVE},
> + {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0}
> +};
>
> static int
> -showDomains(virConnectPtr conn)
> +showObject(virConnectPtr conn, objectTable entry)
> {
> - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains;
> - char **nameList = NULL;
> + int ret = 0, numActive, numInactive, numObjects, i;
> + void **objects = NULL;
>
> - numActiveDomains = virConnectNumOfDomains(conn);
> - if (-1 == numActiveDomains) {
> + if ((numActive = (*entry.activeFcn)(conn)) < 0) {
> ret = 1;
> - printf("Failed to get number of active domains\n");
> + printf("Failed to get number of active %s\n", entry.objName);
> showError(conn);
> goto out;
> }
>
> - numInactiveDomains = virConnectNumOfDefinedDomains(conn);
> - if (-1 == numInactiveDomains) {
> + if ((numInactive = (*entry.inactiveFcn)(conn)) < 0) {
> ret = 1;
> - printf("Failed to get number of inactive domains\n");
> + printf("Failed to get number of inactive %s\n", entry.objName);
> showError(conn);
> goto out;
> }
>
> - printf("There are %d active and %d inactive domains\n",
> - numActiveDomains, numInactiveDomains);
> + printf("There are %d active and %d inactive %s:\n",
> + numActive, numInactive, entry.objName);
>
> - nameList = malloc(sizeof(*nameList) * numInactiveDomains);
> -
> - if (NULL == nameList) {
> + if ((numObjects = (*entry.allFcn)(conn, &objects, entry.flags)) < 0) {
> ret = 1;
> - printf("Could not allocate memory for list of inactive domains\n");
> - goto out;
> - }
> -
> - numNames = virConnectListDefinedDomains(conn,
> - nameList,
> - numInactiveDomains);
> -
> - if (-1 == numNames) {
> - ret = 1;
> - printf("Could not get list of defined domains from hypervisor\n");
> + printf("Failed to get all %s\n", entry.objName);
> showError(conn);
> goto out;
> }
> -
> - if (numNames > 0) {
> - printf("Inactive domains:\n");
> - }
> -
> - for (i = 0 ; i < numNames ; i++) {
> - printf(" %s\n", *(nameList + i));
> - /* The API documentation doesn't say so, but the names
> - * returned by virConnectListDefinedDomains are strdup'd and
> - * must be freed here. */
> - free(*(nameList + i));
> + for (i = 0; i < numObjects; i++) {
> + int active = (*entry.isActiveFcn)(objects[i]);
> + /* Sure the name could be longer, but for a quick example this works */
> + printf("\t%8s (%s)\n",
> + (*entry.nameFcn)(objects[i]),
> + (active == 1 ? "active" : "non-active"));
> +
> + /* We have to free the entry */
> + (*entry.freeFcn)(objects[i]);
> }
> -
> + free(objects);
> out:
> - free(nameList);
> return ret;
> }
>
> -
> int
> main(int argc, char *argv[])
> {
> - int ret = 0;
> + int ret = 0, i;
> virConnectPtr conn;
> char *uri;
>
> @@ -163,7 +191,7 @@ main(int argc, char *argv[])
> * except, possibly, the URI of the hypervisor. */
> conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0);
>
> - if (NULL == conn) {
> + if (!conn) {
> ret = 1;
> printf("No connection to hypervisor\n");
> showError(conn);
> @@ -171,7 +199,7 @@ main(int argc, char *argv[])
> }
>
> uri = virConnectGetURI(conn);
> - if (NULL == uri) {
> + if (!uri) {
> ret = 1;
> printf("Failed to get URI for hypervisor connection\n");
> showError(conn);
> @@ -181,18 +209,21 @@ main(int argc, char *argv[])
> printf("Connected to hypervisor at \"%s\"\n", uri);
> free(uri);
>
> - if (0 != showHypervisorInfo(conn)) {
> + if (showHypervisorInfo(conn) < 0) {
> ret = 1;
> goto disconnect;
> }
>
> - if (0 != showDomains(conn)) {
> - ret = 1;
> - goto disconnect;
> + i = 0;
> + while (fcnTable[i].objName) {
> + ret = showObject(conn, fcnTable[i]);
> + if (ret != 0)
> + goto disconnect;
> + i++;
> }
>
> disconnect:
> - if (0 != virConnectClose(conn)) {
> + if (virConnectClose(conn) < 0) {
> printf("Failed to disconnect from hypervisor\n");
> showError(conn);
> ret = 1;
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 1e78500..e717f16 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -8304,19 +8304,18 @@ error:
> * VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, for filtering based on whether
> * a domain has snapshots.
> *
> - * Returns the number of domains found or -1 and sets domains to NULL in case
> - * of error. On success, the array stored into @doms is guaranteed to have an
> + * Returns the number of domains found or -1 and sets domains to NULL in case of
> + * error. On success, the array stored into @domains is guaranteed to have an
> * extra allocated element set to NULL but not included in the return count, to
> * make iteration easier. The caller is responsible for calling virDomainFree()
> - * on each array element, then calling free() on @doms.
> + * on each array element, then calling free() on @domains.
> *
> * Example of usage:
> * virDomainPtr *domains;
> - * virDomainPtr dom;
> * int i;
> * int ret;
> - * unsigned int flags = VIR_CONNECT_LIST_RUNNING |
> - * VIR_CONNECT_LIST_PERSISTENT;
> + * unsigned int flags = VIR_CONNECT_LIST_DOMAINS_RUNNING |
> + * VIR_CONNECT_LIST_DOMAINS_PERSISTENT;
> *
> * ret = virConnectListAllDomains(conn, &domains, flags);
> * if (ret < 0)
> @@ -10226,7 +10225,7 @@ error:
> * this command is inherently racy; a network can be started between a call
> * to virConnectNumOfNetworks() and this call; you are only guaranteed that
> * all currently active networks were listed if the return is less than
> - * @maxnames.
> + * @maxnames. The client must call free() on each returned name.
> */
> int
> virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames)
> @@ -11208,7 +11207,7 @@ error:
> * this command is inherently racy; a interface can be started between a call
> * to virConnectNumOfInterfaces() and this call; you are only guaranteed that
> * all currently active interfaces were listed if the return is less than
> - * @maxnames.
> + * @maxnames. The client must call free() on each returned name.
> */
> int
> virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames)
> @@ -12076,7 +12075,7 @@ error:
> * this command is inherently racy; a pool can be started between a call to
> * virConnectNumOfStoragePools() and this call; you are only guaranteed
> * that all currently active pools were listed if the return is less than
> - * @maxnames.
> + * @maxnames. The client must call free() on each returned name.
> */
> int
> virConnectListStoragePools(virConnectPtr conn,
> @@ -18285,7 +18284,7 @@ error:
> * the results, see virDomainListAllSnapshots().
> *
> * Returns the number of domain snapshots found or -1 in case of error.
> - * The caller is responsible for freeing each member of the array.
> + * The caller is responsible to call free() for each member of the array.
> */
> int
> virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
> @@ -18534,7 +18533,7 @@ error:
> * the results, see virDomainSnapshotListAllChildren().
> *
> * Returns the number of domain snapshots found or -1 in case of error.
> - * The caller is responsible for freeing each member of the array.
> + * The caller is responsible to call free() for each member of the array.
> */
> int
> virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
> --
> 1.7.11.7
>
> --
> 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