[PATCH 0 of 3] Set PreviousInstance properly when generating an indication
This fix is rather involved and includes some duplicate code. This is because a lot of the existing indications code will get refactored when we move to using domain events instead of an event loop that polls. In the meantime, this just makes sure we return valid instances.
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1246320303 25200 # Node ID 3bf5c713feb1b2bf6094c829e7f423480a05904c # Parent 6ed64506d55ef019359c6d7420ce927fd49855ac Reorganize code CSIndication - be sure to declare filters earlier in the file This is necessary for the raise_indication() function (see second patch). The raise_indication() function needs access to the filters list. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 6ed64506d55e -r 3bf5c713feb1 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:04:49 2009 -0700 +++ b/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:03 2009 -0700 @@ -628,15 +628,6 @@ return(CMPIStatus){CMPI_RC_OK, NULL}; } -static struct std_indication_handler csi = { - .raise_fn = NULL, - .trigger_fn = trigger_indication, - .activate_fn = ActivateFilter, - .deactivate_fn = DeActivateFilter, - .enable_fn = EnableIndications, - .disable_fn = DisableIndications, -}; - DECLARE_FILTER(xen_created, "Xen_ComputerSystemCreatedIndication"); DECLARE_FILTER(xen_deleted, "Xen_ComputerSystemDeletedIndication"); DECLARE_FILTER(xen_modified, "Xen_ComputerSystemModifiedIndication"); @@ -660,6 +651,14 @@ NULL, }; +static struct std_indication_handler csi = { + .raise_fn = NULL, + .trigger_fn = trigger_indication, + .activate_fn = ActivateFilter, + .deactivate_fn = DeActivateFilter, + .enable_fn = EnableIndications, + .disable_fn = DisableIndications, +}; DEFAULT_IND_CLEANUP(); DEFAULT_AF();
+1 Sharad Mishra System x Enablement Linux Technology Center IBM Kaitlin Rupert <kaitlin@linux.vn et.ibm.com> To Sent by: libvirt-cim@redhat.com libvirt-cim-bounc cc es@redhat.com Subject [Libvirt-cim] [PATCH 1 of 3] 06/29/2009 05:05 Reorganize code CSIndication - be PM sure to declare filters earlier in the file Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redh at.com> # HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1246320303 25200 # Node ID 3bf5c713feb1b2bf6094c829e7f423480a05904c # Parent 6ed64506d55ef019359c6d7420ce927fd49855ac Reorganize code CSIndication - be sure to declare filters earlier in the file This is necessary for the raise_indication() function (see second patch). The raise_indication() function needs access to the filters list. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 6ed64506d55e -r 3bf5c713feb1 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:04:49 2009 -0700 +++ b/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:03 2009 -0700 @@ -628,15 +628,6 @@ return(CMPIStatus){CMPI_RC_OK, NULL}; } -static struct std_indication_handler csi = { - .raise_fn = NULL, - .trigger_fn = trigger_indication, - .activate_fn = ActivateFilter, - .deactivate_fn = DeActivateFilter, - .enable_fn = EnableIndications, - .disable_fn = DisableIndications, -}; - DECLARE_FILTER(xen_created, "Xen_ComputerSystemCreatedIndication"); DECLARE_FILTER(xen_deleted, "Xen_ComputerSystemDeletedIndication"); DECLARE_FILTER(xen_modified, "Xen_ComputerSystemModifiedIndication"); @@ -660,6 +651,14 @@ NULL, }; +static struct std_indication_handler csi = { + .raise_fn = NULL, + .trigger_fn = trigger_indication, + .activate_fn = ActivateFilter, + .deactivate_fn = DeActivateFilter, + .enable_fn = EnableIndications, + .disable_fn = DisableIndications, +}; DEFAULT_IND_CLEANUP(); DEFAULT_AF(); _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1246320305 25200 # Node ID ca56533192e5a5715c66a8d9ebe0d0fedfe02bd3 # Parent 3bf5c713feb1b2bf6094c829e7f423480a05904c Add raise_indication() to CSIndication Currently, the event loop in ComputerSystemIndication gets the XML of the guest when the guest has been modified. However, by the time the event loop is called, the the guest has already been modified. This is because we only generate the indication if the modification is successful. Therefore, we need a function that allows us to raise an indication (instead of trigger in this case). This allows us to set the PreviousInstance attribute of the indication instance we pass to the raise call. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 3bf5c713feb1 -r ca56533192e5 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:03 2009 -0700 +++ b/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:05 2009 -0700 @@ -651,8 +651,124 @@ NULL, }; +static CMPIInstance *get_prev_inst(const CMPIBroker *broker, + const CMPIInstance *ind, + CMPIStatus *s) +{ + CMPIData data; + CMPIInstance *prev_inst = NULL; + + data = CMGetProperty(ind, "PreviousInstance", s); + if (s->rc != CMPI_RC_OK || CMIsNullValue(data)) { + cu_statusf(broker, s, + CMPI_RC_ERR_NO_SUCH_PROPERTY, + "Unable to get PreviousInstance of the indication"); + goto out; + } + + if (data.type != CMPI_instance) { + cu_statusf(broker, s, + CMPI_RC_ERR_TYPE_MISMATCH, + "Indication SourceInstance is of unexpected type"); + goto out; + } + + prev_inst = data.value.inst; + + out: + return prev_inst; +} + +static CMPIStatus raise_indication(const CMPIBroker *broker, + const CMPIContext *ctx, + const CMPIInstance *ind) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *prev_inst; + CMPIInstance *src_inst; + CMPIObjectPath *ref = NULL; + struct std_indication_ctx *_ctx = NULL; + struct ind_args *args = NULL; + char *prefix = NULL; + bool rc; + + if (!lifecycle_enabled) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "CSI not enabled, skipping indication delivery"); + goto out; + } + + prev_inst = get_prev_inst(broker, ind, &s); + if (s.rc != CMPI_RC_OK || CMIsNullObject(prev_inst)) + goto out; + + ref = CMGetObjectPath(prev_inst, &s); + if (s.rc != CMPI_RC_OK) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to get a reference to the guest"); + goto out; + } + + /* FIXME: This is a Pegasus work around. Pegsus loses the namespace + when an ObjectPath is pulled from an instance */ + if (STREQ(NAMESPACE(ref), "")) + CMSetNameSpace(ref, "root/virt"); + + s = get_domain_by_ref(broker, ref, &src_inst); + if (s.rc != CMPI_RC_OK || CMIsNullObject(src_inst)) + goto out; + + _ctx = malloc(sizeof(struct std_indication_ctx)); + if (_ctx == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to allocate indication context"); + goto out; + } + + _ctx->brkr = broker; + _ctx->handler = NULL; + _ctx->filters = filters; + _ctx->enabled = lifecycle_enabled; + + args = malloc(sizeof(struct ind_args)); + if (args == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to allocate ind_args"); + goto out; + } + + args->ns = strdup(NAMESPACE(ref)); + args->classname = strdup(CLASSNAME(ref)); + args->_ctx = _ctx; + + prefix = class_prefix_name(args->classname); + + rc = _do_indication(broker, ctx, prev_inst, src_inst, + CS_MODIFIED, prefix, args); + + if (!rc) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to generate indication"); + } + + out: + if (args != NULL) + stdi_free_ind_args(&args); + + if (_ctx != NULL) + free(_ctx); + + free(prefix); + return s; +} + static struct std_indication_handler csi = { - .raise_fn = NULL, + .raise_fn = raise_indication, .trigger_fn = trigger_indication, .activate_fn = ActivateFilter, .deactivate_fn = DeActivateFilter,
+ + /* FIXME: This is a Pegasus work around. Pegsus loses the namespace + when an ObjectPath is pulled from an instance */ + if (STREQ(NAMESPACE(ref), "")) + CMSetNameSpace(ref, "root/virt"); +
Is it always "root/virt"? Seems like we are hard coding here. -Sharad
Sharad Mishra wrote:
+ + /* FIXME: This is a Pegasus work around. Pegsus loses the namespace + when an ObjectPath is pulled from an instance */ + if (STREQ(NAMESPACE(ref), "")) + CMSetNameSpace(ref, "root/virt"); +
Is it always "root/virt"? Seems like we are hard coding here.
-Sharad
Agreed... this is an unfortunate work around for a Pegasus bug. We should never need to hard code the namespace. The namespace should be embedded in the ObjectPath appropriately. However, Pegasus has a bug in which the namespace is striped from the ObjectPath whenever you pull it from an instance. So until that issue is fixed, we don't have any other way to set the namespace appropriately. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
+1 Sharad Mishra System x Enablement Linux Technology Center IBM Kaitlin Rupert <kaitlin@linux.vn et.ibm.com> To Sent by: libvirt-cim@redhat.com libvirt-cim-bounc cc es@redhat.com Subject [Libvirt-cim] [PATCH 2 of 3] Add 06/29/2009 05:05 raise_indication() to CSIndication PM Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redh at.com> # HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1246320305 25200 # Node ID ca56533192e5a5715c66a8d9ebe0d0fedfe02bd3 # Parent 3bf5c713feb1b2bf6094c829e7f423480a05904c Add raise_indication() to CSIndication Currently, the event loop in ComputerSystemIndication gets the XML of the guest when the guest has been modified. However, by the time the event loop is called, the the guest has already been modified. This is because we only generate the indication if the modification is successful. Therefore, we need a function that allows us to raise an indication (instead of trigger in this case). This allows us to set the PreviousInstance attribute of the indication instance we pass to the raise call. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 3bf5c713feb1 -r ca56533192e5 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:03 2009 -0700 +++ b/src/Virt_ComputerSystemIndication.c Mon Jun 29 17:05:05 2009 -0700 @@ -651,8 +651,124 @@ NULL, }; +static CMPIInstance *get_prev_inst(const CMPIBroker *broker, + const CMPIInstance *ind, + CMPIStatus *s) +{ + CMPIData data; + CMPIInstance *prev_inst = NULL; + + data = CMGetProperty(ind, "PreviousInstance", s); + if (s->rc != CMPI_RC_OK || CMIsNullValue(data)) { + cu_statusf(broker, s, + CMPI_RC_ERR_NO_SUCH_PROPERTY, + "Unable to get PreviousInstance of the indication"); + goto out; + } + + if (data.type != CMPI_instance) { + cu_statusf(broker, s, + CMPI_RC_ERR_TYPE_MISMATCH, + "Indication SourceInstance is of unexpected type"); + goto out; + } + + prev_inst = data.value.inst; + + out: + return prev_inst; +} + +static CMPIStatus raise_indication(const CMPIBroker *broker, + const CMPIContext *ctx, + const CMPIInstance *ind) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *prev_inst; + CMPIInstance *src_inst; + CMPIObjectPath *ref = NULL; + struct std_indication_ctx *_ctx = NULL; + struct ind_args *args = NULL; + char *prefix = NULL; + bool rc; + + if (!lifecycle_enabled) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "CSI not enabled, skipping indication delivery"); + goto out; + } + + prev_inst = get_prev_inst(broker, ind, &s); + if (s.rc != CMPI_RC_OK || CMIsNullObject(prev_inst)) + goto out; + + ref = CMGetObjectPath(prev_inst, &s); + if (s.rc != CMPI_RC_OK) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to get a reference to the guest"); + goto out; + } + + /* FIXME: This is a Pegasus work around. Pegsus loses the namespace + when an ObjectPath is pulled from an instance */ + if (STREQ(NAMESPACE(ref), "")) + CMSetNameSpace(ref, "root/virt"); + + s = get_domain_by_ref(broker, ref, &src_inst); + if (s.rc != CMPI_RC_OK || CMIsNullObject(src_inst)) + goto out; + + _ctx = malloc(sizeof(struct std_indication_ctx)); + if (_ctx == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to allocate indication context"); + goto out; + } + + _ctx->brkr = broker; + _ctx->handler = NULL; + _ctx->filters = filters; + _ctx->enabled = lifecycle_enabled; + + args = malloc(sizeof(struct ind_args)); + if (args == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to allocate ind_args"); + goto out; + } + + args->ns = strdup(NAMESPACE(ref)); + args->classname = strdup(CLASSNAME(ref)); + args->_ctx = _ctx; + + prefix = class_prefix_name(args->classname); + + rc = _do_indication(broker, ctx, prev_inst, src_inst, + CS_MODIFIED, prefix, args); + + if (!rc) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to generate indication"); + } + + out: + if (args != NULL) + stdi_free_ind_args(&args); + + if (_ctx != NULL) + free(_ctx); + + free(prefix); + return s; +} + static struct std_indication_handler csi = { - .raise_fn = NULL, + .raise_fn = raise_indication, .trigger_fn = trigger_indication, .activate_fn = ActivateFilter, .deactivate_fn = DeActivateFilter, _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1245869168 25200 # Node ID 7e1613aaff99b7ff536a9bd0277345bf2d62f4f1 # Parent ca56533192e5a5715c66a8d9ebe0d0fedfe02bd3 When a guest is modified, raise an indication and set the PreviousInstance prop Be sure to get the instance of the guest to modifying the guest. This value is needed when we generate an indication. To test this: Modify a guest and make sure the SourceInstance and PreviousInstance values differ and are correctly set. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r ca56533192e5 -r 7e1613aaff99 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Mon Jun 29 17:05:05 2009 -0700 +++ b/src/Virt_ComputerSystem.c Wed Jun 24 11:46:08 2009 -0700 @@ -680,6 +680,39 @@ DEFAULT_EQ(); DEFAULT_INST_CLEANUP(); +static bool trigger_mod_indication(const CMPIContext *context, + CMPIInstance *prev_inst, + const CMPIObjectPath *ref) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + const char *ind_name = "ComputerSystemModifiedIndication"; + CMPIInstance *ind = NULL; + char *type = NULL; + + CU_DEBUG("Preparing ComputerSystem indication"); + + ind = get_typed_instance(_BROKER, + CLASSNAME(ref), + ind_name, + NAMESPACE(ref)); + if (ind == NULL) { + CU_DEBUG("Failed to create ind '%s'", ind_name); + goto out; + } + + CU_DEBUG("Setting PreviousInstance"); + CMSetProperty(ind, "PreviousInstance", + (CMPIValue *)&prev_inst, CMPI_instance); + + type = get_typed_class(CLASSNAME(ref), ind_name); + + s = stdi_raise_indication(_BROKER, context, type, NAMESPACE(ref), ind); + + out: + free(type); + return s.rc == CMPI_RC_OK; +} + static int xen_scheduler_params(struct infostore_ctx *ctx, virSchedParameter **params) { @@ -1068,10 +1101,12 @@ CMPIArgs *argsout) { CMPIStatus s; + CMPIInstance *prev_inst = NULL; uint16_t state; int ret; const char *name = NULL; uint32_t rc = 1; + bool ind_rc; ret = cu_get_u16_arg(argsin, "RequestedState", &state); if (ret != CMPI_RC_OK) { @@ -1088,22 +1123,24 @@ goto out; } + /* 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, + CMPI_RC_ERR_INVALID_PARAMETER, + "Unable to get instance for guest '%s'", + name); + goto out; + } + s = __state_change(name, state, reference); if (s.rc == CMPI_RC_OK) { - char *type = NULL; - - type = get_typed_class(CLASSNAME(reference), - "ComputerSystemModifiedIndication"); - - /* Failure to raise the indication is okay */ - stdi_trigger_indication(_BROKER, - context, - type, - NAMESPACE(reference)); + ind_rc= trigger_mod_indication(context, prev_inst, reference); + if (!ind_rc) + CU_DEBUG("Unable to trigger indication"); + rc = 0; - - free(type); } out: CMReturnData(results, &rc, CMPI_uint32);
+1 Sharad Mishra System x Enablement Linux Technology Center IBM Kaitlin Rupert <kaitlin@linux.vn et.ibm.com> To Sent by: libvirt-cim@redhat.com libvirt-cim-bounc cc es@redhat.com Subject [Libvirt-cim] [PATCH 3 of 3] When a 06/29/2009 05:05 guest is modified, raise an PM indication and set the PreviousInstance prop Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redh at.com> # HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1245869168 25200 # Node ID 7e1613aaff99b7ff536a9bd0277345bf2d62f4f1 # Parent ca56533192e5a5715c66a8d9ebe0d0fedfe02bd3 When a guest is modified, raise an indication and set the PreviousInstance prop Be sure to get the instance of the guest to modifying the guest. This value is needed when we generate an indication. To test this: Modify a guest and make sure the SourceInstance and PreviousInstance values differ and are correctly set. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r ca56533192e5 -r 7e1613aaff99 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Mon Jun 29 17:05:05 2009 -0700 +++ b/src/Virt_ComputerSystem.c Wed Jun 24 11:46:08 2009 -0700 @@ -680,6 +680,39 @@ DEFAULT_EQ(); DEFAULT_INST_CLEANUP(); +static bool trigger_mod_indication(const CMPIContext *context, + CMPIInstance *prev_inst, + const CMPIObjectPath *ref) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + const char *ind_name = "ComputerSystemModifiedIndication"; + CMPIInstance *ind = NULL; + char *type = NULL; + + CU_DEBUG("Preparing ComputerSystem indication"); + + ind = get_typed_instance(_BROKER, + CLASSNAME(ref), + ind_name, + NAMESPACE(ref)); + if (ind == NULL) { + CU_DEBUG("Failed to create ind '%s'", ind_name); + goto out; + } + + CU_DEBUG("Setting PreviousInstance"); + CMSetProperty(ind, "PreviousInstance", + (CMPIValue *)&prev_inst, CMPI_instance); + + type = get_typed_class(CLASSNAME(ref), ind_name); + + s = stdi_raise_indication(_BROKER, context, type, NAMESPACE(ref), ind); + + out: + free(type); + return s.rc == CMPI_RC_OK; +} + static int xen_scheduler_params(struct infostore_ctx *ctx, virSchedParameter **params) { @@ -1068,10 +1101,12 @@ CMPIArgs *argsout) { CMPIStatus s; + CMPIInstance *prev_inst = NULL; uint16_t state; int ret; const char *name = NULL; uint32_t rc = 1; + bool ind_rc; ret = cu_get_u16_arg(argsin, "RequestedState", &state); if (ret != CMPI_RC_OK) { @@ -1088,22 +1123,24 @@ goto out; } + /* 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, + CMPI_RC_ERR_INVALID_PARAMETER, + "Unable to get instance for guest '%s'", + name); + goto out; + } + s = __state_change(name, state, reference); if (s.rc == CMPI_RC_OK) { - char *type = NULL; - - type = get_typed_class(CLASSNAME(reference), - "ComputerSystemModifiedIndication"); - - /* Failure to raise the indication is okay */ - stdi_trigger_indication(_BROKER, - context, - type, - NAMESPACE(reference)); + ind_rc= trigger_mod_indication(context, prev_inst, reference); + if (!ind_rc) + CU_DEBUG("Unable to trigger indication"); + rc = 0; - - free(type); } out: CMReturnData(results, &rc, CMPI_uint32); _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
participants (2)
-
Kaitlin Rupert -
Sharad Mishra