Re: [PATCH v3] storage_backend_rbd: check the return value of rados_conf_set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





At 2016-11-24 01:29:45, "John Ferlan" <jferlan@xxxxxxxxxx> wrote:
>
>
>On 11/12/2016 04:22 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao <chenhanxiao@xxxxxxxxx>
>> 
>> We forget to check the return value of some rados_conf_set.
>> 
>> Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxx>
>> ---
>> v2: add another missing return value check
>> v3: fix a copy-paste error
>> 
>>  src/storage/storage_backend_rbd.c | 28 ++++++++++++++++++++++++----
>>  1 file changed, 24 insertions(+), 4 deletions(-)
>> 
>
>Take one more step... Rather than replicating the same message in so
>many different places, why not create a helper
>
>static int
>virStorageBackendRBDRADOSConfSet(rados_t cluster,
>                                 const char *option,
>                                 const char *value)
>{
>    VIR_DEBUG("Setting RADOS option '%s' to '%s'",
>              option, value);
>    if (rados_conf_set(cluster, option, value) < 0) {
>        virReportError(VIR_ERR_INTERNAL_ERROR,
>                       _("failed to set RADOS option: %s"),
>                       option);
>        return -1;
>    }
>    return 0;
>}
>
>
>And all those other places just have:
>
>    if (virStorageBackendRBDRADOSConfSet(ptr->cluster, arg2, arg3) < 0)
>        goto cleanup;
>
>And be sure also remove the VIR_DEBUG("Found cephx key: %s", rados_key);
>
>Also, try to not have lines longer than 80 cols... makes it tough to
>read the code.
>

Thanks for the review.
I'll send a new patch soon.

Regards,
- Chen

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux