diff mbox series

[for-4.15,1/2] libs/foreignmem: Drop useless and/or misleading logging

Message ID 20210203163750.7564-1-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series [for-4.15,1/2] libs/foreignmem: Drop useless and/or misleading logging | expand

Commit Message

Andrew Cooper Feb. 3, 2021, 4:37 p.m. UTC
These log lines are all in response to single system calls, and do not provide
any information which the immediate caller can't determine themselves.  It is
however exceedinly rude to put junk like this onto stderr, especially as
system call failures are not even error conditions in certain circumstances.

The FreeBSD logging has stale function names in, and solaris shouldn't have
passed code review to start with.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Manuel Bouyer <bouyer@netbsd.org>

The errno constructs in osdep_xenforeignmemory_map_resource() are addressed in
the following patch, to avoid adding complexity to this one.

This reduces the quantity of noise from unit tests, where certain syscall
failures are definitely not errors.
---
 tools/libs/foreignmemory/freebsd.c | 7 ++-----
 tools/libs/foreignmemory/linux.c   | 6 +-----
 tools/libs/foreignmemory/netbsd.c  | 2 --
 tools/libs/foreignmemory/solaris.c | 2 +-
 4 files changed, 4 insertions(+), 13 deletions(-)

Comments

Ian Jackson Feb. 3, 2021, 5:16 p.m. UTC | #1
Andrew Cooper writes ("[PATCH for-4.15 1/2] libs/foreignmem: Drop useless and/or misleading logging"):
> These log lines are all in response to single system calls, and do not provide
> any information which the immediate caller can't determine themselves.  It is
> however exceedinly rude to put junk like this onto stderr, especially as
> system call failures are not even error conditions in certain circumstances.
> 
> The FreeBSD logging has stale function names in, and solaris shouldn't have
> passed code review to start with.
> 
> No functional change.

Thanks.

Reviewed-by: Ian Jackson <iwj@xenproject.org>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>

>          int saved_errno = errno;
> -        PERROR("XXXXXXXX");
> +

That's particularly wtf...

Ian.
Andrew Cooper Feb. 3, 2021, 5:26 p.m. UTC | #2
On 03/02/2021 17:16, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.15 1/2] libs/foreignmem: Drop useless and/or misleading logging"):
>> These log lines are all in response to single system calls, and do not provide
>> any information which the immediate caller can't determine themselves.  It is
>> however exceedinly rude to put junk like this onto stderr, especially as
>> system call failures are not even error conditions in certain circumstances.
>>
>> The FreeBSD logging has stale function names in, and solaris shouldn't have
>> passed code review to start with.
>>
>> No functional change.
> Thanks.
>
> Reviewed-by: Ian Jackson <iwj@xenproject.org>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Thanks,

>
>>          int saved_errno = errno;
>> -        PERROR("XXXXXXXX");
>> +
> That's particularly wtf...

My thoughts exactly.

~Andrew
diff mbox series

Patch

diff --git a/tools/libs/foreignmemory/freebsd.c b/tools/libs/foreignmemory/freebsd.c
index 9a2796f0b7..04bfa806b0 100644
--- a/tools/libs/foreignmemory/freebsd.c
+++ b/tools/libs/foreignmemory/freebsd.c
@@ -65,10 +65,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 
     addr = mmap(addr, num << PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
     if ( addr == MAP_FAILED )
-    {
-        PERROR("xc_map_foreign_bulk: mmap failed");
         return NULL;
-    }
 
     ioctlx.num = num;
     ioctlx.dom = dom;
@@ -80,7 +77,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     if ( rc < 0 )
     {
         int saved_errno = errno;
-        PERROR("xc_map_foreign_bulk: ioctl failed");
+
         (void)munmap(addr, num << PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
@@ -137,7 +134,7 @@  int osdep_xenforeignmemory_map_resource(xenforeignmemory_handle *fmem,
         int saved_errno;
 
         if ( errno != ENOSYS )
-            PERROR("mmap resource ioctl failed");
+            ;
         else
             errno = EOPNOTSUPP;
 
diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index d0eead1196..050b9ed3a5 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -171,10 +171,7 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     addr = mmap(addr, num << PAGE_SHIFT, prot, flags | MAP_SHARED,
                 fd, 0);
     if ( addr == MAP_FAILED )
-    {
-        PERROR("mmap failed");
         return NULL;
-    }
 
     ioctlx.num = num;
     ioctlx.dom = dom;
@@ -273,7 +270,6 @@  void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
 
-        PERROR("ioctl failed");
         (void)munmap(addr, num << PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
@@ -330,7 +326,7 @@  int osdep_xenforeignmemory_map_resource(
         int saved_errno;
 
         if ( errno != fmem->unimpl_errno && errno != EOPNOTSUPP )
-            PERROR("ioctl failed");
+            ;
         else
             errno = EOPNOTSUPP;
 
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index 4ae60aafdd..565682e064 100644
--- a/tools/libs/foreignmemory/netbsd.c
+++ b/tools/libs/foreignmemory/netbsd.c
@@ -147,8 +147,6 @@  int osdep_xenforeignmemory_map_resource(
     rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
     if ( rc )
     {
-        PERROR("ioctl failed");
-
         if ( fres->addr )
         {
             int saved_errno = errno;
diff --git a/tools/libs/foreignmemory/solaris.c b/tools/libs/foreignmemory/solaris.c
index ee8aae4fbd..958fb01f6d 100644
--- a/tools/libs/foreignmemory/solaris.c
+++ b/tools/libs/foreignmemory/solaris.c
@@ -83,7 +83,7 @@  void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
     if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
     {
         int saved_errno = errno;
-        PERROR("XXXXXXXX");
+
         (void)munmap(addr, num*XC_PAGE_SIZE);
         errno = saved_errno;
         return NULL;