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

[Osier Yang]

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