Eduardo Otubo wrote:
Sorry all about my last email, the subject should be "[RFC] Power
Hypervisor
Libvirt support". There should be an error here.
Thanks again,
Hello,
I don't have access to a phyp machine, so unfortunately, I'm unable to
test this out. However, I've done a quick read through and have a few
comments.
> diff --git a/src/phyp_conf.c b/src/phyp_conf.c
> new file mode 100644
> index 0000000..4317936
> --- /dev/null
> +++ b/src/phyp_conf.c
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright IBM Corp. 2009
> + *
> + * phyp_driver.c: ssh layer to access Power Hypervisors
This file is phyp_conf.c, not phyp_driver.c
> 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
This is phyp_conf.h, not phyp_driver.c
> +#include "domain_conf.h"
> +/* Main driver state */
> +typedef struct __phyp_driver phyp_driver_t;
> +struct __phyp_driver {
> + virMutex lock;
> +
> + virCapsPtr *caps;
> +
> + virDomainObjList *domains;
> +
> + char *configDir;
> + char *autostartDir;
> + char *stateDir;
> + char *logDir;
> + int have_netns;
> +};
I don't see this struct / typedef being used anywhere. Will this be
used for something in the future?
> +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);
You don't check ret to see if an error occured.
> +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;
Any reason not to use a virDomainObjPtr to hold these values?
> +
> + if (VIR_ALLOC_N(lpar_name, 100 * sizeof(char)))
> + return NULL;
> +
> + if (VIR_ALLOC_N(lpar_uuid, 100 * sizeof(char)))
> + return NULL;
You then wouldn't need to use a magic number here. You could just do an
alloc of the virDomainObjPtr object itself.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin@xxxxxxxxxxxxxxxxxx
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list