Em Qua, 2009-05-06 às 09:44 +0100, Daniel P. Berrange escreveu:
> On Mon, May 04, 2009 at 05:50:03PM -0300, Eduardo Otubo wrote:
> >
> > > > +
> > > > +/* 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;
> > > > + virBuffer cmd = VIR_BUFFER_INITIALIZER;
> > > > +
> > > > + virBufferVSprintf(&cmd,
> > > > + "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> > > > + managed_system, name);
> > > > + const char *tex_ret =
> > > > + __inner_exec_command(ssh_session, virBufferContentAndReset(&cmd),
> > > > + &exit_status);
> > > > +
> > > > + virBufferContentAndReset(&cmd);
> > >
> > > Huh ? you're supposed to get the resulting char *, and then free it
> > > later once you're done with the data. Here youre just leaking memory
> > > I'm afraid
> > >
> > > same thing for most of the commands in that file.
> >
> > 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?
>
> The virBufferContentAndReset() method returns you the internal char *
> string, and resets the virBuffer state to its inital value. You are
> now owner of the char * string, and are responsible for free'ing it
> when done.
>
> You should also check virBufferError() and report OOM error if it fails.
> So, in the above example. what you'd want todo is
>
> static int
> phypGetLparID(SSH_SESSION * ssh_session, const char *managed_system,
> const char *name)
> {
> int exit_status = 0;
> virBuffer cmd = VIR_BUFFER_INITIALIZER;
> char *buf;
>
> virBufferVSprintf(&cmd,
> "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> managed_system, name);
> if (virBufferError(&cmd)) {
> virReportOOMError(conn);
> return NULL;
> }
>
> buf = virBufferContentAndReset(&cmd);
> const char *tex_ret =
> __inner_exec_command(ssh_session, buf
> &exit_status);
> VIR_FREE(buf);
> }
>
>
> That all said, in this particular function I it is overkill to use
> the virBuffer APIs, since you've only got a single printf() call
> to make. virBuffer is more appropriate when you have 2 or more
> printfs() or strcats() to make. If just doing a single printf,
> then use virAsprintf,
>
> eg
>
> static int
> phypGetLparID(SSH_SESSION * ssh_session, const char *managed_system,
> const char *name)
> {
> int exit_status = 0;
> char *buf;
>
> if (virAsprintf(&buf,
> "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> managed_system, name) < 0) {
> virReportOOMError(conn);
> return NULL;
> }
>
> const char *tex_ret =
> __inner_exec_command(ssh_session, buf
> &exit_status);
> VIR_FREE(buf);
> }
>
>
> Regards,
> Daniel
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@xxxxxxxxxxxxxxxxxx
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list