[Libvirt-cim] [PATCH] Add LibvirtVersion to VSMS

Kaitlin Rupert kaitlin at linux.vnet.ibm.com
Mon Aug 10 20:56:31 UTC 2009


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 at linux.vnet.ibm.com




More information about the Libvirt-cim mailing list