[libvirt] [PATCH 3/3] storage: Fix checking whether source filesystem is mounted

[John Ferlan]

On 02/07/2017 09:16 AM, Erik Skultety wrote:
> Right now, we use simple string comparison both on the source paths
> (mount's output vs pool's source) and the target (mount's mnt_dir vs
> pool's target). The problem are symlinks and mount indeed returns
> symlinks in its output, e.g. /dev/mappper/lvm_symlink. The same goes for
> the pool's source/target, so in order to successfully compare these two
> replace plain string comparison with virFileComparePaths which will
> resolve all symlinks and canonicalize the paths prior to comparison.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1417203
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/storage/storage_backend_fs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index fe4705b..fae1c03 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -301,6 +301,7 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
>      FILE *mtab;
>      struct mntent ent;
>      char buf[1024];
> +    int rc1, rc2;
>  
>      if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) {
>          virReportSystemError(errno,
> @@ -313,8 +314,15 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
>          if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
>              goto cleanup;
>  
> -        if (STREQ(ent.mnt_dir, pool->def->target.path) &&
> -            STREQ(ent.mnt_fsname, src)) {
> +        /* compare both mount destinations and sources to be sure the mounted
> +         * FS pool is really the one we're looking for
> +         */
> +        if ((rc1 = virFileComparePaths(ent.mnt_dir,
> +                                       pool->def->target.path)) < 0 ||
> +            (rc2 = virFileComparePaths(ent.mnt_fsname, src)) < 0)
> +            goto cleanup;
> +
> +        if (rc1 == 0 && rc2 == 0) {

With changes to previous to return 1 on match, then this gets adjusted
too...

ACK - I think you can alter this appropriately

John
>              ret = 1;
>              goto cleanup;
>          }
>