Re: [libvirt] [RFC] Power Hypervisor Libvirt support
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(a)linux.vnet.ibm.com