3:36 p.m.
On 06/23/2010 04:49 PM, Eduardo Otubo wrote:
This is the first patch, just over the semantic changes over the code
itself.
Added the whole storage management stack and fixed the echo $(( expr)) to just
increment the variable in the return of the function.
Hope we can get it acknowledged in time for the next release :)
Thanks for all the comments so far!
+static char *
+phypGetLparProfile(virConnectPtr conn, int lpar_id)
+{
+ 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 system_type = phyp_driver->system_type;
+ int exit_status = 0;
+ char *cmd = NULL;
+ char *ret = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAddLit(&buf, "lssyscfg");
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, " -m %s ", managed_system);
I removed a trailing space here...
+ virBufferVSprintf(&buf,
+ " -r prof --filter lpar_ids=%d -F name|head -n 1",
...since this line always provides a space.
+static int
+phypGetVIOSNextSlotNumber(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 system_type = phyp_driver->system_type;
+ int vios_id = phyp_driver->vios_id;
+ int exit_status = 0;
+ char *char_ptr;
+ char *cmd = NULL;
+ char *ret = NULL;
+ char *profile = NULL;
+ int slot = 0;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (!(profile = phypGetLparProfile(conn, vios_id))) {
+ VIR_ERROR("%s", "Unable to get VIOS profile name.");
New code should start clean, so I added _() here (basically, I squashed
in all the formatting changes from your original 2/2 patch that were
left out when rebasing your two patches to play 2/2 first into my commit
stream). I had to add it in a lot of places, to silence 'make
syntax-check'.
+ goto err;
+ }
+
+ virBufferAddLit(&buf, "lssyscfg");
+
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, " -m %s ", managed_system);
+
+ virBufferVSprintf(&buf, "-r prof --filter "
Even worse, for the non-HMC case, you try to execute "lssyscfg-r prof
..." (there is no lssyscfg-r program, you meant lssyscfg with a first
argument of -r), so I did a global search and replace to clean up all of
these commands to have consistent spacing.
+ "profile_names=%s -F
virtual_eth_adapters,"
+ "virtual_opti_pool_id,virtual_scsi_adapters,"
+ "virtual_serial_adapters|sed -e 's/\"//g' -e
"
+ "'s/,/\\n/g'|sed -e
's/\\(^[0-9][0-9]\\*\\).*$/\\1/'"
+ "|sort|tail -n 1", profile);
+
+ if (virBufferError(&buf)) {
+ virBufferFreeAndReset(&buf);
+ virReportOOMError();
+ return -1;
+ }
+
+ cmd = virBufferContentAndReset(&buf);
+
+ ret = phypExec(session, cmd, &exit_status, conn);
+
+ if (exit_status < 0 || ret == NULL)
+ goto err;
+
+ if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1)
+ goto err;
+
+ VIR_FREE(cmd);
+ VIR_FREE(ret);
+ return ++slot;
I did s/++slot/slot + 1/ since there is no need to modify slot in place
- it goes out of scope after this return.
+int
+phypAttachDevice(virDomainPtr domain, const char *xml)
+{
I saw no reason for this to be a public function, so I removed it (and
many like it) from phyp_driver.h, and marked it static here. If some
file outside of phyp_driver.c needs to use it in the future, we can
worry about exporting it then (and proper exporting, even if it is an
internal-use only symbol, probably involves touching some .syms files).
virDriver phypDriver = {
VIR_DRV_PHYP, "PHYP", phypOpen, /* open */
phypClose, /* close */
@@ -1725,7 +2204,7 @@ virDriver phypDriver = {
NULL, /* domainCreateWithFlags */
NULL, /* domainDefineXML */
NULL, /* domainUndefine */
- NULL, /* domainAttachDevice */
+ phypAttachDevice, /* domainAttachDevice */
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
@@ -1779,6 +2258,1199 @@ virDriver phypDriver = {
NULL, /* domainSnapshotDelete */
};
+virStorageDriver phypStorageDriver = {
I moved this (and phypDriver) later in the file, and marked them static,
to match with the fact that I marked a lot of functions static as part
of reducing scope.
Then it got to be quite unwieldy, so I'm now focusing on preparing a
patch series that tackles this project in a more incremental manner.
I'll post them for review soon (it no longer looks enough like your
original for me to feel justified with acking your version but pushing
mine, so I have to wait for you to confirm that my refactoring works).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org