Hi,
On 06-08-17 16:15, Knut St. Osmundsen wrote:
Hi Hans,
if I understand you correctly you suggest converting VBox status codes
(currently around 2000) to linux errno.h (a few hundreds) in ring-0 and
have ring-3 convert the errno.h back to VBox status codes.
Yes that is what I propose.
Because the potential information loss, that's not something I would like to do.
The problem is that positive return values being error codes is really
REALLY unusual for an ioctl, so I would really like to clean this up, as
part of my work to mainline the vboxguest driver. Leaving this as is
will like get a lot of push back from the mainline kernel community.
Most vboxguest userspace code is already prepared to deal with errno-s instead
of VBox status codes (there are already paths returning those), the only
exception being the libGL code which calls ioctl directly instead of
calling it through vbglR3DoIOCtl and there actually is a TODO comment in
the GL code to move to vbglR3DoIOCtl. So what my changes do is simply make
things consistent and always return an errno.
As for the information loss, all userspace does with the error codes in 99% of
the case is log them and when debugging we can also get to the ring0 status
codes if we need them and I've already made sure that the few codes which does
have special handling in userspace survive being translated to and from an
errno.
So I urge you to please reconsider my patches, as I definitely don't want
to diverge from what you are doing, but I also really want to fix the
positive return code is an error thing.
Ideally, I'd like each I/O control to have a return field for the VBox
status code, like we do in VBoxDrv on the host side. However, that's
quite a bit of work to implement at this point, thus the return code fun.
Right, I would prefer not to do this, note that HGCM_CALL which seems to
be one of the most used ioctls already does this.
Regards,
Hans
On 2017-08-01 5:19 PM, Hans de Goede wrote:
Returning VERR_* style vbox-runtime codes as positive values is an ugly
hack and unlike any other ioctl call on Linux, breaking standard
expectations of ioctls.
All Guest Additions user-space code calls the ioctl through the
vbglR3DoIOCtl wrapper, which calls on RTErrConvertFromErrno on negative
return values. The one exception is the src/VBox/GuestHost/OpenGL/util
code, which gets fixed in this commit to deal with standard errno errors.
All Guest Additions user-space code has been audited to check that any
error codes requiring special handling survive being translated to an
errno and back again by using RTErrConvertToErrno + RTErrConvertFromErrno.
This means that we can return all errors as standard -errno values using
RTErrConvertToErrno and vbglR3DoIOCtl will convert them back to VERR_*
style vbox-runtime codes with any errors requiring special handling
surviving as is.
This commit removes the positive VERR_* style vbox-runtime codes
and makes vgdrvLinuxIOCtl always return standard Linux errno values.
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c | 7 ++++++-
src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp | 4 +++-
src/VBox/GuestHost/OpenGL/util/vboxhgcm.c | 2 ++
src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c | 2 ++
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
index 1ba3ba6..d48eb9e 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
@@ -818,7 +818,12 @@ static int vgdrvLinuxIOCtl(struct inode *pInode, struct
file *pFilp, unsigned in
else
{
Log(("vgdrvLinuxIOCtl: pFilp=%p uCmd=%#x ulArg=%p failed,
rc=%d\n", pFilp, uCmd, (void *)ulArg, rc));
- rc = -rc; Assert(rc > 0); /* Positive returns == negated VBox
error status codes. */
+ /* Userspace code relies on VERR_* style vbox-runtime codes,
+ * vbglR3DoIOCtl will convert them back to VERR_* style
+ * vbox-runtime codes. This means that any VERR_* codes which
+ * require special handling must survive the to and from errno
+ * conversion as is. */
+ rc = -RTErrConvertToErrno(rc);
}
}
else
diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
index 28da83d..61b1c00 100644
--- a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
+++ b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
@@ -433,7 +433,9 @@ int vbglR3DoIOCtl(unsigned iFunction, void *pvData, size_t
cbData)
if (RT_LIKELY(rc == 0))
return VINF_SUCCESS;
- /* Positive values are negated VBox error status codes. */
+ /* Older kernel guest drivers use to return (negated) VBox error status
+ * codes as positive values, for now we still handle this case in case
+ * we end up running with an older guest driver (20170730) */
if (rc > 0)
rc = -rc;
else
diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
index fae2f22..813f3ec 100644
--- a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
+++ b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
@@ -650,6 +650,8 @@ static int crVBoxHGCMCall(CRConnection *conn, void *pvData,
unsigned cbData)
return VINF_SUCCESS;
}
# ifdef RT_OS_LINUX
+ if (rc < 0)
+ rc = -RTErrConvertFromErrno(errno);
if (rc >= 0) /* positive values are negated VBox error status codes. */
{
crWarning("vboxCall failed with VBox status code %d\n", -rc);
diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
index 7032381..d9c4818 100644
--- a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
+++ b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
@@ -277,6 +277,8 @@ static int crVBoxHGCMCall(void *pvData, unsigned cbData)
return VINF_SUCCESS;
}
# ifdef RT_OS_LINUX
+ if (rc < 0)
+ rc = -RTErrConvertFromErrno(errno);
if (rc >= 0) /* positive values are negated VBox error status codes. */
{
crWarning("vboxCall failed with VBox status code %d\n", -rc);
_______________________________________________
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev
