Re: [libvirt] [RFC] Power Hypervisor Libvirt support

> +#include "domain_conf.h" > +/* Main driver state */ > +typedef struct __phyp_driver phyp_driver_t; > +struct __phyp_driver { > + virMutex lock; > + > + virCapsPtr *caps; > + > + virDomainObjList *domains; > + > + char *configDir; > + char *autostartDir; > + char *stateDir; > + char *logDir; > + int have_netns; > +}; I don't see this struct / typedef being used anywhere. Will this be used for something in the future?

> +static int > +phypListDomains(virConnectPtr conn, int *ids, int nids) > +{ > + SSH_SESSION *ssh_session = conn->privateData; > + const char *managed_system = conn->uri->server; > + int ret = 0; > + char id_c[10]; > + unsigned int i = 0, j = 0; > + char *cmd; > + char *textual_return; > + > + if (VIR_ALLOC_N(cmd, MAXSIZE+sizeof(managed_system))) { > + return PHYP_NO_MEM; > + } > + > + if (VIR_ALLOC_N(textual_return, MAXSIZE)) { > + return PHYP_NO_MEM; > + } > + > + nids = 0; > + > + memset(id_c, 0, 10); > + sprintf(cmd, "lssyscfg -r lpar -m %s -F lpar_id", managed_system); > + ret = __inner_exec_command(ssh_session, cmd, textual_return); You don't check ret to see if an error occured.

> +static virDomainPtr > +phypDomainLookupByID(virConnectPtr conn, int lpar_id) > +{ > + SSH_SESSION *ssh_session = conn->privateData; > + virDomainPtr dom = NULL; > + const char *managed_system = conn->uri->server; > + char *lpar_name; > + unsigned char *lpar_uuid; Any reason not to use a virDomainObjPtr to hold these values?

> + > + if (VIR_ALLOC_N(lpar_name, 100 * sizeof(char))) > + return NULL; > + > + if (VIR_ALLOC_N(lpar_uuid, 100 * sizeof(char))) > + return NULL; You then wouldn't need to use a magic number here. You could just do an alloc of the virDomainObjPtr object itself.