[sheepdog] [libvirt] [Patch]Fix bugs of Sheepdog storage driver

[Osier Yang]

[-- Private only --]

I got this after replying to your patch:

尊敬的jyang at redhat.com：

此邮件旨在通知您：您尝试联系的网上论坛（“cloudxy”）可能并不存在，或者您 
没有向该网上论坛发帖的权限。以下是有关您无法发帖的详细原因：

  * 您可能未正确拼写该网上论坛的名称，或该网上论坛名称的格式错误。
  * 论坛版主可能已删除该网上论坛。
  * 您可能需要先加入该网上论坛才能获得发帖权限。
  * 该网上论坛可能没有开启发帖权限。

如果您有关于此网上论坛或其他任何 Google 网上论坛的问题，请访问帮助中心， 
网址为 http://groups.google.com/support/?hl=zh-CN_US。

此致

Google Groups小组敬上

That's bad if one could get one notice like this each time after
reviewing your patch. :-) Not sure how to get rid of this, but
I think it's caused by the permission, so please either change
the google group to public, or don't cc to the list when posting
patch.

Osier

On 2013年02月08日 13:00, Osier Yang wrote:
> On 2013年02月08日 12:33, harryxiyou at gmail.com wrote:
>
> You sent same patch multiple times. It might be caused by the mail
> delay though, I.E, you don't see the one sent out earlier. Perhaps
> you should check you mailbox setting to avoid the small flood.
>
> This one is the best, as it's sent out using git send-email, and has
> commit message.
>
>> virStorageBackendSheepdogCreateVol calls virCommandRun whihout checking
>> the return value,
>
> This is not problem, as virCommandRun returns either 0 or -1, which is
> the same result virStorageBackendSheepdogCreateVol expects.
>
> which this return value determines us whether we should
>> call virStorageBackendSheepdogRefreshVol func. And we should also give a
>> return value to virStorageBackendSheepdogRefreshVol func because it may
>> cause errors.
>
> This is a problem indeed.
>
> Following patch can fix these bugs. This patch have checked
>> by 'make syntax-check'.
>
> So, consider to change the commit log to:
>
> Don't try to refresh the volume if creating volume fails.
>
> Which is more clear I think.
>
>>
>> Signed-off-by: Harry Wei<harryxiyou at gmail.com>
>> ---
>> src/storage/storage_backend_sheepdog.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_sheepdog.c
>> b/src/storage/storage_backend_sheepdog.c
>> index cd18f33..f987604 100644
>> --- a/src/storage/storage_backend_sheepdog.c
>> +++ b/src/storage/storage_backend_sheepdog.c
>> @@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr
>> conn ATTRIBUTE_UNUSED,
>> virCommandAddArgFormat(cmd, "%llu", vol->capacity);
>> virStorageBackendSheepdogAddHostArg(cmd, pool);
>> ret = virCommandRun(cmd, NULL);
>> + if (ret< 0)
>> + goto cleanup;
>
> We prefer:
>
> if ((ret = virCommandRun(cmd, NULL)) < 0)
> goto cleanup;
>
> However, you can avoid using the "ret" here by initialize it
> to "-1", with that you can simply do:
>
> if (virCommandRun(cmd, NULL) < 0)
> goto cleanup;
>
>>
>> - virStorageBackendSheepdogRefreshVol(conn, pool, vol);
>> + ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol);
>
> And:
>
> if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
> goto cleanup;
>
> ret = 0;
>
>>
>> +cleanup:
>> virCommandFree(cmd);
>> return ret;
>> }
>
> Osier
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list