[PATCH 0 of 2] Make ProcRASD check running domains for VCPU count
This patch set makes ProcRASD get the hypervisor's count of VCPUs when a domain is running. This helps us show the state of the system instead of the stored configuration when the guest is running. Because of some Xen quirks, the two can get out of sync and so it's best to make sure we're returning the correct number when a guest is online.
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1225145197 25200 # Node ID 10bd66c67a6ad4eebe6a0f8dc81db2b4912d5650 # Parent e043f46f299ec3dd3d1e48e51e57437fa90db136 Add a domain_vcpu_count() function to return the number of active VCPUs for a given domain. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r e043f46f299e -r 10bd66c67a6a libxkutil/misc_util.c --- a/libxkutil/misc_util.c Wed Oct 22 08:28:37 2008 -0700 +++ b/libxkutil/misc_util.c Mon Oct 27 15:06:37 2008 -0700 @@ -541,6 +541,45 @@ return result; } +int domain_vcpu_count(virDomainPtr dom) +{ + virVcpuInfoPtr info = NULL; + int max; + int count; + int actual = 0; + int i; + virConnectPtr conn = NULL; + + conn = virDomainGetConnect(dom); + if (conn == NULL) { + CU_DEBUG("Failed to get connection from domain"); + return -1; + } + + max = virConnectGetMaxVcpus(conn, virConnectGetType(conn)); + if (max <= 0) { + CU_DEBUG("Failed to get max vcpu count"); + return -1; + } + + info = calloc(max, sizeof(*info)); + if (info == NULL) { + CU_DEBUG("Failed to allocate %i vcpuinfo structures", max); + return -1; + } + + count = virDomainGetVcpus(dom, info, max, NULL, 0); + + free(info); + + for (i = 0; i < count; i++) { + if (info[i].cpu != -1) + actual++; + } + + return actual; +} + /* * Local Variables: * mode: C diff -r e043f46f299e -r 10bd66c67a6a libxkutil/misc_util.h --- a/libxkutil/misc_util.h Wed Oct 22 08:28:37 2008 -0700 +++ b/libxkutil/misc_util.h Mon Oct 27 15:06:37 2008 -0700 @@ -124,6 +124,8 @@ bool check_refs_pfx_match(const CMPIObjectPath *refa, const CMPIObjectPath *refb); +int domain_vcpu_count(virDomainPtr dom); + #define LIBVIRT_CIM_DEFAULT_MAKEREF() \ static CMPIInstance* make_ref(const CMPIObjectPath *source_ref, \ const CMPIInstance *target_inst, \
+ info = calloc(max, sizeof(*info)); + if (info == NULL) { + CU_DEBUG("Failed to allocate %i vcpuinfo structures", max); + return -1; + } + + count = virDomainGetVcpus(dom, info, max, NULL, 0); + + free(info);
You free info here..
+ + for (i = 0; i < count; i++) { + if (info[i].cpu != -1)
But use one of its fields in the compare here.
+ actual++; + } + + return actual; +}
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
+ info = calloc(max, sizeof(*info)); + if (info == NULL) { + CU_DEBUG("Failed to allocate %i vcpuinfo structures", max); + return -1; + } + + count = virDomainGetVcpus(dom, info, max, NULL, 0); + + free(info);
KR> You free info here..
+ + for (i = 0; i < count; i++) { + if (info[i].cpu != -1)
KR> But use one of its fields in the compare here. Yeah, that would be bad. What happened was, I didn't realize at first that we had to actually examine the .cpu field of the struct to get a count of *active* vcpus, so I initially was just returning count. Later, I added the loop to count the active ones, and clearly added it after the free() instead of before :) I'll send a replacement. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1225145383 25200 # Node ID 7bcc5c38cca4093f70ed50601b45c67cf1a0085a # Parent 10bd66c67a6ad4eebe6a0f8dc81db2b4912d5650 Make ProcRASD properly reflect the number of VCPUs a domain currently has, instead of just the number that the libvirt XML definition claims. Since Xen doesn't always honor vcpu add/remove actions, this is required to make sure we're reflecting the state of the system when a domain is running. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 10bd66c67a6a -r 7bcc5c38cca4 src/Virt_RASD.c --- a/src/Virt_RASD.c Mon Oct 27 15:06:37 2008 -0700 +++ b/src/Virt_RASD.c Mon Oct 27 15:09:43 2008 -0700 @@ -96,6 +96,7 @@ static CMPIStatus set_proc_rasd_params(const CMPIBroker *broker, const CMPIObjectPath *ref, + struct virt_device *dev, const char *domain, CMPIInstance *inst) { @@ -105,6 +106,7 @@ struct infostore_ctx *info; uint32_t weight; uint64_t limit; + uint64_t count; conn = connect_by_classname(broker, CLASSNAME(ref), &s); if (conn == NULL) @@ -117,6 +119,17 @@ "Domain `%s' not found while getting info", domain); goto out; } + + if (domain_online(dom)) + count = domain_vcpu_count(dom); + else + count = dev->dev.vcpu.quantity; + + if (count > 0) + CMSetProperty(inst, + "VirtualQuantity", + (CMPIValue *)&count, + CMPI_uint64); info = infostore_open(dom); if (info == NULL) { @@ -306,14 +319,7 @@ CMSetProperty(inst, "Limit", (CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64); } else if (dev->type == CIM_RES_TYPE_PROC) { - if (dev->dev.vcpu.quantity > 0) { - CMSetProperty(inst, - "VirtualQuantity", - (CMPIValue *)&dev->dev.vcpu.quantity, - CMPI_uint64); - } - - set_proc_rasd_params(broker, ref, host, inst); + set_proc_rasd_params(broker, ref, dev, host, inst); } /* FIXME: Put the HostResource in place */
Dan Smith wrote:
This patch set makes ProcRASD get the hypervisor's count of VCPUs when a domain is running. This helps us show the state of the system instead of the stored configuration when the guest is running. Because of some Xen quirks, the two can get out of sync and so it's best to make sure we're returning the correct number when a guest is online.
+1 for this set. I've tried increasing and decreasing # vcpus on both halted and running domains under xen. It is invalid to set vcpus > max vcpus on running domain in xen. Attempting this results in an exception in xend that is lost somewhere along the way, perhaps libvirt. Anyhow, it is unrelated to this set. Thanks, Jim
participants (3)
-
Dan Smith -
Jim Fehlig -
Kaitlin Rupert