[PATCH 0 of 2] [CU] Migration Indication related libcmpi changes
These provide libcmpiutil support copying instances and raising indications, which are required by the new MigrationIndication provider.
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1201795147 18000 # Node ID ec7428b062f45db8eef730c5398d061d8731ce60 # Parent ab0e3b05a10e7b5db7eb493920f01bb402d2feae [CU] Add dup_instance function to libcmpiutil It appears there are some situations where we have to duplicate an instance; this seemed like the kind of thing that would belong in libcmpiutil. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r ab0e3b05a10e -r ec7428b062f4 instance_util.c --- a/instance_util.c Wed Jan 30 11:15:32 2008 -0800 +++ b/instance_util.c Thu Jan 31 10:59:07 2008 -0500 @@ -204,6 +204,28 @@ CMPIStatus cu_copy_prop(const CMPIBroker return s; } +CMPIStatus cu_dup_instance(const CMPIBroker *broker, + CMPIInstance *src, + CMPIInstance **dest) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIObjectPath *ref; + + ref = CMGetObjectPath(src, NULL); + if ((s.rc != CMPI_RC_OK) || CMIsNullObject(ref)) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Could not get objectpath from instance"); + goto out; + } + + *dest = NULL; + *dest = CMNewInstance(broker, ref, &s); + + out: + return s; +} + /* * Local Variables: * mode: C diff -r ab0e3b05a10e -r ec7428b062f4 libcmpiutil.h --- a/libcmpiutil.h Wed Jan 30 11:15:32 2008 -0800 +++ b/libcmpiutil.h Thu Jan 31 10:59:07 2008 -0500 @@ -164,6 +164,18 @@ CMPIrc cu_get_u16_path(const CMPIObjectP CMPIrc cu_get_u16_path(const CMPIObjectPath *reference, const char *key, uint16_t *target); + +/** + * Create a copy of an instance + * + * @param src Source instance + * @param dest Destination instance + * @returns {CMPI_RC_OK, NULL} if success, CMPI_RC ERR_FAILED and + * error message otherwise + */ +CMPIStatus cu_dup_instance(const CMPIBroker *broker, + CMPIInstance *src, + CMPIInstance **dest); /* Forward declaration */ struct inst_list;
JG> +CMPIStatus cu_dup_instance(const CMPIBroker *broker, JG> + CMPIInstance *src, JG> + CMPIInstance **dest) JG> +{ JG> + CMPIStatus s = {CMPI_RC_OK, NULL}; JG> + CMPIObjectPath *ref; JG> + JG> + ref = CMGetObjectPath(src, NULL); JG> + if ((s.rc != CMPI_RC_OK) || CMIsNullObject(ref)) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Could not get objectpath from instance"); JG> + goto out; JG> + } JG> + JG> + *dest = NULL; Why set it to NULL and then set it to the instance right after this? JG> + *dest = CMNewInstance(broker, ref, &s); JG> + JG> + out: JG> + return s; JG> +} What about all the properties of the instance? This gets a new instance structure, yes, but all it has is an object path. Maybe that is all you need at the moment, but I don't think this does what other consumers of the library would think by the name :) It should be easy to loop through all the properties and copy them over. It will look similar to the array copy/filter code in Kaitlin's libcmpiutil CO patch. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> +CMPIStatus cu_dup_instance(const CMPIBroker *broker, JG> + CMPIInstance *src, JG> + CMPIInstance **dest) JG> +{ JG> + CMPIStatus s = {CMPI_RC_OK, NULL}; JG> + CMPIObjectPath *ref; JG> + JG> + ref = CMGetObjectPath(src, NULL); JG> + if ((s.rc != CMPI_RC_OK) || CMIsNullObject(ref)) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Could not get objectpath from instance"); JG> + goto out; JG> + } JG> + JG> + *dest = NULL;
Why set it to NULL and then set it to the instance right after this?
I think I was going with "not sure what *dest becomes if NewInstance" fails, and so I figured better safe than sorry. I'd rather have it NULL than undefined.
JG> + *dest = CMNewInstance(broker, ref, &s); JG> + JG> + out: JG> + return s; JG> +}
What about all the properties of the instance? This gets a new instance structure, yes, but all it has is an object path. Maybe that is all you need at the moment, but I don't think this does what other consumers of the library would think by the name :)
It should be easy to loop through all the properties and copy them over. It will look similar to the array copy/filter code in Kaitlin's libcmpiutil CO patch.
I'm pleading CIM ignorance on this one. I thought the ObjectPath encompassed the properties, and thought that my testing showed it happening (I don't manually move the JobState over but it is est), but I could have been wrong. -- -Jay
JG> I'm pleading CIM ignorance on this one. I thought the ObjectPath JG> encompassed the properties, and thought that my testing showed it JG> happening (I don't manually move the JobState over but it is est), JG> but I could have been wrong. You're only half ignorant here :) The ObjectPath has the key properties, but not the rest. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> I'm pleading CIM ignorance on this one. I thought the ObjectPath JG> encompassed the properties, and thought that my testing showed it JG> happening (I don't manually move the JobState over but it is est), JG> but I could have been wrong.
You're only half ignorant here :)
The ObjectPath has the key properties, but not the rest.
But it can also be, that some of the none-key properties contain a value. This happens, if within the DMTF's mof files a default value was defined. Might this be for JobState ? -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
Heidi Eckhart wrote:
Dan Smith wrote:
JG> I'm pleading CIM ignorance on this one. I thought the ObjectPath JG> encompassed the properties, and thought that my testing showed it JG> happening (I don't manually move the JobState over but it is est), JG> but I could have been wrong.
You're only half ignorant here :)
The ObjectPath has the key properties, but not the rest.
But it can also be, that some of the none-key properties contain a value. This happens, if within the DMTF's mof files a default value was defined. Might this be for JobState ?
Right on the money. I was looking for something that didn't appear to copy correctly, in order to confirm the fix when I'm done working on cu_dup_instance, and JobState was the first one I found. -- -Jay
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1201882988 18000 # Node ID 8669586635358e4858e56fe2dec8587af784cd47 # Parent ec7428b062f45db8eef730c5398d061d8731ce60 [CU] Improve std_indication's raise functionality Turns out STDI_IndicationMIStub was not creating the CMPIFooMIFTs correctly; this patch fixes that. It also adds a default_raise function to std_indication. This is for indication providers that don't need to do anything other than call CBDeliverIndication, which will most likely be the majority. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r ec7428b062f4 -r 866958663535 std_indication.c --- a/std_indication.c Thu Jan 31 10:59:07 2008 -0500 +++ b/std_indication.c Fri Feb 01 11:23:08 2008 -0500 @@ -19,6 +19,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #include <string.h> +#include <stdbool.h> #include <cmpidt.h> #include <cmpift.h> @@ -40,19 +41,40 @@ static CMPIStatus trigger(struct std_ind return ctx->handler->trigger_fn(context); } +static CMPIStatus default_raise(const CMPIBroker *broker, + const CMPIContext *context, + CMPIInstance *ind) +{ + CMPIObjectPath *ref; + CMPIStatus s = {CMPI_RC_OK, NULL}; + + ref = CMGetObjectPath(ind, NULL); + + CBDeliverIndication(broker, + context, + NAMESPACE(ref), + ind); + return s; +} + static CMPIStatus raise(struct std_indication_ctx *ctx, const CMPIContext *context, const CMPIArgs *argsin) { CMPIInstance *inst; - if (ctx->handler->raise_fn == NULL) - return (CMPIStatus){CMPI_RC_OK, NULL}; + if (!ctx->enabled) { + CU_DEBUG("Indication disabled, not raising."); + return (CMPIStatus) {CMPI_RC_OK, NULL}; + } if (cu_get_inst_arg(argsin, "Indication", &inst) != CMPI_RC_OK) return (CMPIStatus){CMPI_RC_ERR_FAILED, NULL}; - return ctx->handler->raise_fn(context, inst); + if (ctx->handler->raise_fn == NULL) + return default_raise(ctx->brkr, context, inst); + + return ctx->handler->raise_fn(ctx->brkr, context, inst); } CMPIStatus stdi_handler(CMPIMethodMI *self, diff -r ec7428b062f4 -r 866958663535 std_indication.h --- a/std_indication.h Thu Jan 31 10:59:07 2008 -0500 +++ b/std_indication.h Fri Feb 01 11:23:08 2008 -0500 @@ -24,6 +24,7 @@ #include <cmpidt.h> #include <cmpift.h> #include <cmpimacs.h> +#include <stdio.h> #include "libcmpiutil.h" #include "std_invokemethod.h" @@ -51,7 +52,8 @@ CMPIStatus stdi_cleanup(CMPIMethodMI *se const CMPIContext *context, CMPIBoolean terminating); -typedef CMPIStatus (*raise_indication_t)(const CMPIContext *ctx, +typedef CMPIStatus (*raise_indication_t)(const CMPIBroker *broker, + const CMPIContext *ctx, const CMPIInstance *ind); typedef CMPIStatus (*trigger_indication_t)(const CMPIContext *ctx); @@ -64,14 +66,44 @@ struct std_indication_ctx { struct std_indication_ctx { const CMPIBroker *brkr; struct std_indication_handler *handler; + bool enabled; }; #define STDI_IndicationMIStub(pfx, pn, _broker, hook, _handler) \ + static struct std_indication_ctx _ctx = { \ + .brkr = NULL, \ + .handler = _handler, \ + .enabled = false, \ + }; \ + \ + static CMPIIndicationMIFT indMIFT__ = { \ + CMPICurrentVersion, \ + CMPICurrentVersion, \ + "Indication" #pn, \ + pfx##IndicationCleanup, \ + pfx##AuthorizeFilter, \ + pfx##MustPoll, \ + pfx##ActivateFilter, \ + pfx##DeActivateFilter, \ + CMIndicationMIStubExtensions(pfx) \ + }; \ CMPIIndicationMI * \ pn##_Create_IndicationMI(const CMPIBroker *, \ - const CMPIContext *, \ - CMPIStatus *); \ - CMIndicationMIStub(pfx, pn, _broker, hook); \ + const CMPIContext *, \ + CMPIStatus *); \ + CMPIIndicationMI * \ + pn##_Create_IndicationMI(const CMPIBroker *brkr, \ + const CMPIContext *ctx, \ + CMPIStatus *rc) { \ + static CMPIIndicationMI mi = { \ + &_ctx, \ + &indMIFT__, \ + }; \ + _ctx.brkr = brkr; \ + _broker = brkr; \ + hook; \ + return &mi; \ + } \ \ static CMPIMethodMIFT methMIFT__ = { \ CMPICurrentVersion, \ @@ -88,13 +120,11 @@ struct std_indication_ctx { CMPIMethodMI *pn##_Create_MethodMI(const CMPIBroker *brkr, \ const CMPIContext *ctx, \ CMPIStatus *rc) { \ - static struct std_indication_ctx _ctx; \ static CMPIMethodMI mi = { \ &_ctx, \ &methMIFT__, \ }; \ _ctx.brkr = brkr; \ - _ctx.handler = _handler; \ _broker = brkr; \ hook; \ return &mi; \
JG> # HG changeset patch JG> # User Jay Gagnon <grendel@linux.vnet.ibm.com> JG> # Date 1201882988 18000 JG> # Node ID 8669586635358e4858e56fe2dec8587af784cd47 JG> # Parent ec7428b062f45db8eef730c5398d061d8731ce60 JG> [CU] Improve std_indication's raise functionality This one looks okay to me, and I think I can apply it independently of the other, correct? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
JG> # HG changeset patch JG> # User Jay Gagnon <grendel@linux.vnet.ibm.com> JG> # Date 1201882988 18000 JG> # Node ID 8669586635358e4858e56fe2dec8587af784cd47 JG> # Parent ec7428b062f45db8eef730c5398d061d8731ce60 JG> [CU] Improve std_indication's raise functionality
This one looks okay to me, and I think I can apply it independently of the other, correct?
------------------------------------------------------------------------
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim Yea, this should be able to go in independently. Migration is the only indication that uses raise, so we won't be pulling the rug out from under anybody.
-- -Jay
This set looks good. +1 -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (4)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert