[Qemu-devel] Make 9pfs buildable for Windows

[Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

Good day,

this is the patch series to make 9pfs buildable (not working!) on Windows.
It is compiling and starts, but obvousily 9pfs is not working - in fact it
spits out a initializing error.

Next step: Implement the directory and file handling stuff.

Quoting myself:

"
The biggest issue seems to be the *at stuff. I'll
try to workaround this via getting the directories' path from the file
descriptor with the /proc (as it is already done in the 9pfs_utils) -
luckily,the mingw environment emulates the /proc. If this doesn't work
I've another idea (the file descriptors needs to be "registered" with the
path (and saved in a sparse vector or map with the fd as key and the path
as value). The "big" solution would be to write a 9p_local_windows.c from
scratch, but I would like to avoid it.
"

I know that some people want to have compiling and working together, but I
hope(!) that I get a first working version next week.

Best regards,
Michael Fritscher

[Qemu-devel] [PATCH 01/18] Add definitions needed by file-op-9p.h for Windows

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 include/sysemu/os-win32.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index ff18b23db1..15cc94513b 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -196,4 +196,30 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
 ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
                            struct sockaddr *addr, socklen_t *addrlen);
 
+typedef uint32_t uid_t;
+typedef uint32_t gid_t;
+
+/* from http://man7.org/linux/man-pages/man2/statfs.2.html */
+typedef uint32_t __fsword_t;
+typedef uint32_t fsblkcnt_t;
+typedef uint32_t fsfilcnt_t;
+
+/* from linux/include/uapi/asm-generic/posix_types.h */
+typedef struct {
+    long __val[2];
+} fsid_t;
+
+struct statfs {
+    __fsword_t f_type;
+    __fsword_t f_bsize;
+    fsblkcnt_t f_blocks;
+    fsblkcnt_t f_bfree;
+    fsblkcnt_t f_bavail;
+    fsfilcnt_t f_files;
+    fsfilcnt_t f_ffree;
+    fsid_t f_fsid;
+    __fsword_t f_namelen;
+    __fsword_t f_frsize;
+    __fsword_t f_flags;
+};
 #endif
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 01/18] Add definitions needed by file-op-9p.h for Windows

[Greg Kurz]

On Fri, 29 Sep 2017 13:13:06 +0200
Michael Fritscher <michael@fritscher.net> wrote:

> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  include/sysemu/os-win32.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index ff18b23db1..15cc94513b 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -196,4 +196,30 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
>  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
>                             struct sockaddr *addr, socklen_t *addrlen);
> 
> +typedef uint32_t uid_t;
> +typedef uint32_t gid_t;
> +
> +/* from http://man7.org/linux/man-pages/man2/statfs.2.html */
> +typedef uint32_t __fsword_t;
> +typedef uint32_t fsblkcnt_t;
> +typedef uint32_t fsfilcnt_t;
> +
> +/* from linux/include/uapi/asm-generic/posix_types.h */
> +typedef struct {
> +    long __val[2];
> +} fsid_t;
> +
> +struct statfs {
> +    __fsword_t f_type;
> +    __fsword_t f_bsize;
> +    fsblkcnt_t f_blocks;
> +    fsblkcnt_t f_bfree;
> +    fsblkcnt_t f_bavail;
> +    fsfilcnt_t f_files;
> +    fsfilcnt_t f_ffree;
> +    fsid_t f_fsid;
> +    __fsword_t f_namelen;
> +    __fsword_t f_frsize;
> +    __fsword_t f_flags;
> +};
>  #endif

Since this statfs definition is likely to be only ever used by fsdev, I'm
wondering if all the related typedefs would better sit in fsdev/file-op-9p.h
directly (ie, move the lines to a #else block in the next patch).

[Qemu-devel] [PATCH 02/18] #include <sys/vfs.h> is not available under Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 fsdev/file-op-9p.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 474c79d003..3fc7b0532f 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -16,7 +16,9 @@
 
 #include <dirent.h>
 #include <utime.h>
-#include <sys/vfs.h>
+#ifndef _WIN32
+# include <sys/vfs.h>
+#endif
 #include "qemu-fsdev-throttle.h"
 
 #define SM_LOCAL_MODE_BITS    0600
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 02/18] #include <sys/vfs.h> is not available under Windows.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  fsdev/file-op-9p.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 474c79d003..3fc7b0532f 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -16,7 +16,9 @@
>  
>  #include <dirent.h>
>  #include <utime.h>
> -#include <sys/vfs.h>
> +#ifndef _WIN32
> +# include <sys/vfs.h>
> +#endif
>  #include "qemu-fsdev-throttle.h"
>  
>  #define SM_LOCAL_MODE_BITS    0600
> 

Like patch 4, this only makes sense in the same patch that stops using
function from /usr/include/sys/vfs.h.

Paolo

[Qemu-devel] [PATCH 03/18] Disable the proxy fsdev under Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 fsdev/qemu-fsdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 266e442b87..9f6d33365d 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -26,7 +26,9 @@ static FsDriverTable FsDrivers[] = {
     { .name = "handle", .ops = &handle_ops},
 #endif
     { .name = "synth", .ops = &synth_ops},
+#ifndef WIN32
     { .name = "proxy", .ops = &proxy_ops},
+#endif
 };
 
 int qemu_fsdev_add(QemuOpts *opts)
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 03/18] Disable the proxy fsdev under Windows.

[Greg Kurz]

On Fri, 29 Sep 2017 13:13:08 +0200
Michael Fritscher <michael@fritscher.net> wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>

What's the justification for this patch ?

> ---
>  fsdev/qemu-fsdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
> index 266e442b87..9f6d33365d 100644
> --- a/fsdev/qemu-fsdev.c
> +++ b/fsdev/qemu-fsdev.c
> @@ -26,7 +26,9 @@ static FsDriverTable FsDrivers[] = {
>      { .name = "handle", .ops = &handle_ops},
>  #endif
>      { .name = "synth", .ops = &synth_ops},
> +#ifndef WIN32
>      { .name = "proxy", .ops = &proxy_ops},
> +#endif
>  };
> 
>  int qemu_fsdev_add(QemuOpts *opts)

Re: [Qemu-devel] [PATCH 03/18] Disable the proxy fsdev under Windows.

[Michael Fritscher]

> On Fri, 29 Sep 2017 13:13:08 +0200
> Michael Fritscher <michael@fritscher.net> wrote:
>> Signed-off-by: Michael Fritscher <michael@fritscher.net>
>
> What's the justification for this patch ?
It had too much linuxisms in it to fix compiling - so I disabled the proxy
variant altogether. Reactivating it could be done in a version 2.0 ;-)

Best regards,
Michael Fritscher

Re: [Qemu-devel] [PATCH 03/18] Disable the proxy fsdev under Windows.

[Greg Kurz]

On Mon, 6 Nov 2017 16:17:29 +0100
"Michael Fritscher" <michael@fritscher.net> wrote:

> > On Fri, 29 Sep 2017 13:13:08 +0200
> > Michael Fritscher <michael@fritscher.net> wrote:  
> >> Signed-off-by: Michael Fritscher <michael@fritscher.net>  
> >
> > What's the justification for this patch ?  
> It had too much linuxisms in it to fix compiling - so I disabled the proxy
> variant altogether. Reactivating it could be done in a version 2.0 ;-)
> 

That's what I was guessing. Fair enough but it deserves some justification in
the commit log anyway :)

BTW, the main justification for the proxy backend is to provide "extra"
security (the helper chroots into the shared directory). I don't know if
this makes sense on a windows host.

Also, if you go for a windows specific backend as suggested in another
mail, you should probably disable all the other existing backends as
well.

> Best regards,
> Michael Fritscher
> 
> 

[Qemu-devel] [PATCH 04/18] Don't include sys/resource.h on Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index d1cfeaf10e..c2dd782620 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -3,7 +3,9 @@
 
 #include <dirent.h>
 #include <utime.h>
-#include <sys/resource.h>
+#ifndef _WIN32
+# include <sys/resource.h>
+#endif
 #include "fsdev/file-op-9p.h"
 #include "fsdev/9p-iov-marshal.h"
 #include "qemu/thread.h"
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 04/18] Don't include sys/resource.h on Windows.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  hw/9pfs/9p.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index d1cfeaf10e..c2dd782620 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -3,7 +3,9 @@
>  
>  #include <dirent.h>
>  #include <utime.h>
> -#include <sys/resource.h>
> +#ifndef _WIN32
> +# include <sys/resource.h>
> +#endif
>  #include "fsdev/file-op-9p.h"
>  #include "fsdev/9p-iov-marshal.h"
>  #include "qemu/thread.h"
> 

This is probably related to patch 14, and should be included in there.

Paolo

[Qemu-devel] [PATCH 05/18] Add definitions for 9p.c.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 include/sysemu/os-win32.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 15cc94513b..d344516987 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -222,4 +222,50 @@ struct statfs {
     __fsword_t f_frsize;
     __fsword_t f_flags;
 };
+
+/* from http://octave.org/doxygen/3.4/fcntl_8h.html */
+#define O_NOCTTY 0
+#define O_NDELAY 0
+#define O_NONBLOCK O_NDELAY
+#define O_DSYNC 0
+#define O_DIRECT 0
+#define O_DIRECTORY 0
+#define O_NOFOLLOW 0
+#define O_NOATIME 0
+#define O_SYNC 0
+#define O_ASYNC 0
+
+#define FASYNC 0
+
+#define AT_REMOVEDIR 1
+
+#define NAME_MAX 260
+
+/* from linux/limits.h */
+#define XATTR_SIZE_MAX 65536
+
+/* from linux/kdev_t.h */
+#define MINORBITS 20
+#define MINORMASK ((1U << MINORBITS) - 1)
+
+#define major(dev) ((unsigned int) ((dev) >> MINORBITS))
+#define minor(dev) ((unsigned int) ((dev) & MINORMASK))
+#define makedev(ma, mi) (((ma) << MINORBITS) | (mi))
+
+/* from linux/include/linux/stat.h */
+#define UTIME_NOW ((1l << 30) - 1l)
+#define UTIME_OMIT ((1l << 30) - 2l)
+
+/* from uapi/linux/stat.h */
+#define S_IFSOCK 0140000
+#define S_IFLNK  0120000
+#define S_ISUID  0004000
+#define S_ISGID  0002000
+#define S_ISVTX  0001000
+
+#define S_ISLNK(m)  (((m) & S_IFMT) == S_IFLNK)
+#define S_ISSOCK(m) (((m) & S_IFMT) == S_IFSOCK)
+
+/* from glibc/dirent/dirent.h */
+#define DT_UNKNOWN 0
 #endif
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 06/18] Stub out functions for 9pfs.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 include/sysemu/os-win32.h | 226 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 226 insertions(+)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index d344516987..26bb0dae66 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -268,4 +268,230 @@ struct statfs {
 
 /* from glibc/dirent/dirent.h */
 #define DT_UNKNOWN 0
+
+/* from gnulib */
+/* Find the first occurrence of C in S or the final NUL byte.  */
+static inline char *
+strchrnul(const char *s, int c_in)
+{
+    /* On 32-bit hardware, choosing longword to be a 32-bit unsigned
+       long instead of a 64-bit uintmax_t tends to give better
+       performance. On 64-bit hardware, unsigned long is generally 64
+       bits already. Change this typedef to experiment with
+       performance. */
+    typedef unsigned long int longword;
+
+    const unsigned char *char_ptr;
+    const longword *longword_ptr;
+    longword repeated_one;
+    longword repeated_c;
+    unsigned char c;
+
+    c = (unsigned char) c_in;
+
+    /* Handle the first few bytes by reading one byte at a time.
+       Do this until CHAR_PTR is aligned on a longword boundary. */
+    for (char_ptr = (const unsigned char *) s;
+             (size_t) char_ptr % sizeof(longword) != 0;
+             ++char_ptr) {
+        if (!*char_ptr || *char_ptr == c) {
+            return (char *) char_ptr;
+        }
+    }
+
+    longword_ptr = (const longword *) char_ptr;
+
+    /* All these elucidatory comments refer to 4-byte longwords,
+       but the theory applies equally well to any size longwords. */
+
+    /* Compute auxiliary longword values:
+       repeated_one is a value which has a 1 in every byte.
+       repeated_c has c in every byte. */
+    repeated_one = 0x01010101;
+    repeated_c = c | (c << 8);
+    repeated_c |= repeated_c << 16;
+    if (0xffffffffU < (longword) -1) {
+        repeated_one |= repeated_one << 31 << 1;
+        repeated_c |= repeated_c << 31 << 1;
+        if (8 < sizeof(longword)) {
+            size_t i;
+
+            for (i = 64; i < sizeof(longword) * 8; i *= 2) {
+                repeated_one |= repeated_one << i;
+                repeated_c |= repeated_c << i;
+            }
+        }
+    }
+
+    /* Instead of the traditional loop which tests each byte, we will
+       test a longword at a time. The tricky part is testing if *any of
+       the four* bytes in the longword in question are equal to NUL or
+       c.    We first use an xor with repeated_c. This reduces the task
+       to testing whether *any of the four* bytes in longword1 or
+       longword2 is zero.
+
+       Let's consider longword1.    We compute tmp =
+           ((longword1 - repeated_one) & ~longword1) & (repeated_one << 7).
+       That is, we perform the following operations:
+           1. Subtract repeated_one.
+           2. & ~longword1.
+           3. & a mask consisting of 0x80 in every byte.
+       Consider what happens in each byte:
+           - If a byte of longword1 is zero, step 1 and 2 transform it into 0xff,
+               and step 3 transforms it into 0x80. A carry can also be propagated
+               to more significant bytes.
+           - If a byte of longword1 is nonzero, let its lowest 1 bit be at
+               position k (0 <= k <= 7); so the lowest k bits are 0. After step 1,
+               the byte ends in a single bit of value 0 and k bits of value 1.
+               After step 2, the result is just k bits of value 1: 2^k - 1. After
+               step 3, the result is 0. And no carry is produced.
+       So, if longword1 has only non-zero bytes, tmp is zero.
+       Whereas if longword1 has a zero byte, call j the position of the least
+       significant zero byte. Then the result has a zero at positions 0, ...,
+       j-1 and a 0x80 at position j. We cannot predict the result at the more
+       significant bytes (positions j+1..3), but it does not matter since we
+       already have a non-zero bit at position 8*j+7.
+
+       The test whether any byte in longword1 or longword2 is zero is equivalent
+       to testing whether tmp1 is nonzero or tmp2 is nonzero. We can combine
+       this into a single test, whether (tmp1 | tmp2) is nonzero.
+
+       This test can read more than one byte beyond the end of a string,
+       depending on where the terminating NUL is encountered. However,
+       this is considered safe since the initialization phase ensured
+       that the read will be aligned, therefore, the read will not cross
+       page boundaries and will not cause a fault. */
+
+    while (1) {
+        longword longword1 = *longword_ptr ^ repeated_c;
+        longword longword2 = *longword_ptr;
+
+        if (((((longword1 - repeated_one) & ~longword1)
+            | ((longword2 - repeated_one) & ~longword2))
+            & (repeated_one << 7)) != 0) {
+                break;
+            }
+        longword_ptr++;
+    }
+
+    char_ptr = (const unsigned char *) longword_ptr;
+
+    /* At this point, we know that one of the sizeof(longword) bytes
+       starting at char_ptr is == 0 or == c. On little-endian machines,
+       we could determine the first such byte without any further memory
+       accesses, just by looking at the tmp result from the last loop
+       iteration. But this does not work on big-endian machines.
+       Choose code that works in both cases. */
+
+    char_ptr = (unsigned char *) longword_ptr;
+    while (*char_ptr && (*char_ptr != c)) {
+        char_ptr++;
+    }
+    return (char *) char_ptr;
+}
+
+static inline int openat(int dirfd, const char *pathname, int flags, ...)
+{
+    return 0;
+}
+
+static inline int renameat(int olddirfd, const char *oldpath,
+                           int newdirfd, const char *newpath)
+{
+    return 0;
+}
+
+static inline int unlinkat(int dirfd, const char *pathname, int flags)
+{
+    return 0;
+}
+
+static inline int fstatat(int dirfd, const char *pathname, struct stat *buf,
+                          int flags)
+{
+    memset(buf, 0, sizeof(struct stat));
+    return 0;
+}
+
+static inline int mkdirat(int dirfd, const char *pathname, mode_t mode)
+{
+    return 0;
+}
+
+static inline ssize_t readlinkat(int dirfd, const char *pathname,
+                                 char *buf, size_t bufsiz)
+{
+    return 0;
+}
+
+static inline int mknodat(int dirfd, const char *pathname, mode_t mode, dev_t dev)
+{
+    return 0;
+}
+
+static inline int symlinkat(const char *target, int newdirfd, const char *linkpath)
+{
+    return 0;
+}
+
+static inline int linkat(int olddirfd, const char *oldpath,
+                         int newdirfd, const char *newpath, int flags)
+{
+    return 0;
+}
+
+static inline int fchownat(int dirfd, const char *pathname,
+                           uid_t owner, gid_t group, int flags)
+{
+    return 0;
+}
+
+static inline int fstatfs(int fd, struct statfs *buf)
+{
+    memset(buf, 0, sizeof(struct statfs));
+    buf->f_type = 0x01021997; /* V9FS_MAGIC */
+    buf->f_bsize = 4096;
+    buf->f_blocks = 4000000;
+    buf->f_bfree = 3000000;
+    buf->f_bavail = 2999000;
+    buf->f_files = 1000000;
+    buf->f_ffree = 800000;
+    buf->f_namelen = NAME_MAX;
+    return 0;
+}
+
+static inline int utimensat(int dirfd, const char *pathname,
+                            const struct timespec times[2], int flags)
+{
+    return 0;
+}
+
+/* pretend success */
+static inline int setxattr(const char *path, const char *name,
+                    const void *value, size_t size, int flags)
+{
+    return 0;
+}
+
+static inline ssize_t fgetxattr(int fd, const char *name,
+                         void *value, size_t size)
+{
+    return 0;
+}
+
+static inline ssize_t lgetxattr(const char *path, const char *name,
+                                void *value, size_t size)
+{
+    return 0;
+}
+
+/* from fcntl.h*/
+#define F_SETFL 6
+#define AT_SYMLINK_NOFOLLOW 0x100
+
+static inline int fcntl(int fd, int cmd, ... /* arg */)
+{
+    return 0;
+}
+
 #endif
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 07/18] Fix unused variable error and unsuded function if FS_IOC_GETVERSION is not defined.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-local.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index e51af87309..4c8e0f3296 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1372,10 +1372,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
     return ret;
 }
 
