[libvirt] [PATCH] storage: Fix bug of fs pool destroying

Peter Krempa pkrempa at redhat.com
Wed Nov 21 13:20:52 UTC 2012


On 11/21/12 04:22, Osier Yang wrote:
> Regression introduced by commit 258e06c85b7, "ret" could be set to 1
> or 0 by virStorageBackendFileSystemIsMounted before goto cleanup.
> This could mislead the callers (up to the public API
> virStoragePoolDestroy) to return success even the underlying umount
> command fails.
> ---
>   src/storage/storage_backend_fs.c |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 10daee3..cbcbe34 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -449,6 +449,7 @@ static int
>   virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) {
>       virCommandPtr cmd = NULL;
>       int ret = -1;
> +    int rc;
>
>       if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
>           if (pool->def->source.nhost != 1) {
> @@ -475,8 +476,8 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) {
>       }
>
>       /* Short-circuit if already unmounted */
> -    if ((ret = virStorageBackendFileSystemIsMounted(pool)) != 1) {
> -        if (ret < 0)
> +    if ((rc = virStorageBackendFileSystemIsMounted(pool)) != 1) {
> +        if (rc < 0)

This if statement is redundant and could be replaced with
	return rc;


>               return -1;
>           else
>               return 0;

ACK with that change.

Peter




More information about the libvir-list mailing list