Re: [Qemu-devel] [PATCH v2] [WIP] [RFC ]Add initial 9pfs support for Windows hosts v2

[Greg Kurz]

Hi Michael,

I have some more comments that should end up in a preliminary patch.

Cheers.

--
Greg

On Tue, 12 Apr 2016 09:52:00 +0200
Michael Fritscher <address@hidden> wrote:

> It was tested on Windows & Linux hosts, on the later no obvious regressions 
> could be found. The guest was a Knoppix 7.6.0 live cd.
> 
> This is WIP and a RFC - it isn't meant to be upstreamed yet. The error_printf 
> are only for debugging and will be deleted in the final patch.
> 
> There are some comments with "mifritscher" - I'm not quite sure how to handle 
> these problems (e.g. on mingw, this in 32 bit, while on Linux this is 16 bit 
> and so on). Additionally, some places from which I suspect problems are 
> marked with a #warning.
> 
> Changes against v1:
>   * fixed open (added binary flag) -> read/write is fine now, file
> creation was fixed as well
>   * added inode calculation using own crude hash algorithm
>   * fixed xattr returning -1  + EOPNOTSUP -> fixes e.g. executing files
>   * fixed unlinkat / remove für directories by trying rmdir as well
>   * prepend 777 mode on files (for executing)
> 
> What does work:
>   * reading/writing "big" files (5MB, verified with md5sum)
>   * creating files
>   * executing files
>   * mkdir
>   * rm / rmdir
>   * cp files
>   * running gcc
>   * cp first 250 files & directories from /etc
>   * tar it from and to the 9p mount
>   * create a 100 MB swap file and do a mkswap on it
> 
> What does not work:
>   * swapon - no real surprise (doesn't work on Linux as well - with the same 
> error message) ;-) (swapon returns -1 EINVAL)
>   * link / symlink (there is no real aequivalent for this on windows)
>   * mknod (dito)
>   * chown (dito - prepend success for compatibility)
> 
> State of v1:
> What works:
>    * mount
>    * stat
>    * chdir
>    * readdir (aka ls)
>    * read of small files (<10k)
>    * overwrite
> 
> What does not work:
>    * create new files (in 90% problems with path handling (old path occurs)
>    * read / probably write big files
> 
> 
> Signed-off-by: Michael Fritscher <address@hidden>
> ---
> On https://github.com/mifritscher/qemu/tree/9pfs_windows is a git tree with 
> the patch.
> 
> I gave a shot at xattr (try to include xattr stuff) but it ended up even more 
> #ifdefs. So I gave up at this.
> Now I'm waiting on comments etc. If there are no big objections, I'll remove 
> the debug output and make a patch which is meant for upstreaming.
> 
> Thanks to everybody which helped me!
> 
>  Makefile.objs              |   1 +
>  configure                  |  15 ++-
>  fsdev/9p-iov-marshal.c     |   2 +
>  fsdev/9p-marshal.c         |   4 +-
>  fsdev/file-op-9p.h         |  35 ++++-
>  fsdev/qemu-fsdev.c         |   2 +
>  hw/9pfs/9p-local.c         | 326 
> +++++++++++++++++++++++++++++++++++++++++----
>  hw/9pfs/9p-synth.c         |  10 +-
>  hw/9pfs/9p.c               | 102 +++++++++++++-
>  hw/9pfs/9p.h               |  70 +++++++++-
>  hw/9pfs/Makefile.objs      |   7 +-
>  hw/9pfs/codir.c            |  31 +++++
>  hw/9pfs/cofile.c           |  15 +++
>  hw/9pfs/cofs.c             |  15 +++
>  hw/9pfs/coxattr.c          |   6 +
>  hw/9pfs/virtio-9p-device.c |   4 +-
>  16 files changed, 600 insertions(+), 45 deletions(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 8f705f6..6fd02bc 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -48,6 +48,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/
>  common-obj-y += qemu-char.o #aio.o
> diff --git a/configure b/configure
> index 5db29f0..a4797c3 100755
> --- a/configure
> +++ b/configure
> @@ -4566,12 +4566,21 @@ if test "$want_tools" = "yes" ; then
>  fi
>  if test "$softmmu" = yes ; then
>    if test "$virtfs" != no ; then
> -    if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; 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 is supported only on Linux and requires 
> libcap-devel and libattr-devel"
> +        error_exit "VirtFS is only supported on Linux or Windows"
>        fi
>        virtfs=no
>      fi
> diff --git a/fsdev/9p-iov-marshal.c b/fsdev/9p-iov-marshal.c
> index fb40bdf..1a292c6 100644
> --- a/fsdev/9p-iov-marshal.c
> +++ b/fsdev/9p-iov-marshal.c
> @@ -15,7 +15,9 @@
>  #include <glib.h>
>  #include <glib/gprintf.h>
>  #include <utime.h>
> +#ifndef _WIN32
>  #include <sys/uio.h>
> +#endif
> 
>  #include "9p-iov-marshal.h"
>  #include "qemu/bswap.h"
> diff --git a/fsdev/9p-marshal.c b/fsdev/9p-marshal.c
> index 183d366..081cb88 100644
> --- a/fsdev/9p-marshal.c
> +++ b/fsdev/9p-marshal.c
> @@ -16,7 +16,9 @@
>  #include <glib/gprintf.h>
>  #include <dirent.h>
>  #include <utime.h>
> -#include <sys/uio.h>
> +#ifndef WIN32
> +    #include <sys/uio.h>
> +#endif
> 
>  #include "9p-marshal.h"
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index b8c2602..446c6a5 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -14,12 +14,43 @@
>  #define _FILEOP_H
>  #include <dirent.h>
>  #include <utime.h>
> -#include <sys/uio.h>
> -#include <sys/vfs.h>
> +#ifndef _WIN32
> +    #include <sys/uio.h>
> +    #include <sys/vfs.h>

This should be included by include/sysemu/os-posix.h, and you just need to add 
a 
forward declaration for struct statfs before struct FileOperations somewhere 
below.

> +#endif
> 
>  #define SM_LOCAL_MODE_BITS    0600
>  #define SM_LOCAL_DIR_MODE_BITS    0700
> 
> +#ifdef _WIN32
> +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;
> +};

All this block belongs to include/sysemu/os-win32.h.

> +#endif
> +