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