+#ifdef FS_IOC_GETVERSION
 static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
                                 mode_t st_mode, uint64_t *st_gen)
 {
-#ifdef FS_IOC_GETVERSION
     int err;
     V9fsFidOpenState fid_open;
 
@@ -1394,15 +1394,14 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
     err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
     local_close(ctx, &fid_open);
     return err;
-#else
-    errno = ENOTTY;
-    return -1;
-#endif
 }
+#endif
 
 static int local_init(FsContext *ctx)
 {
+#ifdef FS_IOC_GETVERSION
     struct statfs stbuf;
+#endif
     LocalData *data = g_malloc(sizeof(*data));
 
     data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 07/18] Fix unused variable error and unsuded function if FS_IOC_GETVERSION is not defined.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  hw/9pfs/9p-local.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index e51af87309..4c8e0f3296 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1372,10 +1372,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
>      return ret;
>  }
>  
> +#ifdef FS_IOC_GETVERSION
>  static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
>                                  mode_t st_mode, uint64_t *st_gen)
>  {
> -#ifdef FS_IOC_GETVERSION
>      int err;
>      V9fsFidOpenState fid_open;
>  
> @@ -1394,15 +1394,14 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
>      err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
>      local_close(ctx, &fid_open);
>      return err;
> -#else
> -    errno = ENOTTY;
> -    return -1;
> -#endif
>  }
> +#endif
>  
>  static int local_init(FsContext *ctx)
>  {
> +#ifdef FS_IOC_GETVERSION
>      struct statfs stbuf;
> +#endif
>      LocalData *data = g_malloc(sizeof(*data));
>  
>      data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
> 

This is probably the change in which patch 2 should be merged.  With the
#include added here, the patch is fine.

Paolo

[Qemu-devel] [PATCH 08/18] Stub 9pfs xattr functions for Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-local.c |  4 +++-
 hw/9pfs/9p-xattr.h | 30 ++++++++++++++++++++++++++++++
 hw/9pfs/9p.c       |  4 +++-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 4c8e0f3296..c4feca27bd 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -22,7 +22,9 @@
 #include <grp.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-#include "qemu/xattr.h"
+#ifndef _WIN32
+# include "qemu/xattr.h"
+#endif
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include <libgen.h>
diff --git a/hw/9pfs/9p-xattr.h b/hw/9pfs/9p-xattr.h
index 0d83996575..1c42c8dba0 100644
--- a/hw/9pfs/9p-xattr.h
+++ b/hw/9pfs/9p-xattr.h
@@ -14,7 +14,9 @@
 #ifndef QEMU_9P_XATTR_H
 #define QEMU_9P_XATTR_H
 
+#ifndef _WIN32
 #include "qemu/xattr.h"
+#endif
 
 typedef struct xattr_operations
 {
@@ -49,6 +51,33 @@ extern XattrOperations *mapped_xattr_ops[];
 extern XattrOperations *passthrough_xattr_ops[];
 extern XattrOperations *none_xattr_ops[];
 
+#ifdef _WIN32
+static inline ssize_t v9fs_get_xattr(FsContext *ctx, const char *path,
+                                     const char *name, void *value, size_t size)
+{
+    return 0;
+}
+
+static inline ssize_t v9fs_list_xattr(FsContext *ctx, const char *path,
+                                      void *value, size_t vsize)
+{
+    return 0;
+}
+
+static inline int v9fs_set_xattr(FsContext *ctx, const char *path,
+                                 const char *name, void *value, size_t size,
+                                 int flags)
+{
+    return 0;
+}
+
+static inline int v9fs_remove_xattr(FsContext *ctx, const char *path,
+                                    const char *name)
+{
+    return 0;
+}
+
+#else
 ssize_t v9fs_get_xattr(FsContext *ctx, const char *path, const char *name,
                        void *value, size_t size);
 ssize_t v9fs_list_xattr(FsContext *ctx, const char *path, void *value,
@@ -56,6 +85,7 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path, void *value,
 int v9fs_set_xattr(FsContext *ctx, const char *path, const char *name,
                           void *value, size_t size, int flags);
 int v9fs_remove_xattr(FsContext *ctx, const char *path, const char *name);
+#endif
 
 ssize_t pt_listxattr(FsContext *ctx, const char *path, char *name, void *value,
                      size_t size);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 23ac7bb532..53f812ec06 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -20,7 +20,9 @@
 #include "qemu/sockets.h"
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
-#include "9p-xattr.h"
+#ifndef _WIN32
+# include "9p-xattr.h"
+#endif
 #include "coth.h"
 #include "trace.h"
 #include "migration/blocker.h"
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 09/18] Dont initialize fields which aren't available on Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-synth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index df0a8de08a..e1bf0438b8 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -150,8 +150,10 @@ static void synth_fill_statbuf(V9fsSynthNode *node, struct stat *stbuf)
     stbuf->st_gid = 0;
     stbuf->st_rdev = 0;
     stbuf->st_size = 0;
+#ifndef _WIN32
     stbuf->st_blksize = 0;
     stbuf->st_blocks = 0;
+#endif
     stbuf->st_atime = 0;
     stbuf->st_mtime = 0;
     stbuf->st_ctime = 0;
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 09/18] Dont initialize fields which aren't available on Windows.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  hw/9pfs/9p-synth.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index df0a8de08a..e1bf0438b8 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -150,8 +150,10 @@ static void synth_fill_statbuf(V9fsSynthNode *node, struct stat *stbuf)
>      stbuf->st_gid = 0;
>      stbuf->st_rdev = 0;
>      stbuf->st_size = 0;
> +#ifndef _WIN32
>      stbuf->st_blksize = 0;
>      stbuf->st_blocks = 0;
> +#endif
>      stbuf->st_atime = 0;
>      stbuf->st_mtime = 0;
>      stbuf->st_ctime = 0;
> 

Just memset the whole stbuf instead, and then overwrite
st_ino/st_mode/st_nlink.

Paolo

[Qemu-devel] [PATCH 10/18] dirent has no d_off on Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-synth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index e1bf0438b8..7b8cc3b6e9 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -222,7 +222,9 @@ static void synth_direntry(V9fsSynthNode *node,
 {
     strcpy(entry->d_name, node->name);
     entry->d_ino = node->attr->inode;
+#ifndef _WIN32
     entry->d_off = off + 1;
+#endif
 }
 
 static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 11/18] Sete ctx->xops to null on Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-local.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index c4feca27bd..fdc0a5bc76 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1429,6 +1429,9 @@ static int local_init(FsContext *ctx)
     }
 #endif
 
+#ifdef _WIN32
+    ctx->xops = 0;
+#else
     if (ctx->export_flags & V9FS_SM_PASSTHROUGH) {
         ctx->xops = passthrough_xattr_ops;
     } else if (ctx->export_flags & V9FS_SM_MAPPED) {
@@ -1442,6 +1445,7 @@ static int local_init(FsContext *ctx)
          */
         ctx->xops = passthrough_xattr_ops;
     }
+#endif
     ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT;
 
     ctx->private = data;
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 11/18] Sete ctx->xops to null on Windows.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
>  
> +#ifdef _WIN32
> +    ctx->xops = 0;
> +#else
>      if (ctx->export_flags & V9FS_SM_PASSTHROUGH) {
>          ctx->xops = passthrough_xattr_ops;
>      } else if (ctx->export_flags & V9FS_SM_MAPPED) {
> @@ -1442,6 +1445,7 @@ static int local_init(FsContext *ctx)
>           */
>          ctx->xops = passthrough_xattr_ops;
>      }
> +#endif

Why not none_xattr_ops?

Paolo

[Qemu-devel] [PATCH 12/18] Buildfix in 9p-util.c.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c
index f709c27a1f..3330b59ef2 100644
--- a/hw/9pfs/9p-util.c
+++ b/hw/9pfs/9p-util.c
@@ -11,7 +11,9 @@
  */
 
 #include "qemu/osdep.h"
+#ifndef _WIN32
 #include "qemu/xattr.h"
+#endif
 #include "9p-util.h"
 
 ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 12/18] Buildfix in 9p-util.c.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> +#ifndef _WIN32
>  #include "qemu/xattr.h"
> +#endif

This #ifndef should be in include/qemu/xattr.h itself.

Paolo

[Qemu-devel] [PATCH 13/18] fsetxattrat_nofollow doesn't seem to be defined on Windows - disable it.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-local.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index fdc0a5bc76..bf6fb70b81 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -397,6 +397,9 @@ static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
 
 static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
 {
+#ifdef _WIN32
+    return 0;
+#else
     int err;
 
     if (credp->fc_uid != -1) {
@@ -432,6 +435,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
         }
     }
     return 0;
+#endif
 }
 
 static int local_set_cred_passthrough(FsContext *fs_ctx, int dirfd,
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 14/18] Disable rlimit under Windows

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 53f812ec06..e0ef1a2a6b 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3630,6 +3630,7 @@ void v9fs_reset(V9fsState *s)
 
 static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
 {
+#ifndef _WIN32
     struct rlimit rlim;
     if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
         error_report("Failed to get the resource limit");
@@ -3637,4 +3638,5 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
     }
     open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);
     open_fd_rc = rlim.rlim_cur/2;
+#endif
 }
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 14/18] Disable rlimit under Windows

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>
> ---
>  hw/9pfs/9p.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 53f812ec06..e0ef1a2a6b 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3630,6 +3630,7 @@ void v9fs_reset(V9fsState *s)
>  
>  static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
>  {
> +#ifndef _WIN32
>      struct rlimit rlim;
>      if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
>          error_report("Failed to get the resource limit");
> @@ -3637,4 +3638,5 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
>      }
>      open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);
>      open_fd_rc = rlim.rlim_cur/2;
> +#endif
>  }
> 

There should be a function in util/oslib-* that returns the rlim_cur value.

9p obviously needs a nonzero value in open_fd_hw and open_fd_rc, so the
last two lines need to be there for Win32 as well.  Perhaps a stub
implementation that just returns a fixed number (could be 1000?) is okay
for Windows.


Paolo

[Qemu-devel] [PATCH 15/18] Fix unavailable fields in stbuf under Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e0ef1a2a6b..f4ccb45f64 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -886,14 +886,34 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
     v9lstat->st_gid = stbuf->st_gid;
     v9lstat->st_rdev = stbuf->st_rdev;
     v9lstat->st_size = stbuf->st_size;
+#ifdef _WIN32
+    /* Blksize is the optimal EA-block,
+       while blocks always refers to 512 byte blocks
+    */
+    v9lstat->st_blksize = 4096;
+    v9lstat->st_blocks = ((stbuf->st_size + 1) / 512) + 1;
+#else
     v9lstat->st_blksize = stbuf->st_blksize;
     v9lstat->st_blocks = stbuf->st_blocks;
+#endif
     v9lstat->st_atime_sec = stbuf->st_atime;
+#ifdef _WIN32
+    v9lstat->st_atime_nsec = 0;
+#else
     v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
+#endif
     v9lstat->st_mtime_sec = stbuf->st_mtime;
+#ifdef _WIN32
+    v9lstat->st_mtime_nsec = 0;
+#else
     v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
+#endif
     v9lstat->st_ctime_sec = stbuf->st_ctime;
+#ifdef _WIN32
+    v9lstat->st_ctime_nsec = 0;
+#else
     v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec;
+#endif
     /* Currently we only support BASIC fields in stat */
     v9lstat->st_result_mask = P9_STATS_BASIC;
 
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 15/18] Fix unavailable fields in stbuf under Windows.

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> @@ -886,14 +886,34 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
>      v9lstat->st_gid = stbuf->st_gid;
>      v9lstat->st_rdev = stbuf->st_rdev;
>      v9lstat->st_size = stbuf->st_size;
> +#ifdef _WIN32
> +    /* Blksize is the optimal EA-block,
> +       while blocks always refers to 512 byte blocks
> +    */
> +    v9lstat->st_blksize = 4096;
> +    v9lstat->st_blocks = ((stbuf->st_size + 1) / 512) + 1;
> +#else
>      v9lstat->st_blksize = stbuf->st_blksize;
>      v9lstat->st_blocks = stbuf->st_blocks;
> +#endif
>      v9lstat->st_atime_sec = stbuf->st_atime;
> +#ifdef _WIN32
> +    v9lstat->st_atime_nsec = 0;
> +#else
>      v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
> +#endif
>      v9lstat->st_mtime_sec = stbuf->st_mtime;
> +#ifdef _WIN32
> +    v9lstat->st_mtime_nsec = 0;
> +#else
>      v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
> +#endif
>      v9lstat->st_ctime_sec = stbuf->st_ctime;
> +#ifdef _WIN32
> +    v9lstat->st_ctime_nsec = 0;
> +#else
>      v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec;
> +#endif
>      /* Currently we only support BASIC fields in stat */
>      v9lstat->st_result_mask = P9_STATS_BASIC;
>  

Likewise, I think it should be struct FileOperations's lstat member that
uses struct V9fsStatDotl instead of struct stat.  Then
stat_to_v9stat_dotl would be hidden inside 9p-local.c (and 9p-local.c
isn't going to be used by Windows at all, so this patch disappears).

Paolo

Re: [Qemu-devel] [PATCH 15/18] Fix unavailable fields in stbuf under Windows.

[Michael Fritscher]

Hi,

> Likewise, I think it should be struct FileOperations's lstat member that
> uses struct V9fsStatDotl instead of struct stat.  Then
> stat_to_v9stat_dotl would be hidden inside 9p-local.c (and 9p-local.c
> isn't going to be used by Windows at all, so this patch disappears).
>
> Paolo
>
At first: thanks for reviewing!

Yes, the main question for me at this stage is:
  * use the 9p-local.c and try workaround the limitations (e.g. lack of
the *at functions) by creating stubs)
  * make a own 9p-local-windows.c and use ntdlls functions, which is a bit
more ugly.

I decribed these options a moment ago in the other mail in more detail.
For me both way are fine.

Best regards,
Michael Fritscher

Re: [Qemu-devel] [PATCH 15/18] Fix unavailable fields in stbuf under Windows.

[Paolo Bonzini]

On 06/11/2017 16:23, Michael Fritscher wrote:
>> Likewise, I think it should be struct FileOperations's lstat member that
>> uses struct V9fsStatDotl instead of struct stat.  Then
>> stat_to_v9stat_dotl would be hidden inside 9p-local.c (and 9p-local.c
>> isn't going to be used by Windows at all, so this patch disappears).
>>
>> Paolo
>>
> At first: thanks for reviewing!
> 
> Yes, the main question for me at this stage is:
>   * use the 9p-local.c and try workaround the limitations (e.g. lack of
> the *at functions) by creating stubs)
>   * make a own 9p-local-windows.c and use ntdlls functions, which is a bit
> more ugly.
> 
> I decribed these options a moment ago in the other mail in more detail.
> For me both way are fine.

Given the limitations of the Windows standard C library, I think the
latter makes more sense.  It could be kernel32 or ntdll, that doesn't
really matter (though ntdll is a better match for 9p and I think it's
worth it); but msvcrt is probably just as ugly as ntdll and even more
limited than kernel32.

Paolo

[Qemu-devel] [PATCH 16/18] Workaround for missing dent->d_type/d_off under Windows

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index f4ccb45f64..4e07bfc71b 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1794,7 +1794,11 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
         count += len;
         v9fs_stat_free(&v9stat);
         v9fs_path_free(&path);
+#ifdef _WIN32
+        saved_dir_pos = v9fs_co_telldir(pdu, fidp);
+#else
         saved_dir_pos = dent->d_off;
+#endif
     }
 
     v9fs_readdir_unlock(&fidp->fs.dir);
@@ -1946,9 +1950,15 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
         qid.version = 0;
 
         /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
+#ifdef _WIN32
+        len = pdu_marshal(pdu, 11 + count, "Qqbs",
+                          &qid, v9fs_co_telldir(pdu, fidp),
+                          DT_UNKNOWN, &name);
+#else
         len = pdu_marshal(pdu, 11 + count, "Qqbs",
                           &qid, dent->d_off,
                           dent->d_type, &name);
+#endif
 
         v9fs_readdir_unlock(&fidp->fs.dir);
 
@@ -1959,7 +1969,11 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
         }
         count += len;
         v9fs_string_free(&name);
+#ifdef _WIN32
+        saved_dir_pos = v9fs_co_telldir(pdu, fidp);
+#else
         saved_dir_pos = dent->d_off;
+#endif
     }
 
     v9fs_readdir_unlock(&fidp->fs.dir);
-- 
2.13.2.windows.1

Re: [Qemu-devel] [PATCH 16/18] Workaround for missing dent->d_type/d_off under Windows

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> Signed-off-by: Michael Fritscher <michael@fritscher.net>

I think you should use your own "struct V9fsDirent" instead of "struct
dirent".

Paolo

> ---
>  hw/9pfs/9p.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index f4ccb45f64..4e07bfc71b 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1794,7 +1794,11 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          count += len;
>          v9fs_stat_free(&v9stat);
>          v9fs_path_free(&path);
> +#ifdef _WIN32
> +        saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> +#else
>          saved_dir_pos = dent->d_off;
> +#endif
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -1946,9 +1950,15 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>          qid.version = 0;
>  
>          /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
> +#ifdef _WIN32
> +        len = pdu_marshal(pdu, 11 + count, "Qqbs",
> +                          &qid, v9fs_co_telldir(pdu, fidp),
> +                          DT_UNKNOWN, &name);
> +#else
>          len = pdu_marshal(pdu, 11 + count, "Qqbs",
>                            &qid, dent->d_off,
>                            dent->d_type, &name);
> +#endif
>  
>          v9fs_readdir_unlock(&fidp->fs.dir);
>  
> @@ -1959,7 +1969,11 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>          }
>          count += len;
>          v9fs_string_free(&name);
> +#ifdef _WIN32
> +        saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> +#else
>          saved_dir_pos = dent->d_off;
> +#endif
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);
> 

[Qemu-devel] [PATCH 17/18] Compile fixes for Windows.

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 hw/9pfs/9p-local.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index bf6fb70b81..afe07dd142 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -17,22 +17,26 @@
 #include "9p-xattr.h"
 #include "9p-util.h"
 #include "fsdev/qemu-fsdev.h"   /* local_ops */
+#ifndef _WIN32
 #include <arpa/inet.h>
 #include <pwd.h>
 #include <grp.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#endif
 #ifndef _WIN32
-# include "qemu/xattr.h"
+#include "qemu/xattr.h"
 #endif
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include <libgen.h>
+#ifndef _WIN32
 #include <linux/fs.h>
