On 2013年02月08日 13:57, Osier Yang wrote:
On 2013年02月08日 13:42, harryxiyou wrote:
On Fri, Feb 8, 2013 at 1:06 PM, Osier Yang<jyang@redhat.com> wrote: [...]
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.
Thanks, it's because i add "cloudxy@googlegroups.com" ML to the CC'ed list.
Yes, see:
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.
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.
Sorry, i am not familiar with 'git send-email --compose' well. So it sends several times. I will fix this problem.
[...]
So, consider to change the commit log to:
Don't try to refresh the volume if creating volume fails.
This one is well for me, thanks.
Signed-off-by: Harry Wei<harryxiyou@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; }
We cannot do like
No true.
[...] if (virCommandRun(cmd, NULL)< 0) goto cleanup; [...] if (virStorageBackendSheepdogRefreshVol(conn, pool, vol)< 0) goto cleanup; [...] +cleanup: virCommandFree(cmd); return ret;
We must initialize "ret",
That's why I said in previous replyment "initialize ret", I mean this (see the top of the function):
int ret = -1;
And with setting "ret" to 0 before cleanup:
ret = 0; cleanup: virCommandFree(cmd); return ret;
Btw, if you look further more on virStorageBackendSheepdogRefreshVol, you will see it has bugs too. It can be another patch though.
because if either virCommandRun and
virStorageBackendSheepdogRefreshVol has some errors, they will return '-1' to "ret". Then execute "return ret", which who calls virStorageBackendSheepdogCreateVol will know it has happened some errors. So we should modify them like this.
[...]
if ((ret = virCommandRun(cmd, NULL))< 0) goto cleanup; [...] if (ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol))< 0) goto cleanup;
cleanup: virCommandFree(cmd); return ret; }
Osier Yang, do you agree with me?
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list