[Libvirt-cim] [PATCH 2/2] ComputerSystem: Reboot/Shutdown state changes as jobs
Daniel Veillard
veillard at redhat.com
Tue Jul 10 06:02:09 UTC 2012
On Wed, Jun 27, 2012 at 03:58:02PM -0300, Eduardo Lima (Etrunko) wrote:
> From: "Eduardo Lima (Etrunko)" <eblima at br.ibm.com>
>
> For Reboot and Shutdown, th RequestStateChange method returns immediately with
typo th -> the
> 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 at br.ibm.com>
> ---
> schema/ComputerSystem.mof | 9 ++
> src/Virt_ComputerSystem.c | 326 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 325 insertions(+), 10 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..9546da2 100644
> --- a/src/Virt_ComputerSystem.c
> +++ b/src/Virt_ComputerSystem.c
> @@ -30,23 +30,40 @@
> #include <cmpift.h>
> #include <cmpimacs.h>
>
> +#include <uuid.h>
> #include <libvirt/libvirt.h>
> +#include <libvirt/virterror.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;
> + CMPIInstance *inst;
> + char *dom_name;
> + uint16_t dom_state;
> + uint16_t status; /* job status */
> +};
> +
> +static bool events_registered = false;
> +
> /* Set the "Name" property of an instance from a domain */
> static int set_name_from_dom(virDomainPtr dom, CMPIInstance *instance)
> {
> @@ -1189,19 +1206,20 @@ static CMPIStatus __state_change(const char *name,
> s = state_change_enable(dom, &info);
> else if (state == CIM_STATE_DISABLED)
> s = state_change_disable(dom, &info);
> - else if (state == CIM_STATE_SHUTDOWN)
> - s = state_change_shutdown(dom, &info);
> else if (state == CIM_STATE_PAUSED)
> s = state_change_pause(dom, &info);
> - else if (state == CIM_STATE_REBOOT)
> - s = state_change_reboot(dom, &info);
> else if (state == CIM_STATE_RESET)
> s = state_change_reset(dom, &info);
> + else if (state == CIM_STATE_SHUTDOWN || state == CIM_STATE_REBOOT)
> + s.rc = CIM_SVPC_RETURN_JOB_STARTED;
> else
> cu_statusf(_BROKER, &s,
> 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 +1233,265 @@ 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_NEW;
> +
> + 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 *) "New", 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);
> + if ((*job)->context == NULL) {
> + CU_DEBUG("Failed to create thread context");
> + cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED,
> + "Unable to create thread context");
> + goto out;
> + }
> +
> + (*job)->inst = CBGetInstance(_BROKER, (*job)->context, (*job)->obj_path, NULL, &s);
> + if (((*job)->inst == NULL) || (s.rc != CMPI_RC_OK)) {
> + CU_DEBUG("Failed to get job instance (%i)", s.rc);
> + goto out;
> + }
> +
> + out:
> + free(type);
> + free(cn);
> + free(ns);
Hum, we rely here on "free(NULL)' to do the right thing, but that's
okay for all recent implementations :-)
> + return s;
> +}
> +
> +static void state_change_reboot_cb(virConnectPtr conn,
> + virDomainPtr dom,
> + void *data)
> +{
> + state_change_job_t *job = (state_change_job_t *) data;
> + job->status = CIM_JOB_STATE_COMPLETED;
> + CU_DEBUG("state_change_reboot_cb(): Reboot complete");
> +}
> +
> +static void state_change_shutdown_cb(virConnectPtr conn,
> + virDomainPtr dom,
> + int event,
> + int detail,
> + void *data)
> +{
> + state_change_job_t *job = (state_change_job_t *) data;
> + if (event == VIR_DOMAIN_EVENT_SHUTDOWN) {
> + job->status = CIM_JOB_STATE_COMPLETED;
> + CU_DEBUG("state_change_shutdown_cb(): Shutdown complete");
> + }
> +}
> +
> +static CMPI_THREAD_RETURN state_change_thread(void *data)
> +{
> + CMPIStatus s;
> + state_change_job_t *job = (state_change_job_t *) data;
> + virConnectPtr conn = NULL;
> + virDomainPtr dom = NULL;
> + virDomainInfo info;
> + int job_cb = -1;
> +
> + /* Set job state */
> + CBAttachThread(_BROKER, job->context);
> + CU_DEBUG("State change job %s started", job->uuid);
> +
> + job->status = CIM_JOB_STATE_STARTING;
> + CMSetProperty(job->inst, "JobState",
> + (CMPIValue *) &(job->status), CMPI_uint16);
> + CMSetProperty(job->inst, "Status",
> + (CMPIValue *) "Starting", 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 err;
> + }
> +
> + dom = virDomainLookupByName(conn, job->dom_name);
> + if (dom == NULL) {
> + CU_DEBUG("Unable to get domain '%s'", job->dom_name);
> + goto err;
> + }
> +
> + if (virDomainGetInfo(dom, &info) != 0) {
> + CU_DEBUG("Unable to get domain info for '%s'", job->dom_name);
> + goto err;
> + }
> +
> + if (job->dom_state == CIM_STATE_REBOOT) {
> + job_cb = virConnectDomainEventRegisterAny(conn, NULL,
> + VIR_DOMAIN_EVENT_ID_REBOOT,
> + VIR_DOMAIN_EVENT_CALLBACK(state_change_reboot_cb),
> + job, NULL);
> +
> + if (job_cb == -1) {
> + CU_DEBUG("Unable to connect domain reboot callback");
> + goto err;
> + }
> +
> + s = state_change_reboot(dom, &info);
> +
> + if (s.rc != CMPI_RC_OK) {
> + CU_DEBUG("Unable to trigger domain reboot: '%s'",
> + CMGetCharPtr(s.msg));
> + goto err;
> + }
> + } else if (job->dom_state == CIM_STATE_SHUTDOWN) {
> + job_cb = virConnectDomainEventRegisterAny(conn, NULL,
> + VIR_DOMAIN_EVENT_ID_LIFECYCLE,
> + VIR_DOMAIN_EVENT_CALLBACK(state_change_shutdown_cb),
> + job, NULL);
> +
> + if (job_cb == -1) {
> + CU_DEBUG("Unable to connect domain shutdown callback");
> + goto err;
> + }
> +
> + s = state_change_shutdown(dom, &info);
> +
> + if (s.rc != CMPI_RC_OK) {
> + CU_DEBUG("Unable to trigger domain shutdown: '%s'",
> + CMGetCharPtr(s.msg));
> + goto err;
> + }
> + } else {
> + CU_DEBUG("Unrecognized state '%d'", job->dom_state);
> + goto err;
> + }
> +
> + job->status = CIM_JOB_STATE_RUNNING;
> + CMSetProperty(job->inst, "JobState",
> + (CMPIValue *) &(job->status), CMPI_uint16);
> + CMSetProperty(job->inst, "Status",
> + (CMPIValue *) "Running", CMPI_chars);
> +
> + CU_DEBUG("Entering mainloop implementation");
> + /* Wait for operation (shutdown/reboot) to complete */
> + while (job->status == CIM_JOB_STATE_RUNNING) {
> + if (virEventRunDefaultImpl() < 0) {
> + virErrorPtr err = virGetLastError();
> + CU_DEBUG("Failed to run event loop: %s\n",
> + err && err->message ? err->message : "Unknown error");
> + goto err;
> + }
> + }
> +
> + CU_DEBUG("Job completed");
> + CMSetProperty(job->inst, "JobState",
> + (CMPIValue *) &(job->status), CMPI_uint16);
> + CMSetProperty(job->inst, "Status",
> + (CMPIValue *) "Completed", CMPI_chars);
> +
> + goto out;
> +
> + err:
> + job->status = CIM_JOB_STATE_EXCEPTION;
> + CMSetProperty(job->inst, "JobState",
> + (CMPIValue *) &(job->status), CMPI_uint16);
> + CMSetProperty(job->inst, "Status",
> + (CMPIValue *) "Error", CMPI_chars);
> +
> + out:
> + if (job_cb != -1)
> + virConnectDomainEventDeregisterAny(conn, job_cb);
> +
> + virDomainFree(dom);
> + virConnectClose(conn);
> +
> + CBDetachThread(_BROKER, job->context);
> + free(job->dom_name);
> + free(job);
> + return NULL;
> +}
> +
> static CMPIStatus state_change(CMPIMethodMI *self,
> const CMPIContext *context,
> const CMPIResult *results,
> @@ -1244,7 +1521,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 +1534,36 @@ static CMPIStatus state_change(CMPIMethodMI *self,
>
> s = __state_change(name, state, reference);
>
> + rc = s.rc;
> +
> if (s.rc == CMPI_RC_OK)
> - rc = 0;
> + goto out;
> +
> + if (s.rc == CIM_SVPC_RETURN_JOB_STARTED) {
> + state_change_job_t *job = NULL;
> +
> + if (events_registered == false) {
> + events_registered = true;
> + if (virEventRegisterDefaultImpl() != 0) {
> + CU_DEBUG("virEventRegisterDefaultImpl() failed");
> + goto out;
> + }
> + }
> +
> + s = create_state_change_job(reference, context, &job, state);
> + if (s.rc != CMPI_RC_OK) {
> + rc = s.rc;
> + 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);
> + }
>
> out:
> CMReturnData(results, &rc, CMPI_uint32);
Okay, fairly complex but that's DMTF style :-)
What i don't see is how/where the object associated to the job are freed
is there some generic mechanism handling all of this, or are we missing
some kind of routine which would be called asynchronously to free the
data associated to the job ?
I don't know, if you're sure that everything is deallocated then fine,
ACK :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the Libvirt-cim
mailing list