+#endif
 #ifdef CONFIG_LINUX_MAGIC_H
 #include <linux/magic.h>
-#endif
 #include <sys/ioctl.h>
+#endif
 
 #ifndef XFS_SUPER_MAGIC
 #define XFS_SUPER_MAGIC  0x58465342
@@ -300,8 +304,10 @@ update_map_file:
 
     map_fd = fileno(fp);
     assert(map_fd != -1);
+#ifndef _WIN32
     ret = fchmod(map_fd, 0600);
     assert(ret == 0);
+#endif
 
     if (credp->fc_uid != -1) {
         uid = credp->fc_uid;
@@ -309,7 +315,8 @@ update_map_file:
     if (credp->fc_gid != -1) {
         gid = credp->fc_gid;
     }
-    if (credp->fc_mode != -1) {
+    /* On Windows it is unsigned... */
+    if ((int16_t)(credp->fc_mode) != -1) {
         mode = credp->fc_mode;
     }
     if (credp->fc_rdev != -1) {
@@ -368,8 +375,12 @@ static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
     if (fd == -1) {
         return -1;
     }
+#ifndef _WIN32
     ret = fchmod(fd, mode);
 #else
+    ret = 0;
+#endif
+#else
     /* Access modes are ignored when O_PATH is supported. If name is a symbolic
      * link, O_PATH | O_NOFOLLOW causes openat(2) to return a file descriptor
      * referring to the symbolic link.
@@ -418,7 +429,8 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
             return err;
         }
     }
-    if (credp->fc_mode != -1) {
+    /* On Windows it is unsigned... */
+    if ((int16_t)(credp->fc_mode) != -1) {
         uint32_t tmp_mode = cpu_to_le32(credp->fc_mode);
         err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.mode", &tmp_mode,
                                    sizeof(mode_t), 0);
@@ -518,8 +530,23 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path,
 static int local_opendir(FsContext *ctx,
                          V9fsPath *fs_path, V9fsFidOpenState *fs)
 {
+   DIR *stream;
+/* there is no fdopendir available on Windows... */
+#ifdef _WIN32
+    char *buffer;
+    char *path = fs_path->data;
+
+    buffer = rpath(ctx, path);
+    error_printf("Directory (opendir) which should be opened: %s\n", buffer);
+    stream = opendir(buffer);
+    g_free(buffer);
+    if (!stream) {
+        return -1;
+    }
+    fs->dir.stream = stream;
+    return 0;
+#else
     int dirfd;
-    DIR *stream;
 
     dirfd = local_opendir_nofollow(ctx, fs_path->data);
     if (dirfd == -1) {
@@ -533,6 +560,7 @@ static int local_opendir(FsContext *ctx,
     }
     fs->dir.stream = stream;
     return 0;
+#endif
 }
 
 static void local_rewinddir(FsContext *ctx, V9fsFidOpenState *fs)
@@ -562,13 +590,17 @@ again:
     }
 
     if (ctx->export_flags & V9FS_SM_MAPPED) {
+#ifndef _WIN32
         entry->d_type = DT_UNKNOWN;
+#endif
     } else if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
         if (local_is_mapped_file_metadata(ctx, entry->d_name)) {
             /* skip the meta data */
             goto again;
         }
+#ifndef _WIN32
         entry->d_type = DT_UNKNOWN;
+#endif
     }
 
     return entry;
@@ -763,7 +795,11 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
     int err, fd;
 
     if (fid_type == P9_FID_DIR) {
+#ifndef _WIN32
         fd = dirfd(fs->dir.stream);
+#else
+        fd = fs->fd;
+#endif
     } else {
         fd = fs->fd;
     }
@@ -1161,7 +1197,11 @@ static int local_fsync(FsContext *ctx, int fid_type,
     int fd;
 
     if (fid_type == P9_FID_DIR) {
+#ifndef _WIN32
         fd = dirfd(fs->dir.stream);
+#else
+        fd = fs->fd;
+#endif
     } else {
         fd = fs->fd;
     }
-- 
2.13.2.windows.1

[Qemu-devel] [PATCH 18/18] Enable 9pfs for Windows in configure / makefiles

[Michael Fritscher]

Signed-off-by: Michael Fritscher <michael@fritscher.net>
---
 Makefile.objs         |  1 +
 configure             | 23 +++++++++++++++++------
 hw/9pfs/Makefile.objs | 11 +++++++----
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index bdfa3b6177..1ab197f48c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -47,6 +47,7 @@ common-obj-$(CONFIG_WIN32) += os-win32.o
 common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
+common-obj-$(CONFIG_WIN32) += fsdev/
 
 common-obj-y += migration/
 
diff --git a/configure b/configure
index 6587e8014b..74ca56c060 100755
--- a/configure
+++ b/configure
@@ -5067,13 +5067,22 @@ if test "$want_tools" = "yes" ; then
   fi
 fi
 if test "$softmmu" = yes ; then
-  if test "$linux" = yes; then
-    if test "$virtfs" != no && test "$cap" = yes && test "$attr" = yes ; then
+  if test "$virtfs" != no ; then
+    if test "$linux" = yes; then
+      if test "$cap" = yes && test "$attr" = yes ; then
+        virtfs=yes
+        tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
+      else
+        if test "$virtfs" = yes; then
+          error_exit "VirtFS requires libcap devel and libattr devel on Linux"
+        fi
+        virtfs=no
+      fi
+    elif test "$mingw32" = yes; then
       virtfs=yes
-      tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
     else
       if test "$virtfs" = yes; then
-        error_exit "VirtFS requires libcap devel and libattr devel"
+        error_exit "VirtFS is only supported on Linux or Windows"
       fi
       virtfs=no
     fi
@@ -5085,10 +5094,12 @@ if test "$softmmu" = yes ; then
       fi
       mpath=no
     fi
-    tools="$tools scsi/qemu-pr-helper\$(EXESUF)"
+    if test "$linux" = yes; then
+      tools="$tools scsi/qemu-pr-helper\$(EXESUF)"
+    fi
   else
     if test "$virtfs" = yes; then
-      error_exit "VirtFS is supported only on Linux"
+      error_exit "VirtFS is supported only on Linux or Windows"
     fi
     virtfs=no
     if test "$mpath" = yes; then
diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index fd90b62900..d11100857d 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -1,10 +1,13 @@
-common-obj-y  = 9p.o 9p-util.o
-common-obj-y += 9p-local.o 9p-xattr.o
-common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
+common-obj-y  = 9p.o
+common-obj-$(CONFIG_LINUX) += 9p-util.o
+common-obj-$(CONFIG_LINUX) += 9p-local.o 9p-xattr.o
+common-obj-$(CONFIG_LINUX) += 9p-xattr-user.o 9p-posix-acl.o
+common-obj-$(CONFIG_WIN32) += 9p-util.o
+common-obj-$(CONFIG_WIN32) += 9p-local.o
 common-obj-y += coth.o cofs.o codir.o cofile.o
 common-obj-y += coxattr.o 9p-synth.o
 common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
-common-obj-y += 9p-proxy.o
+common-obj-$(CONFIG_POSIX) += 9p-proxy.o
 common-obj-$(CONFIG_XEN) += xen-9p-backend.o
 
 obj-$(CONFIG_VIRTIO) += virtio-9p-device.o
-- 
2.13.2.windows.1

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Paolo Bonzini]

On 29/09/2017 13:13, Michael Fritscher wrote:
> "
> The biggest issue seems to be the *at stuff. I'll
> try to workaround this via getting the directories' path from the file
> descriptor with the /proc (as it is already done in the 9pfs_utils) -
> luckily,the mingw environment emulates the /proc. If this doesn't work
> I've another idea (the file descriptors needs to be "registered" with the
> path (and saved in a sparse vector or map with the fd as key and the path
> as value). The "big" solution would be to write a 9p_local_windows.c from
> scratch, but I would like to avoid it.
> "

Yes, that's pretty much the only way to do it; it's not the easiest
thing because you have to use NT kernel APIs (NtCreateFile) rather than
e.g. CreateFile.  Likewise for NtQueryAttributesFile,
NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(

Paolo

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

> On 29/09/2017 13:13, Michael Fritscher wrote:
>> "
>> The biggest issue seems to be the *at stuff. I'll
>> try to workaround this via getting the directories' path from the file
>> descriptor with the /proc (as it is already done in the 9pfs_utils) -
>> luckily,the mingw environment emulates the /proc. If this doesn't work
>> I've another idea (the file descriptors needs to be "registered" with
>> the
>> path (and saved in a sparse vector or map with the fd as key and the
>> path
>> as value). The "big" solution would be to write a 9p_local_windows.c
>> from
>> scratch, but I would like to avoid it.
>> "
>
> Yes, that's pretty much the only way to do it; it's not the easiest
> thing because you have to use NT kernel APIs (NtCreateFile) rather than
> e.g. CreateFile.  Likewise for NtQueryAttributesFile,
> NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(
>
> Paolo
>

Hi,

why do I need the NT* functions instead of the "normal" ones from
kernel32? And it was working even with the posix functions some time ago
(Which I'll port to current qemu atm)

>From the "host side", 9pfs does nothing special I think.

Best regards,
Michael Fritscher

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Paolo Bonzini]

On 29/09/2017 16:14, Michael Fritscher wrote:
>> Yes, that's pretty much the only way to do it; it's not the easiest
>> thing because you have to use NT kernel APIs (NtCreateFile) rather than
>> e.g. CreateFile.  Likewise for NtQueryAttributesFile,
>> NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(
>>
>> Paolo
>>
> Hi,
> 
> why do I need the NT* functions instead of the "normal" ones from
> kernel32? And it was working even with the posix functions some time ago
> (Which I'll port to current qemu atm)

openat lets you create a file from a directory handle and relative path.
 CreateFile doesn't let you do that, only the ntdll functions can.

Paolo

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

> On 29/09/2017 16:14, Michael Fritscher wrote:
>>> Yes, that's pretty much the only way to do it; it's not the easiest
>>> thing because you have to use NT kernel APIs (NtCreateFile) rather than
>>> e.g. CreateFile.  Likewise for NtQueryAttributesFile,
>>> NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(
>>>
>>> Paolo
>>>
>> Hi,
>>
>> why do I need the NT* functions instead of the "normal" ones from
>> kernel32? And it was working even with the posix functions some time ago
>> (Which I'll port to current qemu atm)
>
> openat lets you create a file from a directory handle and relative path.
>  CreateFile doesn't let you do that, only the ntdll functions can.
>
> Paolo

Yes, my plan was to emulate this (as it was done in qemu a year or so ago
in Linux as well). So I can do that or use direct Win32/NT api. But then I
should rewrite 9p_local completely and make a 9p_windows I guess?

Michael

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

Hi,

I've no fear to NT API :-D

surprisingly I'm the first guy which try to import things from ntdll it
seems.

I've a working PoC, which can open a directory successfully - see below.
Will code like this ever by acceptable for merging? Else: other ideas? :-)
And should I follow this way or try to emulate the relative path thing
myself? Your choose :-)

Best regards,
Michael Fritscher

-----------------------

    //Main info sources:
http://resources.infosecinstitute.com/calling-ntdll-functions-directly/
&
https://googleprojectzero.blogspot.de/2016/02/the-definitive-guide-on-win32-to-nt.html
    error_printf("Try to open %s\n", ctx->fs_root);

    typedef NTSTATUS  (__stdcall *NT_OPEN_FILE)(_Out_ PHANDLE FileHandle,
_In_ ACCESS_MASK DesiredAccess, IN POBJECT_ATTRIBUTES
ObjectAttributes, _Out_ PIO_STATUS_BLOCK IoStatusBlock, _In_ ULONG
ShareAccess, _In_ ULONG OpenOptions);
    NT_OPEN_FILE NtOpenFileStruct;

    typedef NTSTATUS (__stdcall
*RTL_ANSI_STRING_TO_UNICODE_STRING)(_Inout_ PUNICODE_STRING
DestinationString, _In_ PCANSI_STRING SourceString, _In_ BOOLEAN
AllocateDestinationString);
    RTL_ANSI_STRING_TO_UNICODE_STRING RtlAnsiStringToUnicodeStringStruct;

    // typedef BOOLEAN (__stdcall
*RTL_DOS_PATH_NAME_TO_RELATIVE_NT_PATH_NAME_U)(_In_ PCWSTR
DosFileName, _Out_ PUNICODE_STRING NtFileName, _Out_opt_  PWSTR*
FilePath, _Out_opt_ PRTL_RELATIVE_NAME RelativeName);
    // RTL_DOS_PATH_NAME_TO_RELATIVE_NT_PATH_NAME_U
RtlDosPathNameToRelativeNtPathName_U;

    //TODO: PRTL_RELATIVE_NAME_U
    typedef BOOLEAN (__stdcall
*RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U)(_In_opt_z_ PCWSTR DosPathName,
_Out_ PUNICODE_STRING NtPathName, _Out_opt_ PCWSTR * NtFileNamePart,
_Out_opt_ PVOID DirectoryInfo);
    RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U RtlDosPathNameToNtPathName_UStruct;

    typedef VOID (__stdcall *RTL_FREE_UNICODE_STRING)(_Inout_
PUNICODE_STRING UnicodeString);
    RTL_FREE_UNICODE_STRING RtlFreeUnicodeStringStruct;

    /* load the ntdll.dll */
    HMODULE hModule = LoadLibrary("ntdll.dll");

    NtOpenFileStruct = (NT_OPEN_FILE)GetProcAddress(hModule, "NtOpenFile");
    if(NtOpenFileStruct == NULL) {
        error_printf("Error: could not find the function NtOpenFile in
library ntdll.dll.");
        exit(-1);
    }
    error_printf("NtOpenFile is located at 0x%p in ntdll.dll.\n",
NtOpenFileStruct);

    RtlAnsiStringToUnicodeStringStruct =
(RTL_ANSI_STRING_TO_UNICODE_STRING)GetProcAddress(hModule,
"RtlAnsiStringToUnicodeString");
    if(RtlAnsiStringToUnicodeStringStruct == NULL) {
        error_printf("Error: could not find the function
RtlAnsiStringToUnicodeString in library ntdll.dll.");
        exit(-1);
    }

    RtlDosPathNameToNtPathName_UStruct =
(RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U)GetProcAddress(hModule,
"RtlDosPathNameToNtPathName_U");
    if(RtlDosPathNameToNtPathName_UStruct == NULL) {
        error_printf("Error: could not find the function
RtlAnsiStringToUnicodeString in library ntdll.dll.");
        exit(-1);
    }

    RtlFreeUnicodeStringStruct =
(RTL_FREE_UNICODE_STRING)GetProcAddress(hModule,
"RtlFreeUnicodeString");
    if(RtlFreeUnicodeStringStruct == NULL) {
        error_printf("Error: could not find the function RtlInitAnsiString
in library ntdll.dll.");
        exit(-1);
    }

    /* create the string in the right format */
    UNICODE_STRING filename_UNICODE;

    wchar_t filename_WIDECHAR[4096];

    MultiByteToWideChar(CP_ACP, 0, ctx->fs_root, -1, filename_WIDECHAR,
sizeof(filename_WIDECHAR));
    RtlDosPathNameToNtPathName_UStruct(filename_WIDECHAR,
&filename_UNICODE, NULL, NULL);

    /* initialize OBJECT_ATTRIBUTES */
    OBJECT_ATTRIBUTES obja;
    InitializeObjectAttributes(&obja, &filename_UNICODE,
OBJ_CASE_INSENSITIVE, NULL, NULL);

    /* call NtOpenFile */
    HANDLE file = NULL;
    ULONG shareAccess = 0;
    ULONG openOptions = FILE_DIRECTORY_FILE;
    IO_STATUS_BLOCK statusBlock;
    NTSTATUS stat = NtOpenFileStruct(&file, GENERIC_READ |
FILE_READ_ATTRIBUTES, &obja, &statusBlock, shareAccess, openOptions);
    if(NT_SUCCESS(stat)) {
        error_printf("File successfully opened.\n");
    }
    else {
        error_printf("File could not be opened: %lx.\n", stat);
    }

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

> On 29/09/2017 16:14, Michael Fritscher wrote:
>>> Yes, that's pretty much the only way to do it; it's not the easiest
>>> thing because you have to use NT kernel APIs (NtCreateFile) rather than
>>> e.g. CreateFile.  Likewise for NtQueryAttributesFile,
>>> NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(
>>>
>>> Paolo
>>>
>> Hi,
>>
>> why do I need the NT* functions instead of the "normal" ones from
>> kernel32? And it was working even with the posix functions some time ago
>> (Which I'll port to current qemu atm)
>
> openat lets you create a file from a directory handle and relative path.
>  CreateFile doesn't let you do that, only the ntdll functions can.
>
> Paolo
>

Hi,

dumb question: what is the advantage of openat vs. open - only the thing
that someone doesn't need to build the path together by hand?

If I understand the man page of openat correctly, it does _not_ prevent
someone to break out of the jail by using e.g. ../../../blah .
If this assumption is correctly perhaps it is better to avoid using the
*at function family (as it was some time ago) and sanitize the path (by
somehow canonizing it and than check if the beginning is ok).

Then I could use the "normal" posix function again and avoid using the NT*
Functions directly which is not soooo nice for various reasons.

Best regards,
Michael Fritscher

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

>
> Hi,
>
> dumb question: what is the advantage of openat vs. open - only the thing
> that someone doesn't need to build the path together by hand?
>
> If I understand the man page of openat correctly, it does _not_ prevent
> someone to break out of the jail by using e.g. ../../../blah .
> If this assumption is correctly perhaps it is better to avoid using the
> *at function family (as it was some time ago) and sanitize the path (by
> somehow canonizing it and than check if the beginning is ok).
>
> Then I could use the "normal" posix function again and avoid using the NT*
> Functions directly which is not soooo nice for various reasons.
>
> Best regards,
> Michael Fritscher
>

Hi again,

I see one thing: symlinks somewhere in the path (which seemed to be the
reason introducing the *at family). But I think that this can be handled
by canonlizing the path, too. realpath should do the job quite well.

Best regards,
Michael Fritscher

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Greg Kurz]

On Sun, 15 Oct 2017 21:13:34 +0200
"Michael Fritscher" <michael@fritscher.net> wrote:

> >
> > Hi,
> >
> > dumb question: what is the advantage of openat vs. open - only the thing
> > that someone doesn't need to build the path together by hand?
> >
> > If I understand the man page of openat correctly, it does _not_ prevent
> > someone to break out of the jail by using e.g. ../../../blah .
> > If this assumption is correctly perhaps it is better to avoid using the
> > *at function family (as it was some time ago) and sanitize the path (by
> > somehow canonizing it and than check if the beginning is ok).
> >
> > Then I could use the "normal" posix function again and avoid using the NT*
> > Functions directly which is not soooo nice for various reasons.
> >
> > Best regards,
> > Michael Fritscher
> >  
> 
> Hi again,
> 
> I see one thing: symlinks somewhere in the path (which seemed to be the
> reason introducing the *at family). But I think that this can be handled
> by canonlizing the path, too. realpath should do the job quite well.
> 

Unfortunately now because we have TOCTOU condition here: some path element
could be replaced by a symlink after realpath() but before we actually pass
the resulting path to a syscall.

> Best regards,
> Michael Fritscher
> 

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

On 15.10.2017 21:50, Greg Kurz wrote:

>> Hi again,
>>
>> I see one thing: symlinks somewhere in the path (which seemed to be the
>> reason introducing the *at family). But I think that this can be handled
>> by canonlizing the path, too. realpath should do the job quite well.
>>
> 
> Unfortunately now because we have TOCTOU condition here: some path element
> could be replaced by a symlink after realpath() but before we actually pass
> the resulting path to a syscall.

Ah, yes, you are right. So lets go on the "hard way" (ntdll) again.

This approach seems to work in general, but somehow I get stack
corrumption or something like that after returning from a function which
makes the relativ NTOpenFile call. But should be fixable. Then the big
work piece is to emulate posix behavior with NT* functions *g*

Michael Fritscher

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

>> I see one thing: symlinks somewhere in the path (which seemed to be the
>> reason introducing the *at family). But I think that this can be handled
>> by canonlizing the path, too. realpath should do the job quite well.
>>
>
> Unfortunately now because we have TOCTOU condition here: some path element
> could be replaced by a symlink after realpath() but before we actually
> pass
> the resulting path to a syscall.
>

Hi,

my mistake was that thinking that O_NOFOLLOW prevents following any
symlink in the path, but it prevents only following if the pathname itself
is a symlink...

Best regards,
Michael Frischer

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Greg Kurz]

On Sun, 15 Oct 2017 21:02:56 +0200
"Michael Fritscher" <michael@fritscher.net> wrote:

> > On 29/09/2017 16:14, Michael Fritscher wrote:  
> >>> Yes, that's pretty much the only way to do it; it's not the easiest
> >>> thing because you have to use NT kernel APIs (NtCreateFile) rather than
> >>> e.g. CreateFile.  Likewise for NtQueryAttributesFile,
> >>> NtQueryDirectoryObject, etc. NtOpenDirectoryObject. :(
> >>>
> >>> Paolo
> >>>  
> >> Hi,
> >>
> >> why do I need the NT* functions instead of the "normal" ones from
> >> kernel32? And it was working even with the posix functions some time ago
> >> (Which I'll port to current qemu atm)  
> >
> > openat lets you create a file from a directory handle and relative path.
> >  CreateFile doesn't let you do that, only the ntdll functions can.
> >
> > Paolo
> >  
> 
> Hi,
> 

Hi,

> dumb question: what is the advantage of openat vs. open - only the thing
> that someone doesn't need to build the path together by hand?
> 
> If I understand the man page of openat correctly, it does _not_ prevent
> someone to break out of the jail by using e.g. ../../../blah .

Look at v9fs_walk():

        if (not_same_qid(&pdu->s->root_qid, &qid) ||
            strcmp("..", wnames[name_idx].data)) {
            err = v9fs_co_name_to_path(pdu, &dpath, wnames[name_idx].data,
                                       &path);

If we have reached the jail root and the path element is "..", we skip it,
ie, it isn't possible to go beyond the jail root.

Cheers,

--
Greg

> If this assumption is correctly perhaps it is better to avoid using the
> *at function family (as it was some time ago) and sanitize the path (by
> somehow canonizing it and than check if the beginning is ok).
> 
> Then I could use the "normal" posix function again and avoid using the NT*
> Functions directly which is not soooo nice for various reasons.
> 
> Best regards,
> Michael Fritscher
> 

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Greg Kurz]

On Fri, 29 Sep 2017 13:13:05 +0200
Michael Fritscher <michael@fritscher.net> wrote:

> Good day,
> 

Hi,

Sorry for the late reply... Since I have only limited bandwidth, I'll only
be able to provide some limited feedback.

A global remark to start with: it is a usual practice to prefix each patch
title with the component's name, as explained here:

https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message

eg, patch titles in this series should start with something like with fsdev: or
9pfs: or oslib-win32:

> this is the patch series to make 9pfs buildable (not working!) on Windows.
> It is compiling and starts, but obvousily 9pfs is not working - in fact it
> spits out a initializing error.
> 

What's the error message ?

> Next step: Implement the directory and file handling stuff.
> 
> Quoting myself:
> 
> "
> The biggest issue seems to be the *at stuff. I'll
> try to workaround this via getting the directories' path from the file
> descriptor with the /proc (as it is already done in the 9pfs_utils) -
> luckily,the mingw environment emulates the /proc. If this doesn't work
> I've another idea (the file descriptors needs to be "registered" with the
> path (and saved in a sparse vector or map with the fd as key and the path
> as value). The "big" solution would be to write a 9p_local_windows.c from
> scratch, but I would like to avoid it.
> "
> 
> I know that some people want to have compiling and working together, but I
> hope(!) that I get a first working version next week.
> 

Any updates ? Not sure people will spend time reviewing this series until
something actually works.

> Best regards,
> Michael Fritscher
> 

Cheers,

--
Greg

PS: if you repost, please use my maintainer's email address groug@kaod.org

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Michael Fritscher]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

Hi,

> Sorry for the late reply... Since I have only limited bandwidth, I'll only
> be able to provide some limited feedback.
No problem!

>
> A global remark to start with: it is a usual practice to prefix each patch
> title with the component's name, as explained here:
>
> https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
>
> eg, patch titles in this series should start with something like with
> fsdev: or
> 9pfs: or oslib-win32:
Yes, I've seen that I forgot this a few minutes too late.


>> this is the patch series to make 9pfs buildable (not working!) on
>> Windows.
>> It is compiling and starts, but obvousily 9pfs is not working - in fact
>> it
>> spits out a initializing error.
>>
>
> What's the error message ?
Sadly I havn't a binary here - it was somthing like "couldn't initialize
the 9pfs". This came only if the cmdline tried to use 9pfs, so it was no
regression. The technical cause was that it tried to use a incompatible
path.

>
> Any updates ? Not sure people will spend time reviewing this series until
> something actually works.
>

The main thing was the question whether to make a sort of "hack" to
emulate the *at commands (which is the way used by the patch, the main
idea using /proc/<pid>/fd information) or make a 9pfs_local_windows
variant which uses the native ntdll api. Which is ... another style of
codeing, let me say it this way...

To get an idea I've attached the "main" code with my experiments to open
the directory. The main question is: Is this kind of code ok? Sadly it
seems the only way to access the ntdll's function, which in turn is the
only way to have *at aquivalents.

Yes, the code is a bit dirty (very WIP). Additionally, I seem to destroy
the stack, because after running this function qemu exists. Fixing this is
my next step.

Best regards,
Michael Fritscher

[-- Attachment #2: 9pfs_windows_code.c --]
[-- Type: text/plain, Size: 7754 bytes --]

#include <windows.h>
#include <winternl.h>

typedef NTSTATUS  (__stdcall *NT_OPEN_FILE)(_Out_ PHANDLE FileHandle, _In_ ACCESS_MASK DesiredAccess, IN POBJECT_ATTRIBUTES ObjectAttributes, _Out_ PIO_STATUS_BLOCK IoStatusBlock, _In_ ULONG ShareAccess, _In_ ULONG OpenOptions);
NT_OPEN_FILE NtOpenFileStruct;

// typedef VOID (__stdcall *RTL_INIT_ANSI_STRING)(_Out_ PANSI_STRING DestinationString, _In_opt_ PCSZ SourceString);
// RTL_INIT_ANSI_STRING RtlInitAnsiStringStruct;

// typedef VOID (__stdcall *RTL_INIT_STRING)(_Out_ PSTRING DestinationString, _In_opt_ PCSZ SourceString);
// RTL_INIT_STRING RtlInitStringStruct;

typedef NTSTATUS (__stdcall *RTL_ANSI_STRING_TO_UNICODE_STRING)(_Inout_ PUNICODE_STRING DestinationString, _In_ PCANSI_STRING SourceString, _In_ BOOLEAN AllocateDestinationString);
RTL_ANSI_STRING_TO_UNICODE_STRING RtlAnsiStringToUnicodeStringStruct;

// typedef BOOLEAN (__stdcall *RTL_DOS_PATH_NAME_TO_RELATIVE_NT_PATH_NAME_U)(_In_ PCWSTR DosFileName, _Out_ PUNICODE_STRING NtFileName, _Out_opt_  PWSTR* FilePath, _Out_opt_ PRTL_RELATIVE_NAME RelativeName);
// RTL_DOS_PATH_NAME_TO_RELATIVE_NT_PATH_NAME_U RtlDosPathNameToRelativeNtPathName_UStruct;

typedef BOOLEAN (__stdcall *RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U)(_In_opt_z_ PCWSTR DosPathName, _Out_ PUNICODE_STRING NtPathName, _Out_opt_ PCWSTR * NtFileNamePart, _Out_opt_ PVOID DirectoryInfo);
RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U RtlDosPathNameToNtPathName_UStruct;

typedef VOID  (__stdcall *RTL_INIT_UNICODE_STRING)(_Inout_ PUNICODE_STRING DestinationString, _In_opt_ PCWSTR SourceString);
RTL_INIT_UNICODE_STRING RtlInitUnicodeStringStruct;

typedef VOID (__stdcall *RTL_FREE_UNICODE_STRING)(_Inout_ PUNICODE_STRING UnicodeString);
RTL_FREE_UNICODE_STRING RtlFreeUnicodeStringStruct;

static void local_init_ntdll(void);

static void local_init_ntdll(void)
{
    /* load the ntdll.dll */
    HMODULE hModule = LoadLibrary("ntdll.dll");

    NtOpenFileStruct = (NT_OPEN_FILE)GetProcAddress(hModule, "NtOpenFile");
    if(NtOpenFileStruct == NULL) {
        error_printf("Error: could not find the function NtOpenFile in library ntdll.dll.");
        exit(-1);
    }
    error_printf("NtOpenFile is located at 0x%p in ntdll.dll.\n", NtOpenFileStruct);

    //RtlInitAnsiStringStruct = (RTL_INIT_ANSI_STRING)GetProcAddress(hModule, "RtlInitAnsiString");
    //if(NtOpenFileStruct == NULL) {
    //    error_printf("Error: could not find the function RtlInitAnsiString in library ntdll.dll.");
    //    exit(-1);
    //}

    //RtlInitStringStruct = (RTL_INIT_ANSI_STRING)GetProcAddress(hModule, "RtlInitString");
    //if(NtOpenFileStruct == NULL) {
    //    error_printf("Error: could not find the function RtlInitString in library ntdll.dll.");
    //    exit(-1);
    //}

    RtlAnsiStringToUnicodeStringStruct = (RTL_ANSI_STRING_TO_UNICODE_STRING)GetProcAddress(hModule, "RtlAnsiStringToUnicodeString");
    if(RtlAnsiStringToUnicodeStringStruct == NULL) {
        error_printf("Error: could not find the function RtlAnsiStringToUnicodeString in library ntdll.dll.");
        exit(-1);
    }

    // RtlDosPathNameToRelativeNtPathName_UStruct = (RTL_DOS_PATH_NAME_TO_RELATIVE_NT_PATH_NAME_U)GetProcAddress(hModule, "RtlDosPathNameToRelativeNtPathName_U");
    // if(RtlDosPathNameToRelativeNtPathName_UStruct == NULL) {
    //     error_printf("Error: could not find the function RtlDosPathNameToRelativeNtPathName in library ntdll.dll.");
    //     exit(-1);
    // }

    RtlDosPathNameToNtPathName_UStruct = (RTL_DOS_PATH_NAME_TO_NT_PATH_NAME_U)GetProcAddress(hModule, "RtlDosPathNameToNtPathName_U");
    if(RtlDosPathNameToNtPathName_UStruct == NULL) {
        error_printf("Error: could not find the function RtlDosPathNameToNtPathName in library ntdll.dll.");
        exit(-1);
    }

    RtlInitUnicodeStringStruct = (RTL_INIT_UNICODE_STRING)GetProcAddress(hModule, "RtlInitUnicodeString");
    if(RtlInitUnicodeStringStruct == NULL) {
        error_printf("Error: could not find the function RtlInitUnicodeString in library ntdll.dll.");
        exit(-1);
    }

    RtlFreeUnicodeStringStruct = (RTL_FREE_UNICODE_STRING)GetProcAddress(hModule, "RtlFreeUnicodeString");
    if(RtlFreeUnicodeStringStruct == NULL) {
        error_printf("Error: could not find the function RtlFreeUnicodeString in library ntdll.dll.");
        exit(-1);
    }

}

static HANDLE local_open_internal(FsContext *fs_ctx, const char* relative)
{
    LocalData *data = fs_ctx->private;

    error_printf("Try to open %s\n", relative);
	if (relative[0] == '.' && relative[1] == '\0') {
		return data->mount_handle;
	}

    /* create the string in the right format */
    //STRING filename;
    //ANSI_STRING filename_ANSI;
    UNICODE_STRING filename_UNICODE;

    //RtlInitStringStruct(&filename, ctx->fs_root);

    wchar_t filename_WIDECHAR[4096];

    MultiByteToWideChar(CP_ACP, 0, relative, -1, filename_WIDECHAR, sizeof(filename_WIDECHAR));
    RtlInitUnicodeStringStruct(&filename_UNICODE, filename_WIDECHAR);

    /* initialize OBJECT_ATTRIBUTES */
    OBJECT_ATTRIBUTES obja;
    InitializeObjectAttributes(&obja, &filename_UNICODE, OBJ_CASE_INSENSITIVE, data->mount_handle, NULL);
 
    /* call NtOpenFile */
    HANDLE file = NULL;
    ULONG shareAccess = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; //0;
    ULONG openOptions = 0; //FILE_NON_DIRECTORY_FILE; //FILE_DIRECTORY_FILE;
    IO_STATUS_BLOCK statusBlock;
    NTSTATUS stat = NtOpenFileStruct(&file, GENERIC_READ | FILE_READ_ATTRIBUTES, &obja, &statusBlock, shareAccess, openOptions);
    if(NT_SUCCESS(stat)) {
        error_printf("File successfully opened.\n");
    }
    else {
        error_printf("File could not be opened: %lx.\n", stat);
    }

    RtlFreeUnicodeStringStruct(&filename_UNICODE);

    return file;
}


static int local_init(FsContext *ctx)
{
#ifdef FS_IOC_GETVERSION
    struct statfs stbuf;
#endif
    LocalData *data = g_malloc(sizeof(*data));

    local_init_ntdll();

    //Main info sources: http://resources.infosecinstitute.com/calling-ntdll-functions-directly/ & https://googleprojectzero.blogspot.de/2016/02/the-definitive-guide-on-win32-to-nt.html
    error_printf("Try to open %s\n", ctx->fs_root);



    /* create the string in the right format */
    //STRING filename;
    //ANSI_STRING filename_ANSI;
    UNICODE_STRING filename_UNICODE;

    //RtlInitStringStruct(&filename, ctx->fs_root);

    wchar_t filename_WIDECHAR[4096];

    MultiByteToWideChar(CP_ACP, 0, ctx->fs_root, -1, filename_WIDECHAR, sizeof(filename_WIDECHAR));
    RtlDosPathNameToNtPathName_UStruct(filename_WIDECHAR, &filename_UNICODE, NULL, NULL);

    /* initialize OBJECT_ATTRIBUTES */
    OBJECT_ATTRIBUTES obja;
    InitializeObjectAttributes(&obja, &filename_UNICODE, OBJ_CASE_INSENSITIVE, NULL, NULL);
 
    /* call NtOpenFile */
    HANDLE file = NULL;
    ULONG shareAccess = 0;
    ULONG openOptions = FILE_DIRECTORY_FILE;
    IO_STATUS_BLOCK statusBlock;
    NTSTATUS stat = NtOpenFileStruct(&file, GENERIC_READ | FILE_READ_ATTRIBUTES, &obja, &statusBlock, shareAccess, openOptions);
    if(NT_SUCCESS(stat)) {
        error_printf("File successfully opened.\n");
    }
    else {
        error_printf("File could not be opened: %lx.\n", stat);
        goto err;
    }

    RtlFreeUnicodeStringStruct(&filename_UNICODE);

	
	// local_open_internal(file, "blah");
	// local_open_internal(file, "blub.txt");
	

    data->mount_handle = file;

    ctx->xops = 0;
    ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT;

    ctx->private = data;
    return 0;

err:
    g_free(data);
    return -1;
}

Re: [Qemu-devel] Make 9pfs buildable for Windows

[Paolo Bonzini]

On 06/11/2017 16:14, Michael Fritscher wrote:
> The main thing was the question whether to make a sort of "hack" to
> emulate the *at commands (which is the way used by the patch, the main
> idea using /proc/<pid>/fd information) or make a 9pfs_local_windows
> variant which uses the native ntdll api. Which is ... another style of
> codeing, let me say it this way...

Yeah, that's expected.

> To get an idea I've attached the "main" code with my experiments to open
> the directory. The main question is: Is this kind of code ok? Sadly it
> seems the only way to access the ntdll's function, which in turn is the
> only way to have *at aquivalents.

Just one note, it's probably better to use UTF-8 for the codepage instead
of CP_ACP:

    MultiByteToWideChar(CP_ACP, 0, relative, -1, filename_WIDECHAR, sizeof(filename_WIDECHAR));
    RtlInitUnicodeStringStruct(&filename_UNICODE, filename_WIDECHAR);

> Yes, the code is a bit dirty (very WIP). Additionally, I seem to destroy
> the stack, because after running this function qemu exists. Fixing this is
> my next step.

For what it's worth, the Wine code for kernel32 (see
https://github.com/wine-mirror/wine/blob/master/dlls/kernel32/file.c)
might be of some help.

> NT_OPEN_FILE NtOpenFileStruct;

Better:

static NT_OPEN_FILE p_NtOpenFile;

Paolo

> 
> Best regards,
> Michael Fritscher
>