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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
