Re: [Libvirt-cim] [PATCH 2/3] ComputerSystem: Make Shutdown state change a job

From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com&gt; For Shutdown, the RequestStateChange method returns immediately with return code 0 (successful) even though the state change is still not completed. According to the DMTF specification DSP1052 (Computer System Profile) the RequestStateChange() method should return 0x1000 and a corresponding job reference in the return parameters which can be polled for completion. Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com&gt; --- schema/ComputerSystem.mof | 9 ++ src/Virt_ComputerSystem.c | 236 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 238 insertions(+), 7 deletions(-) diff --git a/schema/ComputerSystem.mof b/schema/ComputerSystem.mof index 10cb8c4..886c085 100644 --- a/schema/ComputerSystem.mof +++ b/schema/ComputerSystem.mof @@ -1,5 +1,14 @@ // Copyright IBM Corp. 2007 +class Xen_ComputerSystemStateChangeJob : CIM_ConcreteJob { +}; + +class KVM_ComputerSystemStateChangeJob : CIM_ConcreteJob { +}; + +class LXC_ComputerSystemStateChangeJob : CIM_ConcreteJob { +}; + [Description ( "A class derived from CIM_ComputerSystem to represent " "the Xen virtual machines/domains running on the system."), diff --git a/src/Virt_ComputerSystem.c b/src/Virt_ComputerSystem.c index e6c7e55..778809d 100644 --- a/src/Virt_ComputerSystem.c +++ b/src/Virt_ComputerSystem.c @@ -30,23 +30,37 @@ #include <cmpift.h> #include <cmpimacs.h> +#include <uuid.h> #include <libvirt/libvirt.h> -#include "cs_util.h" #include <libcmpiutil/libcmpiutil.h> -#include "misc_util.h" -#include "infostore.h" -#include "device_parsing.h" #include <libcmpiutil/std_invokemethod.h> #include <libcmpiutil/std_instance.h> #include <libcmpiutil/std_indication.h> +#include "cs_util.h" +#include "misc_util.h" +#include "infostore.h" +#include "device_parsing.h" +#include "svpc_types.h" + #include "Virt_ComputerSystem.h" #include "Virt_HostSystem.h" #include "Virt_VirtualSystemSnapshotService.h" + const static CMPIBroker *_BROKER; +typedef struct _state_change_job state_change_job_t; +struct _state_change_job { + char uuid[VIR_UUID_STRING_BUFLEN]; + CMPIContext *context; + CMPIObjectPath *obj_path; + char *dom_name; + uint16_t dom_state; + uint16_t status; /* job status */ +}; + /* Set the "Name" property of an instance from a domain */ static int set_name_from_dom(virDomainPtr dom, CMPIInstance *instance) { @@ -1190,7 +1204,7 @@ static CMPIStatus __state_change(const char *name, else if (state == CIM_STATE_DISABLED) s = state_change_disable(dom, &info); else if (state == CIM_STATE_SHUTDOWN) - s = state_change_shutdown(dom, &info); + s.rc = CIM_SVPC_RETURN_JOB_STARTED; else if (state == CIM_STATE_PAUSED) s = state_change_pause(dom, &info); else if (state == CIM_STATE_REBOOT) @@ -1202,6 +1216,9 @@ static CMPIStatus __state_change(const char *name, CMPI_RC_ERR_NOT_SUPPORTED, "State not supported"); + if (s.rc != CMPI_RC_OK && s.rc != CIM_SVPC_RETURN_JOB_STARTED) + goto out; + infostore = infostore_open(dom); if (infostore != NULL) { infostore_set_u64(infostore, "reqstate", (uint64_t)state); @@ -1215,6 +1232,191 @@ static CMPIStatus __state_change(const char *name, return s; } +static CMPIStatus create_state_change_job(const CMPIObjectPath *ref, + const CMPIContext *context, + state_change_job_t **job, + uint16_t state) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *job_inst; + CMPIDateTime *start; + CMPIBoolean autodelete = true; + CMPIObjectPath *obj_path; + uuid_t uuid; + char *type = NULL, *cn = NULL, *ns = NULL; + + start = CMNewDateTime(_BROKER, &s); + if ((s.rc != CMPI_RC_OK) || CMIsNullObject(start)) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to get job start time"); + goto out; + } + + cn = strdup(CLASSNAME(ref)); + type = get_typed_class(cn, "ComputerSystemStateChangeJob"); + + obj_path = CMNewObjectPath(_BROKER, ns, type, &s); + if ((s.rc != CMPI_RC_OK) || (CMIsNullObject(obj_path))) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to get new object path"); + goto out; + } + + job_inst = CMNewInstance(_BROKER, obj_path, &s); + if ((s.rc != CMPI_RC_OK) || (CMIsNullObject(job_inst))) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to get new instance object"); + goto out; + } + + /* Alloc job struct */ + *job = calloc(1, sizeof(**job)); + if (*job == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to allocate memory for job structure"); + goto out; + } + + (*job)->dom_state = state; + (*job)->status = CIM_JOB_STATE_STARTING; + + uuid_generate(uuid); + uuid_unparse(uuid, (*job)->uuid); + + /* Set Properties */ + CMSetProperty(job_inst, "InstanceID", + (CMPIValue *)(*job)->uuid, CMPI_chars); + CMSetProperty(job_inst, "Name", + (CMPIValue *) "ComputerSystemStateChange", CMPI_chars); + CMSetProperty(job_inst, "StartTime", + (CMPIValue *)&start, CMPI_dateTime); + CMSetProperty(job_inst, "JobState", + (CMPIValue *)&((*job)->status), CMPI_uint16); + CMSetProperty(job_inst, "Status", + (CMPIValue *) "Starting", CMPI_chars); + CMSetProperty(job_inst, "DeleteOnCompletion", + (CMPIValue *)&autodelete, CMPI_boolean); + + obj_path = CMGetObjectPath(job_inst, &s); + if ((obj_path == NULL) || (s.rc != CMPI_RC_OK)) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to get path for ComputerSystemStateChangeJob instance"); + goto out; + } + + CMSetNameSpace(obj_path, ns); + + CU_DEBUG("Creating ComputerSystemStateChangeJob instance: %s", + CMGetCharPtr(CMObjectPathToString(obj_path, NULL))); + + obj_path = CBCreateInstance(_BROKER, context, obj_path, job_inst, &s); + if ((s.rc != CMPI_RC_OK) || (CMIsNullObject(obj_path))) { + CU_DEBUG("Failed to create ComputerSystemStateChangeJob instance: %i", s.rc); + goto out; + } + + ns = strdup(NAMESPACE(ref)); + CMSetNameSpace(obj_path, ns); + + (*job)->obj_path = obj_path; + (*job)->context = CBPrepareAttachThread(_BROKER, context); + + out: + free(type); + free(cn); + free(ns); + return s; +} + +static CMPI_THREAD_RETURN state_change_thread(void *data) +{ + CMPIStatus s; + CMPIInstance *inst = NULL; + state_change_job_t *job = (state_change_job_t *) data; + virConnectPtr conn = NULL; + virDomainPtr dom = NULL; + virDomainInfo info; + + if (job->dom_state != CIM_STATE_SHUTDOWN) { + CU_DEBUG("Unrecognized state '%d'", job->dom_state); + goto end; + } + + /* Set job state */ + CBAttachThread(_BROKER, job->context); + CU_DEBUG("State change job %s started", job->uuid); + job->status = CIM_JOB_STATE_RUNNING; + + inst = CBGetInstance(_BROKER, job->context, job->obj_path, NULL, &s); + if ((inst == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to get job instance (%i)", s.rc); + return NULL; + } + + CMSetProperty(inst, "JobState", + (CMPIValue *)&(job->status), CMPI_uint16); + CMSetProperty(inst, "Status", + (CMPIValue *) "Running", CMPI_chars); + + /* Connect to domain event callback */ + conn = connect_by_classname(_BROKER, CLASSNAME(job->obj_path), &s); + if (conn == NULL) { + CU_DEBUG("Unable to connect to '%s' hypervisor", + CLASSNAME(job->obj_path)); + goto out; + } + + dom = virDomainLookupByName(conn, job->dom_name); + if (dom == NULL) { + CU_DEBUG("Unable to get domain '%s'", job->dom_name); + goto out; + } + + if (virDomainGetInfo(dom, &info) != 0) { + CU_DEBUG("Unable to get domain info for '%s'", job->dom_name); + goto out; + } + + s = state_change_shutdown(dom, &info); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Unable to trigger domain shutdown: '%s'", + CMGetCharPtr(s.msg)); + goto out; + } + + /* Wait for operation (shutdown/reboot) to complete */ + while (info.state != VIR_DOMAIN_SHUTOFF) { + usleep(100 * 1000); + virDomainGetInfo(dom, &info); + } + + CU_DEBUG("Job completed"); + + /* Set job state */ + if (job->status == CIM_JOB_STATE_COMPLETED) { + CMSetProperty(inst, "JobState", + (CMPIValue *)&(job->status), CMPI_uint16); + CMSetProperty(inst, "Status", + (CMPIValue *) "Completed", CMPI_chars); + } + + out: + virDomainFree(dom); + virConnectClose(conn); + + CBDetachThread(_BROKER, job->context); + free(job->dom_name); + free(job); + + end: + return NULL; +} + static CMPIStatus state_change(CMPIMethodMI *self, const CMPIContext *context, const CMPIResult *results, @@ -1244,7 +1446,8 @@ static CMPIStatus state_change(CMPIMethodMI *self, goto out; } - /* Retain original instance of the guest to use for the PreviousInstance attribute when generating an indication. */ + /* Retain original instance of the guest to use for the PreviousInstance + attribute when generating an indication. */ s = get_domain_by_name(_BROKER, reference, name, &prev_inst); if (s.rc != CMPI_RC_OK || prev_inst == NULL) { cu_statusf(_BROKER, &s, @@ -1256,8 +1459,27 @@ static CMPIStatus state_change(CMPIMethodMI *self, s = __state_change(name, state, reference); - if (s.rc == CMPI_RC_OK) + if (s.rc == CMPI_RC_OK) { rc = 0; + goto out; + } + + if (s.rc == CIM_SVPC_RETURN_JOB_STARTED) { + state_change_job_t *job = NULL; + s = create_state_change_job(reference, context, &job, state); + if (s.rc != CMPI_RC_OK) { + free(job); + goto out; + } + + job->dom_name = strdup(name); + + _BROKER->xft->newThread(state_change_thread, job, 0); + + CMAddArg(argsout, "Job", (CMPIValue *)&(job->obj_path), + CMPI_ref); + rc = s.rc; + } out: CMReturnData(results, &rc, CMPI_uint32);