3:17 p.m.
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1195510519 18000
# Node ID 9e000f9f9096cad7a40b1221b467fe99efc4b93f
# Parent 79f43a4ffc38d7227e17432b971639ae20f6f6ed
Fix AllocationCapabilities to make EnumInstances and EnumInstanceNames work.
I don't have a ton of experience with instance providers, so this might require a
little extra scrutiny as I'm sure there are several rookie mistakes.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 79f43a4ffc38 -r 9e000f9f9096 src/Makefile.am
--- a/src/Makefile.am Mon Nov 19 16:19:06 2007 -0500
+++ b/src/Makefile.am Mon Nov 19 17:15:19 2007 -0500
@@ -91,9 +91,9 @@ libVirt_ElementCapabilities_la_LIBADD =
-lVirt_ComputerSystem \
-lVirt_HostSystem
-libVirt_AllocationCapabilities_la_DEPENDENCIES = libVirt_RASD.la
+libVirt_AllocationCapabilities_la_DEPENDENCIES = libVirt_RASD.la libVirt_DevicePool.la
libVirt_AllocationCapabilities_la_SOURCES = Virt_AllocationCapabilities.c
-libVirt_AllocationCapabilities_la_LIBADD = -lVirt_RASD
+libVirt_AllocationCapabilities_la_LIBADD = -lVirt_RASD -lVirt_DevicePool
libVirt_SettingsDefineCapabilities_la_DEPENDENCIES = libVirt_RASD.la
libVirt_DevicePool.la
libVirt_SettingsDefineCapabilities_la_SOURCES = Virt_SettingsDefineCapabilities.c
diff -r 79f43a4ffc38 -r 9e000f9f9096 src/Virt_AllocationCapabilities.c
--- a/src/Virt_AllocationCapabilities.c Mon Nov 19 16:19:06 2007 -0500
+++ b/src/Virt_AllocationCapabilities.c Mon Nov 19 17:15:19 2007 -0500
@@ -32,6 +32,7 @@
#include "Virt_AllocationCapabilities.h"
#include "Virt_RASD.h"
+#include "Virt_DevicePool.h"
const static CMPIBroker *_BROKER;
@@ -83,6 +84,88 @@ static CMPIStatus return_alloc_cap(const
else
CMReturnInstance(results, inst);
out:
+ return s;
+}
+
+static CMPIStatus alloc_cap_instances(const CMPIBroker *broker,
+ const CMPIObjectPath *ref,
+ const CMPIResult *results,
+ bool names_only,
+ const char **properties)
+{
+ int i;
+ virConnectPtr conn;
+ CMPIInstance *inst;
+ struct inst_list alloc_cap_list;
+ struct inst_list device_pool_list;
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+
+ CU_DEBUG("In alloc_cap_instances()");
+
+ inst_list_init(&device_pool_list);
+ inst_list_init(&alloc_cap_list);
+
+ if (!provider_is_responsible(broker, ref, &s))
+ goto out;
+
+ conn = lv_connect(_BROKER, &s);
+ if (conn == NULL)
+ goto out;
+
+ for (i = 0; device_pool_names[i]; i++) {
+ s = get_pool_by_type(broker,
+ conn,
+ device_pool_names[i],
+ NAMESPACE(ref),
+ &device_pool_list);
+ if (s.rc != CMPI_RC_OK) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Error fetching device pools");
+ goto out;
+ }
+ }
+
+ for (i = 0; i < device_pool_list.cur; i++) {
+ inst = get_typed_instance(broker,
+ "AllocationCapabilities",
+ NAMESPACE(ref));
+ if (inst == NULL) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Could not get alloc_cap instance");
+ goto out;
+ }
+
+ s = cu_copy_prop(broker, device_pool_list.list[i], inst,
+ "InstanceID", NULL);
+ if (s.rc != CMPI_RC_OK) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Error copying InstanceID");
+ goto out;
+ }
+
+ s = cu_copy_prop(broker, device_pool_list.list[i], inst,
+ "ResourceType", NULL);
+ if (s.rc != CMPI_RC_OK) {
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Error copying InstanceID");
+ goto out;
+ }
+
+ inst_list_add(&alloc_cap_list, inst);
+ }
+
+ if (names_only)
+ cu_return_instance_names(results, &alloc_cap_list);
+ else
+ cu_return_instances(results, &alloc_cap_list);
+
+ out:
+ inst_list_free(&alloc_cap_list);
+ inst_list_free(&device_pool_list);
return s;
}
@@ -95,8 +178,23 @@ static CMPIStatus GetInstance(CMPIInstan
return return_alloc_cap(reference, results, 0);
}
-DEFAULT_EI();
-DEFAULT_EIN();
+static CMPIStatus EnumInstanceNames(CMPIInstanceMI *self,
+ const CMPIContext *context,
+ const CMPIResult *results,
+ const CMPIObjectPath *reference)
+{
+ return alloc_cap_instances(_BROKER, reference, results, true, NULL);
+}
+
+static CMPIStatus EnumInstances(CMPIInstanceMI *self,
+ const CMPIContext *context,
+ const CMPIResult *results,
+ const CMPIObjectPath *reference,
+ const char **properties)
+{
+ return alloc_cap_instances(_BROKER, reference, results, false, properties);
+}
+
DEFAULT_CI();
DEFAULT_MI();
DEFAULT_DI();
4:45 p.m.
JG> + for (i = 0; device_pool_names[i]; i++) {
JG> + s = get_pool_by_type(broker,
JG> + conn,
JG> + device_pool_names[i],
JG> + NAMESPACE(ref),
JG> + &device_pool_list);
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error fetching device pools");
JG> + goto out;
JG> + }
JG> + }
We do this elsewhere too. Perhaps we need a get_all_pools() function
in Virt_DevicePool?
JG> +
JG> + for (i = 0; i < device_pool_list.cur; i++) {
JG> + inst = get_typed_instance(broker,
JG> + "AllocationCapabilities",
JG> + NAMESPACE(ref));
JG> + if (inst == NULL) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Could not get alloc_cap instance");
JG> + goto out;
JG> + }
JG> +
JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst,
JG> + "InstanceID", NULL);
This function looks great, but I think we're missing the patch that
adds it to libcmpiutil :)
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error copying InstanceID");
JG> + goto out;
JG> + }
JG> +
JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst,
JG> + "ResourceType", NULL);
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error copying InstanceID");
JG> + goto out;
JG> + }
JG> +
JG> + inst_list_add(&alloc_cap_list, inst);
JG> + }
To reduce the size of this loop, and to be in line with our other
instance providers, I think it would be good to define a function that
looks something like this:
CMPIStatus get_ac_instance(CMPIInstance *pool, CMPIInstance **ac);
This would encapsulate things a bit and also would provide a clean
hook for ElementCapabilities, which will need to, given a pool, get an
instance of AllocationCapabilities.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
8:57 a.m.
Dan Smith wrote:
JG> + for (i = 0; device_pool_names[i]; i++) {
JG> + s = get_pool_by_type(broker,
JG> + conn,
JG> + device_pool_names[i],
JG> + NAMESPACE(ref),
JG> + &device_pool_list);
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error fetching device pools");
JG> + goto out;
JG> + }
JG> + }
We do this elsewhere too. Perhaps we need a get_all_pools() function
in Virt_DevicePool?
Sounds reasonable. I'll do that.
JG> +
JG> + for (i = 0; i < device_pool_list.cur; i++) {
JG> + inst = get_typed_instance(broker,
JG> + "AllocationCapabilities",
JG> + NAMESPACE(ref));
JG> + if (inst == NULL) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Could not get alloc_cap
instance");
JG> + goto out;
JG> + }
JG> +
JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst,
JG> + "InstanceID", NULL);
This function looks great, but I think we're missing the patch that
adds it to libcmpiutil :)
Oops. I'll send that out when I send the next iteration of this.
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error copying InstanceID");
JG> + goto out;
JG> + }
JG> +
JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst,
JG> + "ResourceType", NULL);
JG> + if (s.rc != CMPI_RC_OK) {
JG> + cu_statusf(broker, &s,
JG> + CMPI_RC_ERR_FAILED,
JG> + "Error copying InstanceID");
JG> + goto out;
JG> + }
JG> +
JG> + inst_list_add(&alloc_cap_list, inst);
JG> + }
To reduce the size of this loop, and to be in line with our other
instance providers, I think it would be good to define a function that
looks something like this:
CMPIStatus get_ac_instance(CMPIInstance *pool, CMPIInstance **ac);
This would encapsulate things a bit and also would provide a clean
hook for ElementCapabilities, which will need to, given a pool, get an
instance of AllocationCapabilities.
Sure, no problem. Given all the inexplicable struggle getting that bit
working I guess I didn't want to look at it anymore, so the lack of
encapsulation didn't really strike me. Easy fix though, and good call.
--
-Jay
8:59 a.m.
Kaitlin Rupert wrote:
Jay Gagnon wrote:
> + conn = lv_connect(_BROKER, &s);
> + if (conn == NULL)
> + goto out;
>
>
Just a minor suggestion.. should lv_connect() be
connect_by_classname() instead? =)
Honestly, I wasn't really sure on if there had been a resolution on that
front yet, and it was working fine for testing purposes with lv_connect,
so I figured I could always fix it later when the final word came down
on lv_connect versus connect_by_classname. Have we settled on one yet?
--
-Jay
9:12 a.m.
JG> Honestly, I wasn't really sure on if there had been a resolution
JG> on that front yet, and it was working fine for testing purposes
JG> with lv_connect, so I figured I could always fix it later when the
JG> final word came down on lv_connect versus connect_by_classname.
JG> Have we settled on one yet?
Yep, you should tweak this to use connect_by_classname() now, as I
just pushed that set out to the tree.
Thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
6242
days inactive
6243
days old
5 comments
3 participants
participants (3)
-
Dan Smith -
Jay Gagnon -
Kaitlin Rupert