On Tue, Oct 09, 2018 at 15:48:45 +0200, Michal Privoznik wrote:
Introduce some basic test cases for virFileIsSharedFS(). More
will be added later. In order to achieve desired result, mocks
for setmntent() and statfs() need to be invented because the
first thing that virFileIsSharedFS() does is calling the latter.
If it finds a FUSE mount it'll call the former.
The mock might look a bit complicated, but in fact it's quite
simple. The test sets LIBVIRT_MTAB env variable to hold the
absolute path to a file containing mount table. Then, statfs()
returns matching FS it finds, and setmntent() is there just to
replace /proc/mounts with the file the test wants to load.
Adding this test also exposed a bug we have - because we assume
the given path points to a file we cut off what we assume is a
file name to obtain directory path and only then we call
statfs(). This is buggy because the passed path could be already
a mount point.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/Makefile.am | 7 +-
tests/virfiledata/mounts3.txt | 33 ++++++++
tests/virfilemock.c | 154 ++++++++++++++++++++++++++++++++++
tests/virfiletest.c | 62 +++++++++++++-
4 files changed, 254 insertions(+), 2 deletions(-)
create mode 100644 tests/virfiledata/mounts3.txt
create mode 100644 tests/virfilemock.c
...
diff --git a/tests/virfilemock.c b/tests/virfilemock.c
new file mode 100644
index 0000000000..d458616899
--- /dev/null
+++ b/tests/virfilemock.c
...
+static int
+statfs_mock(const char *mtab,
+ const char *path,
+ struct statfs *buf)
+{
+ FILE *f;
+ struct mntent mb;
+ char mntbuf[1024];
+ int ret = -1;
+
+ if (!(f = real_setmntent(mtab, "r"))) {
+ fprintf(stderr, "Unable to open %s", mtab);
+ abort();
I think just returning -1 would be better. The caller will take care of
reporting the error.
+ }
+
+ while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) {
+ int ftype;
+
+ if (STRNEQ(mb.mnt_dir, path))
+ continue;
I was wondering how this can work as we call statfs on an arbitrary path
rather than on the exact mount point. But it works because
virFileIsSharedFSType calls statfs in a loop checking smaller part of
the path in each iteration. That said, the mocked function is not
generic, but it's good enough for our use case.
...
With the change suggested above
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>