Re: [PATCH v3 07/17] hw/9pfs: Support getting current directory offset for Windows

[Christian Schoenebeck]

On Monday, December 19, 2022 11:20:11 AM CET Bin Meng wrote:
> From: Guohuai Shi <guohuai.shi@windriver.com>
> 
> On Windows 'struct dirent' does not have current directory offset.
> Update qemu_dirent_off() to support Windows.
> 
> While we are here, add a build time check to error out if a new
> host does not implement this helper.
> 
> Signed-off-by: Guohuai Shi <guohuai.shi@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
> (no changes since v1)
> 
>  hw/9pfs/9p-util.h       | 11 ++++++++---
>  hw/9pfs/9p-util-win32.c |  7 +++++++
>  hw/9pfs/9p.c            |  4 ++--
>  hw/9pfs/codir.c         |  2 +-
>  4 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 90420a7578..e395936b30 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -127,6 +127,7 @@ int unlinkat_win32(int dirfd, const char *pathname, int 
> flags);
>  int statfs_win32(const char *root_path, struct statfs *stbuf);
>  int openat_dir(int dirfd, const char *name);
>  int openat_file(int dirfd, const char *name, int flags, mode_t mode);
> +off_t qemu_dirent_off_win32(void *s, void *fs);
>  #endif
>  
>  static inline void close_preserve_errno(int fd)
> @@ -200,12 +201,16 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char 
> *filename,
>   * so ensure it is manually injected earlier and call here when
>   * needed.
>   */
> -static inline off_t qemu_dirent_off(struct dirent *dent)
> +static inline off_t qemu_dirent_off(struct dirent *dent, void *s, void *fs)
>  {

Not sure why you chose void* here.

> -#ifdef CONFIG_DARWIN
> +#if defined(CONFIG_DARWIN)
>      return dent->d_seekoff;
> -#else
> +#elif defined(CONFIG_LINUX)
>      return dent->d_off;
> +#elif defined(CONFIG_WIN32)
> +    return qemu_dirent_off_win32(s, fs);
> +#else
> +#error Missing qemu_dirent_off() implementation for this host system
>  #endif
>  }
>  
> diff --git a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c
> index 7a270a7bd5..3592e057ce 100644
> --- a/hw/9pfs/9p-util-win32.c
> +++ b/hw/9pfs/9p-util-win32.c
> @@ -929,3 +929,10 @@ int qemu_mknodat(int dirfd, const char *filename, mode_t 
> mode, dev_t dev)
>      errno = ENOTSUP;
>      return -1;
>  }
> +
> +off_t qemu_dirent_off_win32(void *s, void *fs)
> +{
> +    V9fsState *v9fs = s;
> +
> +    return v9fs->ops->telldir(&v9fs->ctx, (V9fsFidOpenState *)fs);
> +}

That's a bit tricky. So far (i.e. for Linux host, macOS host) we are storing
the directory offset directly to the dirent struct. We are not using telldir()
as the stream might have mutated in the meantime, hence calling telldir()
might return a value that does no longer correlate to dirent in question.

Hence my idea was to use the same hack for Windows as we did for macOS, where
we simply misuse a dirent field known to be not used, on macOS that's
d_seekoff which we are misusing for that purpose.

Looking at the mingw dirent.h header though, there is not much we can choose
from. d_reclen is not used, but too short, d_ino is not used by mingw, but
currently we actually read this field in server common code to assemble a file
ID for guest. I mean it is always zero on Windows, so we could still misuse
it, but we would need to inject more hacks there. :/

> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 072cf67956..be247eeb30 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -2336,7 +2336,7 @@ static int coroutine_fn 
> v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          count += len;
>          v9fs_stat_free(&v9stat);
>          v9fs_path_free(&path);
> -        saved_dir_pos = qemu_dirent_off(dent);
> +        saved_dir_pos = qemu_dirent_off(dent, pdu->s, &fidp->fs);
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -2537,7 +2537,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, 
> V9fsFidState *fidp,
>              qid.version = 0;
>          }
>  
> -        off = qemu_dirent_off(dent);
> +        off = qemu_dirent_off(dent, pdu->s, &fidp->fs);
>          v9fs_string_init(&name);
>          v9fs_string_sprintf(&name, "%s", dent->d_name);
>  
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 93ba44fb75..d40515a607 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -168,7 +168,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState 
> *fidp,
>          }
>  
>          size += len;
> -        saved_dir_pos = qemu_dirent_off(dent);
> +        saved_dir_pos = qemu_dirent_off(dent, s, &fidp->fs);
>      }
>  
>      /* restore (last) saved position */
>