[PATCH 0 of 3] #3 - Add configurable enum_devices() to Device provider and Adopt interface changes to SD & EAFP
6:26 a.m.
This patch set implements the configurable enum_devices() interface to the Device
provider. This slight interface change is adopted to the associations SystemDevice and
ElementAllocatedFromPool. Both associations profit from the easy access to the device
instances.
Diff to patch set 1:
- fixed style issues
Diff to patch set 2:
- moved adoption of new resource types CIM_RES_TYPE_foo into separate patch set
6:26 a.m.
New subject: [PATCH 1 of 3] Device: rename dom_device to enum_devices and make it configurable
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1204892341 -3600
# Node ID 3afbff37cc3946a1f584fff572717f75aa8facc2
# Parent d09896378f5dc5dfd70054a9c514682a23cd4fd3
Device: rename dom_device to enum_devices and make it configurable
- update resource type to CIM_RES_TYPE_foo
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r d09896378f5d -r 3afbff37cc39 src/Virt_Device.c
--- a/src/Virt_Device.c Fri Mar 07 13:19:00 2008 +0100
+++ b/src/Virt_Device.c Fri Mar 07 13:19:01 2008 +0100
@@ -236,22 +236,22 @@ static CMPIInstance *device_instance(con
{
CMPIInstance *instance;
- if (dev->type == VIRT_DEV_NET)
+ if (dev->type == CIM_RES_TYPE_NET)
instance = net_instance(broker,
&dev->dev.net,
dom,
ns);
- else if (dev->type == VIRT_DEV_DISK)
+ else if (dev->type == CIM_RES_TYPE_DISK)
instance = disk_instance(broker,
&dev->dev.disk,
dom,
ns);
- else if (dev->type == VIRT_DEV_MEM)
+ else if (dev->type == CIM_RES_TYPE_MEM)
instance = mem_instance(broker,
&dev->dev.mem,
dom,
ns);
- else if (dev->type == VIRT_DEV_VCPU)
+ else if (dev->type == CIM_RES_TYPE_PROC)
instance = vcpu_instance(broker,
&dev->dev.vcpu,
dom,
@@ -271,23 +271,24 @@ uint16_t device_type_from_classname(cons
uint16_t device_type_from_classname(const char *classname)
{
if (strstr(classname, "NetworkPort"))
- return VIRT_DEV_NET;
+ return CIM_RES_TYPE_NET;
else if (strstr(classname, "LogicalDisk"))
- return VIRT_DEV_DISK;
+ return CIM_RES_TYPE_DISK;
else if (strstr(classname, "Memory"))
- return VIRT_DEV_MEM;
+ return CIM_RES_TYPE_MEM;
else if (strstr(classname, "Processor"))
- return VIRT_DEV_VCPU;
+ return CIM_RES_TYPE_PROC;
else
- return VIRT_DEV_UNKNOWN;
-}
-
-int dom_devices(const CMPIBroker *broker,
- virDomainPtr dom,
- const char *ns,
- int type,
- struct inst_list *list)
-{
+ return CIM_RES_TYPE_UNKNOWN;
+}
+
+static CMPIStatus _get_devices(const CMPIBroker *broker,
+ const CMPIObjectPath *reference,
+ const virDomainPtr dom,
+ const uint16_t type,
+ struct inst_list *list)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
int count;
int i;
struct virt_device *devs = NULL;
@@ -299,7 +300,10 @@ int dom_devices(const CMPIBroker *broker
for (i = 0; i < count; i++) {
CMPIInstance *dev = NULL;
- dev = device_instance(broker, &devs[i], dom, ns);
+ dev = device_instance(broker,
+ &devs[i],
+ dom,
+ NAMESPACE(reference));
if (dev)
inst_list_add(list, dev);
@@ -308,57 +312,106 @@ int dom_devices(const CMPIBroker *broker
out:
free(devs);
-
- return 1;
-}
-
-static int dom_list_devices(virConnectPtr conn,
- const CMPIObjectPath *ref,
- struct inst_list *list)
-{
- virDomainPtr *doms;
- int ndom;
+ return s;
+}
+
+static CMPIStatus _enum_devices(const CMPIBroker *broker,
+ const CMPIObjectPath *reference,
+ const virDomainPtr dom,
+ const uint16_t type,
+ struct inst_list *list)
+{
+ CMPIStatus s;
+
+ if (type == CIM_RES_TYPE_ALL) {
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_PROC,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_MEM,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_NET,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_DISK,
+ list);
+ }
+ else
+ s = _get_devices(broker,
+ reference,
+ dom,
+ type,
+ list);
+
+ return s;
+}
+
+CMPIStatus enum_devices(const CMPIBroker *broker,
+ const CMPIObjectPath *reference,
+ const char *domain,
+ const uint16_t type,
+ struct inst_list *list)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ virConnectPtr conn = NULL;
+ virDomainPtr *doms = NULL;
+ int count = 1;
int i;
- int type;
-
- type = device_type_from_classname(CLASSNAME(ref));
-
- ndom = get_domain_list(conn, &doms);
- if (ndom == 0)
- return 1;
- else if (ndom < 0)
- return 0;
-
- for (i = 0; i < ndom; i++) {
- dom_devices(_BROKER, doms[i], NAMESPACE(ref), type, list);
- }
-
- return 1;
-}
-
-static CMPIStatus enum_devices(const CMPIObjectPath *reference,
- const CMPIResult *results,
- int names_only)
-{
- CMPIStatus s;
- virConnectPtr conn;
+
+ conn = connect_by_classname(broker, CLASSNAME(reference), &s);
+ if (conn == NULL)
+ goto out;
+
+ if (domain) {
+ doms = calloc(1, sizeof(virDomainPtr));
+ doms[0] = virDomainLookupByName(conn, domain);
+ }
+ else
+ count = get_domain_list(conn, &doms);
+
+ for (i = 0; i < count; i++) {
+ s = _enum_devices(broker,
+ reference,
+ doms[i],
+ type,
+ list);
+
+ virDomainFree(doms[i]);
+ }
+
+ out:
+ virConnectClose(conn);
+ free(doms);
+
+ return s;
+}
+
+static CMPIStatus return_enum_devices(const CMPIObjectPath *reference,
+ const CMPIResult *results,
+ int names_only)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct inst_list list;
if (!provider_is_responsible(_BROKER, reference, &s))
- return s;
+ goto out;
inst_list_init(&list);
- conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
- if (conn == NULL)
- return s;
-
- if (!dom_list_devices(conn, reference, &list)) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Failed to list domains");
- return s;
- }
+ s = enum_devices(_BROKER, reference, NULL,
+ device_type_from_classname(CLASSNAME(reference)),
+ &list);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
if (list.cur == 0)
goto out;
@@ -371,11 +424,6 @@ static CMPIStatus enum_devices(const CMP
inst_list_free(&list);
out:
- cu_statusf(_BROKER, &s,
- CMPI_RC_OK,
- "");
- virConnectClose(conn);
-
return s;
}
@@ -524,7 +572,7 @@ static CMPIStatus EnumInstanceNames(CMPI
const CMPIResult *results,
const CMPIObjectPath *reference)
{
- return enum_devices(reference, results, 1);
+ return return_enum_devices(reference, results, true);
}
static CMPIStatus EnumInstances(CMPIInstanceMI *self,
@@ -533,7 +581,7 @@ static CMPIStatus EnumInstances(CMPIInst
const CMPIObjectPath *reference,
const char **properties)
{
- return enum_devices(reference, results, 0);
+ return return_enum_devices(reference, results, false);
}
static CMPIStatus GetInstance(CMPIInstanceMI *self,
diff -r d09896378f5d -r 3afbff37cc39 src/Virt_Device.h
--- a/src/Virt_Device.h Fri Mar 07 13:19:00 2008 +0100
+++ b/src/Virt_Device.h Fri Mar 07 13:19:01 2008 +0100
@@ -27,19 +27,18 @@
* Return a list of devices for a given domain
*
* @param broker A pointer to the CIM broker
- * @param dom The domain in question
- * @param ref The namespace
- * @param list A pointer to an array of CMPIInstance objects (should
- * be NULL initially)
- * @param cur The number of items in the list (0 initially)
- * @param max The size of the list (0 initially)
- * @returns Nonzero on success
+ * @param reference Defines the libvirt connection to use
+ * @param domain The domain id (NULL means for all domains)
+ * @param type The device type or CIM_RES_TYPE_ALL to get
+ * all devices
+ * @param list A pointer to an array of CMPIInstance objects
+ * (should be NULL initially)
*/
-int dom_devices(const CMPIBroker *broker,
- virDomainPtr dom,
- const char *ns,
- int type,
- struct inst_list *list);
+CMPIStatus enum_devices(const CMPIBroker *broker,
+ const CMPIObjectPath *reference,
+ const char *domain,
+ const uint16_t type,
+ struct inst_list *list);
/**
* Returns the device instance defined by the reference
11:54 a.m.
New subject: [PATCH 1 of 3] Device: rename dom_device to enum_devices and make it configurable
+static CMPIStatus _enum_devices(const CMPIBroker *broker,
+ const CMPIObjectPath *reference,
+ const virDomainPtr dom,
+ const uint16_t type,
+ struct inst_list *list)
+{
+ CMPIStatus s;
+
+ if (type == CIM_RES_TYPE_ALL) {
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_PROC,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_MEM,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_NET,
+ list);
+ s = _get_devices(broker,
+ reference,
+ dom,
+ CIM_RES_TYPE_DISK,
+ list);
+ }
Would it be better to have a list of all the resource types, and then
call _get_devices() in a loop? This is probably fine as is, and I don't
suspect we'll add more resource types in the future, so it's probably
not worth worrying about.
Thoughts?
+
+static CMPIStatus return_enum_devices(const CMPIObjectPath *reference,
+ const CMPIResult *results,
+ int names_only)
+{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct inst_list list;
if (!provider_is_responsible(_BROKER, reference, &s))
- return s;
+ goto out;
inst_list_init(&list);
- conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s);
- if (conn == NULL)
- return s;
-
- if (!dom_list_devices(conn, reference, &list)) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Failed to list domains");
- return s;
- }
+ s = enum_devices(_BROKER, reference, NULL,
+ device_type_from_classname(CLASSNAME(reference)),
+ &list);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
if (list.cur == 0)
goto out;
Should inst_list_free(&list); go after the out:?
Even if list.cur == 0, we still probably want to set list->list = NULL
to be sure.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
5:30 a.m.
New subject: [PATCH 1 of 3] Device: rename dom_device to enum_devices and make it configurable
Kaitlin Rupert wrote:
Would it be better to have a list of all the resource types, and then
call _get_devices() in a loop? This is probably fine as is, and I
don't suspect we'll add more resource types in the future, so it's
probably not worth worrying about.
Thoughts?
Good idea. I will update svpc_types with an array for all resource types
and use this one to loop over here. I will send this out as a separate
patch set.
Should inst_list_free(&list); go after the out:?
Even if list.cur == 0, we still probably want to set list->list = NULL
to be sure.
I had a look into libcmpiutil and its possible to remove this check
(list.cur==0) as cu_return_instance_names() and cu_return_instances()
take care of this case. Thanks for making me aware of this.
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
1:03 p.m.
New subject: [PATCH 1 of 3] Device: rename dom_device to enum_devices and make it configurable
HE> + s = enum_devices(_BROKER, reference, NULL,
HE> + device_type_from_classname(CLASSNAME(reference)),
HE> + &list);
This should be one argument per line.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
6:26 a.m.
New subject: [PATCH 2 of 3] SD: adopt Device interface changes
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1204892341 -3600
# Node ID 366b317afbf382f85ed75b86b6df9eae30dd6c94
# Parent 3afbff37cc3946a1f584fff572717f75aa8facc2
SD: adopt Device interface changes
- update resource type to CIM_RES_TYPE_foo
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 3afbff37cc39 -r 366b317afbf3 src/Virt_SystemDevice.c
--- a/src/Virt_SystemDevice.c Fri Mar 07 13:19:01 2008 +0100
+++ b/src/Virt_SystemDevice.c Fri Mar 07 13:19:01 2008 +0100
@@ -48,55 +48,6 @@
const static CMPIBroker *_BROKER;
-#define DEV_TYPE_COUNT 4
-const static int device_types[DEV_TYPE_COUNT] =
- {VIRT_DEV_NET,
- VIRT_DEV_DISK,
- VIRT_DEV_MEM,
- VIRT_DEV_VCPU,
- };
-
-static int get_dom_devices(const char *name,
- struct inst_list *list,
- int type,
- const char *host_cn,
- const char *ns)
-{
- virConnectPtr conn = NULL;
- virDomainPtr dom = NULL;
- CMPIStatus s;
- int ret = 0;
-
- conn = connect_by_classname(_BROKER, host_cn, &s);
- if (conn == NULL)
- goto out;
-
- dom = virDomainLookupByName(conn, name);
- if (dom == NULL)
- goto out;
-
- ret = dom_devices(_BROKER, dom, ns, type, list);
-
- virDomainFree(dom);
- out:
- virConnectClose(conn);
-
- return ret;
-}
-
-static int get_all_devices(const char *name,
- struct inst_list *list,
- const char *host_cn,
- const char *ns)
-{
- int i;
-
- for (i = 0; i < DEV_TYPE_COUNT; i++)
- get_dom_devices(name, list, device_types[i], host_cn, ns);
-
- return i;
-}
-
static CMPIStatus sys_to_dev(const CMPIObjectPath *ref,
struct std_assoc_info *info,
struct inst_list *list)
@@ -104,7 +55,6 @@ static CMPIStatus sys_to_dev(const CMPIO
const char *host = NULL;
CMPIStatus s = {CMPI_RC_OK, NULL};
CMPIInstance *inst = NULL;
- int ret;
if (!match_hypervisor_prefix(ref, info))
return s;
@@ -120,20 +70,7 @@ static CMPIStatus sys_to_dev(const CMPIO
goto out;
}
- ret = get_all_devices(host,
- list,
- CLASSNAME(ref),
- NAMESPACE(ref));
-
- if (ret >= 0) {
- cu_statusf(_BROKER, &s,
- CMPI_RC_OK,
- "");
- } else {
- cu_statusf(_BROKER, &s,
- CMPI_RC_ERR_FAILED,
- "Unable to get devices");
- }
+ s = enum_devices(_BROKER, ref, host, CIM_RES_TYPE_ALL, list);
out:
return s;
6:26 a.m.
New subject: [PATCH 3 of 3] EAFP: adopt Device interface changes
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1204892342 -3600
# Node ID 50f47a071fb1d4668a5eb1b3e0c649f87ce8f376
# Parent 366b317afbf382f85ed75b86b6df9eae30dd6c94
EAFP: adopt Device interface changes
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 366b317afbf3 -r 50f47a071fb1 src/Virt_ElementAllocatedFromPool.c
--- a/src/Virt_ElementAllocatedFromPool.c Fri Mar 07 13:19:01 2008 +0100
+++ b/src/Virt_ElementAllocatedFromPool.c Fri Mar 07 13:19:02 2008 +0100
@@ -77,7 +77,7 @@ static CMPIStatus vdev_to_pool(const CMP
goto out;
type = class_to_type(ref);
- if (type == 0) {
+ if (type == CIM_RES_TYPE_UNKNOWN) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Unknown device type");
@@ -138,47 +138,6 @@ static int filter_by_pool(struct inst_li
return dest->cur;
}
-static int devs_from_pool(uint16_t type,
- const CMPIObjectPath *ref,
- const char *poolid,
- struct inst_list *list)
-{
- CMPIStatus s;
- virConnectPtr conn = NULL;
- virDomainPtr *doms = NULL;
- int count;
- int i;
- const char *ns = NAMESPACE(ref);
- const char *cn = CLASSNAME(ref);
-
- conn = connect_by_classname(_BROKER, cn, &s);
- if (conn == NULL)
- return 0;
-
- count = get_domain_list(conn, &doms);
-
- for (i = 0; i < count; i++) {
- const char *name;
- struct inst_list tmp;
-
- inst_list_init(&tmp);
-
- name = virDomainGetName(doms[i]);
-
- dom_devices(_BROKER, doms[i], ns, type, &tmp);
-
- filter_by_pool(list, &tmp, type, poolid);
-
- inst_list_free(&tmp);
- virDomainFree(doms[i]);
- }
-
- free(doms);
- virConnectClose(conn);
-
- return count;
-}
-
static CMPIStatus pool_to_vdev(const CMPIObjectPath *ref,
struct std_assoc_info *info,
struct inst_list *list)
@@ -186,7 +145,8 @@ static CMPIStatus pool_to_vdev(const CMP
const char *poolid;
CMPIStatus s = {CMPI_RC_OK, NULL};
uint16_t type;
- CMPIInstance *inst;
+ CMPIInstance *inst = NULL;
+ struct inst_list tmp;
if (!match_hypervisor_prefix(ref, info))
return s;
@@ -210,7 +170,15 @@ static CMPIStatus pool_to_vdev(const CMP
goto out;
}
- devs_from_pool(type, ref, poolid, list);
+ inst_list_init(&tmp);
+
+ s = enum_devices(_BROKER, ref, NULL, type, &tmp);
+ if (s.rc != CMPI_RC_OK)
+ goto out;
+
+ filter_by_pool(list, &tmp, type, poolid);
+
+ inst_list_free(&tmp);
out:
return s;
10:32 a.m.
New subject: [PATCH 3 of 3] EAFP: adopt Device interface changes
HE> # HG changeset patch
HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
HE> # Date 1204892342 -3600
HE> # Node ID 50f47a071fb1d4668a5eb1b3e0c649f87ce8f376
HE> # Parent 366b317afbf382f85ed75b86b6df9eae30dd6c94
HE> EAFP: adopt Device interface changes
This patch is throwing a reject for me:
src/Virt_ElementAllocatedFromPool.c
Hunk #1 succeeded at 73 with fuzz 1 (offset -4 lines).
Hunk #2 FAILED at 134.
Hunk #4 succeeded at 168 with fuzz 2 (offset -2 lines).
1 out of 4 hunks FAILED -- saving rejects to file
src/Virt_ElementAllocatedFromPool.c.rej
The actual rejected hunk is:
***************
*** 134,180 ****
return dest->cur;
}
- static int devs_from_pool(uint16_t type,
- const CMPIObjectPath *ref,
- const char *poolid,
- struct inst_list *list)
- {
- CMPIStatus s;
- virConnectPtr conn = NULL;
- virDomainPtr *doms = NULL;
- int count;
- int i;
- const char *ns = NAMESPACE(ref);
- const char *cn = CLASSNAME(ref);
-
- conn = connect_by_classname(_BROKER, cn, &s);
- if (conn == NULL)
- return 0;
-
- count = get_domain_list(conn, &doms);
-
- for (i = 0; i < count; i++) {
- const char *name;
- struct inst_list tmp;
-
- inst_list_init(&tmp);
-
- name = virDomainGetName(doms[i]);
-
- dom_devices(_BROKER, doms[i], ns, type, &tmp);
-
- filter_by_pool(list, &tmp, type, poolid);
-
- inst_list_free(&tmp);
- virDomainFree(doms[i]);
- }
-
- free(doms);
- virConnectClose(conn);
-
- return count;
- }
-
static CMPIStatus pool_to_vdev(const CMPIObjectPath *ref,
struct std_assoc_info *info,
struct inst_list *list)
--- 134,139 ----
return dest->cur;
}
static CMPIStatus pool_to_vdev(const CMPIObjectPath *ref,
struct std_assoc_info *info,
struct inst_list *list)
I applied this after this set:
Update resource types to CIM_RES_TYPE_foo and Adopt changes to
affected providers
Did I mess up the order? Thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
11:24 a.m.
New subject: [PATCH 3 of 3] EAFP: adopt Device interface changes
I applied this after this set:
Update resource types to CIM_RES_TYPE_foo and Adopt changes to
affected providers
Did I mess up the order? Thanks!
This applied alright for me. Here's the order I applied in:
[PATCH 0 of 7] Update resource types to CIM_RES_TYPE_foo and Adopt
changes to affected providers
[PATCH 0 of 6] #3 - Adopt get_() common look and feel to DevicePool and
Adopted interface changes to assocs
[PATCH 0 of 3] #3 - Add configurable enum_devices() to Device provider
and Adopt interface changes to SD & EAFP
Sounds like the same order though.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
1:10 p.m.
New subject: [PATCH 3 of 3] EAFP: adopt Device interface changes
KR> This applied alright for me. Here's the order I applied in:
KR> [PATCH 0 of 7] Update resource types to CIM_RES_TYPE_foo and Adopt
KR> changes to affected providers
KR> [PATCH 0 of 6] #3 - Adopt get_() common look and feel to DevicePool
KR> and Adopted interface changes to assocs
KR> [PATCH 0 of 3] #3 - Add configurable enum_devices() to Device provider
KR> and Adopt interface changes to SD & EAFP
Okay, I was missing the 6-set in the middle, as it showed up much
later in my mailer.
Thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
6130
days inactive
6133
days old
10 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert