[libvirt] [RFC] Power Hypervisor Libvirt support
Eduardo Otubo
otubo at linux.vnet.ibm.com
Fri Mar 27 15:31:43 UTC 2009
Hello Kaitlin,
> > +#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?
I am still planning how I should store all driver information, so I
think I should keep this for now.
>
>
> > +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.
Already fixed, but thanks anyway.
>
> > +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?
Good point. I should follow the patterns and start using virDomainObj to
store name and uuid. But unfortunately ssh_session and the
managed_system name I still need to keep them as I did.
>
>
> > +
> > + 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.
Yes, this problem will be solved when I change it to virDomainObj.
Thanks again for the comments.
[]'s
--
Eduardo Otubo
Software Engineer
Linux Technology Center
IBM Systems & Technology Group
Mobile: +55 19 8135 0885
otubo at linux.vnet.ibm.com
More information about the libvir-list
mailing list