2:51 p.m.
So this is a huge set, and I'm sorry for dumping it all at once, but I kept bouncing
between the various patches as I worked on it, so sending any one out earlier probably
would have backfired. This brings SettingsDefineCapabilities to a working state, as well
as does all the background work. Clients should now be able to follow the chain of
associations all the way from a HostSystem instance to a series of RASD instances that
tell them the min, max, increment (read: granulatiry), and default values for NetworkPort,
Disk, Memory, and Processor.
Once this set is processed and approved I have some plans to try and make things a little
more compact, as there is a reasonable amount of duplication in some places, and a couple
of cases could use some polish, but with this set they should all be working, so it was
about time to get it out for review.
2:51 p.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557266 18000
# Node ID fd6deb234772b44fe549a3513a47115a01e20f7d
# Parent bd1b1067d106ecc1546c3b2436a43f59f8eaba15
Turns out that the sdc_rasd_prop functions might need to report status or see the passed
in reference. This updates the typedef and the memory functions, since those are already
in the tree.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r bd1b1067d106 -r fd6deb234772 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 10:43:48 2007 -0800
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:27:46 2007 -0500
@@ -96,84 +96,96 @@ static bool free_rasd_prop_list(struct s
return true;
}
-static struct sdc_rasd_prop *mem_max(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+static struct sdc_rasd_prop *mem_max(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ struct sdc_rasd_prop *rasd = NULL;
uint64_t max_vq = MAX_MEM;
- struct sdc_rasd_prop max[] = {
+ struct sdc_rasd_prop tmp[] = {
{"InstanceID", (CMPIValue *)"Maximum", CMPI_chars},
{"AllocationUnits", (CMPIValue *)"MegaBytes",
CMPI_chars},
{"VirtualQuantity", (CMPIValue *)&max_vq, CMPI_uint64},
PROP_END
};
- ret = dup_rasd_prop_list(max, &rasd);
- if (ret)
- return rasd;
- else
- return NULL;
-}
-
-static struct sdc_rasd_prop *mem_min(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *mem_min(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ struct sdc_rasd_prop *rasd = NULL;
uint64_t min_vq = 64;
- struct sdc_rasd_prop min[] = {
+ struct sdc_rasd_prop tmp[] = {
{"InstanceID", (CMPIValue *)"Minimum", CMPI_chars},
{"AllocationUnits", (CMPIValue *)"MegaBytes",
CMPI_chars},
{"VirtualQuantity", (CMPIValue *)&min_vq, CMPI_uint64},
PROP_END
};
- ret = dup_rasd_prop_list(min, &rasd);
- if (ret)
- return rasd;
- else
- return NULL;
-}
-
-static struct sdc_rasd_prop *mem_def(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *mem_def(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ struct sdc_rasd_prop *rasd = NULL;
uint64_t def_vq = 256;
- struct sdc_rasd_prop def[] = {
+ struct sdc_rasd_prop tmp[] = {
{"InstanceID", (CMPIValue *)"Default", CMPI_chars},
{"AllocationUnits", (CMPIValue *)"MegaBytes",
CMPI_chars},
{"VirtualQuantity", (CMPIValue *)&def_vq, CMPI_uint64},
PROP_END
};
- ret = dup_rasd_prop_list(def, &rasd);
- if (ret)
- return rasd;
- else
- return NULL;
-}
-
-static struct sdc_rasd_prop *mem_inc(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *mem_inc(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ struct sdc_rasd_prop *rasd = NULL;
uint64_t inc_vq = 1;
- struct sdc_rasd_prop inc[] = {
+ struct sdc_rasd_prop tmp[] = {
{"InstanceID", (CMPIValue *)"Increment",
CMPI_chars},
{"AllocationUnits", (CMPIValue *)"MegaBytes",
CMPI_chars},
{"VirtualQuantity", (CMPIValue *)&inc_vq, CMPI_uint64},
PROP_END
};
- ret = dup_rasd_prop_list(inc, &rasd);
- if (ret)
- return rasd;
- else
- return NULL;
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
}
static struct sdc_rasd mem = {
@@ -190,17 +202,17 @@ static struct sdc_rasd *sdc_rasd_list[]
};
static CMPIInstance *sdc_rasd_inst(const CMPIBroker *broker,
+ CMPIStatus *s,
const CMPIObjectPath *ref,
struct sdc_rasd *rasd,
sdc_rasd_type type)
{
- CMPIStatus s = {CMPI_RC_OK, NULL};
CMPIInstance *inst = NULL;
struct sdc_rasd_prop *prop_list;
int i;
char *inst_id;
uint16_t resource_type;
- /* Defaults for the following three values are from
+ /* Defaults for the following are from
CIM_SettingsDefineCapabilities.mof. */
uint16_t policy = SDC_POLICY_INDEPENDENT;
uint16_t role = SDC_ROLE_SUPPORTED;
@@ -209,43 +221,40 @@ static CMPIInstance *sdc_rasd_inst(const
case SDC_RASD_MIN:
if (rasd->min == NULL)
goto out;
- prop_list = rasd->min();
+ prop_list = rasd->min(ref, s);
inst_id = "Minimum";
range = SDC_RANGE_MIN;
break;
case SDC_RASD_MAX:
if (rasd->max == NULL)
goto out;
- prop_list = rasd->max();
+ prop_list = rasd->max(ref, s);
inst_id = "Maximum";
range = SDC_RANGE_MAX;
break;
case SDC_RASD_INC:
if (rasd->inc == NULL)
goto out;
- prop_list = rasd->inc();
+ prop_list = rasd->inc(ref, s);
inst_id = "Increment";
range = SDC_RANGE_INC;
break;
case SDC_RASD_DEF:
if (rasd->def == NULL)
goto out;
- prop_list = rasd->def();
+ prop_list = rasd->def(ref, s);
inst_id = "Default";
role = SDC_ROLE_DEFAULT;
range = SDC_RANGE_POINT;
break;
default:
- CMSetStatusWithChars(broker, &s, CMPI_RC_ERR_FAILED,
+ CMSetStatusWithChars(broker, s, CMPI_RC_ERR_FAILED,
"Unsupported sdc_rasd type.");
goto out;
}
- if (prop_list == NULL) {
- CMSetStatusWithChars(broker, &s, CMPI_RC_ERR_FAILED,
- "Failed to get prop_list.");
+ if (s->rc != CMPI_RC_OK)
goto out;
- }
inst = get_typed_instance(broker,
"ResourceAllocationSettingData",
@@ -291,7 +300,11 @@ static CMPIStatus sdc_rasds_for_type(con
if (rasd) {
for (i = SDC_RASD_MIN; i <= SDC_RASD_INC; i++) {
- inst = sdc_rasd_inst(_BROKER, ref, rasd, i);
+ inst = sdc_rasd_inst(_BROKER, &s, ref, rasd, i);
+ if (s.rc != CMPI_RC_OK) {
+ CU_DEBUG("Problem getting inst.");
+ goto out;
+ }
CU_DEBUG("Got inst.\n");
if (inst != NULL) {
inst_list_add(list, inst);
@@ -307,6 +320,7 @@ static CMPIStatus sdc_rasds_for_type(con
"Unsupported device type.");
}
+ out:
return s;
}
diff -r bd1b1067d106 -r fd6deb234772 src/Virt_SettingsDefineCapabilities.h
--- a/src/Virt_SettingsDefineCapabilities.h Thu Nov 08 10:43:48 2007 -0800
+++ b/src/Virt_SettingsDefineCapabilities.h Thu Nov 08 16:27:46 2007 -0500
@@ -50,7 +50,8 @@ struct sdc_rasd_prop {
CMPIType type;
};
-typedef struct sdc_rasd_prop *(*rasd_prop_func_t)(void);
+typedef struct sdc_rasd_prop *(*rasd_prop_func_t)(const CMPIObjectPath *ref,
+ CMPIStatus *s);
struct sdc_rasd {
uint16_t resource_type;
4:18 p.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557266 18000
JG> # Node ID fd6deb234772b44fe549a3513a47115a01e20f7d
JG> # Parent bd1b1067d106ecc1546c3b2436a43f59f8eaba15
JG> Turns out that the sdc_rasd_prop functions might need to report status or see the
passed in reference. This updates the typedef and the memory functions, since those are
already in the tree.
This one looks pretty straightforward. It looks like we could apply
this one independently if we wanted to, right? A rather
self-contained change, considering the size of the rest of the set.
JG> + if (!ret) {
JG> + cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
JG> + "Could not copy RASD.");
JG> + }
This is a total nit, so forgive my OCD, but I think that everywhere
else, the style of a cu_statusf() call is:
cu_statusf(broker, status, <newline>
CMPI_RC_ERR_FOO, <newline>
"Error message", [opt if short, else newline],
[opt]);
Could I convince you to change these? :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:18 a.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
Dan Smith wrote:
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557266 18000
JG> # Node ID fd6deb234772b44fe549a3513a47115a01e20f7d
JG> # Parent bd1b1067d106ecc1546c3b2436a43f59f8eaba15
JG> Turns out that the sdc_rasd_prop functions might need to report status or see the
passed in reference. This updates the typedef and the memory functions, since those are
already in the tree.
This one looks pretty straightforward. It looks like we could apply
this one independently if we wanted to, right? A rather
self-contained change, considering the size of the rest of the set.
This sets up the new function header for all the callbacks. I think it
can be applied separately in that it can be applied before the other
ones, but if we are going to put those in then this one will have to be
there. Did that answer the question?
JG> + if (!ret) {
JG> + cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
JG> + "Could not copy RASD.");
JG> + }
This is a total nit, so forgive my OCD, but I think that everywhere
else, the style of a cu_statusf() call is:
cu_statusf(broker, status, <newline>
CMPI_RC_ERR_FOO, <newline>
"Error message", [opt if short, else newline],
[opt]);
Could I convince you to change these? :)
Yea, no prob.
--
-Jay
5:41 p.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
Jay Gagnon wrote:
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557266 18000
# Node ID fd6deb234772b44fe549a3513a47115a01e20f7d
# Parent bd1b1067d106ecc1546c3b2436a43f59f8eaba15
Turns out that the sdc_rasd_prop functions might need to report status or see the passed
in reference. This updates the typedef and the memory functions, since those are already
in the tree.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r bd1b1067d106 -r fd6deb234772 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 10:43:48 2007 -0800
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:27:46 2007 -0500
@@ -96,84 +96,96 @@ static bool free_rasd_prop_list(struct s
return true;
}
-static struct sdc_rasd_prop *mem_max(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+static struct sdc_rasd_prop *mem_max(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ struct sdc_rasd_prop *rasd = NULL;
uint64_t max_vq = MAX_MEM;
- struct sdc_rasd_prop max[] = {
+ struct sdc_rasd_prop tmp[] = {
{"InstanceID", (CMPIValue *)"Maximum", CMPI_chars},
{"AllocationUnits", (CMPIValue *)"MegaBytes",
CMPI_chars},
{"VirtualQuantity", (CMPIValue *)&max_vq, CMPI_uint64},
PROP_END
};
- ret = dup_rasd_prop_list(max, &rasd);
- if (ret)
- return rasd;
- else
- return NULL;
-}
-
-static struct sdc_rasd_prop *mem_min(void)
-{
- bool ret;
- struct sdc_rasd_prop *rasd;
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
Why is the ref param needed here? Is that something you'll be adding
later?
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
9:21 a.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
Kaitlin Rupert wrote:
Jay Gagnon wrote:
> # HG changeset patch
> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
> # Date 1194557266 18000
> # Node ID fd6deb234772b44fe549a3513a47115a01e20f7d
> # Parent bd1b1067d106ecc1546c3b2436a43f59f8eaba15
> Turns out that the sdc_rasd_prop functions might need to report
> status or see the passed in reference. This updates the typedef and
> the memory functions, since those are already in the tree.
> Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
>
>
>
Why is the ref param needed here? Is that something you'll be adding
later?
For the most part, the ref param is unneeded, but some of the callbacks
(disk_max for example) need to look at the ref to determine which Pool
they are being asked about. All of the callbacks need to have the same
header because the sdc_rasd struct stores function pointers for the
callbacks and I really didn't want to do it as the always foreboding
(void *).
--
-Jay
9:31 a.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
JG> All of the callbacks need to have the same header because the
JG> sdc_rasd struct stores function pointers for the callbacks
Right, you're doing the correct thing here, IMHO.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
11:16 a.m.
New subject: [PATCH 1 of 6] Turns out that the sdc_rasd_prop functions might need to report status or see the passed in reference. This updates the typedef and the memory functions, since those are already in the tree
Dan Smith wrote:
JG> All of the callbacks need to have the same header because the
JG> sdc_rasd struct stores function pointers for the callbacks
Right, you're doing the correct thing here, IMHO.
Yep, agreed. I didn't see ref used in any of the functions in the first
patch, so I was confused. But I see why it's useful now. =)
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
2:51 p.m.
New subject: [PATCH 2 of 6] Need to reorder the .la lines in makefile since SettingsDefineCapabilities now depends on DevicePool
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557273 18000
# Node ID 543a0790d8615551153950de8f2f2fe3de107cf3
# Parent fd6deb234772b44fe549a3513a47115a01e20f7d
Need to reorder the .la lines in makefile since SettingsDefineCapabilities now depends on
DevicePool.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r fd6deb234772 -r 543a0790d861 src/Makefile.am
--- a/src/Makefile.am Thu Nov 08 16:27:46 2007 -0500
+++ b/src/Makefile.am Thu Nov 08 16:27:53 2007 -0500
@@ -33,12 +33,12 @@ provider_LTLIBRARIES = libVirt_ComputerS
libVirt_VirtualSystemManagementCapabilities.la \
libVirt_EnabledLogicalElementCapabilities.la \
libVirt_AllocationCapabilities.la \
- libVirt_SettingsDefineCapabilities.la \
libVirt_VSSD.la \
libVirt_HostedDependency.la \
libVirt_RegisteredProfile.la \
libVirt_ElementConformsToProfile.la \
libVirt_DevicePool.la \
+ libVirt_SettingsDefineCapabilities.la \
libVirt_HostedResourcePool.la \
libVirt_ElementCapabilities.la \
libVirt_ResourcePoolConfigurationService.la \
4:19 p.m.
New subject: [PATCH 2 of 6] Need to reorder the .la lines in makefile since SettingsDefineCapabilities now depends on DevicePool
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557273 18000
JG> # Node ID 543a0790d8615551153950de8f2f2fe3de107cf3
JG> # Parent fd6deb234772b44fe549a3513a47115a01e20f7d
JG> Need to reorder the .la lines in makefile since SettingsDefineCapabilities now
depends on DevicePool.
+1, obviously :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
2:51 p.m.
New subject: [PATCH 3 of 6] Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e. lack of warning when minimum or default is higher than maximum), but is functional
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557358 18000
# Node ID 94308147bed1693443d0741de6a30c5b0f77b0f1
# Parent 543a0790d8615551153950de8f2f2fe3de107cf3
Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e. lack of
warning when minimum or default is higher than maximum), but is functional.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 543a0790d861 -r 94308147bed1 src/Makefile.am
--- a/src/Makefile.am Thu Nov 08 16:27:53 2007 -0500
+++ b/src/Makefile.am Thu Nov 08 16:29:18 2007 -0500
@@ -85,7 +85,7 @@ libVirt_AllocationCapabilities_la_LIBADD
libVirt_AllocationCapabilities_la_LIBADD = -lVirt_RASD
libVirt_SettingsDefineCapabilities_la_SOURCES = Virt_SettingsDefineCapabilities.c
-libVirt_SettingsDefineCapabilities_la_LIBADD = -lVirt_RASD
+libVirt_SettingsDefineCapabilities_la_LIBADD = -lVirt_RASD -lVirt_DevicePool
libVirt_RegisteredProfile_la_SOURCES = Virt_RegisteredProfile.c
diff -r 543a0790d861 -r 94308147bed1 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:27:53 2007 -0500
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:29:18 2007 -0500
@@ -23,6 +23,8 @@
#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
+#include <sys/vfs.h>
+#include <errno.h>
#include "config.h"
@@ -37,8 +39,13 @@
#include "svpc_types.h"
#include "Virt_SettingsDefineCapabilities.h"
+#include "Virt_DevicePool.h"
const static CMPIBroker *_BROKER;
+
+/* These are used in more than one place so they are defined here. */
+#define SDC_DISK_MIN 2000
+#define SDC_DISK_DEF 5000
static bool rasd_prop_copy_value(struct sdc_rasd_prop src,
struct sdc_rasd_prop *dest)
@@ -188,6 +195,137 @@ static struct sdc_rasd_prop *mem_inc(con
return rasd;
}
+static struct sdc_rasd_prop *disk_min(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t disk_size = SDC_DISK_MIN;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Minimum", CMPI_chars},
+ {"AllocationQuantity", (CMPIValue *)"MegaBytes",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&disk_size, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *disk_max(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ char *inst_id;
+ CMPIrc prop_ret;
+ uint16_t free_space;
+ uint64_t free_64;
+ virConnectPtr conn;
+ CMPIInstance *pool_inst;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ inst_id = cu_get_str_path(ref, "InstanceID");
+ if (inst_id == NULL) {
+ CMSetStatusWithChars(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not get InstanceID.");
+ goto out;
+ }
+
+ conn = lv_connect(_BROKER, s);
+ if (s->rc != CMPI_RC_OK) {
+ CMSetStatusWithChars(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not get connection.");
+ goto out;
+ }
+
+ /* Getting the relevant resource pool directly finds the free space
+ for us. It is in the Capacity field. */
+ pool_inst = get_pool_by_id(_BROKER, conn, inst_id, NAMESPACE(ref));
+ if (pool_inst == NULL) {
+ CMSetStatusWithChars(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not get pool instance.");
+ goto out;
+ }
+
+ prop_ret = cu_get_u64_prop(pool_inst, "Capacity", &free_64);
+ if (prop_ret != CMPI_RC_OK) {
+ CMSetStatusWithChars(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not get capacity from instance.");
+ goto out;
+ }
+ CU_DEBUG("Got capacity from pool_inst: %lld", free_64);
+
+ free_space = (uint16_t)free_64;
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Maximum", CMPI_chars},
+ {"AllocationQuantity", (CMPIValue *)"MegaBytes",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&free_space,
CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ out:
+ free(inst_id);
+ return rasd;
+}
+
+static struct sdc_rasd_prop *disk_def(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t disk_size = SDC_DISK_DEF;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Default", CMPI_chars},
+ {"AllocationQuantity", (CMPIValue *)"MegaBytes",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&disk_size, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *disk_inc(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t disk_size = 250;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Increment",
CMPI_chars},
+ {"AllocationQuantity", (CMPIValue *)"MegaBytes",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&disk_size, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
static struct sdc_rasd mem = {
.resource_type = CIM_RASD_TYPE_MEM,
.min = mem_min,
@@ -196,8 +334,17 @@ static struct sdc_rasd mem = {
.inc = mem_inc
};
+static struct sdc_rasd disk = {
+ .resource_type = CIM_RASD_TYPE_DISK,
+ .min = disk_min,
+ .max = disk_max,
+ .def = disk_def,
+ .inc = disk_inc
+};
+
static struct sdc_rasd *sdc_rasd_list[] = {
&mem,
+ &disk,
NULL
};
4:33 p.m.
New subject: [PATCH 3 of 6] Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e. lack of warning when minimum or default is higher than maximum), but is functional
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557358 18000
JG> # Node ID 94308147bed1693443d0741de6a30c5b0f77b0f1
JG> # Parent 543a0790d8615551153950de8f2f2fe3de107cf3
JG> Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e.
lack of warning when minimum or default is higher than maximum), but is functional.
I think this looks okay, but:
JG> +#define SDC_DISK_MIN 2000
JG> +#define SDC_DISK_DEF 5000
Why not make the increment amount also parameterized as such? Seems
like a valid thing to want to change. Unlikely, but valid :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:28 a.m.
New subject: [PATCH 3 of 6] Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e. lack of warning when minimum or default is higher than maximum), but is functional
Dan Smith wrote:
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557358 18000
JG> # Node ID 94308147bed1693443d0741de6a30c5b0f77b0f1
JG> # Parent 543a0790d8615551153950de8f2f2fe3de107cf3
JG> Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e.
lack of warning when minimum or default is higher than maximum), but is functional.
I think this looks okay, but:
JG> +#define SDC_DISK_MIN 2000
JG> +#define SDC_DISK_DEF 5000
Why not make the increment amount also parameterized as such? Seems
like a valid thing to want to change. Unlikely, but valid :)
Sure, that's fine. With those two being defined as constants for disk
it's logical to define the last one as well, for organizational
purposes. That said, the next logical question could be, "Why aren't
the other numbers all defines as well?" My answer to that is, as a
habit I tend to only make things that are (or in this case, will be)
used in more than one place, defines. The way I see it, if I only use a
number once, having it as a define doesn't get me very far, because the
define really just creates a level of "indirection" for whomever decides
the value needs to be changed. As soon as the number is used twice, you
of course need a define so that nobody has to root out all instances of
the value, but for one-time-use-only values I don't generally bother
with defines.
This is a pretty good case for an exception to the rule, though, since
the lack of an INC define could be misleading.
--
-Jay
9:34 a.m.
New subject: [PATCH 3 of 6] Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e. lack of warning when minimum or default is higher than maximum), but is functional
JG> The way I see it, if I only use a number once, having it as a
JG> define doesn't get me very far, because the define really just
JG> creates a level of "indirection" for whomever decides the value
JG> needs to be changed. As soon as the number is used twice, you of
JG> course need a define so that nobody has to root out all instances
JG> of the value, but for one-time-use-only values I don't generally
JG> bother with defines.
Well, even if something is only used once, it makes it easier to
change something into a configure option if it already has that layer
of indirection in place. It also means that someone can easily open
the file, see that the limits are all defined at the top, and not
bother to understand the rest of the function pointer indirection
(which would be harder to follow without context than the #defines
anyway).
The only things that I think make sense to leave buried are things
like increment for NICs, since it's very clear that adding one NIC
will always be the desired granularity, which can't be said for disk
or memory, IMHO.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
2:51 p.m.
New subject: [PATCH 4 of 6] Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become dynamic
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557377 18000
# Node ID 0ed8747c1dfbf14ff7f6e23ff792a35852294fa6
# Parent 94308147bed1693443d0741de6a30c5b0f77b0f1
Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become
dynamic.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 94308147bed1 -r 0ed8747c1dfb src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:29:18 2007 -0500
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:29:37 2007 -0500
@@ -195,6 +195,85 @@ static struct sdc_rasd_prop *mem_inc(con
return rasd;
}
+static struct sdc_rasd_prop *net_min(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_nics = 0;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Minimum", CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_nics, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (ret)
+ return rasd;
+ else
+ return NULL;
+}
+
+static struct sdc_rasd_prop *net_max(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_nics = 6;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Maximum", CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_nics, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (ret)
+ return rasd;
+ else
+ return NULL;
+}
+static struct sdc_rasd_prop *net_def(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_nics = 1;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Default", CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_nics, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (ret)
+ return rasd;
+ else
+ return NULL;
+}
+
+static struct sdc_rasd_prop *net_inc(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_nics = 1;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Increment",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_nics, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (ret)
+ return rasd;
+ else
+ return NULL;
+}
+
static struct sdc_rasd_prop *disk_min(const CMPIObjectPath *ref,
CMPIStatus *s)
{
@@ -334,6 +413,14 @@ static struct sdc_rasd mem = {
.inc = mem_inc
};
+static struct sdc_rasd network = {
+ .resource_type = CIM_RASD_TYPE_NET,
+ .min = net_min,
+ .max = net_max,
+ .def = net_def,
+ .inc = net_inc
+};
+
static struct sdc_rasd disk = {
.resource_type = CIM_RASD_TYPE_DISK,
.min = disk_min,
@@ -344,6 +431,7 @@ static struct sdc_rasd disk = {
static struct sdc_rasd *sdc_rasd_list[] = {
&mem,
+ &network,
&disk,
NULL
};
5:58 p.m.
New subject: [PATCH 4 of 6] Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become dynamic
JG> + uint16_t num_nics = 6;
JG> + struct sdc_rasd_prop *rasd = NULL;
JG> +
JG> + struct sdc_rasd_prop tmp[] = {
JG> + {"InstanceID", (CMPIValue *)"Maximum",
CMPI_chars},
JG> + {"VirtualQuantity", (CMPIValue *)&num_nics,
CMPI_uint16},
JG> + PROP_END
JG> + };
The maximum NIC count is probably different for KVM and Xen, and I
imagine this function should be intelligent in the future. Could we
at least pull the '6' out to a constant like XEN_MAX_VIF or something?
JG> +static struct sdc_rasd_prop *net_def(const CMPIObjectPath *ref,
JG> + CMPIStatus *s)
JG> +{
JG> + bool ret;
JG> + uint16_t num_nics = 1;
JG> + struct sdc_rasd_prop *rasd = NULL;
JG> +
JG> + struct sdc_rasd_prop tmp[] = {
JG> + {"InstanceID", (CMPIValue *)"Default",
CMPI_chars},
JG> + {"VirtualQuantity", (CMPIValue *)&num_nics,
CMPI_uint16},
JG> + PROP_END
JG> + };
JG> +
JG> + ret = dup_rasd_prop_list(tmp, &rasd);
JG> + if (ret)
JG> + return rasd;
JG> + else
JG> + return NULL;
JG> +}
JG> +
JG> +static struct sdc_rasd_prop *net_inc(const CMPIObjectPath *ref,
JG> + CMPIStatus *s)
JG> +{
JG> + bool ret;
JG> + uint16_t num_nics = 1;
JG> + struct sdc_rasd_prop *rasd = NULL;
JG> +
JG> + struct sdc_rasd_prop tmp[] = {
JG> + {"InstanceID", (CMPIValue *)"Increment",
CMPI_chars},
JG> + {"VirtualQuantity", (CMPIValue *)&num_nics,
CMPI_uint16},
JG> + PROP_END
JG> + };
JG> +
JG> + ret = dup_rasd_prop_list(tmp, &rasd);
JG> + if (ret)
JG> + return rasd;
JG> + else
JG> + return NULL;
JG> +}
I think "default" and "increment" in the network case are probably
safe to leave hardcoded to '1' and buried :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:03 a.m.
New subject: [PATCH 4 of 6] Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become dynamic
On Thu, Nov 08, 2007 at 03:58:26PM -0800, Dan Smith wrote:
JG> + uint16_t num_nics = 6;
JG> + struct sdc_rasd_prop *rasd = NULL;
JG> +
JG> + struct sdc_rasd_prop tmp[] = {
JG> + {"InstanceID", (CMPIValue *)"Maximum",
CMPI_chars},
JG> + {"VirtualQuantity", (CMPIValue *)&num_nics,
CMPI_uint16},
JG> + PROP_END
JG> + };
The maximum NIC count is probably different for KVM and Xen, and I
imagine this function should be intelligent in the future. Could we
at least pull the '6' out to a constant like XEN_MAX_VIF or something?
As that bug points out, 6 sounds dangerously low for a virtual interface:
https://bugzilla.redhat.com/show_bug.cgi?id=273421
Any chance to also increase this at least up to 32 ?
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:28 a.m.
New subject: [PATCH 4 of 6] Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become dynamic
DV> Any chance to also increase this at least up to 32 ?
Last I checked, Xen had a hard limit due to a ringbuffer sizing issue
or something; has this changed? We were planning to try to expose
this limit (adjusted per platform) to the client. Note that this is
an advertised maximum that is not strictly enforced.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:39 a.m.
New subject: [PATCH 4 of 6] Add NetworkPort functionality to SettingsDefineCapabilities. All static, max may become dynamic
Dan Smith wrote:
JG> + uint16_t num_nics = 6;
JG> + struct sdc_rasd_prop *rasd = NULL;
JG> +
JG> + struct sdc_rasd_prop tmp[] = {
JG> + {"InstanceID", (CMPIValue *)"Maximum",
CMPI_chars},
JG> + {"VirtualQuantity", (CMPIValue *)&num_nics,
CMPI_uint16},
JG> + PROP_END
JG> + };
The maximum NIC count is probably different for KVM and Xen, and I
imagine this function should be intelligent in the future. Could we
at least pull the '6' out to a constant like XEN_MAX_VIF or something?
I agree that this should probably be made intelligent in the future, but
as I said with regards to the disk patch, I don't necessarily see the
point to making this a defined constant unless we need to use it
elsewhere or we actually add a hardcoded value for KVM. As it is right
now, the value is only ever assigned in that initialization line and
only ever read four lines later, and its use seems clear enough that a
named constant won't be adding any clarity. Not a huge deal, and, "Yea,
that's a good point. Make it a define," is a fine response here, but
that's my opinion on the matter.
--
-Jay
2:51 p.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557443 18000
# Node ID 2d09468e2de7ed9194907bfff1efbb16ba40202d
# Parent 0ed8747c1dfbf14ff7f6e23ff792a35852294fa6
Add processor support to SettingsDefineCapabilities.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 0ed8747c1dfb -r 2d09468e2de7 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:29:37 2007 -0500
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Nov 08 16:30:43 2007 -0500
@@ -26,6 +26,8 @@
#include <sys/vfs.h>
#include <errno.h>
+#include <libvirt.h>
+
#include "config.h"
#include "cmpidt.h"
@@ -195,6 +197,121 @@ static struct sdc_rasd_prop *mem_inc(con
return rasd;
}
+static struct sdc_rasd_prop *proc_min(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_procs = 1;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Minimum", CMPI_chars},
+ {"AllocationUnits", (CMPIValue *)"Processors",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_procs, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *proc_max(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ int cpu_num;
+ FILE *cpuinfo;
+ size_t len = 0;
+ char *line = NULL;
+ uint16_t num_procs = 0;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ CU_DEBUG("In proc_max()");
+
+ cpuinfo = fopen("/proc/cpuinfo", "r");
+ if (cpuinfo == NULL) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy open cpuinfo.");
+ goto out;
+ }
+
+ while (getline(&line, &len, cpuinfo) > 0) {
+ if (sscanf(line, "processor : %d", &cpu_num) == 1) {
+ CU_DEBUG("Line matched. num_procs++");
+ num_procs++;
+ }
+ }
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Maximum", CMPI_chars},
+ {"AllocationUnits", (CMPIValue *)"Processors",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_procs, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ out:
+ free(line);
+ fclose(cpuinfo);
+ return rasd;
+}
+
+static struct sdc_rasd_prop *proc_def(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_procs = 1;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Default", CMPI_chars},
+ {"AllocationUnits", (CMPIValue *)"Processors",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_procs, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
+static struct sdc_rasd_prop *proc_inc(const CMPIObjectPath *ref,
+ CMPIStatus *s)
+{
+ bool ret;
+ uint16_t num_procs = 1;
+ struct sdc_rasd_prop *rasd = NULL;
+
+ struct sdc_rasd_prop tmp[] = {
+ {"InstanceID", (CMPIValue *)"Increment",
CMPI_chars},
+ {"AllocationUnits", (CMPIValue *)"Processors",
CMPI_chars},
+ {"VirtualQuantity", (CMPIValue *)&num_procs, CMPI_uint16},
+ PROP_END
+ };
+
+ ret = dup_rasd_prop_list(tmp, &rasd);
+ if (!ret) {
+ cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED,
+ "Could not copy RASD.");
+ }
+
+ return rasd;
+}
+
static struct sdc_rasd_prop *net_min(const CMPIObjectPath *ref,
CMPIStatus *s)
{
@@ -413,6 +530,14 @@ static struct sdc_rasd mem = {
.inc = mem_inc
};
+static struct sdc_rasd processor = {
+ .resource_type = CIM_RASD_TYPE_PROC,
+ .min = proc_min,
+ .max = proc_max,
+ .def = proc_def,
+ .inc = proc_inc
+};
+
static struct sdc_rasd network = {
.resource_type = CIM_RASD_TYPE_NET,
.min = net_min,
@@ -431,6 +556,7 @@ static struct sdc_rasd disk = {
static struct sdc_rasd *sdc_rasd_list[] = {
&mem,
+ &processor,
&network,
&disk,
NULL
6:09 p.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
JG> + cpuinfo = fopen("/proc/cpuinfo", "r");
This should come from virNodeInfo. Also, it would be good to go ahead
and split out the "how many vcpus is the max for xen?" behavior into a
separate function, because it's possible that other platforms will be
able to overcommit vcpus. This might be a good place to start making
it clear that the behavior is platform-dependent. I think it's okay
to always take the Xen default for the time being, but while you're
reworking this, you might as well split it out too.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:33 a.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
Dan Smith wrote:
JG> + cpuinfo = fopen("/proc/cpuinfo",
"r");
This should come from virNodeInfo.
Okay. Only question before I do that is do we
have virNodeInfo in all
the versions of libvirt we support? The only reason I had to write this
myself at all is that the libvirt function that determines the maximum
number of vcpus came in too late for us.
Also, it would be good to go ahead
and split out the "how many vcpus is the max for xen?" behavior into a
separate function, because it's possible that other platforms will be
able to overcommit vcpus. This might be a good place to start making
it clear that the behavior is platform-dependent. I think it's okay
to always take the Xen default for the time being, but while you're
reworking this, you might as well split it out too.
So for now proc_max will basically just call xen_proc_max but in the
future we might need to check the ref and add another function? If
that's what you meant, that's fine.
--
-Jay
9:26 a.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
Jay Gagnon wrote:
Dan Smith wrote:
> JG> + cpuinfo = fopen("/proc/cpuinfo", "r");
>
> This should come from virNodeInfo.
>
Okay. Only question before I do that is do we have virNodeInfo in all
the versions of libvirt we support? The only reason I had to write this
myself at all is that the libvirt function that determines the maximum
number of vcpus came in too late for us.
Looks like this question got lost in the shuffle. What version of
libvirt are we actually writing against? In other words, of the
versions shipped with the various distributions we're supporting, which
one is the oldest? I want to make sure I don't try to use something
that is too new.
--
-Jay
9:37 a.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
JG> What version of libvirt are we actually writing against? In other
JG> words, of the versions shipped with the various distributions
JG> we're supporting, which one is the oldest? I want to make sure I
JG> don't try to use something that is too new.
I think that for right now, we should aim at the latest libvirt
release. If someone needs to run libvirt-cim on an older stack, we
can evaluate and address issues with a specific version in mind.
That said, I'm not sure what version this node info call was added in,
but it seems rather fundamental, so I would hope it's been around for
a while :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
10:01 a.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
On Mon, Nov 12, 2007 at 10:26:54AM -0500, Jay Gagnon wrote:
Jay Gagnon wrote:
> Dan Smith wrote:
>
>> JG> + cpuinfo = fopen("/proc/cpuinfo", "r");
>>
>> This should come from virNodeInfo.
>>
> Okay. Only question before I do that is do we have virNodeInfo in all
> the versions of libvirt we support? The only reason I had to write this
> myself at all is that the libvirt function that determines the maximum
> number of vcpus came in too late for us.
>
Looks like this question got lost in the shuffle. What version of
libvirt are we actually writing against? In other words, of the
versions shipped with the various distributions we're supporting, which
one is the oldest? I want to make sure I don't try to use something
that is too new.
See virNodeGetInfo in the table
http://libvirt.org/hvsupport.html
it's there since 0.1.0 (at least for Xen), so no problem,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
10:31 a.m.
New subject: [PATCH 5 of 6] Add processor support to SettingsDefineCapabilities
Daniel Veillard wrote:
On Mon, Nov 12, 2007 at 10:26:54AM -0500, Jay Gagnon wrote:
> Jay Gagnon wrote:
>
>> Dan Smith wrote:
>>
>>
>>> JG> + cpuinfo = fopen("/proc/cpuinfo", "r");
>>>
>>> This should come from virNodeInfo.
>>>
>>>
>> Okay. Only question before I do that is do we have virNodeInfo in all
>> the versions of libvirt we support? The only reason I had to write this
>> myself at all is that the libvirt function that determines the maximum
>> number of vcpus came in too late for us.
>>
>>
> Looks like this question got lost in the shuffle. What version of
> libvirt are we actually writing against? In other words, of the
> versions shipped with the various distributions we're supporting, which
> one is the oldest? I want to make sure I don't try to use something
> that is too new.
>
See virNodeGetInfo in the table
http://libvirt.org/hvsupport.html
it's there since 0.1.0 (at least for Xen), so no problem,
Daniel
Heh, yea looks like we should be all right. I hadn't seen that table
before, definitely good to know that exists.
--
-Jay
2:51 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557532 18000
# Node ID cf0a2938ba0b9d2e7af8a5790d0e56cc7564df73
# Parent 2d09468e2de7ed9194907bfff1efbb16ba40202d
Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities
connection. Only supports the pool_to_alloc direction, because it is the only one
that's terribly interesting, but the other direction will need to be added at some
point.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 2d09468e2de7 -r cf0a2938ba0b src/Virt_ElementCapabilities.c
--- a/src/Virt_ElementCapabilities.c Thu Nov 08 16:30:43 2007 -0500
+++ b/src/Virt_ElementCapabilities.c Thu Nov 08 16:32:12 2007 -0500
@@ -179,6 +179,50 @@ static CMPIStatus cap_to_cs(const CMPIOb
return s;
}
+static CMPIStatus alloc_to_pool(const CMPIObjectPath *ref,
+ struct std_assoc_info *info,
+ struct inst_list *list)
+{
+ /* Pool to alloc is more important. That will be done first. */
+ return (CMPIStatus){CMPI_RC_OK, NULL};
+}
+
+static CMPIStatus pool_to_alloc(const CMPIObjectPath *ref,
+ struct std_assoc_info *info,
+ struct inst_list *list)
+{
+ int ret;
+ char *inst_id;
+ uint16_t type;
+ CMPIInstance *inst = NULL;
+ CMPIStatus s = {CMPI_RC_OK};
+
+ inst_id = cu_get_str_path(ref, "InstanceID");
+ if (inst_id == NULL) {
+ CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED,
+ "Could not get InstanceID.");
+ goto out;
+ }
+
+ inst = get_typed_instance(_BROKER, "AllocationCapabilities",
+ NAMESPACE(ref));
+ CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars);
+
+ ret = cu_get_u16_path(ref, "ResourceType", &type);
+ if (ret != 1) {
+ CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED,
+ "Could not get ResourceType.");
+ goto out;
+ }
+ CMSetProperty(inst, "ResourceType", &type, CMPI_uint16);
+
+ inst_list_add(list, inst);
+
+ out:
+ free(inst_id);
+
+ return s;
+}
static CMPIInstance *make_ref(const CMPIObjectPath *ref,
const CMPIInstance *inst,
struct std_assoc_info *info,
@@ -273,6 +317,28 @@ struct std_assoc ele_cap_to_computer_sys
.assoc_class = "CIM_ElementCapabilities",
.handler = cap_to_cs,
+ .make_ref = make_ref
+};
+
+struct std_assoc alloc_cap_to_resource_pool = {
+ .source_class = "CIM_AllocationCapabilities",
+ .source_prop = "Capabilities",
+
+ .target_class = "CIM_ResourcePool",
+ .target_prop = "ManagedElement",
+
+ .handler = alloc_to_pool,
+ .make_ref = make_ref
+};
+
+struct std_assoc resource_pool_to_alloc_cap = {
+ .source_class = "CIM_ResourcePool",
+ .source_prop = "ManagedElement",
+
+ .target_class = "CIM_AllocationCapabilities",
+ .target_prop = "Capabilities",
+
+ .handler = pool_to_alloc,
.make_ref = make_ref
};
@@ -282,6 +348,8 @@ struct std_assoc *assoc_handlers[] = {
&system_to_vsm_cap,
&vsm_cap_to_system,
&ele_cap_to_computer_system,
+ &alloc_cap_to_resource_pool,
+ &resource_pool_to_alloc_cap,
NULL
};
5:59 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
Jay Gagnon wrote:
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1194557532 18000
# Node ID cf0a2938ba0b9d2e7af8a5790d0e56cc7564df73
# Parent 2d09468e2de7ed9194907bfff1efbb16ba40202d
Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities
connection. Only supports the pool_to_alloc direction, because it is the only one
that's terribly interesting, but the other direction will need to be added at some
point.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
+
+ inst = get_typed_instance(_BROKER, "AllocationCapabilities",
+ NAMESPACE(ref));
+ CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars);
+
+ ret = cu_get_u16_path(ref, "ResourceType", &type);
+ if (ret != 1) {
+ CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED,
+ "Could not get ResourceType.");
+ goto out;
+ }
+ CMSetProperty(inst, "ResourceType", &type, CMPI_uint16);
+
+ inst_list_add(list, inst);
+
+ out:
+ free(inst_id);
+
+ return s;
+}
Just a nit-picky thing here... why not call the AllocCapabilities
function that setups the instance? I think using the same InstanceID
for the AllocCapa as the pool might not always return the correct
instance because AllocCapa uses <pool type>/0 as the InstanceID. But
something like the NetworkingPool returns "NetworkPool/xenbr0". So
you'll be creating an instance with "NetworkPool/xenbr0" as the
InstanceID, which would conflict with what EnumInstances from AllocCapa
would return.
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
7:11 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
KR> Just a nit-picky thing here... why not call the AllocCapabilities
KR> function that setups the instance? I think using the same
KR> InstanceID for the AllocCapa as the pool might not always return
KR> the correct instance because AllocCapa uses <pool type>/0 as the
KR> InstanceID. But something like the NetworkingPool returns
KR> "NetworkPool/xenbr0". So you'll be creating an instance with
KR> "NetworkPool/xenbr0" as the InstanceID, which would conflict with
KR> what EnumInstances from AllocCapa would return.
I think AllocationCapabilities needs to change here. It think using
the InstanceID from the pool makes the most sense. I'm not sure what
the existing AllocationCapabilities stuff intended, but I'm assuming
it will need to change to match this new stuff. Perhaps it was a
hold-over from the days of one-pool-per-resource-type?
Good catch, by the way :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
2:32 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
Dan Smith wrote:
KR> Just a nit-picky thing here... why not call the
AllocCapabilities
KR> function that setups the instance? I think using the same
KR> InstanceID for the AllocCapa as the pool might not always return
KR> the correct instance because AllocCapa uses <pool type>/0 as the
KR> InstanceID. But something like the NetworkingPool returns
KR> "NetworkPool/xenbr0". So you'll be creating an instance with
KR> "NetworkPool/xenbr0" as the InstanceID, which would conflict with
KR> what EnumInstances from AllocCapa would return.
I think AllocationCapabilities needs to change here. It think using
the InstanceID from the pool makes the most sense. I'm not sure what
the existing AllocationCapabilities stuff intended, but I'm assuming
it will need to change to match this new stuff. Perhaps it was a
hold-over from the days of one-pool-per-resource-type?
Good catch, by the way :)
Just to be clear, copying InstanceID from the instance of ResourcePool
to the instance of AllocationCapabilities is okay, but I need to change
the AllocationCapabilities provider? That doesn't much surprise me; the
AllocationCapabilities provider itself only exists so I could register
it as a class; I'm honestly not even sure what it's supposed to do on
its own
. Reading through the entire Allocation Capabilities Profile (DSP1043)
provides no insight into what get/enumerateInstance(s) would do.
Reasonable detail is given about how it should be connected to things
via associations, but there isn't much concept of a standalone instance.
--
-Jay
2:46 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
JG> I'm honestly not even sure what it's supposed to do on its own .
JG> Reading through the entire Allocation Capabilities Profile
JG> (DSP1043) provides no insight into what get/enumerateInstance(s)
JG> would do. Reasonable detail is given about how it should be
JG> connected to things via associations, but there isn't much concept
JG> of a standalone instance.
The instance functions for AllocationCapabilities could probably be
left unimplemented without anyone noticing, I imagine. However, I
think that EnumInstances should return an instance of every
AllocationCapabilities that could possibly be had through association
with a pool. Thus, enumerate all the pools, create an AC instance for
each and return them. GetInstance works the same. Should be
trivial.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:27 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
Dan Smith wrote:
JG> I'm honestly not even sure what it's supposed to do on
its own .
JG> Reading through the entire Allocation Capabilities Profile
JG> (DSP1043) provides no insight into what get/enumerateInstance(s)
JG> would do. Reasonable detail is given about how it should be
JG> connected to things via associations, but there isn't much concept
JG> of a standalone instance.
The instance functions for AllocationCapabilities could probably be
left unimplemented without anyone noticing, I imagine. However, I
think that EnumInstances should return an instance of every
AllocationCapabilities that could possibly be had through association
with a pool. Thus, enumerate all the pools, create an AC instance for
each and return them. GetInstance works the same. Should be
trivial.
I can see what you mean with EnumInstances, although if this is how the
DMTF wants things done I still take issue with it since it just feels
like a broken way of doing what the associations do.
--
-Jay
6:18 p.m.
New subject: [PATCH 6 of 6] Add another relationship to ElementCapabilities, the ResourcePool-AllocationCapabilities connection. Only supports the pool_to_alloc direction, because it is the only one that's terribly interesting, but the other direction will need to be added at some point
JG> +static CMPIStatus alloc_to_pool(const CMPIObjectPath *ref,
JG> + struct std_assoc_info *info,
JG> + struct inst_list *list)
JG> +{
JG> + /* Pool to alloc is more important. That will be done first. */
JG> + return (CMPIStatus){CMPI_RC_OK, NULL};
JG> +}
If we're going to leave this out for now, this should instead be
RETURN_UNSUPPORTED().
However, since the InstanceID of a pool and an AllocationCapabilities
are the same, isn't this behavior the same as below? If you set the
below function to be the handler for the alloc_to_pool case, won't it
behave as expected?
JG> +static CMPIStatus pool_to_alloc(const CMPIObjectPath *ref,
JG> + struct std_assoc_info *info,
JG> + struct inst_list *list)
JG> +{
JG> + int ret;
JG> + char *inst_id;
JG> + uint16_t type;
JG> + CMPIInstance *inst = NULL;
JG> + CMPIStatus s = {CMPI_RC_OK};
JG> +
JG> + inst_id = cu_get_str_path(ref, "InstanceID");
JG> + if (inst_id == NULL) {
JG> + CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED,
JG> + "Could not get InstanceID.");
JG> + goto out;
JG> + }
JG> +
JG> + inst = get_typed_instance(_BROKER, "AllocationCapabilities",
JG> + NAMESPACE(ref));
JG> + CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars);
JG> +
JG> + ret = cu_get_u16_path(ref, "ResourceType", &type);
JG> + if (ret != 1) {
JG> + CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED,
JG> + "Could not get ResourceType.");
JG> + goto out;
JG> + }
JG> + CMSetProperty(inst, "ResourceType", &type, CMPI_uint16);
JG> +
JG> + inst_list_add(list, inst);
JG> +
JG> + out:
JG> + free(inst_id);
JG> +
JG> + return s;
JG> +}
This brings up an idea. Maybe it would be helpful for libcmpiutil to
have a function that copies a value from a ref to an instance, or from
an instance to an instance. This would assume that they're the same
type, of course, but the above would be much shorter if we had
something like:
cu_copy_key(&ref, &inst, "InstanceID");
cu_copy_key(&ref, &inst, "ResourceType");
Right? I know there are a couple other places where we need to do
that, so it might be worth knocking something up. Should be trivial
and easy.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
6250
days inactive
6254
days old
32 comments
4 participants
participants (4)
-
Dan Smith -
Daniel Veillard -
Jay Gagnon -
Kaitlin Rupert