[PATCH] RFC: Use of root/interop instead of root/PG_InterOp

As a result of an issue I recently discovered where the most recent update to tog-pegasus on my Fedora 19 box, I have found a set of changes that will will allow my Fedora system to operate as before (mostly). The following is a link to the original discovery - there's 5 followups that are worth reading too. https://www.redhat.com/archives/libvirt-cim/2013-November/msg00008.html I'm not 100% sure the changes contained in these patches will work for all the cases, hence the request for comments as opposed to let's get this in ASAP. There may also be some extraneous adjustments - I was hoping that I could get the make install environment work, but I'm not sure I got everything right there either. At the very least the 'make rpm' will build an rpm which I can install and use. These changes do work for my Fedora 19 environment with tog-pegasus 2.12.1-8 installed. One innocent bystander in all this is the FilterList. It happened to use 'InstanceID' from the base/parent class CIM_ManagedElement. If I've read the description correctly here: http://schemas.dmtf.org/wbem/cim-html/2.34.0/CIM_ManagedElement.html The usage of InstanceID for libvirt-cim would need to go through some sort of renaming within the MOF, but it wasn't really clear "how" that would be accomplished. As an alternative, I created an InstanceUUID property which does work for me, although I'm not sure it's the right solution. The downside to doing this is that the cimtest (and I'm sure any other libvirt-cim client) requires a change to grab the FilterList data from the new different property. This creates an incompatibility situation which I'm not in favor of. Hopefully someone knows the right incantation to tell the FilterList.mof that we're going to use InstanceID --- Makefile.am | 29 ++++++++++++++++++++++++----- libvirt-cim.spec.in | 27 ++++++++++++++++++++++----- provider-register.sh | 18 +++++++++++++++++- schema/FilterList.mof | 4 ++++ src/Virt_FilterList.c | 2 +- 5 files changed, 68 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index 9e8e96b..69b65cf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -78,6 +78,9 @@ INTEROP_MOFS = \ $(top_srcdir)/schema/ReferencedProfile.mof \ $(top_srcdir)/schema/AllocationCapabilities.mof +# The PGINTEROP_MOFS are used by tog-pegasus up through version 2.12.1 +# If support for versions prior to 2.12.1 is removed, then these defs +# can go away PGINTEROP_MOFS = \ $(top_srcdir)/schema/RegisteredProfile.mof \ $(top_srcdir)/schema/ElementConformsToProfile.mof \ @@ -157,6 +160,9 @@ INTEROP_REGS = \ $(top_srcdir)/schema/ElementConformsToProfile.registration \ $(top_srcdir)/schema/ReferencedProfile.registration +# The PGINTEROP_REGS are used by tog-pegasus up through version 2.12.1 +# If support for versions prior to 2.12.1 is removed, then these defs +# can go away PGINTEROP_REGS = \ $(top_srcdir)/schema/RegisteredProfile.registration \ $(top_srcdir)/schema/ElementConformsToProfile.registration \ @@ -181,7 +187,8 @@ EXTRA_DIST = schema $(MOFS) $(REGS) $(INTEROP_MOFS) $(INTEROP_REGS) \ .changeset .revision \ examples/diskpool.conf -# If Pegasus isn't the CIMOM target, then remove the PG_InterOp namespace from the appropriate files +# If Pegasus isn't the CIMOM target, then remove the PG_InterOp namespace +# from the appropriate files install-data-local: $(mkinstalldirs) "$(DESTDIR)$(pkgdatadir)" $(install_sh_DATA) -t "$(DESTDIR)$(pkgdatadir)" $(MOFS) @@ -189,11 +196,12 @@ install-data-local: $(install_sh_DATA) -t "$(DESTDIR)$(pkgdatadir)" $(INTEROP_MOFS) $(install_sh_DATA) -t "$(DESTDIR)$(pkgdatadir)" $(INTEROP_REGS) if [[ @CIMSERVER@ != pegasus ]]; then \ - sed -i '/^# --/,/^# --!/d' $(subst $(top_srcdir)/schema,$(DESTDIR)$(pkgdatadir), $(PGINTEROP_REGS)); \ + sed -i '/^# --/,/^# --!/d' $(subst $(top_srcdir)/schema,$(DESTDIR)$(pkgdatadir), $(PGINTEROP_REGS)); \ + sed -i '/^# --/,/^# --!/d' $(subst $(top_srcdir)/schema,$(DESTDIR)$(pkgdatadir), $(PGINTEROP_MOFS)); \ fi uninstall-local: - @list='$(MOFS) $(REGS) $(INTEROP_MOFS) $(INTEROP_REGS)'; \ + @list='$(MOFS) $(REGS) $(INTEROP_MOFS) $(INTEROP_REGS) $(PGINTEROP_REGS) $(PGINTEROP_MOFS)'; \ for p in $$list; do \ f=`echo "$$p" | sed 's|^.*/||;'`; \ echo " rm -f '$(DESTDIR)$(pkgdatadir)/$$f'"; \ @@ -209,8 +217,19 @@ postinstall: $(SHELL) provider-register.sh -v -t @CIMSERVER@ -n @CIM_VIRT_NS@ -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(MOFS)) $(SHELL) provider-register.sh -v -t @CIMSERVER@ -n root/interop -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(INTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(INTEROP_MOFS)) $(SHELL) provider-register.sh -v -t @CIMSERVER@ -n root/cimv2 -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(CIMV2_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(CIMV2_MOFS)) + # + # We need to check the version - if we're not yet at 2.12.1, then + # we'll register at root/PG_InterOp; otherwise, using just the above + # registration should be sufficient. The actual cutoff root/PG_InterOp + # not being valid was 2.12.1-5; however, --version doesn't give us that + # level of detail. The Pegasus docs imply that usage of root/interop was + # valid as of 2.12.0. + # if [[ @CIMSERVER@ = pegasus ]]; then \ - $(SHELL) provider-register.sh -v -t @CIMSERVER@ -n root/PG_InterOp -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_MOFS)); \ + CIMVER=`@CIMSERVER@ --version | awk -F. '{printf("%02d%02d%02d\n", $1,$2,$3); }'` \ + if [[ $CIMVER -lt 021201 ]]; then \ + $(SHELL) provider-register.sh -v -t @CIMSERVER@ -n root/PG_InterOp -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_MOFS)); \ + fi \ fi virsh -v | grep -q '^0.3' && cp examples/diskpool.conf $(DISK_POOL_CONFIG) || true mkdir -p $(INFO_STORE) @@ -220,7 +239,7 @@ preuninstall: $(SHELL) provider-register.sh -v -d -t @CIMSERVER@ -n root/interop -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(INTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(INTEROP_MOFS)) $(SHELL) provider-register.sh -v -d -t @CIMSERVER@ -n root/cimv2 -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(CIMV2_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(CIMV2_MOFS)) if [[ @CIMSERVER@ = pegasus ]]; then \ - $(SHELL) provider-register.sh -v -d -t @CIMSERVER@ -n root/PG_InterOp -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_MOFS)); \ + $(SHELL) provider-register.sh -v -d -t @CIMSERVER@ -n root/PG_InterOp -r $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_REGS)) -m $(subst $(top_srcdir)/schema,$(pkgdatadir), $(PGINTEROP_MOFS)); \ fi rpm: clean diff --git a/libvirt-cim.spec.in b/libvirt-cim.spec.in index c96451b..0eab663 100644 --- a/libvirt-cim.spec.in +++ b/libvirt-cim.spec.in @@ -199,6 +199,10 @@ rm -fr $RPM_BUILD_ROOT %{_datadir}/%{name}/ReferencedProfile.mof \\\ %{_datadir}/%{name}/AllocationCapabilities.mof +# NOTE: As of Pegasus 2.12.1-5, using root/PG_InterOp will no longer be +# valid. All mofs can just compile into root/interop. However, we +# need to keep these here for 'historical purposes'. +# %define PGINTEROP_REG %{_datadir}/%{name}/RegisteredProfile.registration \\\ %{_datadir}/%{name}/ElementConformsToProfile.registration \\\ %{_datadir}/%{name}/ReferencedProfile.registration @@ -267,12 +271,12 @@ fi %if 0%{?fedora} >= 17 || 0%{?rhel} >= 7 if [ "`systemctl is-active tog-pegasus.service 2> /dev/null`" = "active" ] then - systemctl restart tog-pegasus.service + systemctl restart tog-pegasus.service > /dev/null 2>&1 fi if [ "`systemctl is-active sblim-sfcb.service 2> /dev/null`" = "active" ] then - systemctl restart sblim-sfcb.service + systemctl restart sblim-sfcb.service > /dev/null 2>&1 fi %else /etc/init.d/tog-pegasus condrestart @@ -286,9 +290,22 @@ then %{_datadir}/%{name}/provider-register.sh -t pegasus \ -n root/interop \ -r %{INTEROP_REG} -m %{INTEROP_MOF} -v >/dev/null 2>&1 || true - %{_datadir}/%{name}/provider-register.sh -t pegasus \ - -n root/PG_InterOp \ - -r %{PGINTEROP_REG} -m %{PGINTEROP_MOF} -v >/dev/null 2>&1 || true + # + # We need to check the version - if we're not yet at 2.12.1, then + # we'll register at root/PG_InterOp; otherwise, using just the above + # registration should be sufficient. The actual cutoff root/PG_InterOp + # not being valid was 2.12.1-5; however, --version doesn't give us that + # level of detail. The Pegasus docs imply that usage of root/interop was + # valid as of 2.12.0. + # + CIMVER=`/usr/sbin/cimserver --version | \ + awk -F. '{printf("%02d%02d%02d\n", $1,$2,$3); }'` + if [ $CIMVER -lt 021201 ] + then + %{_datadir}/%{name}/provider-register.sh -t pegasus \ + -n root/PG_InterOp \ + -r %{PGINTEROP_REG} -m %{PGINTEROP_MOF} -v >/dev/null 2>&1 || true + fi %{_datadir}/%{name}/provider-register.sh -t pegasus \ -n root/cimv2\ -r %{CIMV2_REG} -m %{CIMV2_MOF} -v >/dev/null 2>&1 || true diff --git a/provider-register.sh b/provider-register.sh index abe8e95..f66fe54 100755 --- a/provider-register.sh +++ b/provider-register.sh @@ -274,7 +274,23 @@ pegasus_install() chatter Registering providers with $state cimserver '('$version')' chatter Installing mofs into namespace $namespace from path $mofpath $CIMMOF -uc -I $mofpath -n $namespace $mymofs && - $CIMMOF -uc -n root/PG_Interop $_REGFILENAME + # + # If compare_version returns false here (e.g. $version is less than + # "2.12.1", then we will compile into root/PG_InterOp; otherwise, + # compile into root/interop. As of 2.12.1-5 using the PG_InterOp + # will fail. Since we cannot get that level of detail out of the + # --version output, "assume" that 2.12.1 -> 2.12.1-4 will be able + # to use the new namespace. The Pegasus docs imply as of 2.12.0 using + # root/interop was preferred. + # + if compare_version "$version" "2.12.1" + then + chatter Installing $_REGFILENAME into root/PG_InterOp + $CIMMOF -uc -n root/PG_Interop $_REGFILENAME + else + chatter Installing $_REGFILENAME into root/interop + $CIMMOF -uc -n root/interop $_REGFILENAME + fi else echo "Failed to build pegasus registration MOF." >&2 return 1 diff --git a/schema/FilterList.mof b/schema/FilterList.mof index 7339db6..b7d8551 100644 --- a/schema/FilterList.mof +++ b/schema/FilterList.mof @@ -10,4 +10,8 @@ class KVM_FilterList : CIM_FilterList MinValue(-1000), MaxValue(1000)] sint16 Priority = 500; + + [Description("The network filter UUID value. This value was formerly " + "stored in the InstanceID field.")] + string InstanceUUID; }; diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c index b248004..6a9112e 100644 --- a/src/Virt_FilterList.c +++ b/src/Virt_FilterList.c @@ -76,7 +76,7 @@ static CMPIInstance *convert_filter_to_instance( CMSetProperty(inst, "SystemName", sys_name, CMPI_chars); CMSetProperty(inst, "SystemCreationClassName", sys_ccname, CMPI_chars); CMSetProperty(inst, "Name", (CMPIValue *)filter->name, CMPI_chars); - CMSetProperty(inst, "InstanceID", (CMPIValue *)filter->uuid, + CMSetProperty(inst, "InstanceUUID", (CMPIValue *)filter->uuid, CMPI_chars); CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16); -- 1.8.3.1

On 11/21/2013 09:03 PM, John Ferlan wrote: ...
diff --git a/schema/FilterList.mof b/schema/FilterList.mof index 7339db6..b7d8551 100644 --- a/schema/FilterList.mof +++ b/schema/FilterList.mof @@ -10,4 +10,8 @@ class KVM_FilterList : CIM_FilterList MinValue(-1000), MaxValue(1000)] sint16 Priority = 500; + + [Description("The network filter UUID value. This value was formerly " + "stored in the InstanceID field.")] + string InstanceUUID; }; diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c index b248004..6a9112e 100644 --- a/src/Virt_FilterList.c +++ b/src/Virt_FilterList.c @@ -76,7 +76,7 @@ static CMPIInstance *convert_filter_to_instance( CMSetProperty(inst, "SystemName", sys_name, CMPI_chars); CMSetProperty(inst, "SystemCreationClassName", sys_ccname, CMPI_chars); CMSetProperty(inst, "Name", (CMPIValue *)filter->name, CMPI_chars); - CMSetProperty(inst, "InstanceID", (CMPIValue *)filter->uuid, + CMSetProperty(inst, "InstanceUUID", (CMPIValue *)filter->uuid, CMPI_chars); CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16);
Hi John, the changes regarding the FilterList seem strange to me. I would not expect any schema changes. Can you elaborate a bit more why these changes are needed, what made you create these changes and how could one recreate a possible error which these changes fix? -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 11/26/2013 07:40 AM, Boris Fiuczynski wrote:
On 11/21/2013 09:03 PM, John Ferlan wrote: ...
diff --git a/schema/FilterList.mof b/schema/FilterList.mof index 7339db6..b7d8551 100644 --- a/schema/FilterList.mof +++ b/schema/FilterList.mof @@ -10,4 +10,8 @@ class KVM_FilterList : CIM_FilterList MinValue(-1000), MaxValue(1000)] sint16 Priority = 500; + + [Description("The network filter UUID value. This value was formerly " + "stored in the InstanceID field.")] + string InstanceUUID; }; diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c index b248004..6a9112e 100644 --- a/src/Virt_FilterList.c +++ b/src/Virt_FilterList.c @@ -76,7 +76,7 @@ static CMPIInstance *convert_filter_to_instance( CMSetProperty(inst, "SystemName", sys_name, CMPI_chars); CMSetProperty(inst, "SystemCreationClassName", sys_ccname, CMPI_chars); CMSetProperty(inst, "Name", (CMPIValue *)filter->name, CMPI_chars); - CMSetProperty(inst, "InstanceID", (CMPIValue *)filter->uuid, + CMSetProperty(inst, "InstanceUUID", (CMPIValue *)filter->uuid, CMPI_chars); CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16);
Hi John, the changes regarding the FilterList seem strange to me. I would not expect any schema changes. Can you elaborate a bit more why these changes are needed, what made you create these changes and how could one recreate a possible error which these changes fix?
BTW: I had a very bad disk crash in my main work environment - I've lost everything... It's going to take me a few days to recover... Unfortunately for me the backups I thought were occurring - weren't... The following is from memory and some notes I have written down... It seems the newer environment doesn't allow setting the "InstanceID" property. That property is a member of CIM_ManagedElement and the link I pointed at in my RFC describes the field. I'm assuming that since we've moved to a more common "root/interop" environment and away from the (more private) "root/PG_InterOp" environment that somehow there needs to be a way to more uniquely identify which provider (namespace) is adjusting that value. Reading that webpage description really didn't help me figure out what the magic incantation might need to be. So as an alternative I created the new property and thus why I called this an RFC because - quite frankly I wasn't sure how to handle it. Note that in the new environment "wbemcli ei http://root:password@localhost/root/virt:KVM_FilterList" failed to show the InstanceID parameter (assuming of course you've applied the rest of the changes to use "root/interop")... Because InstanceID doesn't exist the cimtest for FilterList will fail when accessing 'InstanceID' in the python dictionary/list (damn, I wish I'd sent that one too - now I have to recreate - sigh) John
participants (2)
-
Boris Fiuczynski
-
John Ferlan