On Fri, Mar 20, 2009 at 01:58:50PM -0300, Eduardo Otubo wrote:
> I've been working on a libvirt extension to manage IBM's Power VMs
> (LPARs). The Power systems are managed through management console
> referred to as the HMC or using a management partition (IVM). Both HMC
> and IVM runs an SSH, then you can reach it via command line, and an HTTP
> server, then you can reach it via web browser.
>
> The protocol between the console and the partition (LPAR) is not
> disclosed, therefore I propose the driver to execute commands remoetly
> over an SSH connection to the consoles to manage IBM LPARs.
That seems like a reasonably choice, unless the web server in the
HMC/IVM provided a good formal API like xmlrpc.
> The patch attached is the first scratch of the driver that will interact
> with HMC over a SSH connection. The URI model that is
> used in virsh command line is:
>
> virsh --conect phyp://$user@$server
>
> Some known issues are:
> * Next step is to make the URI like this: phyp://$user@
> $HMC/@managed_system. Almost finished. What it takes now is
> $server = $HMC = $managed_system.
> * Next features in my TODO list are "resume", "stop" and "reboot" the
> LPAR.
So if I'm understanding what I read on the web correctly, the HMC is a
machine that is typically separate from the host running virtualization.
Thus a single HMC can manage multiple hosts.
For the URI scheme, I'd see two possible options
Either use the path component of the URI for managed system name.
phyp://$user@$HMC/$managedsystem
Or use a query parameter
phyp://$user@$HMC/?managedsystem=$managedsystem
I reckon I'd favour the former.
> diff --git a/configure.in b/configure.in
> index 6b2bb5e..201a7dc 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -182,6 +182,8 @@ AC_ARG_WITH([uml],
> [ --with-uml add UML support (on)],[],[with_uml=yes])
> AC_ARG_WITH([openvz],
> [ --with-openvz add OpenVZ support (on)],[],[with_openvz=yes])
> +AC_ARG_WITH([phyp],
> +[ --with-phyp add IBM HMC/IVM support (on)],[],[with_phyp=yes])
> AC_ARG_WITH([lxc],
> [ --with-lxc add Linux Container support (on)],[],[with_lxc=yes])
> AC_ARG_WITH([test],
> @@ -714,6 +716,21 @@ AM_CONDITIONAL([HAVE_NUMACTL], [test "$with_numactl" != "no"])
> AC_SUBST([NUMACTL_CFLAGS])
> AC_SUBST([NUMACTL_LIBS])
>
> +if test "$with_phyp" = "yes"; then
> + AC_CHECK_LIB([ssh],[ssh_new],[
> + LIBSSH_LIBS="$LIBSSH_LIBS -lssh -L/usr/local/lib/"
> + AC_SUBST([LIBSSH_LIBS])],[
> + AC_MSG_ERROR([You must install the libssh to compile Phype driver.])])
> +
> + AC_CHECK_HEADERS([libssh/libssh.h],[
> + LIBSSH_CFLAGS="-I/usr/local/include/libssh"
> + AC_SUBST([LIBSSH_CFLAGS])],[
> + AC_MSG_ERROR([Cannot find libssh headers.Is libssh installed ?])],[])
> + AC_DEFINE_UNQUOTED([WITH_PHYP], 1,
> + [whether IBM HMC / IVM driver is enabled])
> +fi
> +AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"])
For this it is preferable to avoid using hardcoded paths in this way. If
we are going to use libssh2 for phyp driver, then I reckon it woul dbe
desirable to also use it for our existing remote RPC driver too (it currently
just fork/exec's /usr/bin/ssh). Thus I'd recommend adding an explicit arg
for --with-libssh2=$PREFIX. So if someone did
--with-libssh2
it'd just look for it in /usr. While, if they did
--with-libssh2=/usr/local
then it'd search the alternate path given.
Then, when processing your '$with_phyp' arg I'd recommend the following
logic
- If no --with-phyp arg is given
- If libssh2 was detected, enable phyp driver by default
- else disable the phyp driver by default
- If a --with-phyp arg is given
- If libssh2 was detected, then use it
- Else AC_MSG_ERROR() to tell use libssh was missing & is
needed for phyp
- If a --without-phyp arg is given
- diable the phyp driver
> +
> dnl virsh libraries
> AC_CHECK_HEADERS([readline/readline.h])
>
> @@ -1345,6 +1362,7 @@ AC_MSG_NOTICE([ QEMU: $with_qemu])
> AC_MSG_NOTICE([ UML: $with_uml])
> AC_MSG_NOTICE([ OpenVZ: $with_openvz])
> AC_MSG_NOTICE([ LXC: $with_lxc])
> +AC_MSG_NOTICE([ IBM HMC/IVM: $with_phyp])
> AC_MSG_NOTICE([ Test: $with_test])
> AC_MSG_NOTICE([ Remote: $with_remote])
> AC_MSG_NOTICE([ Network: $with_network])
Can you either shortern this to just 'Phyp: $with_phyp', or
fix the indentation for all the other existing lines here,
so everything is aligned sensibly.
> diff --git a/src/phyp_conf.h b/src/phyp_conf.h
> new file mode 100644
> index 0000000..51ca65e
> --- /dev/null
> +++ b/src/phyp_conf.h
> @@ -0,0 +1,44 @@
> +/*
> + * 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 <sys/types.h>
> +#include <config.h>
This should typically be done in the .c file, rather than the .h files.
> +static virDrvOpenStatus
> +phypOpen(virConnectPtr conn,
> + virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> + int flags ATTRIBUTE_UNUSED)
> +{
> + if (!conn || !conn->uri)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + if (conn->uri->scheme == NULL || conn->uri->server == NULL)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + int ssh_auth = 0, exit_status = 0;
> + char *banner;
> + char *password;
> +
> + SSH_SESSION *session;
> + SSH_OPTIONS *opt;
Our general coding style is to keep the variabl declarations at
the start of the nearest enclosing {} block. So either at start
of the function, or start of the while/if/for loop.
> +
> + 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)) {
> + fprintf(stderr, "Connection failed : %s\n",
> + ssh_get_error(session));
> + ssh_disconnect(session);
> + ssh_finalize();
> + exit_status = SSH_CONN_ERR;
> + return VIR_DRV_OPEN_DECLINED;
> + }
> +
> + /*trying to use pub key */
> + if ((ssh_auth =
> + ssh_userauth_autopubkey(session, NULL)) == SSH_AUTH_ERROR) {
> + fprintf(stderr, "Authenticating with pubkey: %s\n",
> + ssh_get_error(session));
> + exit_status = SSH_AUTH_PUBKEY_ERR;
> + return VIR_DRV_OPEN_DECLINED;
> + }
> +
> + if ((banner = ssh_get_issue_banner(session))) {
> + printf("%s\n", banner);
> + VIR_FREE(banner);
> + }
> +
> + if (ssh_auth != SSH_AUTH_SUCCESS) {
> + password = getpass("Password: ");
Rather than calling 'getpass', you should check to see if the caller
has suplied an authentication callback - eg the
'virConnectAuthPtr auth'
parameter which is passed in to this open method. 'getpass' will
prompt on the command line, which isn't suitable for graphical
apps, so you need to use the callback for collecting credentials.
virsh has a default callback implementation that knows how to
prompt for both username and password if requested by then open
method,
You could also use this to prompt for a passphrase if the SSH
private key is not unlocked.
> + if (ssh_userauth_password(session, NULL, password) !=
> + SSH_AUTH_SUCCESS) {
> + fprintf(stderr, "Authentication failed: %s\n",
> + ssh_get_error(session));
> + ssh_disconnect(session);
> + memset(password, 0, strlen(password));
> + exit_status = SSH_AUTH_ERR;
> + return VIR_DRV_OPEN_DECLINED;
> + } else
> + goto exec;
> + } else
> + goto exec;
> +
> + 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;
> +}
> +
> +static int
> +__inner_exec_command(SSH_SESSION * session, char *cmd,
> + char *textual_return)
> +{
> + CHANNEL *channel = channel_new(session);
> + BUFFER *readbuf = buffer_new();
> +
> + int exit_status = 0, temp_return = 0, buffer_size = 0;
> +
> + memset(textual_return, 0, sizeof(textual_return));
> +
> + if (channel_open_session(channel) == SSH_ERROR)
> + return SSH_CHANNEL_OPEN_ERR;
> +
> + if (channel_request_exec(channel, cmd) == SSH_ERROR)
> + return SSH_CHANNEL_EXEC_ERR;
> +
> + while (channel && channel_is_open(channel)) {
> + temp_return = channel_read(channel, readbuf, 0, 0);
> +
> + strncat(textual_return, buffer_get(readbuf),
> + buffer_get_len(readbuf));
> + buffer_size += buffer_size + buffer_get_len(readbuf);
> +
> + if (temp_return == 0) {
> + textual_return[buffer_size + 1] = 0;
> + exit_status = channel_get_exit_status(channel);
> + break;
> + }
> + }
> +
> + if (channel_send_eof(channel) == SSH_ERROR)
> + return SSH_CHANNEL_SEND_EOF_ERR;
> +
> + if (channel_close(channel) == SSH_ERROR)
> + return SSH_CHANNEL_CLOSE_ERR;
> +
> + return exit_status;
> +}
> +
> +static int
> +phypGetLparID(SSH_SESSION * ssh_session, const char *managed_system,
> + const char *name)
> +{
> + char id_c[10];
> + unsigned int i = 0;
> + int ret = 0;
> + char *cmd;
> + char *textual_return;
> +
> + if (VIR_ALLOC_N(cmd, MAXSIZE+sizeof(name)+sizeof(managed_system))) {
> + return PHYP_NO_MEM;
> + }
> +
> + if (VIR_ALLOC_N(textual_return, MAXSIZE)) {
> + return PHYP_NO_MEM;
> + }
> +
> + memset(id_c, 0, sizeof(id_c));
> +
> + sprintf(cmd,
> + "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> + managed_system, name);
In this case you'll find it simpler to use the virAsprintf() function
which is available from src/util.h. This will automatically allocate an
char * buffer large enough for the data your printf'ing. I think you
probably also want to make sure you quote the string args in yuour
SSH commands, and possibly also worry about escaping shell magic
characters.
> + ret = __inner_exec_command(ssh_session, cmd, textual_return);
I'd also recommend against passing in a fixed allocated buffer
'char *textual_return' here. Instead, use the 'virBuffer' object
(from src/buf.h) which implements grow-on-demand buffer with
strong error checking for OOM.
> +
> + if (ret) {
> + CLEANUP;
> + return LPAR_EXEC_ERR;
> + }
> +
> + for (i = 0; textual_return[i] != '\n'; i++)
> + id_c[i] = textual_return[i];
> + id_c[i] = '\0';
> +
> + CLEANUP;
> + return atoi(id_c);
> +}
> +
> +static int
> +phypGetLparNAME(SSH_SESSION * ssh_session, const char *managed_system,
> + unsigned int lpar_id, char *lpar_name)
> +{
> + unsigned int i = 0;
> + int ret = 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;
> + }
> +
> + sprintf(cmd,
> + "lssyscfg -r lpar -m %s --filter lpar_ids=%d -F name",
> + managed_system, lpar_id);
> + ret = __inner_exec_command(ssh_session, cmd, textual_return);
> +
> + if (ret) {
> + CLEANUP;
> + lpar_name = NULL;
> + return LPAR_EXEC_ERR;
> + }
> +
> + for (i = 0; textual_return[i] != '\n'; i++)
> + lpar_name[i] = textual_return[i];
> + lpar_name[i] = '\0';
> +
> + CLEANUP;
> + return ret;
> +}
> +
> +static int
> +phypGetLparUUID(SSH_SESSION * ssh_session, const char *managed_system,
> + unsigned int lpar_id, unsigned char *lpar_uuid)
> +{
> + unsigned int i = 0;
> + int ret = 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;
> + }
> +
> + sprintf(cmd,
> + "lssyscfg -r lpar -m %s --filter lpar_ids=%d -F logical_serial_num",
> + managed_system, lpar_id);
> + ret = __inner_exec_command(ssh_session, cmd, textual_return);
> +
> + if (ret) {
> + CLEANUP;
> + lpar_uuid = NULL;
> + return LPAR_EXEC_ERR;
> + }
> +
> + for (i = 0; textual_return[i] != '\n'; i++)
> + lpar_uuid[i] = textual_return[i];
> + lpar_uuid[i] = '\0';
> +
> + CLEANUP;
> + return ret;
> +}
> +
> +static int
> +phypNumDomains(virConnectPtr conn)
> +{
> + SSH_SESSION *ssh_session = conn->privateData;
> + const char *managed_system = conn->uri->server;
> + int ret = 0;
> + unsigned int i = 0, ndom = 1;
> + 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;
> + }
For errors that occur in driver APIs, you can't return an error code
in this way. See the 'src/libvirt.c' docs for description of the error
code for method - it is usually '-1' or 'NULL' as appropriate.
To give more detail to the caller, you can use one of the error reporting
functions from src/virterror_internal.h
> +
> + sprintf(cmd, "lssyscfg -r lpar -m %s -F lpar_id", managed_system);
> + ret = __inner_exec_command(ssh_session, cmd, textual_return);
> +
> + if (ret) {
> + CLEANUP;
> + return 0;
> + }
> +
> + for (i = 0; textual_return[i] != 0; i++)
> + if (textual_return[i] == '\n')
> + ndom++;
> +
> + VIR_FREE(cmd);
> + VIR_FREE(textual_return);
> + return ndom;
> +}
> +
> +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);
> +
> + for (i = 0; textual_return[i] != 0; i++) {
> + if (textual_return[i] == '\n') {
> + ids[nids] = atoi(id_c);
> + memset(id_c, 0, 10);
> + j = 0;
> + nids++;
> + continue;
> + }
> + id_c[j] = textual_return[i];
> + j++;
> + }
> +
> + VIR_FREE(cmd);
> + VIR_FREE(textual_return);
> + return nids;
> +}
> +
> +static virDomainPtr
> +phypDomainLookupByName(virConnectPtr conn, const char *name)
> +{
> + SSH_SESSION *ssh_session = conn->privateData;
> + virDomainPtr dom = NULL;
> + const char *managed_system = conn->uri->server;
> + unsigned char *lpar_uuid;
> +
> + if (VIR_ALLOC_N(lpar_uuid, 100 * sizeof(char))) {
> + return NULL;
> + }
> + int lpar_id = 0;
> +
> + lpar_id = phypGetLparID(ssh_session, managed_system, name);
> + if (lpar_id == PHYP_NO_MEM)
> + return NULL;
> +
> + if (phypGetLparUUID(ssh_session, managed_system, lpar_id, lpar_uuid) ==
> + PHYP_NO_MEM)
> + 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;
> + const char *managed_system = conn->uri->server;
> + char *lpar_name;
> + unsigned char *lpar_uuid;
> +
> + if (VIR_ALLOC_N(lpar_name, 100 * sizeof(char)))
> + return NULL;
> +
> + if (VIR_ALLOC_N(lpar_uuid, 100 * sizeof(char)))
> + return NULL;
> +
> + if (phypGetLparNAME(ssh_session, managed_system, lpar_id, lpar_name) ==
> + PHYP_NO_MEM)
> + return NULL;
> +
> + if (phypGetLparUUID(ssh_session, managed_system, lpar_id, lpar_uuid) ==
> + 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;
> +}
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list