Hello Daniel, I've been talking to libssh guys and they did a lot of changes trying to be more compliant. What do you think about reconsidering the use of libssh instead of libssh2? It's fixed in svn trunk now. This issue is solved since at least one year ago. This is being fixed and it's almost done. Libssh is ported to win32 and works very fine. In their words "can't agree more with him. SSH1 support was made initialy to support old cisco routers". All of these issues will be available in the libssh-0.3, which will be released in the end of this week, 10/april. Do you think after all these changes libssh will be compliant to be used in libvirt? What do you think? []'s -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com

Hello again, I very am glad with all the feedback I got. The following attachtment is the result of the work using all the suggestions the list made. I also have a few comments over this new RFC. Here is a little Changelog: * Now the URI is recognized as phyp://user@hmc/managed_system. To use path component of the uri struct, I had to implement a little function to strip out the first slash of the string, giving me only 'path', instead of '/path'. * I implemented the option --with-libssh and --with-phyp right according to what Daniel Berrange suggested. * Now all the variable-size string buffers are manipulated with the internal API virBuffer* to make it safer. | +- This made me think and change the __inner_exec_command function. Now it returns char * with all the textual return, and I am passing int * as reference to get the exit status - all the information I need about the remote executed command. | +- And this also made me write a little auxiliar function to strip out the '\n' from the textual return. Known and unsolved issues: * Authentication - need to improve the authenticantion as Daniel Berrange said. Still in progress. * Error codes. * Logging and debugging messages. Any comments are welcome, []'s Em Sex, 2009-03-20 às 13:58 -0300, Eduardo Otubo escreveu: -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com

Hello DV, I agree with your suggestions and everything is fixed now. But I would like to discuss some points. This was a libssh issue This was the only way I could make it work. But now the libssh is fixed and this function is a little different - http://pastebin.com/fbf84426 Here, I just would like to free the Buffer, and this was the best way I find since I couldn't find any better function to manipulate this. How do I simply free a buffer using the internal virBuffer* API? Thanks for the help, -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com

Em Qua, 2009-05-06 às 09:26 +0200, Daniel Veillard escreveu: Hello DV, Nice! Hope you enjoyed your vacations :) In fact I am part of the libssh team, I help them with testing. And yes, we're planning to release a new version of libssh this week with all these fixes. But I still need to make it sure in configure.in. I'll do it right now. All right, got it. Thanks for the tips :) []'s -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com

