At 2016-11-24 01:29:45, "John Ferlan" <jferlan@redhat.com> wrote:
On 11/12/2016 04:22 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
We forget to check the return value of some rados_conf_set.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- 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