New patch, more fixes. Matthias Bolte wrote:
+static virDomainPtr +phypDomainCreateAndStart(virConnectPtr conn, + const char *xml, + unsigned int flags ATTRIBUTE_UNUSED) +{ + + ConnectionData *connection_data = conn->networkPrivateData; + LIBSSH2_SESSION *session = connection_data->session; + virDomainDefPtr def = NULL; + virDomainPtr dom = NULL; + phyp_driverPtr phyp_driver = conn->privateData; + uuid_tablePtr uuid_table = phyp_driver->uuid_table; + lparPtr *lpars = uuid_table->lpars; + unsigned int i = 0; + char *managed_system = phyp_driver->managed_system; + + if (!(def = virDomainDefParseString(conn, phyp_driver->caps, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto err; + + /* checking if this name already exists on this system */ + if (phypGetLparID(session, managed_system, def->name, conn) == -1) { + VIR_WARN("%s", "LPAR name already exists."); + goto err; + } + + /* checking if ID or UUID already exists on this system */ + for (i = 0; i < uuid_table->nlpars; i++) { + if (lpars[i]->id == def->id || lpars[i]->uuid == def->uuid) { + VIR_WARN("%s", "LPAR ID or UUID already exists."); + goto err; + } + }
def->id is always -1 here because you're parsing the domain XML with the VIR_DOMAIN_XML_INACTIVE flag set.
+ dom->conn = conn; + dom->name = def->name; + dom->id = def->id; + memmove(dom->uuid, def->uuid, VIR_UUID_BUFLEN);
You're accessing a NULL pointer here. Just remove this 4 lines of code, because you're setting dom a line below anyway.
+ if ((dom = virGetDomain(conn, def->name, def->uuid)) == NULL) + goto err;
def->id is -1 here. As I understand phypBuildLpar() it calls mksyscfg to actually define a new LPAR with a given ID. You use def->id for this. You're either defining all new LPARs with ID -1 or I misunderstand how this method is working.
I was thinking I could just avoid handling the ID at all here, HMC does it by itself. So, skiping ID and just dealing with the LPAR name.
+ if (phypBuildLpar(conn, def) == -1) + goto err; + + if (phypDomainResume(dom) == -1) + goto err; + + return dom; + + err: + virDomainDefFree(def); + VIR_FREE(dom); + return NULL; +} +
[...]
-void -init_uuid_db(virConnectPtr conn) +int +phypUUIDTable_WriteFile(virConnectPtr conn) { - uuid_dbPtr uuid_db; + phyp_driverPtr phyp_driver = conn->privateData; + uuid_tablePtr uuid_table = phyp_driver->uuid_table; + unsigned int i = 0; + int fd = -1; + char local_file[] = "./uuid_table"; + + if ((fd = creat(local_file, 0755)) == -1) + goto err; + + for (i = 0; i < uuid_table->nlpars; i++) { + if (write + (fd, &uuid_table->lpars[i]->id, + sizeof(uuid_table->lpars[i]->id)) == -1) + VIR_ERROR("%s", "Unable to write information to local file."); + + if (write(fd, uuid_table->lpars[i]->uuid, VIR_UUID_BUFLEN) == -1) + VIR_ERROR("%s", "Unable to write information to local file."); + }
You should goto err if a write fails, because a single failed write will corrupt the while table.
You should instead check for
write(...) != sizeof(uuid_table->lpars[i]->id) and write(...) != VIR_UUID_BUFLEN
because write() may write less bytes than requested.
+ close(fd); + return 0; + + err: + close(fd); + return -1; +} +
You fixed most issues in this version of the patch, but some memory leaks are still there.
All the other things are fixed. Thanks for all the comments. []'s -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 eotubo@linux.vnet.ibm.com