Re: [libvirt] [PATCH 2/2] virpcimock: Mock __open_2()

2019-08-15 Thread Ján Tomko

On Thu, Aug 15, 2019 at 05:15:57PM +0200, Michal Privoznik wrote:

Hold on to your hat, this is going to be a wild ride. As nearly
nothing in glic, nor open() is a real function. Just look into


s/glic/glibc/


bits/fcntl2.h and you'll see that open() is actually a thin
wrapper that calls either __open_alias() or __open_2(). Now,
before 801ebb5edb6 the open() done in
virPCIDeviceConfigOpenInternal() had a constant oflags (we were
opening the pci config with O_RDWR). And since we were not
passing any mode nor O_CREAT the wrapper decided to call
__open_alias() which was open() provided by our mock. So far so
good. But after the referenced commit, the oflags is no longer
compile time constant and therefore the wrapper calls __open_2()
which we don't mock and thus the real __open_2() from glibc was
called and thus we did try to open real path from host's /sys.
This of course fails with variety of errors.

Signed-off-by: Michal Privoznik 
---
tests/virpcimock.c | 29 +
1 file changed, 29 insertions(+)



Thanks for tracking this down!

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] virpcimock: Mock __open_2()

2019-08-15 Thread Erik Skultety
On Thu, Aug 15, 2019 at 05:15:57PM +0200, Michal Privoznik wrote:
> Hold on to your hat, this is going to be a wild ride. As nearly
> nothing in glic, nor open() is a real function. Just look into
> bits/fcntl2.h and you'll see that open() is actually a thin
> wrapper that calls either __open_alias() or __open_2(). Now,
> before 801ebb5edb6 the open() done in
> virPCIDeviceConfigOpenInternal() had a constant oflags (we were
> opening the pci config with O_RDWR). And since we were not
> passing any mode nor O_CREAT the wrapper decided to call
> __open_alias() which was open() provided by our mock. So far so
> good. But after the referenced commit, the oflags is no longer
> compile time constant and therefore the wrapper calls __open_2()
> which we don't mock and thus the real __open_2() from glibc was
> called and thus we did try to open real path from host's /sys.
> This of course fails with variety of errors.
>
> Signed-off-by: Michal Privoznik 
> ---

Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/2] virpcimock: Mock __open_2()

2019-08-15 Thread Michal Privoznik
Hold on to your hat, this is going to be a wild ride. As nearly
nothing in glic, nor open() is a real function. Just look into
bits/fcntl2.h and you'll see that open() is actually a thin
wrapper that calls either __open_alias() or __open_2(). Now,
before 801ebb5edb6 the open() done in
virPCIDeviceConfigOpenInternal() had a constant oflags (we were
opening the pci config with O_RDWR). And since we were not
passing any mode nor O_CREAT the wrapper decided to call
__open_alias() which was open() provided by our mock. So far so
good. But after the referenced commit, the oflags is no longer
compile time constant and therefore the wrapper calls __open_2()
which we don't mock and thus the real __open_2() from glibc was
called and thus we did try to open real path from host's /sys.
This of course fails with variety of errors.

Signed-off-by: Michal Privoznik 
---
 tests/virpcimock.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index beb5e1490d..829d61cd3f 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -32,6 +32,7 @@
 
 static int (*real_access)(const char *path, int mode);
 static int (*real_open)(const char *path, int flags, ...);
+static int (*real___open_2)(const char *path, int flags);
 static int (*real_close)(int fd);
 static DIR * (*real_opendir)(const char *name);
 static char *(*real_virFileCanonicalizePath)(const char *path);
@@ -805,6 +806,7 @@ init_syms(void)
 
 VIR_MOCK_REAL_INIT(access);
 VIR_MOCK_REAL_INIT(open);
+VIR_MOCK_REAL_INIT(__open_2);
 VIR_MOCK_REAL_INIT(close);
 VIR_MOCK_REAL_INIT(opendir);
 VIR_MOCK_REAL_INIT(virFileCanonicalizePath);
@@ -932,6 +934,33 @@ open(const char *path, int flags, ...)
 return ret;
 }
 
+
+int
+__open_2(const char *path, int flags)
+{
+VIR_AUTOFREE(char *) newpath = NULL;
+int ret;
+
+init_syms();
+
+if (STRPREFIX(path, SYSFS_PCI_PREFIX) &&
+getrealpath(&newpath, path) < 0)
+return -1;
+
+ret = real___open_2(newpath ? newpath : path, flags);
+
+/* Catch both: /sys/bus/pci/drivers/... and
+ * /sys/bus/pci/device/.../driver/... */
+if (ret >= 0 && STRPREFIX(path, SYSFS_PCI_PREFIX) &&
+strstr(path, "driver") && add_fd(ret, path) < 0) {
+real_close(ret);
+ret = -1;
+}
+
+return ret;
+}
+
+
 DIR *
 opendir(const char *path)
 {
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list