[PATCH] [RFC] Changes required to work with new std_indication. The trigger changes were very light, which was nice
10:39 p.m.
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1202420324 18000
# Node ID a82dd87a3830eadda678b88bf4bfcfd4f8cb1ca4
# Parent c7dd4a8358a187a3469c3d8a177950625898a227
[RFC] Changes required to work with new std_indication. The trigger changes were very
light, which was nice.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r c7dd4a8358a1 -r a82dd87a3830 src/Virt_ComputerSystemIndication.c
--- a/src/Virt_ComputerSystemIndication.c Wed Feb 06 07:00:00 2008 -0800
+++ b/src/Virt_ComputerSystemIndication.c Thu Feb 07 16:38:44 2008 -0500
@@ -488,7 +488,8 @@ STDI_IndicationMIStub(,
Virt_ComputerSystemIndication,
_BROKER,
libvirt_cim_init(),
- &csi);
+ &csi,
+ NULL);
/*
* Local Variables:
diff -r c7dd4a8358a1 -r a82dd87a3830 src/Virt_ComputerSystemMigrationIndication.c
--- a/src/Virt_ComputerSystemMigrationIndication.c Wed Feb 06 07:00:00 2008 -0800
+++ b/src/Virt_ComputerSystemMigrationIndication.c Thu Feb 07 16:38:44 2008 -0500
@@ -58,7 +58,13 @@ static CMPIStatus ActivateFilter(CMPIInd
CMPIBoolean first)
{
CMPIStatus s = {CMPI_RC_OK, NULL};
+ struct std_indication_ctx *_ctx;
+ _ctx = (struct std_indication_ctx *)mi->hdl;
+ s = stdi_set_ind_state_prop(_ctx,
+ "Xen_ComputerSystemMigrationIndication",
+ STDI_PROP_FILTER,
+ true);
return s;
}
@@ -69,16 +75,31 @@ static CMPIStatus DeActivateFilter(CMPII
const CMPIObjectPath* op,
CMPIBoolean last)
{
- return (CMPIStatus){CMPI_RC_OK, NULL};
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ struct std_indication_ctx *_ctx;
+
+ _ctx = (struct std_indication_ctx *)mi->hdl;
+ s = stdi_set_ind_state_prop(_ctx,
+ "Xen_ComputerSystemMigrationIndication",
+ STDI_PROP_FILTER,
+ true);
+ return s;
}
static _EI_RTYPE EnableIndications(CMPIIndicationMI* mi,
const CMPIContext *ctx)
{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct std_indication_ctx *_ctx;
_ctx = (struct std_indication_ctx *)mi->hdl;
- _ctx->enabled = true;
- CU_DEBUG("ComputerSystemModifiedIndication enabled");
+
+ CU_DEBUG("New enable. Cross your fingers");
+ s = stdi_set_ind_state_prop(_ctx,
+ "Xen_ComputerSystemMigrationIndication",
+ STDI_PROP_ENABLE,
+ true);
+
+ CU_DEBUG("ComputerSystemMigrationIndication enabled");
_EI_RET();
}
@@ -86,10 +107,16 @@ static _EI_RTYPE DisableIndications(CMPI
static _EI_RTYPE DisableIndications(CMPIIndicationMI* mi,
const CMPIContext *ctx)
{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct std_indication_ctx *_ctx;
_ctx = (struct std_indication_ctx *)mi->hdl;
- _ctx->enabled = false;
- CU_DEBUG("ComputerSystemModifiedIndication disabled");
+
+ s = stdi_set_ind_state_prop(_ctx,
+ "Xen_ComputerSystemMigrationIndication",
+ STDI_PROP_ENABLE,
+ false);
+
+ CU_DEBUG("ComputerSystemMigrationIndication disabled");
_EI_RET();
}
@@ -97,6 +124,17 @@ static struct std_indication_handler csi
static struct std_indication_handler csi = {
.raise_fn = NULL,
.trigger_fn = NULL,
+};
+
+static struct std_ind_state migrate = {
+ .ind_name = "Xen_ComputerSystemMigrationIndication",
+ .filter_active = false,
+ .enabled = false,
+};
+
+static struct std_ind_state *states[] = {
+ &migrate,
+ NULL,
};
DEFAULT_IND_CLEANUP();
@@ -107,7 +145,8 @@ STDI_IndicationMIStub(,
Virt_ComputerSystemMigrationIndication,
_BROKER,
libvirt_cim_init(),
- &csi);
+ &csi,
+ states);
/*
* Local Variables:
11:30 a.m.
Jay Gagnon wrote:
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1202420324 18000
# Node ID a82dd87a3830eadda678b88bf4bfcfd4f8cb1ca4
# Parent c7dd4a8358a187a3469c3d8a177950625898a227
[RFC] Changes required to work with new std_indication. The trigger changes were very
light, which was nice.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r c7dd4a8358a1 -r a82dd87a3830 src/Virt_ComputerSystemIndication.c
--- a/src/Virt_ComputerSystemIndication.c Wed Feb 06 07:00:00 2008 -0800
+++ b/src/Virt_ComputerSystemIndication.c Thu Feb 07 16:38:44 2008 -0500
@@ -488,7 +488,8 @@ STDI_IndicationMIStub(,
Virt_ComputerSystemIndication,
_BROKER,
libvirt_cim_init(),
- &csi);
+ &csi,
+ NULL);
/*
* Local Variables:
I'm interested in the other changes that are necessary to the
CSIndication provider to make use of the std_indication reorg - also
make use of the states and not setting it to NULL ;).
diff -r c7dd4a8358a1 -r a82dd87a3830
src/Virt_ComputerSystemMigrationIndication.c
--- a/src/Virt_ComputerSystemMigrationIndication.c Wed Feb 06 07:00:00 2008 -0800
+++ b/src/Virt_ComputerSystemMigrationIndication.c Thu Feb 07 16:38:44 2008 -0500
If I got this right, then the MigrationIndication provider is currently
only switching the migrate.filter_activate and migrate.enabled values.
Potentially it could deliver an indication (by the default_raise
function), but it can not as the trigger_fn is not implemented and so
nothing is monitored, right ?
Well, in sum - I like the approach :) - but I would even go farther. I
think its possible to write one indication engine (that would become
part of libcmpiutil) that handles all the CIM_InstModification,
CIM_InstCreation and CIM_InstDeletion indications at once. This engine
would be a large part of the CSIndication provider and maybe only the
compare functions (dom_in_list, dom_changed) need to stay provider
specific. But that's a guess at the moment and is worth to be evaluated
(now I hope that you volunteer for this job ;) ).
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
3:59 p.m.
Heidi Eckhart wrote:
Jay Gagnon wrote:
> # HG changeset patch
> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
> # Date 1202420324 18000
> # Node ID a82dd87a3830eadda678b88bf4bfcfd4f8cb1ca4
> # Parent c7dd4a8358a187a3469c3d8a177950625898a227
> [RFC] Changes required to work with new std_indication. The trigger
> changes were very light, which was nice.
>
> Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
>
> diff -r c7dd4a8358a1 -r a82dd87a3830 src/Virt_ComputerSystemIndication.c
> --- a/src/Virt_ComputerSystemIndication.c Wed Feb 06 07:00:00 2008
> -0800
> +++ b/src/Virt_ComputerSystemIndication.c Thu Feb 07 16:38:44 2008
> -0500
> @@ -488,7 +488,8 @@ STDI_IndicationMIStub(,
> Virt_ComputerSystemIndication,
> _BROKER,
> libvirt_cim_init(), - &csi);
> + &csi,
> + NULL);
>
> /*
> * Local Variables:
>
I'm interested in the other changes that are necessary to the
CSIndication provider to make use of the std_indication reorg - also
make use of the states and not setting it to NULL ;).
Haha yea it's just set to
NULL here in the interest of using the minimal
amount of change to not break it. When I have everything else in place
I'll make sure CSIndication fully utilizes the new code.
> diff -r c7dd4a8358a1 -r a82dd87a3830
> src/Virt_ComputerSystemMigrationIndication.c
> --- a/src/Virt_ComputerSystemMigrationIndication.c Wed Feb 06
> 07:00:00 2008 -0800
> +++ b/src/Virt_ComputerSystemMigrationIndication.c Thu Feb 07
> 16:38:44 2008 -0500
>
If I got this right, then the MigrationIndication provider is
currently only switching the migrate.filter_activate and
migrate.enabled values. Potentially it could deliver an indication (by
the default_raise function), but it can not as the trigger_fn is not
implemented and so nothing is monitored, right ?
That is correct, and that's
actually the intended behavior. Attempting
to monitor out of band migrations is not a problem I'm terribly
interested in solving at the moment, and the raise functionality is tied
into by VSMigrationService in a previous patch. The end result is that
whenever our providers are asked to do a migration task,
VSMigrationService uses MigrationIndication's raise call to send the
indication.
Well, in sum - I like the approach :) - but I would even go farther. I
think its possible to write one indication engine (that would become
part of libcmpiutil) that handles all the CIM_InstModification,
CIM_InstCreation and CIM_InstDeletion indications at once. This engine
would be a large part of the CSIndication provider and maybe only the
compare functions (dom_in_list, dom_changed) need to stay provider
specific. But that's a guess at the moment and is worth to be
evaluated (now I hope that you volunteer for this job ;) ).
Yes, this will definitely go further. This is basically phase one of three.
--
-Jay
4:06 p.m.
I agree with Heidi. I think you've opened the door to take it a
little further so that the calls in Enable/Disable and
Activate/Deactivate are automated for the simple case. Maybe this is
already planned?
JG> +static struct std_ind_state migrate = {
JG> + .ind_name = "Xen_ComputerSystemMigrationIndication",
JG> + .filter_active = false,
JG> + .enabled = false,
JG> +};
This is a really minor thing, but this definition has some fields that
must always be set to false on initialization to avoid badness. Now,
since these are static, they are all initialized to zero, which should
cover you in the case that someone just doesn't set filter_active and
enabled. However, perhaps setting the precedent with something like a
constructor macro would be appropriate?
#define DECLARE_IND(ident, name) \
static struct std_ind_state ident = { \
.ind_name = name, \
.filter_active = false, \
.enabled = false, \
};
Which would make the above definition look like this:
DECLARE_IND(migrate, "Xen_ComputerSystemMigrationIndication");
Thoughts?
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
4:35 p.m.
Dan Smith wrote:
I agree with Heidi. I think you've opened the door to take it a
little further so that the calls in Enable/Disable and
Activate/Deactivate are automated for the simple case. Maybe this is
already planned?
Yup, those are phase two.
JG> +static struct std_ind_state migrate = {
JG> + .ind_name = "Xen_ComputerSystemMigrationIndication",
JG> + .filter_active = false,
JG> + .enabled = false,
JG> +};
This is a really minor thing, but this definition has some fields that
must always be set to false on initialization to avoid badness. Now,
since these are static, they are all initialized to zero, which should
cover you in the case that someone just doesn't set filter_active and
enabled. However, perhaps setting the precedent with something like a
constructor macro would be appropriate?
#define DECLARE_IND(ident, name) \
static struct std_ind_state ident = { \
.ind_name = name, \
.filter_active = false, \
.enabled = false, \
};
Which would make the above definition look like this:
DECLARE_IND(migrate, "Xen_ComputerSystemMigrationIndication");
Thoughts?
Ooh, I like that. I'll definitely use it.
--
-Jay
6068
days inactive
6069
days old
4 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon