<tt>libvir-list-bounces@redhat.com wrote on 05/13/2010 10:53:53 AM: </tt> <tt>> > Hello all, > > This is the first patch about storage management driver on IBM Power > Hypervisor. I reviewd a couple of times but perhaps I might be missing > something. Any comments are welcome.</tt> <tt>Please send the patches inline.</tt> <tt> > > diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c > index cec99b1..7bd203f 100644 > --- a/src/phyp/phyp_driver.c > +++ b/src/phyp/phyp_driver.c > @@ -1665,6 +1665,262 @@ virDriver phypDriver = { > NULL, /* domainSnapshotDelete */ > }; > > +virStorageDriver phypStorageDriver = { > + .name = "PHYP", > + .open = phypStorageOpen, > + .close = phypStorageClose, > + > + .numOfPools = phypNumOfStoragePools, > + .listPools = phypListStoragePools, > + .numOfDefinedPools = NULL, > + .listDefinedPools = NULL, > + .findPoolSources = NULL, > + .poolLookupByName = phypSPLookupByName, > + .poolLookupByUUID = NULL, > + .poolLookupByVolume = NULL, > + .poolCreateXML = NULL, > + .poolDefineXML = NULL, > + .poolBuild = NULL, > + .poolUndefine = NULL, > + .poolCreate = NULL, > + .poolDestroy = NULL, > + .poolDelete = NULL, > + .poolRefresh = NULL, > + .poolGetInfo = NULL, > + .poolGetXMLDesc = NULL, > + .poolGetAutostart = NULL, > + .poolSetAutostart = NULL, > + .poolNumOfVolumes = NULL, > + .poolListVolumes = NULL, > + > + .volLookupByName = NULL, > + .volLookupByKey = NULL, > + .volLookupByPath = NULL, > + .volCreateXML = NULL, > + .volCreateXMLFrom = NULL, > + .volDelete = NULL, > + .volGetInfo = NULL, > + .volGetXMLDesc = NULL, > + .volGetPath = NULL, > + .poolIsActive = NULL, > + .poolIsPersistent = NULL > +}; > + > +int > +phypGetStoragePoolUUID(unsigned char *sp_uuid, const char *sp_name, > + virConnectPtr conn) > +{ > + ConnectionData *connection_data = conn->networkPrivateData; > + phyp_driverPtr phyp_driver = conn->privateData; > + LIBSSH2_SESSION *session = connection_data->session; > + char *managed_system = phyp_driver->managed_system; > + int vios_id = phyp_driver->vios_id; > + int exit_status = 0; > + char *cmd = NULL; > + char *ret = NULL; > + > + if (virAsprintf(&cmd, > + "viosvrcmd -m %s --id %d -c 'lsdev -dev %s -attr " > + "vgserial_id'|sed '1d'|sed '1d'", > + managed_system, vios_id, sp_name, sp_name) < 0) { > + virReportOOMError(); > + goto err; > + } > + > + ret = phypExec(session, cmd, &exit_status, conn); > + > + if (exit_status < 0 || ret == NULL) > + goto err; > + > + if (memmove(sp_uuid, ret, VIR_UUID_BUFLEN) == NULL) > + goto err; > + > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return 0; > + > + err: > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return -1; > +} > + > +int > +phypGetStoragePoolID(const char *sp_name, virConnectPtr conn) > +{ > + ConnectionData *connection_data = conn->networkPrivateData; > + phyp_driverPtr phyp_driver = conn->privateData; > + LIBSSH2_SESSION *session = connection_data->session; > + char *managed_system = phyp_driver->managed_system; > + int vios_id = phyp_driver->vios_id; > + int exit_status = 0; > + int sp_id = 0; > + char *char_ptr; > + char *cmd = NULL; > + char *ret = NULL; > + > + if (virAsprintf(&cmd, > + "viosvrcmd -m %s --id %d -c 'lssp -field Pool'" > + "|sed '1d'|grep -n %s|sed -e 's/:.*$//g'", > + managed_system, vios_id, sp_name) < 0) { > + virReportOOMError(); > + goto err; > + } > + > + ret = phypExec(session, cmd, &exit_status, conn); > + > + if (exit_status < 0 || ret == NULL) > + goto err; > + > + if (virStrToLong_i(ret, &char_ptr, 10, &sp_id) == -1) > + goto err; </tt> <tt>char_ptr not being used afterward -> could supply NULL instead.</tt> <tt>> + > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return sp_id; > + > + err: > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return -1; > +} > + > +virStoragePoolPtr > +phypSPLookupByName(virConnectPtr conn, const char *sp_name) > +{ > + virStoragePoolPtr sp = NULL; > + int sp_id = 0; > + unsigned char sp_uuid[VIR_UUID_BUFLEN]; > + > + sp_id = phypGetStoragePoolID(sp_name, conn); > + if (sp_id == -1) > + return NULL; > + > + if (phypGetStoragePoolUUID(sp_uuid, sp_name, conn) == -1) > + return NULL; > + > + sp = virGetStoragePool(conn, sp_name, sp_uuid); > + > + if (sp) > + return sp; > + else > + return NULL;</tt> <tt>Doesn't seem necessary to do if - then here.</tt> <tt>Just a 'return sp;' should do the trick.</tt> <tt> > +} > + > +int > +phypNumOfStoragePools(virConnectPtr conn)</tt> <tt>Make this one static since it's accessed through the interface? Or is it exported for some debugging purpose?</tt> <tt> > +{ > + ConnectionData *connection_data = conn->networkPrivateData; > + phyp_driverPtr phyp_driver = conn->privateData; > + LIBSSH2_SESSION *session = connection_data->session; > + int exit_status = 0; > + int nsp = 0; > + char *char_ptr; > + char *cmd = NULL; > + char *ret = NULL; > + char *managed_system = phyp_driver->managed_system; > + int vios_id = phyp_driver->vios_id; > + > + if (virAsprintf(&cmd, > + "viosvrcmd -m %s --id %d -c 'lssp -field " > + "Pool'|sed '1d'|grep -c ^.*$", > + managed_system, vios_id) < 0) { > + virReportOOMError(); > + goto err; > + } > + > + ret = phypExec(session, cmd, &exit_status, conn); > + > + if (exit_status < 0 || ret == NULL) > + goto err; > + > + if (virStrToLong_i(ret, &char_ptr, 10, &nsp) == -1) > + goto err;</tt> <tt>NULL rather than char_ptr?</tt> <tt> > + > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return nsp; > + > + err: > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return -1; > +} > + > +int > +phypListStoragePools(virConnectPtr conn, char **const pools, int npools)</tt> <tt>Static function?</tt> <tt> > +{ > + ConnectionData *connection_data = conn->networkPrivateData; > + phyp_driverPtr phyp_driver = conn->privateData; > + LIBSSH2_SESSION *session = connection_data->session; > + char *managed_system = phyp_driver->managed_system; > + int vios_id = phyp_driver->vios_id; > + int exit_status = 0; > + int got = 0; > + int i; > + char *cmd = NULL; > + char *ret = NULL; > + char *storage_pools = NULL; > + char *char_ptr2 = NULL; > + > + if (virAsprintf > + (&cmd, > + "viosvrcmd -m %s --id %d -c 'lssp -field Pool'|sed '1d'", > + managed_system, vios_id) < 0) { > + virReportOOMError(); > + goto err; > + } > + > + ret = phypExec(session, cmd, &exit_status, conn); > + > + /* I need to parse the textual return in order to get the storage > pools */ > + if (exit_status < 0 || ret == NULL) > + goto err; > + else { > + storage_pools = ret; > + > + while (got < npools) { > + char_ptr2 = strchr(storage_pools, '\n'); > + > + if (char_ptr2) { > + *char_ptr2 = '\0'; > + if ((pools[got++] = strdup(storage_pools)) == NULL) { > + virReportOOMError(); > + goto err; > + } > + char_ptr2++; > + storage_pools = char_ptr2; > + } else > + break; > + } > + } > + > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return got; > + > + err: > + for (i = 0; i < got; i++) > + VIR_FREE(pools[i]); > + VIR_FREE(cmd); > + VIR_FREE(ret); > + return -1; > +} > + > +virDrvOpenStatus > +phypStorageOpen(virConnectPtr conn ATTRIBUTE_UNUSED, > + virConnectAuthPtr auth ATTRIBUTE_UNUSED, > + int flags ATTRIBUTE_UNUSED)</tt> <tt>Static function?</tt> <tt> > +{ > + return VIR_DRV_OPEN_SUCCESS; > +} > + > +int > +phypStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED)</tt> <tt>static function?</tt> <tt> > +{ > + return 0; > +} > + > int > phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) > { > @@ -2209,6 +2465,10 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * > session) > int > phypRegister(void) > { > - virRegisterDriver(&phypDriver); > + if (virRegisterDriver(&phypDriver) < 0 ) > + return -1; > + if(virRegisterStorageDriver(&phypStorageDriver) < 0) > + return -1; > + > return 0; > } > diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h > index f680994..edd0d6c 100644 > --- a/src/phyp/phyp_driver.h > +++ b/src/phyp/phyp_driver.h > @@ -69,6 +69,32 @@ struct _phyp_driver { > char *managed_system; > }; > > + > +/* > + * Common storage functions > + * */ > +int phypGetStoragePoolID(const char *sp_name, virConnectPtr conn); > + > +int phypGetStoragePoolUUID(unsigned char *sp_uuid, const char *sp_name, > + virConnectPtr conn); > + > +virStoragePoolPtr phypSPLookupByName(virConnectPtr conn, const char > *name); > + > +int phypNumOfStoragePools(virConnectPtr conn); > + > +int phypListStoragePools(virConnectPtr conn, char **const pools, > + int npools); > + > +virDrvOpenStatus phypStorageOpen(virConnectPtr conn ATTRIBUTE_UNUSED, > + virConnectAuthPtr auth ATTRIBUTE_UNUSED, > + int flags ATTRIBUTE_UNUSED); > + > +int phypStorageClose(virConnectPtr conn);</tt> <tt>If the don't need to be public then you can remove them from this file here.</tt> <tt> Stefan</tt> <tt> > + > +/* > + * Common driver functions > + * */ > + > int phypCheckSPFreeSapce(virConnectPtr conn, int required_size, char > *sp); > > int phypGetVIOSPartitionID(virConnectPtr conn); </tt>