Re: [Qemu-devel] [Qemu-block] [PATCH] rbd: make the code better readable

2016-10-14 Thread 李秀波


JohnSnow 写到:
>
>
>On 10/14/2016 05:51 AM, Xiubo Li wrote:
>> Make it a bit clear and better readable.
>>
>
>Suggestion: "Make it clearer and more readable."
>
Yes, see the next version.

>>
>>  if (qemu_rbd_set_auth(cluster, secretid, errp) < 0) {
>>  rados_shutdown(cluster);
>
>Did you mean to remove rados_shutdown() here, too?
>
I will fix this.

>>  ret = rbd_create(io_ctx, name, bytes, _order);
>> -rados_ioctx_destroy(io_ctx);
>> -rados_shutdown(cluster);
>>  if (ret < 0) {
>>  error_setg_errno(errp, -ret, "error rbd create");
>> -return ret;
>>  }
>>
>> +rados_ioctx_destroy(io_ctx);
>> +
>> +failed_shutdown:
>
>Since this executes on the non-error pathway too, I might just call
>this
>'shutdown'.
>
Agree.

Thanks very much.

BRS
Xiubo


>> +rados_shutdown(cluster);
>>  return ret;
>>  }
>>
>>
>
>--
>梛s


Re: [Qemu-devel] [Qemu-block] [PATCH] rbd: make the code better readable

2016-10-14 Thread John Snow



On 10/14/2016 05:51 AM, Xiubo Li wrote:

Make it a bit clear and better readable.



Suggestion: "Make it clearer and more readable."


Signed-off-by: Xiubo Li 
---
 block/rbd.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 0a5840d..de8ca1b 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -366,45 +366,45 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 rados_conf_read_file(cluster, NULL);
 } else if (conf[0] != '\0' &&
qemu_rbd_set_conf(cluster, conf, true, _err) < 0) {
-rados_shutdown(cluster);
 error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto failed_shutdown;
 }

 if (conf[0] != '\0' &&
 qemu_rbd_set_conf(cluster, conf, false, _err) < 0) {
-rados_shutdown(cluster);
 error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto failed_shutdown;
 }

 if (qemu_rbd_set_auth(cluster, secretid, errp) < 0) {
 rados_shutdown(cluster);


Did you mean to remove rados_shutdown() here, too?


-return -EIO;
+ret = -EIO;
+goto failed_shutdown;
 }

 ret = rados_connect(cluster);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error connecting");
-rados_shutdown(cluster);
-return ret;
+goto failed_shutdown;
 }

 ret = rados_ioctx_create(cluster, pool, _ctx);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error opening pool %s", pool);
-rados_shutdown(cluster);
-return ret;
+goto failed_shutdown;
 }

 ret = rbd_create(io_ctx, name, bytes, _order);
-rados_ioctx_destroy(io_ctx);
-rados_shutdown(cluster);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error rbd create");
-return ret;
 }

+rados_ioctx_destroy(io_ctx);
+
+failed_shutdown:


Since this executes on the non-error pathway too, I might just call this 
'shutdown'.



+rados_shutdown(cluster);
 return ret;
 }




--
—js