[PATCH 0 of 3] Device: fix getInstance issues

- getInstance with wrong hypervisor segfaults - getInstance returns with FAILED instead of NOT_FOUND - SystemCreationClassName not set in key properties

# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201693929 -3600 # Node ID d7a8461a61dcf13ff9d0ab2220b78d9748818cb4 # Parent 9494f6f1f1677389b6ad5dbfb01776795b483d11 Device: getInstance with wrong hypervisor segfaults wbemgi 'http://localhost:5988/root/virt:Xen_LogicalDisk.DeviceID="qemu1/hda",CreationClassName="KVM_LogicalDisk",SystemName="qemu1",SystemCreationClassName=""' on a KVM system segfaults. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 9494f6f1f167 -r d7a8461a61dc src/Virt_Device.c --- a/src/Virt_Device.c Wed Jan 30 12:42:52 2008 +0100 +++ b/src/Virt_Device.c Wed Jan 30 12:52:09 2008 +0100 @@ -464,8 +464,12 @@ static CMPIStatus get_device(const CMPIO cn = CLASSNAME(reference); conn = connect_by_classname(_BROKER, cn, &s); - if (!conn) - return s; + if (conn == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance"); + goto out; + } inst = instance_from_devid(_BROKER, conn, @@ -481,6 +485,7 @@ static CMPIStatus get_device(const CMPIO "Unable to get device instance"); } + out: virConnectClose(conn); return s;

# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201694304 -3600 # Node ID 0a2eafae729edda6f4cd12171fd28d7fbf5b1c16 # Parent d7a8461a61dcf13ff9d0ab2220b78d9748818cb4 Device: getInstance returns with FAILED instead of NOT_FOUND Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r d7a8461a61dc -r 0a2eafae729e src/Virt_Device.c --- a/src/Virt_Device.c Wed Jan 30 12:52:09 2008 +0100 +++ b/src/Virt_Device.c Wed Jan 30 12:58:24 2008 +0100 @@ -456,7 +456,7 @@ static CMPIStatus get_device(const CMPIO const CMPIResult *results, const char *devid) { - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; virConnectPtr conn; CMPIInstance *inst; const char *cn; @@ -476,14 +476,18 @@ static CMPIStatus get_device(const CMPIO devid, NAMESPACE(reference), device_type_from_classname(cn)); - if (inst) { - CMReturnInstance(results, inst); - CMSetStatus(&s, CMPI_RC_OK); - } else { + if (inst == NULL) { cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to get device instance"); - } + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", devid); + goto out; + } + + s = cu_validate_ref(_BROKER, reference, inst); + if (s.rc != CMPI_RC_OK) + goto out; + + CMReturnInstance(results, inst); out: virConnectClose(conn);

# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201696056 -3600 # Node ID 06f194fc249857f9d523583873065d3417ea5784 # Parent 0a2eafae729edda6f4cd12171fd28d7fbf5b1c16 Device: SystemCreationClassName not set in key properties Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 0a2eafae729e -r 06f194fc2498 src/Virt_Device.c --- a/src/Virt_Device.c Wed Jan 30 12:58:24 2008 +0100 +++ b/src/Virt_Device.c Wed Jan 30 13:27:36 2008 +0100 @@ -84,15 +84,6 @@ static int net_set_hwaddr(CMPIInstance * return 1; } -static int net_set_systemname(CMPIInstance *instance, - const char *domain) -{ - CMSetProperty(instance, "SystemName", - (CMPIValue *)domain, CMPI_chars); - - return 1; -} - static CMPIInstance *net_instance(const CMPIBroker *broker, struct net_device *dev, const virDomainPtr dom, @@ -111,9 +102,6 @@ static CMPIInstance *net_instance(const return NULL; if (!net_set_hwaddr(inst, dev, broker)) - return NULL; - - if (!net_set_systemname(inst, virDomainGetName(dom))) return NULL; return inst; @@ -224,8 +212,19 @@ static int device_set_systemname(CMPIIns static int device_set_systemname(CMPIInstance *instance, const virDomainPtr dom) { + virConnectPtr conn = NULL; + CMSetProperty(instance, "SystemName", (CMPIValue *)virDomainGetName(dom), CMPI_chars); + + conn = virDomainGetConnect(dom); + if (conn) { + char *sccn = NULL; + sccn = get_typed_class(pfx_from_conn(conn), "ComputerSystem"); + CMSetProperty(instance, "SystemCreationClassName", + (CMPIValue *)sccn, CMPI_chars); + free(sccn); + } return 1; }

Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1201696056 -3600 # Node ID 06f194fc249857f9d523583873065d3417ea5784 # Parent 0a2eafae729edda6f4cd12171fd28d7fbf5b1c16 Device: SystemCreationClassName not set in key properties Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r 0a2eafae729e -r 06f194fc2498 src/Virt_Device.c --- a/src/Virt_Device.c Wed Jan 30 12:58:24 2008 +0100 +++ b/src/Virt_Device.c Wed Jan 30 13:27:36 2008 +0100 @@ -84,15 +84,6 @@ static int net_set_hwaddr(CMPIInstance * return 1; }
-static int net_set_systemname(CMPIInstance *instance, - const char *domain) -{ - CMSetProperty(instance, "SystemName", - (CMPIValue *)domain, CMPI_chars); - - return 1; -} - static CMPIInstance *net_instance(const CMPIBroker *broker, struct net_device *dev, const virDomainPtr dom, @@ -111,9 +102,6 @@ static CMPIInstance *net_instance(const return NULL;
if (!net_set_hwaddr(inst, dev, broker)) - return NULL; - - if (!net_set_systemname(inst, virDomainGetName(dom))) return NULL;
return inst; @@ -224,8 +212,19 @@ static int device_set_systemname(CMPIIns static int device_set_systemname(CMPIInstance *instance, const virDomainPtr dom) { + virConnectPtr conn = NULL; + CMSetProperty(instance, "SystemName", (CMPIValue *)virDomainGetName(dom), CMPI_chars); + + conn = virDomainGetConnect(dom); + if (conn) { + char *sccn = NULL; + sccn = get_typed_class(pfx_from_conn(conn), "ComputerSystem"); + CMSetProperty(instance, "SystemCreationClassName", + (CMPIValue *)sccn, CMPI_chars); + free(sccn); + }
return 1; }
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
I'm sure this is just my CIM ignorance showing here, but is ComputerSystem the right base class name there? Isn't this for Device? Also, looks like you forgot the virConnectClose() for that new conn. Other two in the set look good. -- -Jay

Jay Gagnon wrote:
Heidi Eckhart wrote:
@@ -224,8 +212,19 @@ static int device_set_systemname(CMPIIns static int device_set_systemname(CMPIInstance *instance, const virDomainPtr dom) { + virConnectPtr conn = NULL; + CMSetProperty(instance, "SystemName", (CMPIValue *)virDomainGetName(dom), CMPI_chars); + + conn = virDomainGetConnect(dom); + if (conn) { + char *sccn = NULL; + sccn = get_typed_class(pfx_from_conn(conn), "ComputerSystem"); + CMSetProperty(instance, "SystemCreationClassName", + (CMPIValue *)sccn, CMPI_chars); + free(sccn); + }
return 1; }
I'm sure this is just my CIM ignorance showing here, but is ComputerSystem the right base class name there? Isn't this for Device?
Its always good to have a discerning reviewer ;). What's set here is the system's CreationClassName. And the scoping system in that case is the virtual system ... our Xen/KVM_ComputerSystem. So its the key-value pair of ComputerSystem. Xen/KVM_ComputerSystem.Name becomes Xen/KVM_<LogicalDevice>.SystemName Xen/KVM_ComputerSystem.CreationClassName becomes Xen/KVM_<LogicalDevice>.SystemCreationClassName
Also, looks like you forgot the virConnectClose() for that new conn.
Thanks ... a very good catch. I will resend the patch. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor

Heidi Eckhart wrote:
Jay Gagnon wrote:
Heidi Eckhart wrote:
@@ -224,8 +212,19 @@ static int device_set_systemname(CMPIIns static int device_set_systemname(CMPIInstance *instance, const virDomainPtr dom) { + virConnectPtr conn = NULL; + CMSetProperty(instance, "SystemName", (CMPIValue *)virDomainGetName(dom), CMPI_chars); + + conn = virDomainGetConnect(dom); + if (conn) { + char *sccn = NULL; + sccn = get_typed_class(pfx_from_conn(conn), "ComputerSystem"); + CMSetProperty(instance, "SystemCreationClassName", + (CMPIValue *)sccn, CMPI_chars); + free(sccn); + }
return 1; }
I'm sure this is just my CIM ignorance showing here, but is ComputerSystem the right base class name there? Isn't this for Device?
Its always good to have a discerning reviewer ;). What's set here is the system's CreationClassName. And the scoping system in that case is the virtual system ... our Xen/KVM_ComputerSystem. So its the key-value pair of ComputerSystem. Xen/KVM_ComputerSystem.Name becomes Xen/KVM_<LogicalDevice>.SystemName Xen/KVM_ComputerSystem.CreationClassName becomes Xen/KVM_<LogicalDevice>.SystemCreationClassName
Okay, cool. I suspected it might be something like that but wanted to make sure. -- -Jay
participants (2)
-
Heidi Eckhart
-
Jay Gagnon