RE: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message- > From: Shi, Guohuai > Sent: Friday, March 17, 2023 01:28 > To: Christian Schoenebeck ; Greg Kurz > ; qemu-devel@nongnu.org > Cc: Meng, Bin > Subject: RE: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir() > APIs > > > > > -Original Message- > > From: Christian Schoenebeck > > Sent: Thursday, March 16, 2023 19:05 > > To: Greg Kurz ; qemu-devel@nongnu.org > > Cc: Meng, Bin ; Shi, Guohuai > > > > Subject: Re: [PATCH v5 04/16] hw/9pfs: Implement Windows specific > > xxxdir() APIs > > > > 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 Wednesday, March 15, 2023 8:05:34 PM CET Shi, Guohuai wrote: > > > > > > > -Original Message- > > > > From: Christian Schoenebeck > > > > Sent: Wednesday, March 15, 2023 00:06 > > > > To: Greg Kurz ; qemu-devel@nongnu.org > > > > Cc: Shi, Guohuai ; Meng, Bin > > > > > > > > Subject: Re: [PATCH v5 04/16] hw/9pfs: Implement Windows specific > > > > xxxdir() APIs > > > > > > > > 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 Monday, February 20, 2023 11:08:03 AM CET Bin Meng wrote: > > > > > From: Guohuai Shi > > > > > > > > > > This commit implements Windows specific xxxdir() APIs for safety > > > > > directory access. > > > > > > > > That comment is seriously too short for this patch. > > > > > > > > 1. You should describe the behaviour implementation that you have > > > > chosen and why you have chosen it. > > > > > > > > 2. Like already said in the previous version of the patch, you > > > > should place a link to the discussion we had on this issue. > > > > > > > > > Signed-off-by: Guohuai Shi > > > > > Signed-off-by: Bin Meng > > > > > --- > > > > > > > > > > hw/9pfs/9p-util.h | 6 + > > > > > hw/9pfs/9p-util-win32.c | 443 > > > > > > > > > > 2 files changed, 449 insertions(+) > > > > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index > > > > > 0f159fb4ce..c1c251fbd1 100644 > > > > > --- a/hw/9pfs/9p-util.h > > > > > +++ b/hw/9pfs/9p-util.h > > > > > @@ -141,6 +141,12 @@ 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); > > > > > +DIR *opendir_win32(const char *full_file_name); int > > > > > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR > > > > > +*pDir); void rewinddir_win32(DIR *pDir); void seekdir_win32(DIR > > > > > +*pDir, long pos); long telldir_win32(DIR *pDir); > > > > > #endif > > > > > > > > > > static inline void close_preserve_errno(int fd) diff --git > > > > > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index > > > > > a99d579a06..e9408f3c45 100644 > > > > > --- a/hw/9pfs/9p-util-win32.c > > > > > +++ b/hw/9pfs/9p-util-win32.c > > > > > @@ -37,6 +37,16 @@ > > > > > *Windows does not support opendir, the directory fd is created > > > > > by > > > > > *CreateFile and convert to fd by _open_osfhandle(). Keep the fd > > open > > > > will > > > > > *lock and protect the directory (can not be modified or > > > > > replaced) > > > > > + * > > > > > + * 5. Neither Windows native APIs, nor MinGW provide a POSIX > > > > > + compatible > > > > API for > > > > > + *acquiring directory entries in a safe way. Calling those APIs > > > > (native > > > > > + *_findfirst() and _findnext() or MinGW's readdir(), seekdir() > > > > > and > > > > > + *telldir()) direct
RE: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message- > From: Christian Schoenebeck > Sent: Thursday, March 16, 2023 19:05 > To: Greg Kurz ; qemu-devel@nongnu.org > Cc: Meng, Bin ; Shi, Guohuai > > Subject: Re: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir() > APIs > > 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 Wednesday, March 15, 2023 8:05:34 PM CET Shi, Guohuai wrote: > > > > > -Original Message- > > > From: Christian Schoenebeck > > > Sent: Wednesday, March 15, 2023 00:06 > > > To: Greg Kurz ; qemu-devel@nongnu.org > > > Cc: Shi, Guohuai ; Meng, Bin > > > > > > Subject: Re: [PATCH v5 04/16] hw/9pfs: Implement Windows specific > > > xxxdir() APIs > > > > > > 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 Monday, February 20, 2023 11:08:03 AM CET Bin Meng wrote: > > > > From: Guohuai Shi > > > > > > > > This commit implements Windows specific xxxdir() APIs for safety > > > > directory access. > > > > > > That comment is seriously too short for this patch. > > > > > > 1. You should describe the behaviour implementation that you have > > > chosen and why you have chosen it. > > > > > > 2. Like already said in the previous version of the patch, you > > > should place a link to the discussion we had on this issue. > > > > > > > Signed-off-by: Guohuai Shi > > > > Signed-off-by: Bin Meng > > > > --- > > > > > > > > hw/9pfs/9p-util.h | 6 + > > > > hw/9pfs/9p-util-win32.c | 443 > > > > > > > > 2 files changed, 449 insertions(+) > > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index > > > > 0f159fb4ce..c1c251fbd1 100644 > > > > --- a/hw/9pfs/9p-util.h > > > > +++ b/hw/9pfs/9p-util.h > > > > @@ -141,6 +141,12 @@ 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); > > > > +DIR *opendir_win32(const char *full_file_name); int > > > > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR > > > > +*pDir); void rewinddir_win32(DIR *pDir); void seekdir_win32(DIR > > > > +*pDir, long pos); long telldir_win32(DIR *pDir); > > > > #endif > > > > > > > > static inline void close_preserve_errno(int fd) diff --git > > > > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index > > > > a99d579a06..e9408f3c45 100644 > > > > --- a/hw/9pfs/9p-util-win32.c > > > > +++ b/hw/9pfs/9p-util-win32.c > > > > @@ -37,6 +37,16 @@ > > > > *Windows does not support opendir, the directory fd is created by > > > > *CreateFile and convert to fd by _open_osfhandle(). Keep the fd > open > > > will > > > > *lock and protect the directory (can not be modified or replaced) > > > > + * > > > > + * 5. Neither Windows native APIs, nor MinGW provide a POSIX > > > > + compatible > > > API for > > > > + *acquiring directory entries in a safe way. Calling those APIs > > > (native > > > > + *_findfirst() and _findnext() or MinGW's readdir(), seekdir() and > > > > + *telldir()) directly can lead to an inconsistent state if > directory > > > is > > > > + *modified in between, e.g. the same directory appearing more than > > > once > > > > + *in output, or directories not appearing at all in output even > though > > > they > > > > + *were neither newly created nor deleted. POSIX does not define > what > > > happens > > > > + *with deleted or newly created directories in between, but it > > > guarantees a > > > > + *consistent state. > > > > */ > > > > > > > > #include "qemu/osdep.h" > > > > @@ -51,6 +61,25 @@ > > > > > > > > #define V9FS_MAGIC 0x53465
RE: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message-
> From: Christian Schoenebeck
> Sent: Wednesday, March 15, 2023 00:06
> To: Greg Kurz ; qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Meng, Bin
>
> Subject: Re: [PATCH v5 04/16] hw/9pfs: Implement Windows specific xxxdir()
> APIs
>
> 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 Monday, February 20, 2023 11:08:03 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > This commit implements Windows specific xxxdir() APIs for safety
> > directory access.
>
> That comment is seriously too short for this patch.
>
> 1. You should describe the behaviour implementation that you have chosen and
> why you have chosen it.
>
> 2. Like already said in the previous version of the patch, you should place a
> link to the discussion we had on this issue.
>
> > Signed-off-by: Guohuai Shi
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/9pfs/9p-util.h | 6 +
> > hw/9pfs/9p-util-win32.c | 443
> >
> > 2 files changed, 449 insertions(+)
> >
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > 0f159fb4ce..c1c251fbd1 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -141,6 +141,12 @@ 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);
> > +DIR *opendir_win32(const char *full_file_name); int
> > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR *pDir);
> > +void rewinddir_win32(DIR *pDir); void seekdir_win32(DIR *pDir, long
> > +pos); long telldir_win32(DIR *pDir);
> > #endif
> >
> > static inline void close_preserve_errno(int fd) diff --git
> > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index
> > a99d579a06..e9408f3c45 100644
> > --- a/hw/9pfs/9p-util-win32.c
> > +++ b/hw/9pfs/9p-util-win32.c
> > @@ -37,6 +37,16 @@
> > *Windows does not support opendir, the directory fd is created by
> > *CreateFile and convert to fd by _open_osfhandle(). Keep the fd open
> will
> > *lock and protect the directory (can not be modified or replaced)
> > + *
> > + * 5. Neither Windows native APIs, nor MinGW provide a POSIX compatible
> API for
> > + *acquiring directory entries in a safe way. Calling those APIs
> (native
> > + *_findfirst() and _findnext() or MinGW's readdir(), seekdir() and
> > + *telldir()) directly can lead to an inconsistent state if directory
> is
> > + *modified in between, e.g. the same directory appearing more than
> once
> > + *in output, or directories not appearing at all in output even though
> they
> > + *were neither newly created nor deleted. POSIX does not define what
> happens
> > + *with deleted or newly created directories in between, but it
> guarantees a
> > + *consistent state.
> > */
> >
> > #include "qemu/osdep.h"
> > @@ -51,6 +61,25 @@
> >
> > #define V9FS_MAGIC 0x53465039 /* string "9PFS" */
> >
> > +/*
> > + * MinGW and Windows does not provide a safe way to seek directory
> > +while other
> > + * thread is modifying the same directory.
> > + *
> > + * This structure is used to store sorted file id and ensure
> > +directory seek
> > + * consistency.
> > + */
> > +struct dir_win32 {
> > +struct dirent dd_dir;
> > +uint32_t offset;
> > +uint32_t total_entries;
> > +HANDLE hDir;
> > +uint32_t dir_name_len;
> > +uint64_t dot_id;
> > +uint64_t dot_dot_id;
> > +uint64_t *file_id_list;
> > +char dd_name[1];
> > +};
> > +
> > /*
> > * win32_error_to_posix - convert Win32 error to POSIX error number
> > *
> > @@ -977,3 +1006,417 @@ int qemu_mknodat(int dirfd, const char *filename,
> mode_t mode, dev_t dev)
> > errno = ENOTSUP;
> > return -1;
> > }
> > +
> > +static int file_id_compare(const void *id_ptr1, const void *id_ptr2)
> > +{
> > +uint64_t id[2];
> > +
> > +id[0] = *(uint64_t *)id_ptr1;
> > +id[1] = *(uint64_t *)id_ptr2;
> > +
> > +if (id[0] > id[1]) {
> > +return 1;
> > +} else if (id[0] < id[1]) {
> > +return -1;
>
RE: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message-
> From: Christian Schoenebeck
> Sent: Tuesday, February 7, 2023 18:12
> To: Greg Kurz ; qemu-devel@nongnu.org
> Cc: Meng, Bin ; Marc-André Lureau
> ; Daniel P. Berrangé ; Shi,
> Guohuai
> Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir()
> APIs
>
> 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 Monday, February 6, 2023 6:37:16 AM CET Shi, Guohuai wrote:
> [...]
> > > I know, it's an n-square performance issue and what I already wrote
> > > in the summary of the linked original suggestion [1] in v3 before, quote:
> > >
> > > + Relatively straight-forward to implement.
> > >
> > > + No (major) changes in 9pfs code base required.
> > >
> > > - Still n-square performance issue (neglectable to land Windows
> > > host support
> > > IMO).
> > >
> > > o Consistency assured for "most" cases, except one: if hardlinks are
> > > inserted in between then it might fail
> >
> > readdir() on Linux host may also return the deleted entries.
> > And POSIX specification does not mention about the consistency issue.
>
> POSIX explicitly specifies that 1. new and 2. deleted entries may or may not
> appear in result and leaves that implementation specific. That was never our
> concern.
>
> And yes, POSIX does not explicitly discuss consistency concerning entries
> that have neither been added or removed, but this expectation is implied. In
> practice double entries are probably less of an issue, client might be able
> to handle that without misbehaviour (haven't checked this at all yet), but if
> the implementation would lead to chances that entries may *never* appear to
> clients at all, even after refreshing periodically, I mean how could you work
> with a file system like that?
>
> > NTFS file id is the $MFT index id. It will keen unique until file is
> deleted.
> > But the index id may be reuse if delete and re-create many files.
> >
> > Saving file id instead of name will make consistency better, but may not
> cover all status.
> > Because read directory is not a "atomic" operation.
>
> I don't see an issue with that, because these are entries that were either
> added or removed, we don't care about them. And their file IDs would not
> affect fetching the other directory entries that have not been touched in
> between.
>
> And we are also not questioning atomicity here, but consistency.
>
> > > [1] https://lore.kernel.org/qemu-devel/2468168.SvRIHAoRfs@silver/
> > >
> > > The idea was to use that just as a starting point to land Windows
> > > host support ASAP, slower on large dirs compared to other solutions,
> > > yes, but with guaranteed correct and deterministic behaviour. And
> > > then on the long run we would of course replace that with a more
> performant solution.
> > >
> > > I mean, this is really simple to implement, so I would at least test
> > > it. If it really runs horribly slow we could still discuss faster
> > > solutions, which are however all much more tricky.
> > >
> >
> > I did a basic test on Windows host, here is the code:
> >
> > st = clock();
> > pDir = opendir_win32(TEST_DIR);
> >
> > if (pDir == NULL)
> > return -1;
> >
> > while ((pEnt = readdir_win32(pDir)) != NULL)
> > {
> > totals++;
> > }
> > closedir_win32(pDir);
> > ed = clock();
> >
> > printf("total = %d clocks = %d %d\n", totals, ed - st,
> > CLOCKS_PER_SEC);
> >
> > My local storage is SSD disk.
> >
> > Run this test for 100, 1000, 1 entries.
> > For file name cache solution, the time cost is: 2, 9, 44 (in ms).
> > For file id cache solution, the time cost: 3, 438, 4338 (in ms).
> > I already used OpenFileById() to make it faster instead of CreateFile(). If
> I use CreateFile, it need more than 80 seconds.
> >
> > The performance looks like not good.
> > And actually, it would be worse in 9pfs.
> > Because in current design, 9pfs may seek forward and seek back several
> times during reading directory, which may cause the performance worse.
>
> Poor performance, yes, probably a bit worse than I would have expected.
>
> So it is about choosing your poison (potential crash vs. poor performance).
>
> I mean, I am not keen into suggesting any kind of bike shredding for
RE: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message- > From: Christian Schoenebeck > Sent: Saturday, February 4, 2023 01:55 > To: Greg Kurz ; qemu-devel@nongnu.org > Cc: Meng, Bin ; Marc-André Lureau > ; Daniel P. Berrangé > ; Shi, Guohuai > Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() > APIs > > 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, February 3, 2023 5:30:35 PM CET Shi, Guohuai wrote: > > > > > -Original Message- > > > From: Christian Schoenebeck > > > Sent: Friday, February 3, 2023 22:41 > > > To: Greg Kurz ; qemu-devel@nongnu.org > > > Cc: Meng, Bin ; Marc-André Lureau > > > ; Daniel P. Berrangé > > > ; Shi, Guohuai > > > Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific > > > xxxdir() APIs > > > > > > 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, February 3, 2023 2:34:13 PM CET Shi, Guohuai wrote: > > > > > > > > > -Original Message- > > > > > From: Christian Schoenebeck > > > > > Sent: Friday, February 3, 2023 20:25 > > > > > To: Greg Kurz ; qemu-devel@nongnu.org > > > > > Cc: Shi, Guohuai ; Meng, Bin > > > > > ; Marc-André Lureau > > > > > ; Daniel P. Berrangé > > > > > > > > > > Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows > > > > > specific > > > > > xxxdir() APIs > > > > > > > > > > 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 Monday, January 30, 2023 10:51:50 AM CET Bin Meng wrote: > > > > > > From: Guohuai Shi > > > > > > > > > > > > This commit implements Windows specific xxxdir() APIs for > > > > > > safety directory access. > > > > > > > > > > > > > > > > This issue deserves a link to either the previous discussion > > > > > > > > > > Link: > > > > > https://lore.kernel.org/qemu-devel/2830993.GtbaR8S6b6@silver/ > > > > > > > > > > and/or a link to this continuation of the discussion here, as > > > > > it's not a trivial issue, with pros and cons been discussed for > > > > > the individual, possible solutions. > > > > > > > > > > > Signed-off-by: Guohuai Shi > > > > > > Signed-off-by: Bin Meng > > > > > > --- > > > > > > > > > > > > hw/9pfs/9p-util.h | 6 + > > > > > > hw/9pfs/9p-util-win32.c | 296 > > > > > > > > > > > > 2 files changed, 302 insertions(+) > > > > > > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index > > > > > > 0f159fb4ce..c1c251fbd1 100644 > > > > > > --- a/hw/9pfs/9p-util.h > > > > > > +++ b/hw/9pfs/9p-util.h > > > > > > @@ -141,6 +141,12 @@ 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); > > > > > > +DIR *opendir_win32(const char *full_file_name); int > > > > > > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR > > > > > > +*pDir); void rewinddir_win32(DIR *pDir); void > > > > > > +seekdir_win32(DIR *pDir, long pos); long telldir_win32(DIR > > > > > > +*pDir); > > > > > > #endif > > > > > > > > > > > > static inline void close_preserve_errno(int fd) diff --git > > > > > > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index > > > > > > a99d579a06..5503199300 100644 > > > > > > --- a/hw/9pfs/9p-util-win32.c > > > > > >
RE: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message- > From: Christian Schoenebeck > Sent: Friday, February 3, 2023 22:41 > To: Greg Kurz ; qemu-devel@nongnu.org > Cc: Meng, Bin ; Marc-André Lureau > ; Daniel P. Berrangé ; Shi, > Guohuai > Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() > APIs > > 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, February 3, 2023 2:34:13 PM CET Shi, Guohuai wrote: > > > > > -Original Message- > > > From: Christian Schoenebeck > > > Sent: Friday, February 3, 2023 20:25 > > > To: Greg Kurz ; qemu-devel@nongnu.org > > > Cc: Shi, Guohuai ; Meng, Bin > > > ; Marc-André Lureau > > > ; Daniel P. Berrangé > > > > > > Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific > > > xxxdir() APIs > > > > > > 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 Monday, January 30, 2023 10:51:50 AM CET Bin Meng wrote: > > > > From: Guohuai Shi > > > > > > > > This commit implements Windows specific xxxdir() APIs for safety > > > > directory access. > > > > > > > > > > This issue deserves a link to either the previous discussion > > > > > > Link: https://lore.kernel.org/qemu-devel/2830993.GtbaR8S6b6@silver/ > > > > > > and/or a link to this continuation of the discussion here, as it's > > > not a trivial issue, with pros and cons been discussed for the > > > individual, possible solutions. > > > > > > > Signed-off-by: Guohuai Shi > > > > Signed-off-by: Bin Meng > > > > --- > > > > > > > > hw/9pfs/9p-util.h | 6 + > > > > hw/9pfs/9p-util-win32.c | 296 > > > > > > > > 2 files changed, 302 insertions(+) > > > > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index > > > > 0f159fb4ce..c1c251fbd1 100644 > > > > --- a/hw/9pfs/9p-util.h > > > > +++ b/hw/9pfs/9p-util.h > > > > @@ -141,6 +141,12 @@ 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); > > > > +DIR *opendir_win32(const char *full_file_name); int > > > > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR > > > > +*pDir); void rewinddir_win32(DIR *pDir); void seekdir_win32(DIR > > > > +*pDir, long pos); long telldir_win32(DIR *pDir); > > > > #endif > > > > > > > > static inline void close_preserve_errno(int fd) diff --git > > > > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index > > > > a99d579a06..5503199300 100644 > > > > --- a/hw/9pfs/9p-util-win32.c > > > > +++ b/hw/9pfs/9p-util-win32.c > > > > @@ -37,6 +37,13 @@ > > > > *Windows does not support opendir, the directory fd is created by > > > > *CreateFile and convert to fd by _open_osfhandle(). Keep the fd > open > > > will > > > > *lock and protect the directory (can not be modified or replaced) > > > > + * > > > > + * 5. Windows and MinGW does not provide safety directory > > > > + accessing > > > functions. > > > > + *readdir(), seekdir() and telldir() may get or set wrong value > > > because > > > > + *directory entry data is not protected. > > > > > > I would rephrase that sentence, as it doesn't cover the root problem > > > adequately. Maybe something like this: > > > > > > 5. Neither Windows native APIs, nor MinGW provide a POSIX compatible > > > API for acquiring directory entries in a safe way. Calling those > > > APIs (native > > > _findfirst() and _findnext() or MinGW's readdir(), seekdir() and > > > telldir()) directly can lead to an inconsistent state if directory > > > is modified in between, e.g. the same directory appearing more than > > > once in output, or directories not appearing at all in output even > > >
RE: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir() APIs
> -Original Message-
> From: Christian Schoenebeck
> Sent: Friday, February 3, 2023 20:25
> To: Greg Kurz ; qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Meng, Bin
> ; Marc-André Lureau ;
> Daniel P. Berrangé
> Subject: Re: [PATCH v4 04/16] hw/9pfs: Implement Windows specific xxxdir()
> APIs
>
> 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 Monday, January 30, 2023 10:51:50 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > This commit implements Windows specific xxxdir() APIs for safety
> > directory access.
> >
>
> This issue deserves a link to either the previous discussion
>
> Link: https://lore.kernel.org/qemu-devel/2830993.GtbaR8S6b6@silver/
>
> and/or a link to this continuation of the discussion here, as it's not a
> trivial issue, with pros and cons been discussed for the individual, possible
> solutions.
>
> > Signed-off-by: Guohuai Shi
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/9pfs/9p-util.h | 6 +
> > hw/9pfs/9p-util-win32.c | 296
> >
> > 2 files changed, 302 insertions(+)
> >
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > 0f159fb4ce..c1c251fbd1 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -141,6 +141,12 @@ 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);
> > +DIR *opendir_win32(const char *full_file_name); int
> > +closedir_win32(DIR *pDir); struct dirent *readdir_win32(DIR *pDir);
> > +void rewinddir_win32(DIR *pDir); void seekdir_win32(DIR *pDir, long
> > +pos); long telldir_win32(DIR *pDir);
> > #endif
> >
> > static inline void close_preserve_errno(int fd) diff --git
> > a/hw/9pfs/9p-util-win32.c b/hw/9pfs/9p-util-win32.c index
> > a99d579a06..5503199300 100644
> > --- a/hw/9pfs/9p-util-win32.c
> > +++ b/hw/9pfs/9p-util-win32.c
> > @@ -37,6 +37,13 @@
> > *Windows does not support opendir, the directory fd is created by
> > *CreateFile and convert to fd by _open_osfhandle(). Keep the fd open
> will
> > *lock and protect the directory (can not be modified or replaced)
> > + *
> > + * 5. Windows and MinGW does not provide safety directory accessing
> functions.
> > + *readdir(), seekdir() and telldir() may get or set wrong value
> because
> > + *directory entry data is not protected.
>
> I would rephrase that sentence, as it doesn't cover the root problem
> adequately. Maybe something like this:
>
> 5. Neither Windows native APIs, nor MinGW provide a POSIX compatible API for
> acquiring directory entries in a safe way. Calling those APIs (native
> _findfirst() and _findnext() or MinGW's readdir(), seekdir() and telldir())
> directly can lead to an inconsistent state if directory is modified in
> between, e.g. the same directory appearing more than once in output, or
> directories not appearing at all in output even though they were neither
> newly created nor deleted. POSIX does not define what happens with deleted or
> newly created directories in between, but it guarantees a consistent state.
>
> > + *
> > + *This file re-write POSIX directory accessing functions and cache all
> > + *directory entries during opening.
> > */
> >
> > #include "qemu/osdep.h"
> > @@ -51,6 +58,27 @@
> >
> > #define V9FS_MAGIC 0x53465039 /* string "9PFS" */
> >
> > +/*
> > + * MinGW and Windows does not provide safety way to seek directory
> > +while other
> > + * thread is modifying same directory.
> > + *
> > + * The two structures are used to cache all directory entries when opening
> it.
> > + * Cached entries are always returned for read or seek.
> > + */
> > +struct dir_win32_entry {
> > +QSLIST_ENTRY(dir_win32_entry) node;
> > +struct _finddata_t dd_data;
> > +};
> > +
> > +struct dir_win32 {
> > +struct dirent dd_dir;
> > +uint32_t offset;
> > +uint32_t total_entries;
> > +QSLIST_HEAD(, dir_win32_entry) head;
> > +struct dir_win32_entry *current;
> > +char dd_name[1];
> > +};
> > +
> > /*
> > * win32_error_to_posix - convert Win32 error to POSIX error number
> > *
> > @@ -977,3 +1
RE: [PATCH v4 00/16] hw/9pfs: Add 9pfs support for Windows
> From: Marc-André Lureau > Sent: Tuesday, January 31, 2023 23:07 > To: Daniel P. Berrangé > Cc: Meng, Bin ; Greg Kurz ; Christian > Schoenebeck ; qemu-devel@nongnu.org; Shi, Guohuai > ; Laurent > Vivier ; Paolo > Bonzini ; Philippe Mathieu-Daudé ; > Thomas Huth > Subject: Re: [PATCH v4 00/16] hw/9pfs: Add 9pfs support 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. > Hi > > On Tue, Jan 31, 2023 at 6:39 PM Daniel P. Berrangé > <mailto:berra...@redhat.com> wrote: > On Tue, Jan 31, 2023 at 06:31:39PM +0400, Marc-André Lureau wrote: > > Hi > > > > On Mon, Jan 30, 2023 at 1:52 PM Bin Meng <mailto:bin.m...@windriver.com> > > wrote: > > > > > At present there is no Windows support for 9p file system. > > > This series adds initial Windows support for 9p file system. > > > > > > 'local' file system backend driver is supported on Windows, > > > including open, read, write, close, rename, remove, etc. > > > All security models are supported. The mapped (mapped-xattr) > > > security model is implemented using NTFS Alternate Data Stream > > > (ADS) so the 9p export path shall be on an NTFS partition. > > > > > > 'synth' driver is adapted for Windows too so that we can now > > > run qtests on Windows for 9p related regression testing. > > > > > > Example command line to test: > > > > > > "-fsdev local,path=c:\msys64,security_model=mapped,id=p9 -device > > > virtio-9p-pci,fsdev=p9,mount_tag=p9fs" > > > > > > Base-commit: 13356edb87506c148b163b8c7eb0695647d00c2a > > > > > > Changes in v4: > > > - Fixed 9pfs mounted as read-only issue on Windows host, adding a > > > win32_error_to_posix() to translate Windows native API error to > > > POSIX one. > > > - Fixed errors of handling symbolic links > > > - Added forward declaration to avoid using 'void *' > > > - Implemented Windows specific xxxdir() APIs for safe directory access > > > > > > > > Sorry to look a bit late at this series, I don't know what was discussed > > previously. > > > > My general feeling is that a lot of this FS portability work would be > > better handled by using GIO (even though this may add some extra > > dependency). GIO lacks some features on win32 (for example xattributes on > > win32), but they could have been proposed there too and benefiting other > > apps. GIO function is actually same as MinGW APIs, which is not safety as MinGW (discussed in previous versions). https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/dirent/dirent.c#L61 https://github.com/Alexpux/mingw-w64/blob/master/mingw-w64-crt/misc/dirent.c#L42 GIO function also does not handle symbolic links on Windows host, this may cause security issues. GIO functions also use Windows POSIX APIs without extra security checks (does not provide NO_FOLLOW flags): https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gstdio.c#L1050 9pfs need functions like openat() to make sure that the sub-sequence operation is working in the expected parent. So using GIO will still have security issues. > > The currently impl relies on the openat, fstatat, mkdirat, renameat, > utimensat, unlinkat functions. IIRC this was in order to deal with > various security vulnerabilities that exist due to race conditions. > AFAIK, there's no way to achieve the same with GIO as its a higher > level API which doesn't expose this kind of functionality > > Correct me if I am wrong, but that doesn't seem to hold much since the > protocol doesn't keep a context (with associated fds) around. But perhaps GIO > API alone can't provide safe implementations of the FileOperations callbacks? > > Also a lot of 9p-unix specific details may not map easily to the GIO API. How > they can be ported to win32 is certainly a challenge, mostly duplicating the > effort done in GIO to me. Thanks Guohuai
RE: [PATCH v3 07/17] hw/9pfs: Support getting current directory offset for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Wednesday, December 28, 2022 19:51
> To: Greg Kurz ; qemu-devel@nongnu.org
> Cc: Meng, Bin ; Shi, Guohuai
>
> Subject: Re: [PATCH v3 07/17] hw/9pfs: Support getting current directory
> offset 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 Wednesday, December 21, 2022 7:02:43 PM CET Shi, Guohuai wrote:
> >
> > > -Original Message-
> > > From: Christian Schoenebeck
> > > Sent: Wednesday, December 21, 2022 22:48
> > > To: Greg Kurz ; qemu-devel@nongnu.org
> > > Cc: Shi, Guohuai ; Meng, Bin
> > >
> > > Subject: Re: [PATCH v3 07/17] hw/9pfs: Support getting current
> > > directory offset 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 Monday, December 19, 2022 11:20:11 AM CET Bin Meng wrote:
> > > > From: Guohuai Shi
> > > >
> > > > 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
> > > > Signed-off-by: Bin Meng
> > > > ---
> > > >
> > > > (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.
> >
> > It is "V9fsState *" here, but 9p-util.h may be included by some other
> source file which is not have definition of "V9fsState".
> > So change it to "void *" to prevent unnecessary type declarations.
>
> You can anonymously declare the struct to avoid cyclic dependencies (e.g.
> like in 9p.h):
>
> typedef struct V9fsState V9fsState;
>
> Avoid the void.
>
Got it, understood.
> > >
> > > > -#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;
> > > > }
>
RE: [PATCH v3 07/17] hw/9pfs: Support getting current directory offset for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Wednesday, December 21, 2022 22:48
> To: Greg Kurz ; qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Meng, Bin
>
> Subject: Re: [PATCH v3 07/17] hw/9pfs: Support getting current directory
> offset 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 Monday, December 19, 2022 11:20:11 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > 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
> > Signed-off-by: Bin Meng
> > ---
> >
> > (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.
It is "V9fsState *" here, but 9p-util.h may be included by some other source
file which is not have definition of "V9fsState".
So change it to "void *" to prevent unnecessary type declarations.
>
> > -#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(>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. :/
If you check seekdir and telldir() implement in MinGW,
you can see that MinGW (and Windows) does not have a safety way to seek/tell
directory offset.
Because Windows POSIX and native API does not provide a way to seek directory.
Windows native API only allow to read directory forward, but not backward.
So even we store the d_seekoff to some places, the directory may still be
modified.
And Windows file system actually has inode number, MinGW does not introduce it
to MinGW API.
RE: [PATCH v2 07/19] hw/9pfs: Implement Windows specific utilities functions for 9pfs
> -Original Message-
> From: Christian Schoenebeck
> Sent: Thursday, November 17, 2022 23:55
> To: qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Greg Kurz ;
> Meng, Bin
> Subject: Re: [PATCH v2 07/19] hw/9pfs: Implement Windows specific utilities
> functions for 9pfs
>
> 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:13 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > Windows POSIX API and MinGW library do not provide the NO_FOLLOW flag,
> > and do not allow opening a directory by POSIX open(). This causes all
> > xxx_at() functions cannot work directly. However, we can provide
> > Windows handle based functions to emulate xxx_at() functions (e.g.:
> > openat_win32, utimensat_win32, etc.).
> >
> > NTFS ADS (Alternate Data Streams) is used to emulate 9pfs extended
> > attributes on Windows. Symbolic link is only supported when security
> > model is "mapped-xattr" or "mapped-file".
> >
> > Signed-off-by: Guohuai Shi
> > Signed-off-by: Bin Meng
> >
> > ---
> >
> > Changes in v2:
> > - Support symbolic link when security model is "mapped-xattr" or "mapped-
> file"
> >
> > hw/9pfs/9p-local.h | 7 +
> > hw/9pfs/9p-util.h | 38 +-
> > hw/9pfs/9p-local.c | 4 -
> > hw/9pfs/9p-util-win32.c | 934
> >
> > 4 files changed, 978 insertions(+), 5 deletions(-) create mode
> > 100644 hw/9pfs/9p-util-win32.c
> >
> > diff --git a/hw/9pfs/9p-local.h b/hw/9pfs/9p-local.h index
> > 66a21316a0..eb4f39ddc2 100644
> > --- a/hw/9pfs/9p-local.h
> > +++ b/hw/9pfs/9p-local.h
> > @@ -13,6 +13,13 @@
> > #ifndef QEMU_9P_LOCAL_H
> > #define QEMU_9P_LOCAL_H
> >
> > +typedef struct {
> > +QemuFd_t mountfd;
> > +#ifdef CONFIG_WIN32
> > +char *root_path;
> > +#endif
> > +} LocalData;
> > +
> > QemuFd_t local_open_nofollow(FsContext *fs_ctx, const char *path, int
> flags,
> > mode_t mode); QemuFd_t
> > local_opendir_nofollow(FsContext *fs_ctx, const char *path); diff
> > --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > 3d6bd1a51e..5fb854bf61 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -88,26 +88,61 @@ static inline int errno_to_dotl(int err) {
> > return err;
> > }
> >
> > -#ifdef CONFIG_DARWIN
> > +#if defined(CONFIG_DARWIN)
> > #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> > +#elif defined(CONFIG_WIN32)
> > +#define qemu_fgetxattr fgetxattr_win32
> > #else
> > #define qemu_fgetxattr fgetxattr
> > #endif
> >
> > +#ifdef CONFIG_WIN32
> > +#define qemu_openat openat_win32
> > +#define qemu_fstatatfstatat_win32
> > +#define qemu_mkdiratmkdirat_win32
> > +#define qemu_renameat renameat_win32
> > +#define qemu_utimensat utimensat_win32
> > +#define qemu_unlinkat unlinkat_win32
> > +#else
> > #define qemu_openat openat
> > #define qemu_fstatatfstatat
> > #define qemu_mkdiratmkdirat
> > #define qemu_renameat renameat
> > #define qemu_utimensat utimensat
> > #define qemu_unlinkat unlinkat
> > +#endif
> > +
> > +#ifdef CONFIG_WIN32
> > +char *get_full_path_win32(QemuFd_t fd, const char *name); ssize_t
> > +fgetxattr_win32(int fd, const char *name, void *value, size_t size);
> > +QemuFd_t openat_win32(QemuFd_t dirfd, const char *pathname, int flags,
> > + mode_t mode);
> > +int fstatat_win32(QemuFd_t dirfd, const char *pathname,
> > + struct stat *statbuf, int flags); int
> > +mkdirat_win32(QemuFd_t dirfd, const char *pathname, mode_t mode); int
> > +renameat_win32(QemuFd_t olddirfd, const char *oldpath,
> > + QemuFd_t newdirfd, const char *newpath); int
> > +utimensat_win32(QemuFd_t dirfd, const char *pathname,
> > +const struct timespec times[2], int flags); int
> > +unlinkat_win32(QemuFd_t dirfd, const char *pathname, int flags); int
> > +statfs_win32(const char *root_path, struct statfs *stbuf); QemuFd_t
> > +openat_dir(QemuFd_t dirfd, const char *name); QemuFd_t
> > +openat_file(QemuFd_t dirfd, const char *name, int flags,
> > + mode_t mode);
> > +#endif
>
> That's quite a bunch of *_win32() prototypes. Maybe movin
RE: [PATCH v2 06/19] hw/9pfs: Add missing definitions for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Tuesday, November 15, 2022 00:41
> To: qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Greg Kurz ;
> Meng, Bin
> 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
> >
> > 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
> > Signed-off-by: Bin Meng
> >
> > ---
> >
> > 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
> > #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_BITS0600
> > #define SM_LOCAL_DIR_MODE_BITS0700
> >
> > 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
> > #include
> > +#ifndef CONFIG_WIN32
> > #include
> > +#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_MAXMAX_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_REMOVEDIR0x200 /* Remove directory instead of file */
> > +#define O_DIRECTORY 0200
> > +
> > +#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,
> >
>
RE: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Wednesday, November 2, 2022 19:34
> To: qemu-devel@nongnu.org
> Cc: Greg Kurz ; Meng, Bin ; Shi,
> Guohuai
> Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features
> 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 Wednesday, November 2, 2022 4:44:14 AM CET Shi, Guohuai wrote:
> >
> > > -Original Message-
> > > From: Christian Schoenebeck
> > > Sent: Wednesday, November 2, 2022 02:59
> > > To: qemu-devel@nongnu.org
> > > Cc: Greg Kurz ; Meng, Bin ;
> > > Shi, Guohuai
> > > Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags and
> > > features for Windows
> > >
> > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > >
> > > On Tuesday, November 1, 2022 4:34:54 PM CET Shi, Guohuai wrote:
> > > >
> > > > > -Original Message-
> > > > > From: Christian Schoenebeck
> > > > > Sent: Tuesday, November 1, 2022 23:04
> > > > > To: qemu-devel@nongnu.org
> > > > > Cc: Shi, Guohuai ; Greg Kurz
> > > > > ; Meng, Bin
> > > > > Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags
> > > > > and features for Windows
> > > > >
> > > > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > > > >
> > > > > On Monday, October 24, 2022 6:57:52 AM CET Bin Meng wrote:
> > > > > > From: Guohuai Shi
> > > > > >
> > > > > > Some flags and features are not supported on Windows, like
> > > > > > mknod, readlink, file mode, etc. Update the codes for Windows.
> > > > > >
> > > > > > Signed-off-by: Guohuai Shi
> > > > > > Signed-off-by: Bin Meng
> > > > > > ---
> > > > > >
> > > > > > hw/9pfs/9p-util.h | 6 +++-
> > > > > > hw/9pfs/9p.c | 90 ++-
>
> > > > > > 2 files changed, 86 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > > > > > 82b2d0c3e4..3d154e9103 100644
> > > > > > --- a/hw/9pfs/9p-util.h
> > > > > > +++ b/hw/9pfs/9p-util.h
> > > > > > @@ -53,8 +53,10 @@ static inline uint64_t
> > > > > > makedev_dotl(uint32_t dev_major,
> > > > > uint32_t dev_minor)
> > > > > > */
> > > > > > static inline uint64_t host_dev_to_dotl_dev(dev_t dev) {
> > > > > > -#ifdef CONFIG_LINUX
> > > > > > +#if defined(CONFIG_LINUX)
> > > > > > return dev;
> > > > > > +#elif defined(CONFIG_WIN32)
> > > > > > +return 0;
> > > > >
> > > > > Really?
> > > >
> > > > Check MS this document:
> > > > https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/
> > > > fsta
> > > > t-fstat32-fstat64-fstati64-fstat32i64-fstat64i32?view=msvc-170
> > > > st_rdev: If a device, fd; otherwise 0.
> > > > st_dev: If a device, fd; otherwise 0.
> > > >
> > > > So for any file open, it should be 0.
> > >
> > > Yeah, but that function translates a corresponding device ID for
> > > *Linux* guest side. And the intention is to avoid e.g. file ID
> > > collisions on guest side.
> > > Because for a Linux guest, the two-tuple (device number, inode
> > > number) makes a system-wide unique file ID.
> > >
> > > If you just return zero here, that might be OK if only one 9p
> > > directory is exported to guest, but say you have "C:\foo\" exported
> > > and "D:\bar\" exported and mounted via 9p to guest, then guest would
> > > assume every file with the same inode number on those two to be the
> > > same files. But they are not. They are on two different drives actually.
> > >
> >
> > Got it.
> > Windows does not provide any numerical type ID for device, I think the
> > solution could be using driver letter ASC code "C:", "D:", etc.
> >
> > > >
> > > > >
> > &
RE: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities functions for 9pfs
> -Original Message- > From: Christian Schoenebeck > Sent: Wednesday, November 2, 2022 19:06 > To: qemu-devel@nongnu.org > Cc: Greg Kurz ; Meng, Bin ; Shi, > Guohuai > Subject: Re: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities > functions for 9pfs > > 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 Wednesday, November 2, 2022 4:07:35 AM CET Shi, Guohuai wrote: > > > > > -Original Message- > > > From: Christian Schoenebeck > > > Sent: Wednesday, November 2, 2022 02:22 > > > To: qemu-devel@nongnu.org > > > Cc: Greg Kurz ; Meng, Bin ; > > > Shi, Guohuai > > > Subject: Re: [PATCH 07/16] hw/9pfs: Implement Windows specific > > > utilities functions for 9pfs > > > > > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > > > > > On Tuesday, November 1, 2022 4:20:53 PM CET Shi, Guohuai wrote: > > > > > > > [...] > > > > > > > Windows POSIX API and MinGW library do not provide the > > > > > > > NO_FOLLOW flag, and do not allow opening a directory by > > > > > > > POSIX open(). This causes all > > > > > > > xxx_at() functions cannot work directly. However, we can > > > > > > > provide Windows handle based functions to emulate xxx_at() > functions (e.g.: > > > > > > > openat_win32, utimensat_win32, etc.). > > > > > > > > > > > > > > Windows does not support extended attributes. 9pfs for > > > > > > > Windows uses NTFS ADS (Alternate Data Streams) to emulate > > > > > > > extended > > > attributes. > > > > > > > > > > > > > > Windows does not provide POSIX compatible readlink(), and > > > > > > > symbolic link feature in 9pfs will be disabled on Windows. > > > > > > > > > > > > Wouldn't it be more user friendly if the relevant error > > > > > > locations would use something like error_report_once() and > > > > > > suggesting to enable > > > > > > mapped(-xattr) to make 9p symlinks on guest working if desired > > > > > > by the > > > user? > > > > > > > > > > > > Probably this error case would need to wrapped into a > > > > > > dedicated function, otherwise I guess error_report_once() > > > > > > would fire several times by different callers. > > > > > > > > > > > > > > > > Windows (MinGW) does not only support symlink, but also does not > > > > > have symlink definitions. > > > > > Windows does not support symlink flags S_IFLNK. > > > > > > > > > > So even I add symlink support by mapped-xattr, the MinGW library > > > > > does not have symlink flags and get a build error. > > > > > And this flags is defined by Windows header files. > > > > > The impact of adding a new flags to an pre-defined structure > > > > > (struct > > > > > stat) is unknown. > > > > > > > > > > So I think it is not a good idea to do that. > > > > > > > > Because Windows does not support symlink, so error_report_once() > > > > and report > > > it to user will be OK. > > > > But mapped-xattr could not work. > > > > > > Showing an error makes sense for "passthrough" security model, but > > > not for the "mapped" one. > > > > > > Just to avoid misapprehensions: are you aware that there is already > > > a system- agnostic implementation for symlinks if "mapped" is used? > > > > > > When mapped security model is enabled, then creating symlinks on > > > guest will simply create a corresponding *regular* file on host and > > > the content of that regular file on host is the pointing path as raw > > > string. Additionally the symlink flag is added to "virtfs.mode" > > > xattr to mark that regular file as a symlink, a virtual one that is. > > > So this does not require any support for symlinks by either the > underlying host file system, nor by host OS. > > > > > > Likewise interpreting and walking those virtual symlinks in "mapped" > > > mode is also implemented in the local fs dr
RE: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Wednesday, November 2, 2022 02:59
> To: qemu-devel@nongnu.org
> Cc: Greg Kurz ; Meng, Bin ; Shi,
> Guohuai
> Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features
> for Windows
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tuesday, November 1, 2022 4:34:54 PM CET Shi, Guohuai wrote:
> >
> > > -Original Message-
> > > From: Christian Schoenebeck
> > > Sent: Tuesday, November 1, 2022 23:04
> > > To: qemu-devel@nongnu.org
> > > Cc: Shi, Guohuai ; Greg Kurz
> > > ; Meng, Bin
> > > Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags and
> > > features for Windows
> > >
> > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > >
> > > On Monday, October 24, 2022 6:57:52 AM CET Bin Meng wrote:
> > > > From: Guohuai Shi
> > > >
> > > > Some flags and features are not supported on Windows, like mknod,
> > > > readlink, file mode, etc. Update the codes for Windows.
> > > >
> > > > Signed-off-by: Guohuai Shi
> > > > Signed-off-by: Bin Meng
> > > > ---
> > > >
> > > > hw/9pfs/9p-util.h | 6 +++-
> > > > hw/9pfs/9p.c | 90 ++-
> > > > 2 files changed, 86 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > > > 82b2d0c3e4..3d154e9103 100644
> > > > --- a/hw/9pfs/9p-util.h
> > > > +++ b/hw/9pfs/9p-util.h
> > > > @@ -53,8 +53,10 @@ static inline uint64_t makedev_dotl(uint32_t
> > > > dev_major,
> > > uint32_t dev_minor)
> > > > */
> > > > static inline uint64_t host_dev_to_dotl_dev(dev_t dev) { -#ifdef
> > > > CONFIG_LINUX
> > > > +#if defined(CONFIG_LINUX)
> > > > return dev;
> > > > +#elif defined(CONFIG_WIN32)
> > > > +return 0;
> > >
> > > Really?
> >
> > Check MS this document:
> > https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fsta
> > t-fstat32-fstat64-fstati64-fstat32i64-fstat64i32?view=msvc-170
> > st_rdev: If a device, fd; otherwise 0.
> > st_dev: If a device, fd; otherwise 0.
> >
> > So for any file open, it should be 0.
>
> Yeah, but that function translates a corresponding device ID for *Linux*
> guest side. And the intention is to avoid e.g. file ID collisions on guest
> side.
> Because for a Linux guest, the two-tuple (device number, inode number) makes
> a system-wide unique file ID.
>
> If you just return zero here, that might be OK if only one 9p directory is
> exported to guest, but say you have "C:\foo\" exported and "D:\bar\" exported
> and mounted via 9p to guest, then guest would assume every file with the same
> inode number on those two to be the same files. But they are not. They are on
> two different drives actually.
>
Got it.
Windows does not provide any numerical type ID for device,
I think the solution could be using driver letter ASC code "C:", "D:", etc.
> >
> > >
> > > > #else
> > > > return makedev_dotl(major(dev), minor(dev)); #endif @@
> > > > -260,7
> > > > +262,9 @@ static inline struct dirent *qemu_dirent_dup(struct
> > > > +dirent
> > > > *dent) #if defined CONFIG_DARWIN && defined
> > > > CONFIG_PTHREAD_FCHDIR_NP int pthread_fchdir_np(int fd)
> > > > __attribute__((weak_import)); #endif
> > > > +#ifndef CONFIG_WIN32
> > > > int qemu_mknodat(P9_FILE_ID dirfd, const char *filename, mode_t mode,
> > > > dev_t dev);
> > > > +#endif
> > > >
> > > > #endif
> > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index
> > > > 6c4af86240..771aab34ac
> > > > 100644
> > > > --- a/hw/9pfs/9p.c
> > > > +++ b/hw/9pfs/9p.c
> > > > @@ -39,6 +39,11 @@
> > > > #include "qemu/xxhash.h"
> > > > #include
> > > >
> > > > +#ifdef CONFIG_WIN32
> > > > +#define UTIME_NOW ((1l << 30) - 1l)
> > > > +#define UTIME_OMIT ((1l << 30) - 2l) #endif
> > > > +
> > > > int open_fd_hw;
> > > > int total_open_fd;
> > > > static int open_fd_rc;
> > > > @@ -132,13 +137,17 @@ s
RE: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities functions for 9pfs
> -Original Message- > From: Christian Schoenebeck > Sent: Wednesday, November 2, 2022 02:22 > To: qemu-devel@nongnu.org > Cc: Greg Kurz ; Meng, Bin ; Shi, > Guohuai > Subject: Re: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities > functions for 9pfs > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Tuesday, November 1, 2022 4:20:53 PM CET Shi, Guohuai wrote: > > > [...] > > > > > Windows POSIX API and MinGW library do not provide the NO_FOLLOW > > > > > flag, and do not allow opening a directory by POSIX open(). This > > > > > causes all > > > > > xxx_at() functions cannot work directly. However, we can provide > > > > > Windows handle based functions to emulate xxx_at() functions (e.g.: > > > > > openat_win32, utimensat_win32, etc.). > > > > > > > > > > Windows does not support extended attributes. 9pfs for Windows > > > > > uses NTFS ADS (Alternate Data Streams) to emulate extended > attributes. > > > > > > > > > > Windows does not provide POSIX compatible readlink(), and > > > > > symbolic link feature in 9pfs will be disabled on Windows. > > > > > > > > Wouldn't it be more user friendly if the relevant error locations > > > > would use something like error_report_once() and suggesting to > > > > enable > > > > mapped(-xattr) to make 9p symlinks on guest working if desired by the > user? > > > > > > > > Probably this error case would need to wrapped into a dedicated > > > > function, otherwise I guess error_report_once() would fire several > > > > times by different callers. > > > > > > > > > > Windows (MinGW) does not only support symlink, but also does not > > > have symlink definitions. > > > Windows does not support symlink flags S_IFLNK. > > > > > > So even I add symlink support by mapped-xattr, the MinGW library > > > does not have symlink flags and get a build error. > > > And this flags is defined by Windows header files. > > > The impact of adding a new flags to an pre-defined structure (struct > > > stat) is unknown. > > > > > > So I think it is not a good idea to do that. > > > > Because Windows does not support symlink, so error_report_once() and report > it to user will be OK. > > But mapped-xattr could not work. > > Showing an error makes sense for "passthrough" security model, but not for > the "mapped" one. > > Just to avoid misapprehensions: are you aware that there is already a system- > agnostic implementation for symlinks if "mapped" is used? > > When mapped security model is enabled, then creating symlinks on guest will > simply create a corresponding *regular* file on host and the content of that > regular file on host is the pointing path as raw string. Additionally the > symlink flag is added to "virtfs.mode" xattr to mark that regular file as a > symlink, a virtual one that is. So this does not require any support for > symlinks by either the underlying host file system, nor by host OS. > > Likewise interpreting and walking those virtual symlinks in "mapped" mode is > also implemented in the local fs driver already. Yes, symlink can be supported by "mapped" mode. I mean that MinGW does not provide symlink mode flags "S_IFLNK" and some other related functions and defines. You can check with "9p.c": S_ISLNK, S_ISSOCK and S_ISFIFO are not valid on Windows (MinGW) host. So even I enabled symlink supported by "mapped" mode on local-agent code, "9p.c" can not be built. So I disabled symlink totally, because MinGW interface does not support it. To resolve this issue, MinGW should add the missing defines at first. > > Best regards, > Christian Schoenebeck > Thanks Guohuai
RE: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities functions for 9pfs
> -Original Message-
> From: Shi, Guohuai
> Sent: Tuesday, November 1, 2022 23:13
> To: Christian Schoenebeck ; qemu-devel@nongnu.org
> Cc: Greg Kurz ; Meng, Bin
> Subject: RE: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities
> functions for 9pfs
>
>
>
> > -Original Message-
> > From: Christian Schoenebeck
> > Sent: Tuesday, November 1, 2022 22:28
> > To: qemu-devel@nongnu.org
> > Cc: Shi, Guohuai ; Greg Kurz
> > ; Meng, Bin
> > Subject: Re: [PATCH 07/16] hw/9pfs: Implement Windows specific
> > utilities functions for 9pfs
> >
> > [Please note: This e-mail is from an EXTERNAL e-mail address]
> >
> > On Monday, October 24, 2022 6:57:50 AM CET Bin Meng wrote:
> > > From: Guohuai Shi
> > >
> > > Windows POSIX API and MinGW library do not provide the NO_FOLLOW
> > > flag, and do not allow opening a directory by POSIX open(). This
> > > causes all
> > > xxx_at() functions cannot work directly. However, we can provide
> > > Windows handle based functions to emulate xxx_at() functions (e.g.:
> > > openat_win32, utimensat_win32, etc.).
> > >
> > > Windows does not support extended attributes. 9pfs for Windows uses
> > > NTFS ADS (Alternate Data Streams) to emulate extended attributes.
> > >
> > > Windows does not provide POSIX compatible readlink(), and symbolic
> > > link feature in 9pfs will be disabled on Windows.
> >
> > Wouldn't it be more user friendly if the relevant error locations
> > would use something like error_report_once() and suggesting to enable
> > mapped(-xattr) to make 9p symlinks on guest working if desired by the user?
> >
> > Probably this error case would need to wrapped into a dedicated
> > function, otherwise I guess error_report_once() would fire several
> > times by different callers.
> >
>
> Windows (MinGW) does not only support symlink, but also does not have symlink
> definitions.
> Windows does not support symlink flags S_IFLNK.
>
> So even I add symlink support by mapped-xattr, the MinGW library does not
> have symlink flags and get a build error.
> And this flags is defined by Windows header files.
> The impact of adding a new flags to an pre-defined structure (struct stat) is
> unknown.
>
> So I think it is not a good idea to do that.
Because Windows does not support symlink, so error_report_once() and report it
to user will be OK.
But mapped-xattr could not work.
>
> > > Signed-off-by: Guohuai Shi
> > > Signed-off-by: Bin Meng
> > > ---
> > >
> > > hw/9pfs/9p-local.h | 7 +
> > > hw/9pfs/9p-util.h | 40 +-
> > > hw/9pfs/9p-local.c | 4 -
> > > hw/9pfs/9p-util-win32.c | 885
> > >
> > > 4 files changed, 931 insertions(+), 5 deletions(-) create mode
> > > 100644 hw/9pfs/9p-util-win32.c
> > >
> > > diff --git a/hw/9pfs/9p-local.h b/hw/9pfs/9p-local.h index
> > > c8404063e5..02fd894ba3 100644
> > > --- a/hw/9pfs/9p-local.h
> > > +++ b/hw/9pfs/9p-local.h
> > > @@ -15,6 +15,13 @@
> > >
> > > #include "9p-file-id.h"
> > >
> > > +typedef struct {
> > > +P9_FILE_ID mountfd;
> > > +#ifdef CONFIG_WIN32
> > > +char *root_path;
> > > +#endif
> > > +} LocalData;
> > > +
> > > P9_FILE_ID local_open_nofollow(FsContext *fs_ctx, const char *path,
> > > int
> > flags,
> > > mode_t mode); P9_FILE_ID
> > > local_opendir_nofollow(FsContext *fs_ctx, const char *path); diff
> > > --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > > 1e7dc76345..82b2d0c3e4 100644
> > > --- a/hw/9pfs/9p-util.h
> > > +++ b/hw/9pfs/9p-util.h
> > > @@ -90,26 +90,61 @@ static inline int errno_to_dotl(int err) {
> > > return err;
> > > }
> > >
> > > -#ifdef CONFIG_DARWIN
> > > +#if defined(CONFIG_DARWIN)
> > > #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> > > +#elif defined(CONFIG_WIN32)
> > > +#define qemu_fgetxattr fgetxattr_win32
> > > #else
> > > #define qemu_fgetxattr fgetxattr
> > > #endif
> > >
> > > +#ifdef CONFIG_WIN32
> > > +#define qemu_openat openat_win32
> > > +#define qemu_fstatatfstatat_win32
> > > +#define qemu_mkdiratmkdirat_win32
> > > +#define qemu_renameat renameat_win32
> > > +#define qemu_utimensat utime
RE: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features for Windows
> -Original Message-
> From: Christian Schoenebeck
> Sent: Tuesday, November 1, 2022 23:04
> To: qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Greg Kurz ;
> Meng, Bin
> Subject: Re: [PATCH 09/16] hw/9pfs: Disable unsupported flags and features
> for Windows
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Monday, October 24, 2022 6:57:52 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > Some flags and features are not supported on Windows, like mknod,
> > readlink, file mode, etc. Update the codes for Windows.
> >
> > Signed-off-by: Guohuai Shi
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/9pfs/9p-util.h | 6 +++-
> > hw/9pfs/9p.c | 90 ++-
> > 2 files changed, 86 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > 82b2d0c3e4..3d154e9103 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -53,8 +53,10 @@ static inline uint64_t makedev_dotl(uint32_t dev_major,
> uint32_t dev_minor)
> > */
> > static inline uint64_t host_dev_to_dotl_dev(dev_t dev) { -#ifdef
> > CONFIG_LINUX
> > +#if defined(CONFIG_LINUX)
> > return dev;
> > +#elif defined(CONFIG_WIN32)
> > +return 0;
>
> Really?
Check MS this document:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fstat-fstat32-fstat64-fstati64-fstat32i64-fstat64i32?view=msvc-170
st_rdev: If a device, fd; otherwise 0.
st_dev: If a device, fd; otherwise 0.
So for any file open, it should be 0.
>
> > #else
> > return makedev_dotl(major(dev), minor(dev)); #endif @@ -260,7
> > +262,9 @@ static inline struct dirent *qemu_dirent_dup(struct dirent
> > *dent) #if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> > int pthread_fchdir_np(int fd) __attribute__((weak_import)); #endif
> > +#ifndef CONFIG_WIN32
> > int qemu_mknodat(P9_FILE_ID dirfd, const char *filename, mode_t mode,
> > dev_t dev);
> > +#endif
> >
> > #endif
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 6c4af86240..771aab34ac
> > 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -39,6 +39,11 @@
> > #include "qemu/xxhash.h"
> > #include
> >
> > +#ifdef CONFIG_WIN32
> > +#define UTIME_NOW ((1l << 30) - 1l)
> > +#define UTIME_OMIT ((1l << 30) - 2l) #endif
> > +
> > int open_fd_hw;
> > int total_open_fd;
> > static int open_fd_rc;
> > @@ -132,13 +137,17 @@ static int dotl_to_open_flags(int flags)
> > DotlOpenflagMap dotl_oflag_map[] = {
> > { P9_DOTL_CREATE, O_CREAT },
> > { P9_DOTL_EXCL, O_EXCL },
> > +#ifndef CONFIG_WIN32
> > { P9_DOTL_NOCTTY , O_NOCTTY },
> > +#endif
> > { P9_DOTL_TRUNC, O_TRUNC },
> > { P9_DOTL_APPEND, O_APPEND },
> > +#ifndef CONFIG_WIN32
> > { P9_DOTL_NONBLOCK, O_NONBLOCK } ,
> > { P9_DOTL_DSYNC, O_DSYNC },
> > { P9_DOTL_FASYNC, FASYNC },
> > -#ifndef CONFIG_DARWIN
> > +#endif
> > +#ifdef CONFIG_LINUX
>
> Better
>
>#if !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
>
It is OK.
> Otherwise it might automatically opt-out other future platforms
> unintentionally.
>
> > { P9_DOTL_NOATIME, O_NOATIME },
> > /*
> > * On Darwin, we could map to F_NOCACHE, which is @@ -151,8
> > +160,10 @@ static int dotl_to_open_flags(int flags) #endif
> > { P9_DOTL_LARGEFILE, O_LARGEFILE },
> > { P9_DOTL_DIRECTORY, O_DIRECTORY },
> > +#ifndef CONFIG_WIN32
> > { P9_DOTL_NOFOLLOW, O_NOFOLLOW },
> > { P9_DOTL_SYNC, O_SYNC },
> > +#endif
> > };
> >
> > for (i = 0; i < ARRAY_SIZE(dotl_oflag_map); i++) { @@ -179,8
> > +190,11 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
> > * Filter the client open flags
> > */
> > flags = dotl_to_open_flags(oflags);
> > -flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
> > -#ifndef CONFIG_DARWIN
> > +flags &= ~(O_CREAT);
> > +#ifndef CONFIG_WIN32
> > +flags &= ~(O_NOCTTY | O_ASYNC);
> > +#endif
> > +#ifdef CONFIG_LINUX
>
> Same as above: better explicitly opt-out than the other way around.
>
It is OK.
> > /*
> > * Ignore direct disk access hint until the server supports it.
> > */
> > @@ -986,9 +1000,11 @@
RE: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities functions for 9pfs
> -Original Message-
> From: Christian Schoenebeck
> Sent: Tuesday, November 1, 2022 22:28
> To: qemu-devel@nongnu.org
> Cc: Shi, Guohuai ; Greg Kurz ;
> Meng, Bin
> Subject: Re: [PATCH 07/16] hw/9pfs: Implement Windows specific utilities
> functions for 9pfs
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Monday, October 24, 2022 6:57:50 AM CET Bin Meng wrote:
> > From: Guohuai Shi
> >
> > Windows POSIX API and MinGW library do not provide the NO_FOLLOW flag,
> > and do not allow opening a directory by POSIX open(). This causes all
> > xxx_at() functions cannot work directly. However, we can provide
> > Windows handle based functions to emulate xxx_at() functions (e.g.:
> > openat_win32, utimensat_win32, etc.).
> >
> > Windows does not support extended attributes. 9pfs for Windows uses
> > NTFS ADS (Alternate Data Streams) to emulate extended attributes.
> >
> > Windows does not provide POSIX compatible readlink(), and symbolic
> > link feature in 9pfs will be disabled on Windows.
>
> Wouldn't it be more user friendly if the relevant error locations would use
> something like error_report_once() and suggesting to enable mapped(-xattr) to
> make 9p symlinks on guest working if desired by the user?
>
> Probably this error case would need to wrapped into a dedicated function,
> otherwise I guess error_report_once() would fire several times by different
> callers.
>
Windows (MinGW) does not only support symlink, but also does not have symlink
definitions.
Windows does not support symlink flags S_IFLNK.
So even I add symlink support by mapped-xattr, the MinGW library does not have
symlink flags and get a build error.
And this flags is defined by Windows header files.
The impact of adding a new flags to an pre-defined structure (struct stat) is
unknown.
So I think it is not a good idea to do that.
> > Signed-off-by: Guohuai Shi
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/9pfs/9p-local.h | 7 +
> > hw/9pfs/9p-util.h | 40 +-
> > hw/9pfs/9p-local.c | 4 -
> > hw/9pfs/9p-util-win32.c | 885
> >
> > 4 files changed, 931 insertions(+), 5 deletions(-) create mode
> > 100644 hw/9pfs/9p-util-win32.c
> >
> > diff --git a/hw/9pfs/9p-local.h b/hw/9pfs/9p-local.h index
> > c8404063e5..02fd894ba3 100644
> > --- a/hw/9pfs/9p-local.h
> > +++ b/hw/9pfs/9p-local.h
> > @@ -15,6 +15,13 @@
> >
> > #include "9p-file-id.h"
> >
> > +typedef struct {
> > +P9_FILE_ID mountfd;
> > +#ifdef CONFIG_WIN32
> > +char *root_path;
> > +#endif
> > +} LocalData;
> > +
> > P9_FILE_ID local_open_nofollow(FsContext *fs_ctx, const char *path, int
> flags,
> > mode_t mode); P9_FILE_ID
> > local_opendir_nofollow(FsContext *fs_ctx, const char *path); diff
> > --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index
> > 1e7dc76345..82b2d0c3e4 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -90,26 +90,61 @@ static inline int errno_to_dotl(int err) {
> > return err;
> > }
> >
> > -#ifdef CONFIG_DARWIN
> > +#if defined(CONFIG_DARWIN)
> > #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> > +#elif defined(CONFIG_WIN32)
> > +#define qemu_fgetxattr fgetxattr_win32
> > #else
> > #define qemu_fgetxattr fgetxattr
> > #endif
> >
> > +#ifdef CONFIG_WIN32
> > +#define qemu_openat openat_win32
> > +#define qemu_fstatatfstatat_win32
> > +#define qemu_mkdiratmkdirat_win32
> > +#define qemu_renameat renameat_win32
> > +#define qemu_utimensat utimensat_win32
> > +#define qemu_unlinkat unlinkat_win32
> > +#else
> > #define qemu_openat openat
> > #define qemu_fstatatfstatat
> > #define qemu_mkdiratmkdirat
> > #define qemu_renameat renameat
> > #define qemu_utimensat utimensat
> > #define qemu_unlinkat unlinkat
> > +#endif
> > +
> > +#ifdef CONFIG_WIN32
> > +char *get_full_path_win32(P9_FILE_ID fd, const char *name); ssize_t
> > +fgetxattr_win32(int fd, const char *name, void *value, size_t size);
> > +P9_FILE_ID openat_win32(P9_FILE_ID dirfd, const char *pathname, int flags,
> > +mode_t mode); int fstatat_win32(P9_FILE_ID
> > +dirfd, const char *pathname,
> > + struct stat *statbuf, int flags); int
> > +mkdirat_win32(P9_FILE_ID dirfd, const char *pathname, mode_t mode);
> > +int renameat_win32(P9_FILE_ID olddirfd,
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
> -Original Message- > From: Greg Kurz > Sent: 2022年5月11日 20:19 > To: Shi, Guohuai > Cc: Christian Schoenebeck ; qemu-devel@nongnu.org; > Meng, > Bin ; Bin Meng > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Tue, 10 May 2022 15:35:10 + > "Shi, Guohuai" wrote: > > > Let's force on the security issue: > > > > Please don't top post, especially when all previous comments were made inline, > so that someone who steps in this thread now has a chance to catch-up. > > > Firstly, this answer > ( https://stackoverflow.com/questions/32138524/is-there-a-windows-equivalent- > of-openat ) is useless for QEMU. > > It uses Windows native API NtCreateFile() and accesses files by Windows > > handle. > > But 9PFS is using Windows POSIX interface, handle can not be used in POSIX > interface. > > Actually, Windows provide similar APIs like > GetFinalPathNameByHandle()/GetFileInformationByHandle(). > > It can also get file information by Windows handle. > > > > Windows POSIX interface do not support NO_FOLLOW flags, that means, Windows > > POSIX > open() always translate symbolic link. > > > > This precludes any tentative to fix the issue at the QEMU level then. > Maybe there are some knobs to control symlink behavior at the fs level but > this > certainly requires windows knowledge that I don't have. > > > So everything are finally point to one limitation: Windows POSIX interfaces > > do > not support symbolic link and always translate link. > > > > For the security reason, I think it is reasonable to disable symbolic link > > support > on Windows host for 9PFS. > > I can re-work this patch to adding a symbolic link check during path-walk > > operation > and stop it when get a symbolic link. > > > > This would be useless because of TOCTOU : a directory could be replaced by a > symlink > between the check and the actual use of the file. O_NOFOLLOW provides the > atomicity > needed to safely error out on symlinks. Since O_NOFOLLOW only makes sense for > the > rightmost path element, paths from the client have to be broken down into a > succession of *at() syscalls, one for each element. For Windows file system, it would be OK. Windows can not delete a opening file (this is different behavior between Windows file system driver and UNIX-like-inode-based file system). So when 9PFS try to open the final file, the following steps will keep it safe: 1. open the final file by Windows NT APIs and keep the open handle. 2. open the final file by MinGW open(). 3. close NT handle. Windows file system does not allow delete/rename/move a opening file. Even Windows provide "FILE_SHARE_DELETE" flag in its NT API CreateFile(). Windows allow to delete the opening file, but can not re-create same name. The following steps will be failure on Windows: 1. Open a directory by CreateFile() with "FILE_SHARE_DELETE" flag and keep the handle open. 2. Remove the directory. 3. Re-create same name directory/file/links. Windows will get failure on step #3. So I think checking if there is a link in filename would be safety on Window host. Best Regards, Guohuai > > > Best Regards, > > Guohuai > > > > > -Original Message- > > > From: Greg Kurz > > > Sent: 2022年5月10日 22:35 > > > To: Christian Schoenebeck > > > Cc: qemu-devel@nongnu.org; Meng, Bin ; Bin > > > Meng ; Shi, Guohuai > > > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend > > > driver for Windows > > > > > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > > > > > On Tue, 10 May 2022 16:04:28 +0200 > > > Christian Schoenebeck wrote: > > > > > > > On Dienstag, 10. Mai 2022 15:40:06 CEST Greg Kurz wrote: > > > > > On Tue, 10 May 2022 13:54:46 +0200 > > > > > > > > > > Christian Schoenebeck wrote: > > > > > > On Dienstag, 10. Mai 2022 12:18:33 CEST Christian Schoenebeck wrote: > > > > > > > On Dienstag, 10. Mai 2022 04:17:44 CEST Shi, Guohuai wrote: > > > > > > > [...] > > > > > > > > > > > > > > > > > > > I tend to agree with Christian's remarks that this > > > > > > > > > > > > patch is too big and that the choice of > > > > > > > > > > > > introducing right away a new implementation of > > > > &
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
Let's force on the security issue: Firstly, this answer ( https://stackoverflow.com/questions/32138524/is-there-a-windows-equivalent-of-openat ) is useless for QEMU. It uses Windows native API NtCreateFile() and accesses files by Windows handle. But 9PFS is using Windows POSIX interface, handle can not be used in POSIX interface. Actually, Windows provide similar APIs like GetFinalPathNameByHandle()/GetFileInformationByHandle(). It can also get file information by Windows handle. Windows POSIX interface do not support NO_FOLLOW flags, that means, Windows POSIX open() always translate symbolic link. So everything are finally point to one limitation: Windows POSIX interfaces do not support symbolic link and always translate link. For the security reason, I think it is reasonable to disable symbolic link support on Windows host for 9PFS. I can re-work this patch to adding a symbolic link check during path-walk operation and stop it when get a symbolic link. Best Regards, Guohuai > -Original Message- > From: Greg Kurz > Sent: 2022年5月10日 22:35 > To: Christian Schoenebeck > Cc: qemu-devel@nongnu.org; Meng, Bin ; Bin Meng > ; Shi, Guohuai > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Tue, 10 May 2022 16:04:28 +0200 > Christian Schoenebeck wrote: > > > On Dienstag, 10. Mai 2022 15:40:06 CEST Greg Kurz wrote: > > > On Tue, 10 May 2022 13:54:46 +0200 > > > > > > Christian Schoenebeck wrote: > > > > On Dienstag, 10. Mai 2022 12:18:33 CEST Christian Schoenebeck wrote: > > > > > On Dienstag, 10. Mai 2022 04:17:44 CEST Shi, Guohuai wrote: > > > > > [...] > > > > > > > > > > > > > > > I tend to agree with Christian's remarks that this > > > > > > > > > > patch is too big and that the choice of introducing > > > > > > > > > > right away a new implementation of 9p-local for > > > > > > > > > > windows hosts is too bold to start with. We need to > > > > > > > > > > clearly understand what's diverging between windows > > > > > > > > > > and linux in order to make such a decision. You should > > > > > > > > > > first try to introduce the required abstractions to > > > > > > > > > > cope with these differences, so that we can review. > > > > > > > > > > > > > > > > > > Here is the basic introductions of 9PFS for Windows > > > > > > > > > development: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Windows always returns -1 when try to call open() for a > > > > > > > > > directory. > > > > > > > > > Windows (actually MinGW library) only allows opendir() > > > > > > > > > for a directory. > > > > > > > > > > That missing behaviour could be implemented in 9p-util-win.c, > > > > > similar to the missing behaviours of mknodat() for macOS which > > > > > did not support a bunch of things like creating a UNIX socket file and > more: > > > > > > > > > > https://github.com/qemu/qemu/commit/055ab89327bab83f1bd07e9de07f > > > > > 7628643d > > > > > 3d8d> > > > > > > > > > Does MinGW have dirfd() ? > > > > > > > > > > > > > > No. > > > > > > > MinGW does not open any directory. > > > > > > > Here is opendir() source code of MinGW: > > > > > > > https://github.com/mirror/mingw-w64/blob/master/mingw-w64-cr > > > > > > > t/misc/d > > > > > > > iren > > > > > > > t. > > > > > > > c#L42 > > > > > > > > > > > > > > So MinGW do not have a fd associated to a directory. > > > > > > > > > > > > > > > > Windows does not support APIs like "*at" (openat(), > > > > > > > > > renameat(), > > > > > > > > > etc.) > > > > > > > > > > Like already suggested before on your previous RFC version, it > > > > > is possible to use the same workaround as we are using for macOS > > > > > hosts already (which > >
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
> -Original Message- > From: Greg Kurz > Sent: 2022年5月10日 0:20 > To: Shi, Guohuai > Cc: Bin Meng ; Christian Schoenebeck > ; > qemu-devel@nongnu.org; Meng, Bin > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Mon, 9 May 2022 15:09:46 + > "Shi, Guohuai" wrote: > > > > > > > > -Original Message- > > > From: Greg Kurz > > > Sent: 2022年5月9日 22:29 > > > To: Bin Meng > > > Cc: Christian Schoenebeck ; qemu-devel@nongnu.org; > Shi, > > > Guohuai ; Meng, Bin > > > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver > for > > > Windows > > > > > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > > > > > On Mon, 25 Apr 2022 22:27:01 +0800 > > > Bin Meng wrote: > > > > > > > From: Guohuai Shi > > > > > > > > Add a 9p local file system backend driver to support Windows, > > > > including open, read, write, close, rename, remove, etc. > > > > > > > > All security models are supported. The mapped (mapped-xattr) > > > > security model is implemented using NTFS Alternate Data Stream > > > > (ADS) so the 9p export path shall be on an NTFS partition. > > > > > > > > Signed-off-by: Guohuai Shi > > > > Signed-off-by: Bin Meng > > > > --- > > > > > > > > > > Hi ! > > > > > > I tend to agree with Christian's remarks that this patch is too big > > > and that the choice of introducing right away a new implementation > > > of 9p-local for windows hosts is too bold to start with. We need to > > > clearly understand what's diverging between windows and linux in order > > > to make such a decision. You should first try to introduce the required > > > abstractions to cope with these differences, so that we can review. > > > > > > > Here is the basic introductions of 9PFS for Windows development: > > > > Windows always returns -1 when try to call open() for a directory. > > Windows (actually MinGW library) only allows opendir() for a directory. > > Does MinGW have dirfd() ? No. MinGW does not open any directory. Here is opendir() source code of MinGW: https://github.com/mirror/mingw-w64/blob/master/mingw-w64-crt/misc/dirent.c#L42 So MinGW do not have a fd associated to a directory. > > > Windows does not support APIs like "*at" (openat(), renameat(), etc.) > > Ouch... > > > So 9PFS can not use any openat() for opening a sub file or directory in 9P > > mount > directory. > > This commit use merge_fs_path() to build up full filename by string > > concatenation. > > I know that may have a risk of security, but Windows does fully support > > POSIX > APIs. > > > > s/does/doesn't ? > > I understand from your various answers that symlinks aren't > currently supported by window's POSIX API. Is this forever ? > Google do mentions symlinks in windows 10. What's the story > there ? How do they behave ? How would they be exposed to the > client ? Be aware that, even if the client cannot create symlinks, > an existing symlink could be used to escape with rename(). > > If the code "may have a risk of security" then it must be > fixed or avoided in some way before being merged upstream. > > Other thing that comes to mind is that windows hosts should > maybe use the mapped or mapped-file security modes since > they emulate symlinks with a simple file hidden in the > VIRTFS_META_DIR directory. > > Cheers, > > -- > Greg > Windows native API support symbolic link file start from Windows Vista: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createsymboliclinka I mean Windows POSIX APIs do not support symbolic link (MinGW use Win32 POSIX APIs) So we can not create symbolic link by MinGW. Anyway, there is another solution: re-work whole 9PFS code: not only 9p-local.c, but also every file in 9p driver. Replace every MinGW/POSIX APIs (e.g. open, lseek, read, write, close), by Windows Native APIs (e.g. open -> CreateFile, lseek -> SetFilePointer, read -> ReadFile, write -> WriteFile, close -> CloseHandle, etc.) Then 9P can use Windows symbolic link feature. However, I do think it is a good idea to replace everything. Thanks Guohuai > > > Some inlined remarks below anyway. > > > > > > > hw/9pfs/9p-linux-errno.h | 151 + > > > > hw/9pfs/9
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
> -Original Message- > From: Greg Kurz > Sent: 2022年5月9日 22:29 > To: Bin Meng > Cc: Christian Schoenebeck ; qemu-devel@nongnu.org; > Shi, > Guohuai ; Meng, Bin > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Mon, 25 Apr 2022 22:27:01 +0800 > Bin Meng wrote: > > > From: Guohuai Shi > > > > Add a 9p local file system backend driver to support Windows, > > including open, read, write, close, rename, remove, etc. > > > > All security models are supported. The mapped (mapped-xattr) > > security model is implemented using NTFS Alternate Data Stream > > (ADS) so the 9p export path shall be on an NTFS partition. > > > > Signed-off-by: Guohuai Shi > > Signed-off-by: Bin Meng > > --- > > > > Hi ! > > I tend to agree with Christian's remarks that this patch is too big > and that the choice of introducing right away a new implementation > of 9p-local for windows hosts is too bold to start with. We need to > clearly understand what's diverging between windows and linux in order > to make such a decision. You should first try to introduce the required > abstractions to cope with these differences, so that we can review. > Here is the basic introductions of 9PFS for Windows development: Windows always returns -1 when try to call open() for a directory. Windows (actually MinGW library) only allows opendir() for a directory. Windows does not support APIs like "*at" (openat(), renameat(), etc.) So 9PFS can not use any openat() for opening a sub file or directory in 9P mount directory. This commit use merge_fs_path() to build up full filename by string concatenation. I know that may have a risk of security, but Windows does fully support POSIX APIs. > Some inlined remarks below anyway. > > > hw/9pfs/9p-linux-errno.h | 151 + > > hw/9pfs/9p-local.h |4 + > > hw/9pfs/9p-util.h| 41 ++ > > hw/9pfs/9p.h | 23 + > > hw/9pfs/9p-local-win32.c | 1242 ++ > > hw/9pfs/9p-util-win32.c | 303 ++ > > hw/9pfs/9p-xattr.c | 113 > > hw/9pfs/9p.c | 91 ++- > > hw/9pfs/codir.c | 15 + > > 9 files changed, 1982 insertions(+), 1 deletion(-) > > create mode 100644 hw/9pfs/9p-linux-errno.h > > create mode 100644 hw/9pfs/9p-local-win32.c > > create mode 100644 hw/9pfs/9p-util-win32.c > > > > diff --git a/hw/9pfs/9p-linux-errno.h b/hw/9pfs/9p-linux-errno.h > > new file mode 100644 > > index 00..b0d6ac45ac > > --- /dev/null > > +++ b/hw/9pfs/9p-linux-errno.h > > @@ -0,0 +1,151 @@ > > +/* > > + * 9p Linux errno translation definition > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or > > later. > > + * See the COPYING file in the top-level directory. > > + */ > > + > > +#include > > + > > +#ifndef QEMU_9P_LINUX_ERRNO_H > > +#define QEMU_9P_LINUX_ERRNO_H > > + > > +/* > > + * This file contains the Linux errno definitions to translate errnos set > > by > > + * the 9P server (running on Windows) to a corresponding errno value. > > + * > > + * This list should be periodically reviewed and updated; particularly for > > + * errnos that might be set as a result of a file system operation. > > + */ > > + > > +#define L_EPERM 1 /* Operation not permitted */ > > +#define L_ENOENT2 /* No such file or directory */ > > +#define L_ESRCH 3 /* No such process */ > > +#define L_EINTR 4 /* Interrupted system call */ > > +#define L_EIO 5 /* I/O error */ > > +#define L_ENXIO 6 /* No such device or address */ > > +#define L_E2BIG 7 /* Argument list too long */ > > +#define L_ENOEXEC 8 /* Exec format error */ > > +#define L_EBADF 9 /* Bad file number */ > > +#define L_ECHILD10 /* No child processes */ > > +#define L_EAGAIN11 /* Try again */ > > +#define L_ENOMEM12 /* Out of memory */ > > +#define L_EACCES13 /* Permission denied */ > > +#define L_EFAULT14 /* Bad address */ > > +#define L_ENOTBLK 15 /* Block device required */ > > +#define L_EBUSY 16 /* Device or resource busy */ > > +#define L_EEXIST17 /* File exists */ > > +#define L_EXDEV 18 /* Cross-device link */ > > +#define L_ENODEV
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
> -Original Message- > From: Christian Schoenebeck > Sent: 2022年5月5日 19:44 > To: qemu-devel@nongnu.org; Shi, Guohuai ; Greg Kurz > > Cc: Meng, Bin ; Bin Meng > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Mittwoch, 4. Mai 2022 21:34:22 CEST Shi, Guohuai wrote: > [...] > > > > index 00..aab7c9f7b5 > > > > --- /dev/null > > > > +++ b/hw/9pfs/9p-local-win32.c > > > > @@ -0,0 +1,1242 @@ > > > > +/* > > > > + * 9p Windows callback > > > > + * > > > > + * Copyright (c) 2022 Wind River Systems, Inc. > > > > + * > > > > + * Based on hw/9pfs/9p-local.c > > > > + * > > > > + * This work is licensed under the terms of the GNU GPL, version 2. > > > > See > > > > + * the COPYING file in the top-level directory. > > > > + */ > > > > + > > > > +/* > > > > + * Not so fast! You might want to read the 9p developer docs first: > > > > + * https://wiki.qemu.org/Documentation/9p > > > > + */ > > > > + > > > > +#include "qemu/osdep.h" > > > > +#include > > > > +#include > > > > +#include "9p.h" > > > > +#include "9p-local.h" > > > > +#include "9p-xattr.h" > > > > +#include "9p-util.h" > > > > +#include "fsdev/qemu-fsdev.h" /* local_ops */ > > > > +#include "qapi/error.h" > > > > +#include "qemu/cutils.h" > > > > +#include "qemu/error-report.h" > > > > +#include "qemu/option.h" > > > > +#include > > > > > > I'm not sure whether all of this (i.e. 9p-local-win32.c in general) > > > is really needed. I mean yes, it probably does the job, but you > > > basically add a complete separate 'local' backend implementation > > > just for the Windows host platform. > > > > > > My expectation would be to stick to 9p-local.c being OS-agnostic, > > > and only define OS-specific functionality in 9p-util-windows.c if needed. > > > > > > And most importantly: don't duplicate code as far as possible! I > > > mean somebody would need to also review and maintain all of this. > > > > Actually, almost all FileOperations functions are re-written for > > Windows host. > > > > Here is the comparison statistics for 9p-local.c and 9p-local-win32.c: > > Total lines : 1611 > > Changed lines: 590 > > Inserted lines: 138 > > Removed lines: 414 > > > > For function level difference count: > > > > Changed function: 39 > > Unchanged function: 13 > > > > If use "#ifdef _WIN32" to sperate Windows host code, it may need to be > > inserted about 150 code blocks (or 39 code blocks for all changed > > functions). > > > > I am not sure if it is a good idea to insert so many "#ifdef _WIN32", > > it may cause this file not readable. > > > > If stick to 9p-local.c being OS-agnostic, I think it is better to > > create two new files: 9p-local-linux.c and 9p-local-win32.c > > The thing is, as this is presented right now, I can hardly even see where > deviating > behaviour for Windows would be, where not, and I'm missing any explanations > and > reasons for the individual deviations right now. Chances are that you are > unnecessarilly adding duplicate code and unnecessary code deviations. For me > this > 9p-local-win32.c approach looks overly complex and not appropriately > abstracted > on first sight. > > I suggest waiting for Greg to give his opinion on this as well before > continuing. > > Best regards, > Christian Schoenebeck > I can make this commit to be split into several commits: Commit #1: only make Windows host built successfully. Commit #2: provide some basic file system operations and leave other functions not workable (return -1). Commit #3: provide full file system operations. Commit #4: provide security mode "mapped" by NTFS ADS Commit #5: provide security mode "mapped-file" Commit #6: fix Windows (MinGW) directory access API compatible issues However, I think I may still need a standalone file "9p-local-win32.c". Some functions could be moved (or merged) back to 9p-local.c. But there are still some functions are too complex to be merged. Thanks Guohuai
RE: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for Windows
> -Original Message- > From: Christian Schoenebeck > Sent: Thursday, May 5, 2022 02:02 > To: qemu-devel@nongnu.org > Cc: Greg Kurz ; Meng, Bin ; Shi, > Guohuai ; Bin Meng > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver > for Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Montag, 25. April 2022 16:27:01 CEST Bin Meng wrote: > > From: Guohuai Shi > > > > Add a 9p local file system backend driver to support Windows, > > including open, read, write, close, rename, remove, etc. > > > > All security models are supported. The mapped (mapped-xattr) > > security model is implemented using NTFS Alternate Data Stream > > (ADS) so the 9p export path shall be on an NTFS partition. > > > > Signed-off-by: Guohuai Shi > > Signed-off-by: Bin Meng > > --- > > > > hw/9pfs/9p-linux-errno.h | 151 + > > hw/9pfs/9p-local.h |4 + > > hw/9pfs/9p-util.h| 41 ++ > > hw/9pfs/9p.h | 23 + > > hw/9pfs/9p-local-win32.c | 1242 ++ > > hw/9pfs/9p-util-win32.c | 303 ++ > > hw/9pfs/9p-xattr.c | 113 > > hw/9pfs/9p.c | 91 ++- > > hw/9pfs/codir.c | 15 + > > 9 files changed, 1982 insertions(+), 1 deletion(-) > > create mode 100644 hw/9pfs/9p-linux-errno.h > > create mode 100644 hw/9pfs/9p-local-win32.c > > create mode 100644 hw/9pfs/9p-util-win32.c > > This patch is definitely too huge and should be split up into a huge bunch of > separate patches! > > > diff --git a/hw/9pfs/9p-linux-errno.h b/hw/9pfs/9p-linux-errno.h > > new file mode 100644 > > index 00..b0d6ac45ac > > --- /dev/null > > +++ b/hw/9pfs/9p-linux-errno.h > > This file definitely deserves a patch on its own. > > As for its filename: Following our current filename scheme, it would probably > be better be named 9p-errno-linux.h or 9p-errno-dotl.h as this is probably a > 9p protocol version thing. > > > @@ -0,0 +1,151 @@ > > +/* > > + * 9p Linux errno translation definition > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or > > later. + * See the COPYING file in the top-level directory. > > + */ > > + > > +#include > > + > > +#ifndef QEMU_9P_LINUX_ERRNO_H > > +#define QEMU_9P_LINUX_ERRNO_H > > + > > +/* > > + * This file contains the Linux errno definitions to translate errnos set > > by + * the 9P server (running on Windows) to a corresponding errno value. + > > * > > + * This list should be periodically reviewed and updated; particularly for > > + * errnos that might be set as a result of a file system operation. > > + */ > > + > > I would just import the already existing sys/errno.h from the Linux kernel, > with all its copyright header etc. and then with a 2nd patch just prefix the > individual macros with DOTL_* > > > +#define L_EPERM 1 /* Operation not permitted */ > > +#define L_ENOENT2 /* No such file or directory */ > > +#define L_ESRCH 3 /* No such process */ > > +#define L_EINTR 4 /* Interrupted system call */ > > +#define L_EIO 5 /* I/O error */ > > +#define L_ENXIO 6 /* No such device or address */ > > +#define L_E2BIG 7 /* Argument list too long */ > > +#define L_ENOEXEC 8 /* Exec format error */ > > +#define L_EBADF 9 /* Bad file number */ > > +#define L_ECHILD10 /* No child processes */ > > +#define L_EAGAIN11 /* Try again */ > > +#define L_ENOMEM12 /* Out of memory */ > > +#define L_EACCES13 /* Permission denied */ > > +#define L_EFAULT14 /* Bad address */ > > +#define L_ENOTBLK 15 /* Block device required */ > > +#define L_EBUSY 16 /* Device or resource busy */ > > +#define L_EEXIST17 /* File exists */ > > +#define L_EXDEV 18 /* Cross-device link */ > > +#define L_ENODEV19 /* No such device */ > > +#define L_ENOTDIR 20 /* Not a directory */ > > +#define L_EISDIR21 /* Is a directory */ > > +#define L_EINVAL22 /* Invalid argument */ > > +#define L_ENFILE23 /* File table overflow */ > > +#define L_EMFILE24 /* Too many open files */ > > +#define L_ENOTTY25 /* Not a typewriter */ > > +#define L_ETXTBSY 26 /* Text file busy */ > &g
RE: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host
Hi All,
I just finished a basic PoC for mapped-file.
It works! I think I can also support "security_model=map-xattr" by NTFS ADS.
However, I got a limitation of MinGW:
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-crt/misc/dirent.c#L290
MinGW can not handle seekdir() while the directory has changed.
That means, if run command like "rm -rf *", MinGW may not delete all files.
"rm -rf *" need to call readdir()(and call seekdir() in 9P) and unlink(),
and MinGW's seekdir() may seek directory to wrong directory entry index.
I am considering to change 9PFS readdir() implement on Windows host, to fix
this problem.
Thanks,
Guohuai
-Original Message-
From: Mark Cave-Ayland
Sent: Tuesday, April 19, 2022 18:59
To: Christian Schoenebeck
Cc: Shi, Guohuai ; qemu-devel@nongnu.org; Bin Meng
; Greg Kurz
Subject: Re: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host
[Please note: This e-mail is from an EXTERNAL e-mail address]
On 18/04/2022 13:31, Christian Schoenebeck wrote:
> On Montag, 18. April 2022 11:07:33 CEST Mark Cave-Ayland wrote:
>> On 17/04/2022 13:55, Christian Schoenebeck wrote:
>>> On Donnerstag, 14. April 2022 19:25:04 CEST Shi, Guohuai wrote:
>>>>> -Original Message-
>>>>> From: Christian Schoenebeck
>>>>> Sent: 2022年4月14日 19:24
>>>>> To: qemu-devel@nongnu.org; Shi, Guohuai
>>>>>
>>>>> Cc: Bin Meng ; Greg Kurz
>>>>> Subject: Re: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows
>>>>> host
>>>>>
>>>>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>>>>
>>>>> On Mittwoch, 13. April 2022 05:30:57 CEST Shi, Guohuai wrote:
>>>>>>> We have 3 fs drivers: local, synth, proxy. I don't mind about
>>>>>>> proxy, it is in bad shape and we will probably deprecate it in
>>>>>>> near future anyway. But it would be good to have support for the
>>>>>>> synth driver, because we are using it for running test cases and
>>>>>>> fuzzing tests (QA).
>>>
>>> [...]
>>>
>>>> For 9p-synth:
>>>>
>>>> I had enabled 9p-synth.c and built it successfully on Windows platform.
>>>> However, test cases code are not built on Windows host.
>>>> So I think it is useless that enable synth on Windows host (no way
>>>> to run it).
>>>
>>> Please, don't give up too soon. Looking at tests/qtest/meson.build
>>> it starts with:
>>>
>>> # All QTests for now are POSIX-only, but the dependencies are #
>>> really in libqtest, not in the testcases themselves.
>>> if not config_host.has_key('CONFIG_POSIX')
>>>
>>> subdir_done()
>>>
>>> endif
>>>
>>> And looking at tests/qtest/libqtest.c I "think" this should be
>>> working on Windows as well. It uses socket APIs which are available
>>> on Windows. I don't see a real show stopper here for Windows.
>>>
>>> Could you please try if you can compile the tests on Windows? What
>>> we would need is test/qtest/qos-test, we don't need all the other
>>> tests:
>>>
>>> https://wiki.qemu.org/Documentation/9p#Test_Cases
>>>
>>>>>> It is possible that to "map" extend attribute to NTFS stream data.
>>>>>> However, if Windows host media is not NTFS (e.g. FAT) which does
>>>>>> not support stream data, then the "map" can not work.
>>>>>
>>>>> ... yes exactly, it would make sense to use ADS [4] instead of
>>>>> xattr on Windows. ADS are available with NTFS and ReFS and maybe
>>>>> also with exFAT nowadays (?), not sure about the latter though.
>>>>> But I think it is fair enough to assume Windows users to either
>>>>> use NTFS or ReFS. And if they don't, you can still call
>>>>> error_report_once() to make user aware that
>>>>> seucrity_model=mapped(-xattr) requires a fileystem on Windows that
>>>>> supports ADS.
>>>>> [4] https://en.wikipedia.org/wiki/NTFS#Alternate_data_stream_(ADS)
>>>>
>>>> Windows does not support POSIX permission.
>>>> So I think that only allow user to use security_model=none is
>>>> reasonable on Windows host.
>>>
>>> It depends on the use case. I assume your use case are Windows
>>> guests, in that case you don't have the concept of POSIX permissions
>>> neither on guest s
RE: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host
> -Original Message- > From: Christian Schoenebeck > Sent: 2022年4月14日 19:24 > To: qemu-devel@nongnu.org; Shi, Guohuai > Cc: Bin Meng ; Greg Kurz > Subject: Re: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Mittwoch, 13. April 2022 05:30:57 CEST Shi, Guohuai wrote: > > > We have 3 fs drivers: local, synth, proxy. I don't mind about proxy, > > > it is in bad shape and we will probably deprecate it in near future > > > anyway. > > > But it would be good to have support for the synth driver, because > > > we are using it for running test cases and fuzzing tests (QA). > > > > synth driver can not be built on Windows platform (or cross build on > > Linux). So the test cases can not work on Windows. > > Could you please be more specific what kind of challenge you see for making > the > synth driver working on Windows? The synth driver is just a simple mockup > driver > [1] that simulates in-RAM-only a filesystem with a bunch of hard coded dirs > and > files, solely for the purpose to run test cases. So the synth driver does not > interact > with any real filesystem on host at all. My expectation therefore would be > that > it just needs to tweak some header includes and maybe declaring missing POSIX > data > types, which you have done for the local driver already anyway. > For 9p-synth: I had enabled 9p-synth.c and built it successfully on Windows platform. However, test cases code are not built on Windows host. So I think it is useless that enable synth on Windows host (no way to run it). > BTW support for macOS hosts has just been recently added for 9p, I know it is > different as its a POSIX OS, but maybe you might still find the diff [2] > helpful. > > [1] https://wiki.qemu.org/Documentation/9p#9p_Filesystem_Drivers > [2] https://gitlab.com/qemu-project/qemu/-/commit/f45cc81911adc772 > > > > What are the limitations against security_model=mapped on Windows? > > > Keep in mind that with security_model=none you are very limited in > > > what you can do with 9p. > > > > MSYS library does not support extend attribute (e.g. getxattr), And > > does not support POSIX permission APIs (e.g. chmod, chown). > > Security model is useless on Windows host. > > That would be security_model=passthrough, yes, that's not possible with msys. > The recommended way in practice though is using security_model=mapped [3] for > all > systems, which should be possible to achieve with msys as well ... > > [3] https://wiki.qemu.org/Documentation/9psetup#Starting_the_Guest_directly > > > It is possible that to "map" extend attribute to NTFS stream data. > > However, if Windows host media is not NTFS (e.g. FAT) which does not > > support stream data, then the "map" can not work. > > ... yes exactly, it would make sense to use ADS [4] instead of xattr on > Windows. > ADS are available with NTFS and ReFS and maybe also with exFAT nowadays (?), > not > sure about the latter though. But I think it is fair enough to assume Windows > users > to either use NTFS or ReFS. And if they don't, you can still call > error_report_once() > to make user aware that > seucrity_model=mapped(-xattr) requires a fileystem on Windows that supports > ADS. > > [4] https://en.wikipedia.org/wiki/NTFS#Alternate_data_stream_(ADS) > Windows does not support POSIX permission. So I think that only allow user to use security_model=none is reasonable on Windows host. There is a difficulty to support "mapped" or "mapped-file" on Windows host: There are many functions in 9p-code using APIs like "openat", "mkdirat", etc. MSYS does not support that (openat is not valid on Windows host). I remember that 9p replaced "open" by "openat" for a long time. To fully support "security_model=mapped", 9p for Windows need to replace "openat" by "open". This may impact too many functions. I would have a try to enable "mapped" by using ADS, but it looks like a big refactor for 9p-local.c Best Regards, Guohuai
RE: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host
> We have 3 fs drivers: local, synth, proxy. I don't mind about proxy, it is in > bad shape and we will probably deprecate it in near future anyway. But it > would be good to have support for the synth driver, because we are using it > for running test cases and fuzzing tests (QA). synth driver can not be built on Windows platform (or cross build on Linux). So the test cases can not work on Windows. > What are the limitations against security_model=mapped on Windows? Keep in > mind that with security_model=none you are very limited in what you can do > with 9p. MSYS library does not support extend attribute (e.g. getxattr), And does not support POSIX permission APIs (e.g. chmod, chown). Security model is useless on Windows host. It is possible that to "map" extend attribute to NTFS stream data. However, if Windows host media is not NTFS (e.g. FAT) which does not support stream data, then the "map" can not work. Thanks Guohuai -Original Message- From: Bin Meng Sent: 2022年4月13日 11:19 To: Christian Schoenebeck ; Shi, Guohuai Cc: qemu-devel@nongnu.org Developers ; Greg Kurz Subject: Re: [RFC PATCH 0/4] 9pfs: Add 9pfs support for Windows host [Please note: This e-mail is from an EXTERNAL e-mail address] +Guohuai On Tue, Apr 12, 2022 at 8:27 PM Christian Schoenebeck wrote: > > On Freitag, 8. April 2022 19:10:09 CEST Bin Meng wrote: > > At present there is no Windows support for 9p file system. > > This series adds initial Windows support for 9p file system. > > Nice! > > > Only 'local' file system driver backend is supported. security_model > > should be 'none' due to limitations on Windows host. > > We have 3 fs drivers: local, synth, proxy. I don't mind about proxy, > it is in bad shape and we will probably deprecate it in near future > anyway. But it would be good to have support for the synth driver, > because we are using it for running test cases and fuzzing tests (QA). > > What are the limitations against security_model=mapped on Windows? > Keep in mind that with security_model=none you are very limited in > what you can do with 9p. > Regards, Bin
