[PATCH 0 of 3] Adding ElementSettingData.
Cleaned up previous patch - removed buggy InstanceID parsing code from ElementSettingData. Created a new function in libxutil/misc_util.c based on the InstanceID parsing of VSSDComponent. Cleaned up other bugs / style issues as well.
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1195082890 28800 # Node ID b4f0488c0d16a228a3c6fd6f23f7465cd3944123 # Parent 6dc0a1a05c60ad08738c7c48aa9fb6d7dd7cd8be Add function to parse InstanceIDs. This function currently supports InstanceIDs in the format <Org>:<LocalID>, such as "Xen:Domain-0" for VSSD. The functionality is based on the parsing from VSSDComponent, so the code for parsing the InstanceID is replaced by the function. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 6dc0a1a05c60 -r b4f0488c0d16 libxkutil/misc_util.c --- a/libxkutil/misc_util.c Wed Nov 14 15:22:38 2007 -0800 +++ b/libxkutil/misc_util.c Wed Nov 14 15:28:10 2007 -0800 @@ -371,6 +371,27 @@ bool domain_online(virDomainPtr dom) (info.state == VIR_DOMAIN_RUNNING); } +bool parse_id(const CMPIObjectPath *ref, + char **name) +{ + char *id = NULL; + char *pfx = NULL; + int ret; + + id = cu_get_str_path(ref, "InstanceID"); + if (id == NULL) + return false; + + ret = sscanf(id, "%a[^:]:%as", &pfx, name); + + free(id); + free(pfx); + + if ((ret != 2) || (*name == NULL)) + return false; + + return true; +} /* * Local Variables: diff -r 6dc0a1a05c60 -r b4f0488c0d16 libxkutil/misc_util.h --- a/libxkutil/misc_util.h Wed Nov 14 15:22:38 2007 -0800 +++ b/libxkutil/misc_util.h Wed Nov 14 15:28:10 2007 -0800 @@ -89,6 +89,8 @@ char *association_prefix(const char *pro char *association_prefix(const char *provider_name); bool match_pn_to_cn(const char *pn, const char *cn); +bool parse_id(const CMPIObjectPath *ref, char **name); + #define ASSOC_MATCH(pn, cn) \ if (!match_pn_to_cn((pn), (cn))) { \ return (CMPIStatus){CMPI_RC_OK, NULL}; \ diff -r 6dc0a1a05c60 -r b4f0488c0d16 src/Virt_VSSDComponent.c --- a/src/Virt_VSSDComponent.c Wed Nov 14 15:22:38 2007 -0800 +++ b/src/Virt_VSSDComponent.c Wed Nov 14 15:28:10 2007 -0800 @@ -41,10 +41,7 @@ static CMPIStatus vssd_to_rasd(const CMP struct inst_list *list) { CMPIStatus s; - char *id = NULL; - char *pfx = NULL; char *name = NULL; - int ret; int i = 0; int types[] = { CIM_RASD_TYPE_PROC, @@ -56,19 +53,10 @@ static CMPIStatus vssd_to_rasd(const CMP ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Missing InstanceID"); - goto out; - } - - ret = sscanf(id, "%a[^:]:%as", &pfx, &name); - if (ret != 2) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Invalid InstanceID"); + if (!parse_id(ref, &name)) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get system name"); goto out; } @@ -83,8 +71,6 @@ static CMPIStatus vssd_to_rasd(const CMP CMSetStatus(&s, CMPI_RC_OK); out: - free(id); - free(pfx); free(name); return s;
KR> # HG changeset patch KR> # User Kaitlin Rupert <karupert@us.ibm.com> KR> # Date 1195082890 28800 KR> # Node ID b4f0488c0d16a228a3c6fd6f23f7465cd3944123 KR> # Parent 6dc0a1a05c60ad08738c7c48aa9fb6d7dd7cd8be KR> Add function to parse InstanceIDs. Great! KR> +bool parse_id(const CMPIObjectPath *ref, KR> + char **name) KR> +{ KR> + char *id = NULL; KR> + char *pfx = NULL; KR> + int ret; KR> + KR> + id = cu_get_str_path(ref, "InstanceID"); KR> + if (id == NULL) KR> + return false; KR> + KR> + ret = sscanf(id, "%a[^:]:%as", &pfx, name); KR> + KR> + free(id); KR> + free(pfx); KR> + KR> + if ((ret != 2) || (*name == NULL)) KR> + return false; KR> + KR> + return true; KR> +} What do you think about making parse_id() take a string id, and then also exposing a wrapper function called parse_instanceid() that takes the ref? I'd hate to bundle the actual parsing into a function that takes such a complex type. Also, since this is a library function and it could potentially be used in other places down the road, I think it would be better to make it return both sides of the id, as parse_fq_devid() does now. Yes, it will require the caller to free() another string, but I think its better that way. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
Also, since this is a library function and it could potentially be used in other places down the road, I think it would be better to make it return both sides of the id, as parse_fq_devid() does now. Yes, it will require the caller to free() another string, but I think its better that way.
Seems like with another 10 minutes ore so you could make it so that passing a NULL pointer in for either side of the id would just mean that part gets ignored, so you only get the side you are actually interested in. It wouldn't force callers to break from convention but would allow for things to be a touch cleaner in the calling function. -- -Jay
Jay Gagnon wrote:
Dan Smith wrote:
Also, since this is a library function and it could potentially be used in other places down the road, I think it would be better to make it return both sides of the id, as parse_fq_devid() does now. Yes, it will require the caller to free() another string, but I think its better that way.
Seems like with another 10 minutes ore so you could make it so that passing a NULL pointer in for either side of the id would just mean that part gets ignored, so you only get the side you are actually interested in. It wouldn't force callers to break from convention but would allow for things to be a touch cleaner in the calling function.
Both seem like good ideas. I'll see if I can write up something reasonable. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1195082558 28800 # Node ID 6dc0a1a05c60ad08738c7c48aa9fb6d7dd7cd8be # Parent c9915bdf44f5cf439c37f87a216116992a604f49 Updating build scripts to make / install/ and register provider. Also added the necessary mof /registration files. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r c9915bdf44f5 -r 6dc0a1a05c60 Makefile.am --- a/Makefile.am Wed Nov 14 15:22:34 2007 -0800 +++ b/Makefile.am Wed Nov 14 15:22:38 2007 -0800 @@ -31,7 +31,8 @@ MOFS = \ schema/NetPool.mof \ schema/ResourceAllocationFromPool.mof \ schema/ElementAllocatedFromPool.mof \ - schema/HostedService.mof + schema/HostedService.mof \ + schema/ElementSettingData.mof INTEROP_MOFS = \ schema/RegisteredProfile.mof \ @@ -67,7 +68,8 @@ REGS = \ schema/NetPool.registration \ schema/ResourceAllocationFromPool.registration \ schema/ElementAllocatedFromPool.registration \ - schema/HostedService.registration + schema/HostedService.registration \ + schema/ElementSettingData.registration INTEROP_REGS = \ schema/RegisteredProfile.registration \ diff -r c9915bdf44f5 -r 6dc0a1a05c60 schema/ElementSettingData.mof --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/schema/ElementSettingData.mof Wed Nov 14 15:22:38 2007 -0800 @@ -0,0 +1,6 @@ +// Copyright IBM Corp. 2007 +class Xen_ElementSettingData : CIM_ElementSettingData { +}; + +class KVM_ElementSettingData : CIM_ElementSettingData { +}; diff -r c9915bdf44f5 -r 6dc0a1a05c60 schema/ElementSettingData.registration --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/schema/ElementSettingData.registration Wed Nov 14 15:22:38 2007 -0800 @@ -0,0 +1,4 @@ +# Copyright IBM Corp. 2007 +# Classname Namespace ProviderName ProviderModule ProviderTypes ... +Xen_ElementSettingData root/virt Xen_ElementSettingDataProvider Virt_ElementSettingData association +KVM_ElementSettingData root/virt KVM_ElementSettingDataProvider Virt_ElementSettingData association diff -r c9915bdf44f5 -r 6dc0a1a05c60 src/Makefile.am --- a/src/Makefile.am Wed Nov 14 15:22:34 2007 -0800 +++ b/src/Makefile.am Wed Nov 14 15:22:38 2007 -0800 @@ -49,7 +49,8 @@ provider_LTLIBRARIES = libVirt_ComputerS libVirt_SettingsDefineState.la \ libVirt_ResourceAllocationFromPool.la \ libVirt_ElementAllocatedFromPool.la \ - libVirt_HostedService.la + libVirt_HostedService.la \ + libVirt_ElementSettingData.la libVirt_ComputerSystem_la_SOURCES = Virt_ComputerSystem.c libVirt_Device_la_SOURCES = Virt_Device.c @@ -116,3 +117,6 @@ libVirt_ElementAllocatedFromPool_la_LIBA libVirt_HostedService_la_SOURCES = Virt_HostedService.c libVirt_HostedService_la_LIBADD = -lVirt_VirtualSystemManagementService -lVirt_ResourcePoolConfigurationService + +libVirt_ElementSettingData_la_SOURCES = Virt_ElementSettingData.c +libVirt_ElementSettingData_la_LIBADD = -lVirt_VSSD
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1195082554 28800 # Node ID c9915bdf44f5cf439c37f87a216116992a604f49 # Parent aad2a74321d54e26b5b2c89dc8df0e95bd1860f8 Adding ElementSettingData association. This only handles the case defined in section "7.3.4.2 Single-Configuration Implementation Approach" of the VSP. Will update to handle the case(s) defined in the Resource Allocation Profile. The handler name is a bit ugly. I suspect this name will change when I add the RASD pieces. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r aad2a74321d5 -r c9915bdf44f5 src/Virt_ElementSettingData.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/Virt_ElementSettingData.c Wed Nov 14 15:22:34 2007 -0800 @@ -0,0 +1,159 @@ +/* + * Copyright IBM Corp. 2007 + * + * Authors: + * Kaitlin Rupert <karupert@us.ibm.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include <cmpidt.h> +#include <cmpift.h> +#include <cmpimacs.h> + +#include "libcmpiutil.h" +#include "std_association.h" +#include "misc_util.h" + +#include "Virt_VSSD.h" + +const static CMPIBroker *_BROKER; + +static CMPIStatus vssd_to_sd(const CMPIObjectPath *ref, + struct std_assoc_info *info, + struct inst_list *list) +{ + CMPIStatus s; + CMPIInstance *inst; + virConnectPtr conn = NULL; + virDomainPtr dom = NULL; + char *host = NULL; + + ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); + + if (!parse_id(ref, &host)) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get system name"); + goto out; + } + + conn = lv_connect(_BROKER, &s); + if (conn == NULL) + goto out; + + dom = virDomainLookupByName(conn, host); + if (dom == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "No such system `%s'", host); + goto out; + } + + inst = get_vssd_instance(dom, _BROKER, ref); + if (inst == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Error getting VSSD for `%s'", host); + goto out; + } + + inst_list_add(list, inst); + + out: + virDomainFree(dom); + virConnectClose(conn); + free(host); + + return s; +} + +static CMPIInstance *make_ref(const CMPIObjectPath *ref, + const CMPIInstance *inst, + struct std_assoc_info *info, + struct std_assoc *assoc) +{ + CMPIInstance *refinst = NULL; + char *base; + uint16_t prop_value = 1; + + base = class_base_name(assoc->assoc_class); + if (base == NULL) + goto out; + + refinst = get_typed_instance(_BROKER, + base, + NAMESPACE(ref)); + + if (refinst != NULL) { + CMPIObjectPath *instop; + + instop = CMGetObjectPath(inst, NULL); + + set_reference(assoc, refinst, ref, instop); + + /* Set additional properties with values + * defined in the "Virtual System Profile." + */ + CMSetProperty(refinst, "IsDefault", + (CMPIValue *)&prop_value, CMPI_uint16); + + CMSetProperty(refinst, "IsNext", + (CMPIValue *)&prop_value, CMPI_uint16); + + CMSetProperty(refinst, "IsMinimum", + (CMPIValue *)&prop_value, CMPI_uint16); + + CMSetProperty(refinst, "IsMaximum", + (CMPIValue *)&prop_value, CMPI_uint16); + } + +out: + return refinst; +} + +static struct std_assoc vssd_to_sd_fd_bkwd = { + .source_class = "CIM_VirtualSystemSettingData", + .source_prop = "ManagedElement", + + .target_class = "CIM_ManagedElement", + .target_prop = "SettingData", + + .assoc_class = "CIM_ElementSettingData", + + .handler = vssd_to_sd, + .make_ref = make_ref +}; + +static struct std_assoc *handlers[] = { + &vssd_to_sd_fd_bkwd, + NULL +}; + +STDA_AssocMIStub(, Xen_ElementSettingDataProvider, _BROKER, CMNoHook, handlers); +STDA_AssocMIStub(, KVM_ElementSettingDataProvider, _BROKER, CMNoHook, handlers); + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */
KR> +static CMPIInstance *make_ref(const CMPIObjectPath *ref, KR> + const CMPIInstance *inst, KR> + struct std_assoc_info *info, KR> + struct std_assoc *assoc) KR> +{ KR> + CMPIInstance *refinst = NULL; KR> + char *base; KR> + uint16_t prop_value = 1; KR> + KR> + base = class_base_name(assoc->assoc_class); KR> + if (base == NULL) KR> + goto out; KR> + KR> + refinst = get_typed_instance(_BROKER, KR> + base, KR> + NAMESPACE(ref)); KR> + KR> + if (refinst != NULL) { KR> + CMPIObjectPath *instop; KR> + KR> + instop = CMGetObjectPath(inst, NULL); KR> + KR> + set_reference(assoc, refinst, ref, instop); KR> + KR> + /* Set additional properties with values KR> + * defined in the "Virtual System Profile." KR> + */ KR> + CMSetProperty(refinst, "IsDefault", KR> + (CMPIValue *)&prop_value, CMPI_uint16); KR> + KR> + CMSetProperty(refinst, "IsNext", KR> + (CMPIValue *)&prop_value, CMPI_uint16); KR> + KR> + CMSetProperty(refinst, "IsMinimum", KR> + (CMPIValue *)&prop_value, CMPI_uint16); KR> + KR> + CMSetProperty(refinst, "IsMaximum", KR> + (CMPIValue *)&prop_value, CMPI_uint16); KR> + } KR> + KR> +out: KR> + return refinst; KR> +} You leak "base" here. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (3)
-
Dan Smith -
Jay Gagnon -
Kaitlin Rupert