[PATCH] Add LibvirtVersion to VSMS
# HG changeset patch # User Jim Fehlig <jfehlig@novell.com> # Date 1249688466 21600 # Node ID cca9b991128c170d03177e33b88e1b5bac930bd6 # Parent 8c9cb3efdbad40890a5408267bc6a0d7b4b3de6e Add LibvirtVersion to VSMS I've had users ask for libvirt version available through libvirt-cim. This patch add LibvirtVersion property to VSMS, which is a proxy for libvirt and seems the most appropriate place to add the property. I consider VSMSC, but that class describes what the associated service is capable of as opposed to version information about the service. Signed-off-by: Jim Fehlig <jfehlig@novell.com> diff -r 8c9cb3efdbad -r cca9b991128c schema/VirtualSystemManagementService.mof --- a/schema/VirtualSystemManagementService.mof Wed Aug 05 14:07:00 2009 -0300 +++ b/schema/VirtualSystemManagementService.mof Fri Aug 07 17:41:06 2009 -0600 @@ -11,6 +11,9 @@ [Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; }; [Provider("cmpi::Virt_VirtualSystemManagementService")] @@ -24,6 +27,9 @@ [Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; }; [Provider("cmpi::Virt_VirtualSystemManagementService")] @@ -37,4 +43,7 @@ [Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; }; diff -r 8c9cb3efdbad -r cca9b991128c src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Aug 05 14:07:00 2009 -0300 +++ b/src/Virt_VirtualSystemManagementService.c Fri Aug 07 17:41:06 2009 -0600 @@ -2418,8 +2418,10 @@ const char *ccname = NULL; virConnectPtr conn = NULL; unsigned long hv_version = 0; + unsigned long lv_version = 0; const char * hv_type = NULL; char *caption = NULL; + char *lv_version_string = NULL; CMPIArray *array; uint16_t op_status; @@ -2483,6 +2485,25 @@ CMSetProperty(inst, "Caption", (CMPIValue *)"Unknown Hypervisor", CMPI_chars); + if (virGetVersion(&lv_version, hv_type, &hv_version) < 0) { + CU_DEBUG("Unable to get libvirt version"); + lv_version= 0; + hv_version= 0; + } + + if (asprintf(&lv_version_string, "%lu.%lu.%lu", + lv_version / 1000000, + (lv_version % 1000000) / 1000, + (lv_version % 1000000) % 1000) == -1) + lv_version_string = NULL; + + if (lv_version_string != NULL) + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)lv_version_string, CMPI_chars); + else + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)"Unknown libvirt", CMPI_chars); + CMSetProperty(inst, "Name", (CMPIValue *)"Management Service", CMPI_chars); @@ -2522,6 +2543,7 @@ ""); out: free(caption); + free(lv_version_string); virConnectClose(conn); *_inst = inst;
+1 On 08/07/2009 08:42 PM, Jim Fehlig wrote:
# HG changeset patch # User Jim Fehlig<jfehlig@novell.com> # Date 1249688466 21600 # Node ID cca9b991128c170d03177e33b88e1b5bac930bd6 # Parent 8c9cb3efdbad40890a5408267bc6a0d7b4b3de6e Add LibvirtVersion to VSMS
I've had users ask for libvirt version available through libvirt-cim. This patch add LibvirtVersion property to VSMS, which is a proxy for libvirt and seems the most appropriate place to add the property.
I consider VSMSC, but that class describes what the associated service is capable of as opposed to version information about the service.
Signed-off-by: Jim Fehlig<jfehlig@novell.com>
diff -r 8c9cb3efdbad -r cca9b991128c schema/VirtualSystemManagementService.mof --- a/schema/VirtualSystemManagementService.mof Wed Aug 05 14:07:00 2009 -0300 +++ b/schema/VirtualSystemManagementService.mof Fri Aug 07 17:41:06 2009 -0600 @@ -11,6 +11,9 @@
[Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; };
[Provider("cmpi::Virt_VirtualSystemManagementService")] @@ -24,6 +27,9 @@
[Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; };
[Provider("cmpi::Virt_VirtualSystemManagementService")] @@ -37,4 +43,7 @@
[Description("Package Version")] string Release; + + [Description("libvirt Version")] + string LibvirtVersion; }; diff -r 8c9cb3efdbad -r cca9b991128c src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Aug 05 14:07:00 2009 -0300 +++ b/src/Virt_VirtualSystemManagementService.c Fri Aug 07 17:41:06 2009 -0600 @@ -2418,8 +2418,10 @@ const char *ccname = NULL; virConnectPtr conn = NULL; unsigned long hv_version = 0; + unsigned long lv_version = 0; const char * hv_type = NULL; char *caption = NULL; + char *lv_version_string = NULL; CMPIArray *array; uint16_t op_status;
@@ -2483,6 +2485,25 @@ CMSetProperty(inst, "Caption", (CMPIValue *)"Unknown Hypervisor", CMPI_chars);
+ if (virGetVersion(&lv_version, hv_type,&hv_version)< 0) { + CU_DEBUG("Unable to get libvirt version"); + lv_version= 0; + hv_version= 0; + } + + if (asprintf(&lv_version_string, "%lu.%lu.%lu", + lv_version / 1000000, + (lv_version % 1000000) / 1000, + (lv_version % 1000000) % 1000) == -1) + lv_version_string = NULL; + + if (lv_version_string != NULL) + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)lv_version_string, CMPI_chars); + else + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)"Unknown libvirt", CMPI_chars); + CMSetProperty(inst, "Name", (CMPIValue *)"Management Service", CMPI_chars);
@@ -2522,6 +2543,7 @@ ""); out: free(caption); + free(lv_version_string); virConnectClose(conn); *_inst = inst;
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com
Thanks Jim! This looks good, I've got a minor change below...
@@ -2483,6 +2485,25 @@ CMSetProperty(inst, "Caption", (CMPIValue *)"Unknown Hypervisor", CMPI_chars);
+ if (virGetVersion(&lv_version, hv_type, &hv_version) < 0) { + CU_DEBUG("Unable to get libvirt version"); + lv_version= 0; + hv_version= 0; + } + + if (asprintf(&lv_version_string, "%lu.%lu.%lu", + lv_version / 1000000, + (lv_version % 1000000) / 1000, + (lv_version % 1000000) % 1000) == -1) + lv_version_string = NULL; + + if (lv_version_string != NULL) + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)lv_version_string, CMPI_chars); + else + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)"Unknown libvirt", CMPI_chars); +
Instead of the if / else here, in the case when lv_version_string, can you assign it the value of "Unknown libvirt". Then you can just call CMSetProperty() with lv_version_string, as lv_version_string shouldn't be NULL at that point. This approach is more inline with the convention followed up the rest of the code. Also, I think "Unknown libvirt version" is a little more clear. Thanks! -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
Kaitlin Rupert wrote:
Thanks Jim! This looks good, I've got a minor change below...
@@ -2483,6 +2485,25 @@ CMSetProperty(inst, "Caption", (CMPIValue *)"Unknown Hypervisor", CMPI_chars);
+ if (virGetVersion(&lv_version, hv_type, &hv_version) < 0) { + CU_DEBUG("Unable to get libvirt version"); + lv_version= 0; + hv_version= 0; + } + + if (asprintf(&lv_version_string, "%lu.%lu.%lu", + lv_version / 1000000, + (lv_version % 1000000) / 1000, + (lv_version % 1000000) % 1000) == -1) + lv_version_string = NULL; + + if (lv_version_string != NULL) + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)lv_version_string, CMPI_chars); + else + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)"Unknown libvirt", CMPI_chars); +
Instead of the if / else here, in the case when lv_version_string, can you assign it the value of "Unknown libvirt". Then you can just call CMSetProperty() with lv_version_string, as lv_version_string shouldn't be NULL at that point. This approach is more inline with the convention followed up the rest of the code.
I was following the convention established by the Caption property, which includes the hypervisor version. If asprintf() fails (most likely because of memory allocation failure), I would need to dup "Unkown libvirt version" in lv_version_string, which would also fail :-). I could assign the static string to another variable, but I don't think that is any better than the current logic.
Also, I think "Unknown libvirt version" is a little more clear.
Agreed. I'll wait for your advice on the above before submitting a follow up patch. Thanks, Jim
Jim Fehlig wrote:
Kaitlin Rupert wrote:
Thanks Jim! This looks good, I've got a minor change below...
@@ -2483,6 +2485,25 @@ CMSetProperty(inst, "Caption", (CMPIValue *)"Unknown Hypervisor", CMPI_chars);
+ if (virGetVersion(&lv_version, hv_type, &hv_version) < 0) { + CU_DEBUG("Unable to get libvirt version"); + lv_version= 0; + hv_version= 0; + } + + if (asprintf(&lv_version_string, "%lu.%lu.%lu", + lv_version / 1000000, + (lv_version % 1000000) / 1000, + (lv_version % 1000000) % 1000) == -1) + lv_version_string = NULL; + + if (lv_version_string != NULL) + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)lv_version_string, CMPI_chars); + else + CMSetProperty(inst, "LibvirtVersion", + (CMPIValue *)"Unknown libvirt", CMPI_chars); + Instead of the if / else here, in the case when lv_version_string, can you assign it the value of "Unknown libvirt". Then you can just call CMSetProperty() with lv_version_string, as lv_version_string shouldn't be NULL at that point. This approach is more inline with the convention followed up the rest of the code.
I was following the convention established by the Caption property,
As, you're right. My mistake. Generally, we do something like: if (str == NULL) str = strdup(val); //Call CMSetProperty() here
which includes the hypervisor version. If asprintf() fails (most likely because of memory allocation failure), I would need to dup "Unkown libvirt version" in lv_version_string, which would also fail :-). I could assign the static string to another variable, but I don't think that is any better than the current logic.
Yes, agreed. It's fine to leave it as is. I missed seeing that Caption was assigned using this approach.
Also, I think "Unknown libvirt version" is a little more clear.
Agreed. I'll wait for your advice on the above before submitting a follow up patch.
Thanks, Jim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Jim Fehlig -
Kaitlin Rupert -
Richard Maciel