Em Qua, 2009-05-06 às 09:44 +0100, Daniel P. Berrange escreveu: DV and danpb, First of all, thanks for the tips. Is helping me a lot. Here is the phyp_driver.c with the memory leaks fixed with your suggestions. With those things done, do you think this code is enough and compliant to libvirt patterns to be included in the next libvirt release? The only feature we have until now is just to list the LPARs ("Logical PARtitions", the IBM virtual machines for Power). Once this code is safe and goot enough, the implementations of new commands will be much faster and easier. Here is the code: /* * Copyright IBM Corp. 2009 * * phyp_driver.c: ssh layer to access Power Hypervisors * * Authors: * Eduardo Otubo <otubo at linux.vnet.ibm.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #include <config.h> #include <sys/types.h> #include <limits.h> #include <string.h> #include <strings.h> #include <stdio.h> #include <stdarg.h> #include <stdlib.h> #include <unistd.h> #include <errno.h> #include <stdio.h> #include <libssh/libssh.h> #include "internal.h" #include "util.h" #include "datatypes.h" #include "buf.h" #include "memory.h" #include "logging.h" #include "driver.h" #include "libvirt/libvirt.h" #include "virterror_internal.h" #include "phyp_driver.h" #define VIR_FROM_THIS VIR_FROM_PHYP /* * TODO: I still need to implement a way to distinguish * an HMC from an IVM * */ static virDrvOpenStatus phypOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) { int ssh_auth = 0, exit_status = 0; char *banner; SSH_SESSION *session; SSH_OPTIONS *opt; if (!conn || !conn->uri) return VIR_DRV_OPEN_DECLINED; if (conn->uri->scheme == NULL || conn->uri->server == NULL || conn->uri->path == NULL) return VIR_DRV_OPEN_DECLINED; session = ssh_new(); opt = ssh_options_new(); if (!conn->uri->port) conn->uri->port = 22; /*setting some ssh options */ ssh_options_set_host(opt, conn->uri->server); ssh_options_set_port(opt, conn->uri->port); ssh_options_set_username(opt, conn->uri->user); ssh_set_options(session, opt); /*starting ssh connection */ if (ssh_connect(session)) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("connection failed")); ssh_disconnect(session); ssh_finalize(); } /*trying to use pub key */ if ((ssh_auth = ssh_userauth_autopubkey(session, NULL)) == SSH_AUTH_ERROR) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s : %s", _("authenticating with public key ailed"), ssh_get_error(session)); } if ((banner = ssh_get_issue_banner(session))) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", banner); VIR_FREE(banner); } if (ssh_auth != SSH_AUTH_SUCCESS) { int i; int hasPassphrase = 0; virConnectCredential creds[] = { {VIR_CRED_PASSPHRASE, "password", "Password", NULL, NULL, 0}, }; if (!auth || !auth->cb) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("no authentication callback provided")); goto err; } for (i = 0; i < auth->ncredtype; i++) { if (auth->credtype[i] == VIR_CRED_PASSPHRASE) hasPassphrase = 1; } if (!hasPassphrase) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("required credentials are not supported")); goto err; } int res = (auth->cb) (creds, ARRAY_CARDINALITY(creds), auth->cbdata); if (res < 0) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("unable to fetch credentials")); goto err; } char *password = creds[0].result; char *username = conn->uri->user; if (ssh_userauth_password(session, username, password) != SSH_AUTH_SUCCESS) { virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s : % s", "authentication failed", ssh_get_error(session)); ssh_disconnect(session); memset(password, 0, strlen(password)); memset(username, 0, strlen(username)); goto err; } else goto exec; } else goto exec; err: exit_status = SSH_CONN_ERR; return VIR_DRV_OPEN_DECLINED; exec: conn->privateData = session; return VIR_DRV_OPEN_SUCCESS; } static int phypClose(virConnectPtr conn) { SSH_SESSION *ssh_session = conn->privateData; ssh_disconnect(ssh_session); return 0; } /* this functions is the layer that manipulates the ssh channel itself * and executes the commands on the remote machine */ static char * __inner_exec_command(SSH_SESSION * session, char *cmd, int *exit_status) { CHANNEL *channel = channel_new(session); char buf[4096] = { 0 }; virBuffer tex_ret = VIR_BUFFER_INITIALIZER; int ret = 0; if (channel_open_session(channel) == SSH_ERROR) goto err; if (channel_request_exec(channel, cmd) == SSH_ERROR) goto err; if (channel_send_eof(channel) == SSH_ERROR) goto err; while (channel && channel_is_open(channel)) { ret = channel_read(channel, buf, sizeof(buf), 0); if (ret < 0) goto err; if (ret == 0) { channel_send_eof(channel); if (channel_get_exit_status(channel) == -1) goto err; if (channel_close(channel) == SSH_ERROR) goto err; channel_free(channel); channel = NULL; goto exit; } virBufferAdd(&tex_ret, (const char *) &buf, sizeof(buf)); } err: (*exit_status) = SSH_CMD_ERR; return NULL; exit: return virBufferContentAndReset(&tex_ret); } /* return the lpar_id given a name and a managed system name */ static int phypGetLparID(SSH_SESSION * ssh_session, const char *managed_system, const char *name) { int exit_status = 0; int lpar_id = 0; char *char_ptr; char *cmd; if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id", managed_system, name) < 0) return 0; const char *tex_ret = __inner_exec_command(ssh_session, cmd, &exit_status); VIR_FREE(cmd); if (exit_status < 0 || tex_ret == NULL) return 0; if (virStrToLong_i(tex_ret, &char_ptr, 10, &lpar_id) == -1) return 0; return lpar_id; } /* return the lpar name given a lpar_id and a managed system name */ static char * phypGetLparNAME(SSH_SESSION * ssh_session, const char *managed_system, unsigned int lpar_id, int *exit_status) { char *cmd; if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s --filter lpar_ids=%d -F name", managed_system, lpar_id) < 0) return NULL; char *lpar_name = __inner_exec_command(ssh_session, cmd, (int *) exit_status); char *striped_lpar_name = (char *) malloc(sizeof(lpar_name) - 1); stripNewline(striped_lpar_name, lpar_name); VIR_FREE(cmd); if ((*exit_status) < 0 || lpar_name == NULL) return NULL; return striped_lpar_name; } /* return the lpar_uuid (which for now is its logical serial number) * given a lpar id and a managed system name */ static unsigned char * phypGetLparUUID(SSH_SESSION * ssh_session, const char *managed_system, unsigned int lpar_id, int *exit_status) { char *cmd; if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s --filter lpar_ids=%d -F logical_serial_num", managed_system, lpar_id) < 0) return NULL; unsigned char *lpar_uuid = (unsigned char *) __inner_exec_command(ssh_session, cmd, (int *) exit_status); unsigned char *striped_lpar_uuid = (unsigned char *) malloc(sizeof(lpar_uuid) - 1); stripNewline((char *) striped_lpar_uuid, (char *) lpar_uuid); VIR_FREE(cmd); if ((*exit_status) < 0 || lpar_uuid == NULL) return NULL; return striped_lpar_uuid; } static int phypNumDomains(virConnectPtr conn) { int exit_status = 0; int ndom = 0; char *char_ptr; char *cmd; char managed_system[strlen(conn->uri->path) - 1]; SSH_SESSION *ssh_session = conn->privateData; stripPath((char *) &managed_system, conn->uri->path); if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s -F lpar_id|grep -c ^[0-9]*", managed_system) < 0) { virReportOOMError(conn); return 0; } char *ret = __inner_exec_command(ssh_session, cmd, &exit_status); VIR_FREE(cmd); if (exit_status < 0 || ret == NULL) return 0; if (virStrToLong_i(ret, &char_ptr, 10, &ndom) == -1) return 0; return ndom; } static int phypListDomains(virConnectPtr conn, int *ids, int nids) { int exit_status = 0; int got = 0; char *char_ptr; unsigned int i = 0, j = 0; char id_c[10]; char *cmd; char managed_system[strlen(conn->uri->path) - 1]; SSH_SESSION *ssh_session = conn->privateData; stripPath((char *) &managed_system, conn->uri->path); memset(id_c, 0, 10); if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s -F lpar_id", managed_system) < 0) { virReportOOMError(conn); return 0; } char *domains = __inner_exec_command(ssh_session, cmd, &exit_status); /* I need to parse the textual return in order to get the domains */ if (exit_status < 0 || domains == NULL) return 0; else { while (got < nids) { if (domains[i] == '\n') { if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) return 0; memset(id_c, 0, 10); j = 0; got++; } else { id_c[j] = domains[i]; j++; } i++; } } VIR_FREE(cmd); return got; } static virDomainPtr phypDomainLookupByName(virConnectPtr conn, const char *name) { SSH_SESSION *ssh_session = conn->privateData; virDomainPtr dom = NULL; int lpar_id = 0; int exit_status = 0; char managed_system[strlen(conn->uri->path) - 1]; stripPath((char *) &managed_system, conn->uri->path); lpar_id = phypGetLparID(ssh_session, managed_system, name); if (lpar_id == PHYP_NO_MEM) return NULL; unsigned char *lpar_uuid = phypGetLparUUID(ssh_session, managed_system, lpar_id, &exit_status); if (exit_status == PHYP_NO_MEM || lpar_uuid == NULL) return NULL; dom = virGetDomain(conn, name, lpar_uuid); if (dom) dom->id = lpar_id; VIR_FREE(lpar_uuid); return dom; } static virDomainPtr phypDomainLookupByID(virConnectPtr conn, int lpar_id) { SSH_SESSION *ssh_session = conn->privateData; virDomainPtr dom = NULL; int exit_status = 0; char managed_system[strlen(conn->uri->path) - 1]; stripPath((char *) &managed_system, conn->uri->path); char *lpar_name = phypGetLparNAME(ssh_session, managed_system, lpar_id, &exit_status); if (exit_status == PHYP_NO_MEM) return NULL; unsigned char *lpar_uuid = phypGetLparUUID(ssh_session, managed_system, lpar_id, &exit_status); if (exit_status == PHYP_NO_MEM) return NULL; dom = virGetDomain(conn, lpar_name, lpar_uuid); if (dom) dom->id = lpar_id; VIR_FREE(lpar_name); VIR_FREE(lpar_uuid); return dom; } static virDriver phypDriver = { VIR_DRV_PHYP, "PHYP", phypOpen, /* open */ phypClose, /* close */ NULL, /* supports_feature */ NULL, /* type */ NULL, /* version */ NULL, /* getHostname */ NULL, /* getMaxVcpus */ NULL, /* nodeGetInfo */ NULL, /* getCapabilities */ phypListDomains, /* listDomains */ phypNumDomains, /* numOfDomains */ NULL, /* domainCreateXML */ phypDomainLookupByID, /* domainLookupByID */ NULL, /* domainLookupByUUID */ phypDomainLookupByName, /* domainLookupByName */ NULL, /* domainSuspend */ NULL, /* domainResume */ NULL, /* domainShutdown */ NULL, /* domainReboot */ NULL, /* domainDestroy */ NULL, /* domainGetOSType */ NULL, /* domainGetMaxMemory */ NULL, /* domainSetMaxMemory */ NULL, /* domainSetMemory */ NULL, /* domainGetInfo */ NULL, /* domainSave */ NULL, /* domainRestore */ NULL, /* domainCoreDump */ NULL, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ NULL, /* domainGetMaxVcpus */ NULL, /* domainGetSecurityLabel */ NULL, /* nodeGetSecurityModel */ NULL, /* domainDumpXML */ NULL, /* listDefinedDomains */ NULL, /* numOfDefinedDomains */ NULL, /* domainCreate */ NULL, /* domainDefineXML */ NULL, /* domainUndefine */ NULL, /* domainAttachDevice */ NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ NULL, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ NULL, /* domainMigrateFinish2 */ NULL, /* nodeDeviceDettach */ NULL, /* nodeDeviceReAttach */ NULL, /* nodeDeviceReset */ }; int phypRegister(void) { virRegisterDriver(&phypDriver); return 0; } /* function that helps me to strip out the first slash from the * uri: phyp://user@hmc/path * * '/path' becomes 'path' * */ void stripPath(char *striped_path, char *path) { unsigned int i = 0; for (i = 1; i <= strlen(path); i++) striped_path[i - 1] = path[i]; striped_path[i] = '\0'; return; } /* function to strip out the '\n' of the end of some string */ void stripNewline(char *striped_string, char *string) { unsigned int i = 0; for (i = 0; i <= strlen(string); i++) if (string[i] != '\n') striped_string[i] = string[i]; striped_string[strlen(string) - 1] = '\0'; return; } Best regards, []'s -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com

I just opened a new thread in order to discuss the usage of this function and UUID. I adjusted the level of problem as I thought that would be more consistent. (the fixed phyp_driver.c is attatched) Agreed and fixed. ok. The exit_status here refers to the exit status of the command executed remotely and not the return of the function. The return of the '__inner_exec_command' function is NULL or the textual return from the remote command. Agreed. Actually this was at my todo list and its done now. In order to check virBufferError and virReportOOMError I added 'virConnectPtr conn' as parameter to __inner_exec_command function. Everything is recorded now. These two lines opened my mind yesterday and made me remove those two ugly function. I wasn't happy with them and I was trying other ways to handle those chars. The new "stripPath" is reduced to: char *managed_system = conn->uri->path; if (managed_system[0] == '/') managed_system++; And the new "stripNewline" I did as you told me: char *char_ptr = strchr(lpar_uuid, '\n'); if (char_ptr) *char_ptr = '\0'; This is not fixed in the phyp_driver.c that is attatched here, but will be fixed as soon as we discuss that UUID issue in the thread is just opened. This also made me think a better way to handle managed_system without this stack allocation. Also fixed. Fixed! Fixed! -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 otubo(a)linux.vnet.ibm.com