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

[WangXu]

----------------------------------------
> Date: Thu, 9 May 2013 09:03:20 -0400
> From: jferlan at redhat.com
> To: libvirt-cim at redhat.com
> Subject: Re: [Libvirt-cim] [PATCH V2 0/3] libvirt-cim patches
>
> 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.
I'll Cc you in every mail:-)
>
> 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.
It's a static variable so every caller use the same memory space. It needn't
released by me and shouldn't be freed because other caller may use it too. 
>
> 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.
Yes, it maybe a feasible way to deal with this command. I discussed with Wenchao
for our design ideas and at last we decided to use the old code. The reason is that
we think get_xxx method just include fetch value of pointed keywords string and return
it to the caller. It wouldn't contain any logic processing. We'll keep the coding style.
>
> John
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim