diff mbox series

[v2,06/19] hw/9pfs: Add missing definitions for Windows

Message ID 20221111042225.1115931-7-bin.meng@windriver.com (mailing list archive)
State New, archived
Headers show
Series At present there is no Windows support for 9p file system. | expand

Commit Message

Bin Meng Nov. 11, 2022, 4:22 a.m. UTC 
From: Guohuai Shi <guohuai.shi@windriver.com>

Some definitions currently used by the 9pfs codes are only available
on POSIX platforms. Let's add our own ones in preparation to adding
9pfs support for Windows.

Signed-off-by: Guohuai Shi <guohuai.shi@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- Add S_IFLNK related macros to support symbolic link

 fsdev/file-op-9p.h | 33 +++++++++++++++++++++++++++++++++
 hw/9pfs/9p.h       | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

Comments

Christian Schoenebeck Nov. 14, 2022, 4:40 p.m. UTC | #1 
On Friday, November 11, 2022 5:22:12 AM CET Bin Meng wrote:
> From: Guohuai Shi <guohuai.shi@windriver.com>
> 
> Some definitions currently used by the 9pfs codes are only available
> on POSIX platforms. Let's add our own ones in preparation to adding
> 9pfs support for Windows.
> 
> Signed-off-by: Guohuai Shi <guohuai.shi@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - Add S_IFLNK related macros to support symbolic link
> 
>  fsdev/file-op-9p.h | 33 +++++++++++++++++++++++++++++++++
>  hw/9pfs/9p.h       | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 4997677460..7d9a736b66 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -27,6 +27,39 @@
>  # include <sys/mount.h>
>  #endif
>  
> +#ifdef CONFIG_WIN32
> +
> +/* POSIX structure not defined in Windows */
> +
> +typedef uint32_t uid_t;
> +typedef uint32_t gid_t;
> +
> +/* from http://man7.org/linux/man-pages/man2/statfs.2.html */
> +typedef uint32_t __fsword_t;
> +typedef uint32_t fsblkcnt_t;
> +typedef uint32_t fsfilcnt_t;
> +
> +/* from linux/include/uapi/asm-generic/posix_types.h */
> +typedef struct {
> +    long __val[2];
> +} fsid_t;
> +
> +struct statfs {
> +    __fsword_t f_type;
> +    __fsword_t f_bsize;
> +    fsblkcnt_t f_blocks;
> +    fsblkcnt_t f_bfree;
> +    fsblkcnt_t f_bavail;
> +    fsfilcnt_t f_files;
> +    fsfilcnt_t f_ffree;
> +    fsid_t f_fsid;
> +    __fsword_t f_namelen;
> +    __fsword_t f_frsize;
> +    __fsword_t f_flags;
> +};
> +

Does it make sense to define all of these, even though not being used?

> +#endif /* CONFIG_WIN32 */
> +
>  #define SM_LOCAL_MODE_BITS    0600
>  #define SM_LOCAL_DIR_MODE_BITS    0700
>  
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 2fce4140d1..957a7e4ccc 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -3,13 +3,46 @@
>  
>  #include <dirent.h>
>  #include <utime.h>
> +#ifndef CONFIG_WIN32
>  #include <sys/resource.h>
> +#endif
>  #include "fsdev/file-op-9p.h"
>  #include "fsdev/9p-iov-marshal.h"
>  #include "qemu/thread.h"
>  #include "qemu/coroutine.h"
>  #include "qemu/qht.h"
>  
> +#ifdef CONFIG_WIN32
> +
> +#define NAME_MAX            MAX_PATH

That's not quite the same. MAX_PATH on Windows corresponds to PATH_MAX on
POSIX, which is the max. length of an entire path (i.e. drive, multiple
directory names, filename, backslashes). AFAICS MAX_PATH is 260 on Windows.

The max. length of a single filename component OTOH is 255 on Windows by
default. I don't know if there is a macro for the latter, if not, maybe
just hard coding it here for now?

> +
> +/* macros required for build, values do not matter */
> +#define AT_SYMLINK_NOFOLLOW 0x100   /* Do not follow symbolic links */
> +#define AT_REMOVEDIR        0x200   /* Remove directory instead of file */
> +#define O_DIRECTORY         02000000
> +
> +#define makedev(major, minor)   \
> +        ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
> +#define major(dev)  ((unsigned int)(((dev) >> 8) & 0xfff))
> +#define minor(dev)  ((unsigned int)(((dev) & 0xff)))
> +
> +#ifndef S_IFLNK
> +/*
> + * Currenlty Windows/MinGW does not provide the following flag macros,
> + * so define them here for 9p codes.
> + *
> + * Once Windows/MinGW provides them, remove the defines to prevent conflicts.
> + */
> +#define S_IFLNK         0xA000
> +#define S_ISUID         0x0800
> +#define S_ISGID         0x0400
> +#define S_ISVTX         0x0200
> +
> +#define S_ISLNK(mode)   ((mode & S_IFMT) == S_IFLNK)
> +#endif /* S_IFLNK */
> +
> +#endif /* CONFIG_WIN32 */
> +
>  enum {
>      P9_TLERROR = 6,
>      P9_RLERROR,
>
Shi, Guohuai Nov. 16, 2022, 9:01 a.m. UTC | #2 
> -----Original Message-----
> From: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Sent: Tuesday, November 15, 2022 00:41
> To: qemu-devel@nongnu.org
> Cc: Shi, Guohuai <Guohuai.Shi@windriver.com>; Greg Kurz <groug@kaod.org>;
> Meng, Bin <Bin.Meng@windriver.com>
> Subject: Re: [PATCH v2 06/19] hw/9pfs: Add missing definitions for Windows
> 
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
> 
> On Friday, November 11, 2022 5:22:12 AM CET Bin Meng wrote:
> > From: Guohuai Shi <guohuai.shi@windriver.com>
> >
> > Some definitions currently used by the 9pfs codes are only available
> > on POSIX platforms. Let's add our own ones in preparation to adding
> > 9pfs support for Windows.
> >
> > Signed-off-by: Guohuai Shi <guohuai.shi@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v2:
> > - Add S_IFLNK related macros to support symbolic link
> >
> >  fsdev/file-op-9p.h | 33 +++++++++++++++++++++++++++++++++
> >  hw/9pfs/9p.h       | 33 +++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> >
> > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index
> > 4997677460..7d9a736b66 100644
> > --- a/fsdev/file-op-9p.h
> > +++ b/fsdev/file-op-9p.h
> > @@ -27,6 +27,39 @@
> >  # include <sys/mount.h>
> >  #endif
> >
> > +#ifdef CONFIG_WIN32
> > +
> > +/* POSIX structure not defined in Windows */
> > +
> > +typedef uint32_t uid_t;
> > +typedef uint32_t gid_t;
> > +
> > +/* from http://man7.org/linux/man-pages/man2/statfs.2.html */ typedef
> > +uint32_t __fsword_t; typedef uint32_t fsblkcnt_t; typedef uint32_t
> > +fsfilcnt_t;
> > +
> > +/* from linux/include/uapi/asm-generic/posix_types.h */ typedef
> > +struct {
> > +    long __val[2];
> > +} fsid_t;
> > +
> > +struct statfs {
> > +    __fsword_t f_type;
> > +    __fsword_t f_bsize;
> > +    fsblkcnt_t f_blocks;
> > +    fsblkcnt_t f_bfree;
> > +    fsblkcnt_t f_bavail;
> > +    fsfilcnt_t f_files;
> > +    fsfilcnt_t f_ffree;
> > +    fsid_t f_fsid;
> > +    __fsword_t f_namelen;
> > +    __fsword_t f_frsize;
> > +    __fsword_t f_flags;
> > +};
> > +
> 
> Does it make sense to define all of these, even though not being used?

Windows does not have this definition, so use Linux definition can make less impact to 9pfs code.
If not, need to add many macro "#ifdef CONFIG_WIN32" in other places to disable the unsupported code.

> 
> > +#endif /* CONFIG_WIN32 */
> > +
> >  #define SM_LOCAL_MODE_BITS    0600
> >  #define SM_LOCAL_DIR_MODE_BITS    0700
> >
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index 2fce4140d1..957a7e4ccc
> > 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -3,13 +3,46 @@
> >
> >  #include <dirent.h>
> >  #include <utime.h>
> > +#ifndef CONFIG_WIN32
> >  #include <sys/resource.h>
> > +#endif
> >  #include "fsdev/file-op-9p.h"
> >  #include "fsdev/9p-iov-marshal.h"
> >  #include "qemu/thread.h"
> >  #include "qemu/coroutine.h"
> >  #include "qemu/qht.h"
> >
> > +#ifdef CONFIG_WIN32
> > +
> > +#define NAME_MAX            MAX_PATH
> 
> That's not quite the same. MAX_PATH on Windows corresponds to PATH_MAX on
> POSIX, which is the max. length of an entire path (i.e. drive, multiple
> directory names, filename, backslashes). AFAICS MAX_PATH is 260 on Windows.
> 
> The max. length of a single filename component OTOH is 255 on Windows by
> default. I don't know if there is a macro for the latter, if not, maybe just
> hard coding it here for now?
> 

My mistake, it should be 255.

> > +
> > +/* macros required for build, values do not matter */
> > +#define AT_SYMLINK_NOFOLLOW 0x100   /* Do not follow symbolic links */
> > +#define AT_REMOVEDIR        0x200   /* Remove directory instead of file */
> > +#define O_DIRECTORY         02000000
> > +
> > +#define makedev(major, minor)   \
> > +        ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
> > +#define major(dev)  ((unsigned int)(((dev) >> 8) & 0xfff)) #define
> > +minor(dev)  ((unsigned int)(((dev) & 0xff)))
> > +
> > +#ifndef S_IFLNK
> > +/*
> > + * Currenlty Windows/MinGW does not provide the following flag
> > +macros,
> > + * so define them here for 9p codes.
> > + *
> > + * Once Windows/MinGW provides them, remove the defines to prevent
> conflicts.
> > + */
> > +#define S_IFLNK         0xA000
> > +#define S_ISUID         0x0800
> > +#define S_ISGID         0x0400
> > +#define S_ISVTX         0x0200
> > +
> > +#define S_ISLNK(mode)   ((mode & S_IFMT) == S_IFLNK)
> > +#endif /* S_IFLNK */
> > +
> > +#endif /* CONFIG_WIN32 */
> > +
> >  enum {
> >      P9_TLERROR = 6,
> >      P9_RLERROR,
> >
>
Christian Schoenebeck Nov. 16, 2022, 12:52 p.m. UTC | #3 
On Wednesday, November 16, 2022 10:01:39 AM CET Shi, Guohuai wrote:
[...]
> > > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index
> > > 4997677460..7d9a736b66 100644
> > > --- a/fsdev/file-op-9p.h
> > > +++ b/fsdev/file-op-9p.h
> > > @@ -27,6 +27,39 @@
> > >  # include <sys/mount.h>
> > >  #endif
> > >
> > > +#ifdef CONFIG_WIN32
> > > +
> > > +/* POSIX structure not defined in Windows */
> > > +
> > > +typedef uint32_t uid_t;
> > > +typedef uint32_t gid_t;
> > > +
> > > +/* from http://man7.org/linux/man-pages/man2/statfs.2.html */ typedef
> > > +uint32_t __fsword_t; typedef uint32_t fsblkcnt_t; typedef uint32_t
> > > +fsfilcnt_t;
> > > +
> > > +/* from linux/include/uapi/asm-generic/posix_types.h */ typedef
> > > +struct {
> > > +    long __val[2];
> > > +} fsid_t;
> > > +
> > > +struct statfs {
> > > +    __fsword_t f_type;
> > > +    __fsword_t f_bsize;
> > > +    fsblkcnt_t f_blocks;
> > > +    fsblkcnt_t f_bfree;
> > > +    fsblkcnt_t f_bavail;
> > > +    fsfilcnt_t f_files;
> > > +    fsfilcnt_t f_ffree;
> > > +    fsid_t f_fsid;
> > > +    __fsword_t f_namelen;
> > > +    __fsword_t f_frsize;
> > > +    __fsword_t f_flags;
> > > +};
> > > +
> > 
> > Does it make sense to define all of these, even though not being used?
> 
> Windows does not have this definition, so use Linux definition can make less impact to 9pfs code.
> If not, need to add many macro "#ifdef CONFIG_WIN32" in other places to disable the unsupported code.

My bad, I thought most of these were not referenced in code at all, but I just
realized they are indeed. Only exception is probably `f_flags`, but I haven't
checked yet whether you are using that for something new in your patches.

The previous patches LGTM BTW. I still have to look at all following patches
though. So better wait some more days before posting a v3.

Best regards,
Christian Schoenebeck
diff mbox series

Patch

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 4997677460..7d9a736b66 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -27,6 +27,39 @@ 
 # include <sys/mount.h>
 #endif
 
+#ifdef CONFIG_WIN32
+
+/* POSIX structure not defined in Windows */
+
+typedef uint32_t uid_t;
+typedef uint32_t gid_t;
+
+/* from http://man7.org/linux/man-pages/man2/statfs.2.html */
+typedef uint32_t __fsword_t;
+typedef uint32_t fsblkcnt_t;
+typedef uint32_t fsfilcnt_t;
+
+/* from linux/include/uapi/asm-generic/posix_types.h */
+typedef struct {
+    long __val[2];
+} fsid_t;
+
+struct statfs {
+    __fsword_t f_type;
+    __fsword_t f_bsize;
+    fsblkcnt_t f_blocks;
+    fsblkcnt_t f_bfree;
+    fsblkcnt_t f_bavail;
+    fsfilcnt_t f_files;
+    fsfilcnt_t f_ffree;
+    fsid_t f_fsid;
+    __fsword_t f_namelen;
+    __fsword_t f_frsize;
+    __fsword_t f_flags;
+};
+
+#endif /* CONFIG_WIN32 */
+
 #define SM_LOCAL_MODE_BITS    0600
 #define SM_LOCAL_DIR_MODE_BITS    0700
 
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 2fce4140d1..957a7e4ccc 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -3,13 +3,46 @@ 
 
 #include <dirent.h>
 #include <utime.h>
+#ifndef CONFIG_WIN32
 #include <sys/resource.h>
+#endif
 #include "fsdev/file-op-9p.h"
 #include "fsdev/9p-iov-marshal.h"
 #include "qemu/thread.h"
 #include "qemu/coroutine.h"
 #include "qemu/qht.h"
 
+#ifdef CONFIG_WIN32
+
+#define NAME_MAX            MAX_PATH
+
+/* macros required for build, values do not matter */
+#define AT_SYMLINK_NOFOLLOW 0x100   /* Do not follow symbolic links */
+#define AT_REMOVEDIR        0x200   /* Remove directory instead of file */
+#define O_DIRECTORY         02000000
+
+#define makedev(major, minor)   \
+        ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
+#define major(dev)  ((unsigned int)(((dev) >> 8) & 0xfff))
+#define minor(dev)  ((unsigned int)(((dev) & 0xff)))
+
+#ifndef S_IFLNK
+/*
+ * Currenlty Windows/MinGW does not provide the following flag macros,
+ * so define them here for 9p codes.
+ *
+ * Once Windows/MinGW provides them, remove the defines to prevent conflicts.
+ */
+#define S_IFLNK         0xA000
+#define S_ISUID         0x0800
+#define S_ISGID         0x0400
+#define S_ISVTX         0x0200
+
+#define S_ISLNK(mode)   ((mode & S_IFMT) == S_IFLNK)
+#endif /* S_IFLNK */
+
+#endif /* CONFIG_WIN32 */
+
 enum {
     P9_TLERROR = 6,
     P9_RLERROR,