[Libvirt-cim] [PATCH V2 0/3] libvirt-cim patches

[John Ferlan]

On 05/02/2013 03:10 AM, Xu Wang wrote:
> V2 updates:
>     1. adjust comments for force use qemu (disable kvm under nested kvm) 
>     2. fix the problem of cdrom (with blank or null disk) VSMS patch caused
>        in V1
>     3. fix syntax errors for comments
>     4. status return updates 
>     5. add description about 8 maximum items limits about vsi support output 
>   
> 
> Wenchao Xia (1):
>   make force use qemu configurable
> 
> Xu Wang (2):
>   VSMS: tip error for invalid disk resource
>   make lldptool command and support output configurable
> 
>  libvirt-cim.conf                          |   26 ++++++++++
>  libxkutil/misc_util.c                     |   26 ++++++++++
>  libxkutil/misc_util.h                     |    3 +
>  src/Virt_SwitchService.c                  |   66 ++++++++++++++++++++----
>  src/Virt_VirtualSystemManagementService.c |   77 ++++++++++++++++++++++-------
>  5 files changed, 168 insertions(+), 30 deletions(-)
> 
> 

Very strange - this is the 2nd time patch 3/3 didn't make it to my email
even though it's on the list.  In fact my reply nor either of your
responses to my reply made it to the mail server. It's as if it has some
magic word or setting so that the mail server refuses to download it.

In any case, I'm still not convinced there's not a memory leak, but
perhaps a Coverity or Valgrind run would answer that. I'll try to set up
Coverity run on the sources.

Another consideration - perhaps the get_lldptool_query_options() and
get_vsi_support_key_string() should return a char * of the strdup()'d
prop.value string.  You may also want to consider putting the defaults
into those functions too to hide them from the caller.

For example, instead of:

+const char *get_lldptool_query_options(void)
+{
+        static LibvirtcimConfigProperty prop = {
+                          "lldptool_query_options", CONFIG_STRING, {0}, 0};
+
+        libvirt_cim_config_get(&prop);
+        return prop.value_string;
+}
+


go with

char *get_lldptool_query_options(void)
{
        static LibvirtcimConfigProperty prop = {
                      "lldptool_query_options", CONFIG_STRING, {0}, 0};

        libvirt_cim_config_get(&prop);
        if (prop.value_string)
                return strdup(prop.value_string);

        return strdup("-t -g ncb -V evbcfg");
}

The caller checks for NULL return, errors appropriately and can free()
the returned string.

John