Re: [PATCH] 9pfs: improve v9fs_open() tracing

[Christian Schoenebeck]

On Monday, December 30, 2024 10:50:59 AM CET Christian Schoenebeck wrote:
> On Monday, December 16, 2024 11:30:09 AM CET Christian Schoenebeck wrote:
> > Improve tracing of 9p 'Topen' request type by showing open() flags as
> > human-readable text.
> > 
> > E.g. trace output:
> > 
> >   v9fs_open tag 0 id 12 fid 2 mode 100352
> > 
> > would become:
> > 
> >   v9fs_open tag=0 id=12 fid=2 mode=100352(RDONLY|NONBLOCK|DIRECTORY|
> >   TMPFILE|NDELAY)
> > 
> > Therefor add a new utility function qemu_open_flags_tostr() that converts
> > numeric open() flags from host's native O_* flag constants to a string
> > presentation.
> > 
> > 9p2000.L and 9p2000.u protocol variants use different numeric 'mode'
> > constants for 'Topen' requests. Instead of writing string conversion code
> > for both protocol variants, use the already existing conversion functions
> > that convert the mode flags from respective protocol constants to host's
> > native open() numeric flag constants and pass that result to the new
> > string conversion function qemu_open_flags_tostr().
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> >  hw/9pfs/9p-util-generic.c | 44 +++++++++++++++++++++++++++++++++++++++
> >  hw/9pfs/9p-util.h         |  6 ++++++
> >  hw/9pfs/9p.c              |  9 +++++++-
> >  hw/9pfs/meson.build       |  1 +
> >  hw/9pfs/trace-events      |  2 +-
> >  5 files changed, 60 insertions(+), 2 deletions(-)
> >  create mode 100644 hw/9pfs/9p-util-generic.c
> 
> Queued on 9p.next:
> https://github.com/cschoenebeck/qemu/commits/9p.next
> 
> Thanks!
> 
> /Christian

I just realized that evaluation of O_SYNC and O_TMPFILE was wrong, as they
might contain several bits.

And O_NDELAY should be filtered if it is exactly the same as O_NONBLOCK.

This should fix these issues:

diff --git a/hw/9pfs/9p-util-generic.c b/hw/9pfs/9p-util-generic.c
index dff9a42d97..4c1e9c887d 100644
--- a/hw/9pfs/9p-util-generic.c
+++ b/hw/9pfs/9p-util-generic.c
@@ -28,15 +28,21 @@ char *qemu_open_flags_tostr(int flags)
         #ifdef O_CLOEXEC
         (flags & O_CLOEXEC) ? "|CLOEXEC" : "",
         #endif
-        (flags & O_SYNC) ? "|SYNC" : "",
+        #ifdef __O_SYNC
+        (flags & __O_SYNC) ? "|SYNC" : "",
+        #else
+        ((flags & O_SYNC) == O_SYNC) ? "|SYNC" : "",
+        #endif
         #ifdef O_PATH
         (flags & O_PATH) ? "|PATH" : "",
         #endif
-        #ifdef O_TMPFILE
-        (flags & O_TMPFILE) ? "|TMPFILE" : "",
+        #ifdef __O_TMPFILE
+        (flags & __O_TMPFILE) ? "|TMPFILE" : "",
+        #elif defined(O_TMPFILE)
+        ((flags & O_TMPFILE) == O_TMPFILE) ? "|TMPFILE" : "",
         #endif
         /* O_NDELAY is usually just an alias of O_NONBLOCK */
-        #ifdef O_NDELAY
+        #if defined(O_NDELAY) && O_NDELAY != O_NONBLOCK
         (flags & O_NDELAY) ? "|NDELAY" : "",
         #endif
         NULL /* always last (required NULL termination) */

I post a V2 of this patch now.

Best regards,
Christian Schoenebeck