Re: [libvirt] [PATCH 3/3] storage: Fix checking whether source filesystem is mounted
On Thu, Feb 09, 2017 at 06:05:41PM -0500, John Ferlan wrote: > > > 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> > --- > > 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 Sure, thanks. Erik > > John > > ret = 1; > > goto cleanup; > > } > > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] storage: Fix checking whether source filesystem is mounted
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> --- > 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; > } > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] storage: Fix checking whether source filesystem is mounted
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--- 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) { ret = 1; goto cleanup; } -- 2.10.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list