[Qemu-devel] [PATCH v3 00/13] 9p: Add support for Darwin

[Qemu-devel] [PATCH v3 00/13] 9p: Add support for Darwin

[Keno Fischer]

Hi Greg,

this is the rebased version of the patch series adding
support for building the 9p server on Darwin. As you
know a number of patches from the v2 version of this
series are already landed. This is the remaining patches.
Other than rebasing, there is onnly one minor change
in patch 11.

Keno

Keno Fischer (13):
  9p: linux: Fix a couple Linux assumptions
  9p: Rename 9p-util -> 9p-util-linux
  9p: darwin: Handle struct stat(fs) differences
  9p: darwin: Handle struct dirent differences
  9p: darwin: Explicitly cast comparisons of mode_t with -1
  9p: darwin: Ignore O_{NOATIME, DIRECT}
  9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX
  9p: darwin: *xattr_nofollow implementations
  9p: darwin: Compatibility for f/l*xattr
  9p: darwin: Provide a fallback implementation for utimensat
  9p: darwin: Implement compatibility for mknodat
  9p: darwin: virtfs-proxy: Implement setuid code for darwin
  9p: darwin: configure: Allow VirtFS on Darwin

 Makefile                    |   6 ++
 Makefile.objs               |   1 +
 configure                   |  22 +++--
 fsdev/file-op-9p.h          |   2 +-
 fsdev/virtfs-proxy-helper.c | 230 ++++++++++++++++++++++++++++----------------
 hw/9pfs/9p-local.c          |  25 +++--
 hw/9pfs/9p-proxy.c          |  17 +++-
 hw/9pfs/9p-synth.c          |   4 +
 hw/9pfs/9p-util-darwin.c    | 191 ++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p-util-linux.c     |  70 ++++++++++++++
 hw/9pfs/9p-util.c           |  59 ------------
 hw/9pfs/9p-util.h           |  27 ++++++
 hw/9pfs/9p.c                |  71 ++++++++++++--
 hw/9pfs/Makefile.objs       |   4 +-
 include/qemu/statfs.h       |  19 ++++
 include/qemu/xattr.h        |   4 +-
 16 files changed, 579 insertions(+), 173 deletions(-)
 create mode 100644 hw/9pfs/9p-util-darwin.c
 create mode 100644 hw/9pfs/9p-util-linux.c
 delete mode 100644 hw/9pfs/9p-util.c
 create mode 100644 include/qemu/statfs.h

-- 
2.8.1

[Qemu-devel] [PATCH v3 01/13] 9p: linux: Fix a couple Linux assumptions

[Keno Fischer]

From: Keno Fischer <keno@alumni.harvard.edu>

 - Guard Linux only headers.
 - Add qemu/statfs.h header to abstract over the which
   headers are needed for struct statfs
 - Define `ENOATTR` only if not only defined
   (it's defined in system headers on Darwin).

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 fsdev/file-op-9p.h          |  2 +-
 fsdev/virtfs-proxy-helper.c |  4 +++-
 hw/9pfs/9p-local.c          |  2 ++
 include/qemu/statfs.h       | 19 +++++++++++++++++++
 include/qemu/xattr.h        |  4 +++-
 5 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 include/qemu/statfs.h

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 3fa062b..111f804 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -16,7 +16,7 @@
 
 #include <dirent.h>
 #include <utime.h>
-#include <sys/vfs.h>
+#include "qemu/statfs.h"
 #include "qemu-fsdev-throttle.h"
 
 #define SM_LOCAL_MODE_BITS    0600
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 6f132c5..94fb069 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -13,17 +13,19 @@
 #include <sys/resource.h>
 #include <getopt.h>
 #include <syslog.h>
+#ifdef CONFIG_LINUX
 #include <sys/capability.h>
 #include <sys/fsuid.h>
-#include <sys/vfs.h>
 #include <sys/ioctl.h>
 #include <linux/fs.h>
 #ifdef CONFIG_LINUX_MAGIC_H
 #include <linux/magic.h>
 #endif
+#endif
 #include "qemu-common.h"
 #include "qemu/sockets.h"
 #include "qemu/xattr.h"
+#include "qemu/statfs.h"
 #include "9p-iov-marshal.h"
 #include "hw/9pfs/9p-proxy.h"
 #include "fsdev/9p-iov-marshal.h"
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 828e8d6..d713983 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -27,10 +27,12 @@
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include <libgen.h>
+#ifdef CONFIG_LINUX
 #include <linux/fs.h>
 #ifdef CONFIG_LINUX_MAGIC_H
 #include <linux/magic.h>
 #endif
+#endif
 #include <sys/ioctl.h>
 
 #ifndef XFS_SUPER_MAGIC
diff --git a/include/qemu/statfs.h b/include/qemu/statfs.h
new file mode 100644
index 0000000..dde289f
--- /dev/null
+++ b/include/qemu/statfs.h
@@ -0,0 +1,19 @@
+/*
+ * Host statfs header abstraction
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2, or any
+ * later version.  See the COPYING file in the top-level directory.
+ *
+ */
+#ifndef QEMU_STATFS_H
+#define QEMU_STATFS_H
+
+#ifdef CONFIG_LINUX
+# include <sys/vfs.h>
+#endif
+#ifdef CONFIG_DARWIN
+# include <sys/param.h>
+# include <sys/mount.h>
+#endif
+
+#endif
diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h
index a83fe8e..f1d0f7b 100644
--- a/include/qemu/xattr.h
+++ b/include/qemu/xattr.h
@@ -22,7 +22,9 @@
 #ifdef CONFIG_LIBATTR
 #  include <attr/xattr.h>
 #else
-#  define ENOATTR ENODATA
+#  if !defined(ENOATTR)
+#    define ENOATTR ENODATA
+#  endif
 #  include <sys/xattr.h>
 #endif
 
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 01/13] 9p: linux: Fix a couple Linux assumptions

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:45 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> From: Keno Fischer <keno@alumni.harvard.edu>
> 
>  - Guard Linux only headers.
>  - Add qemu/statfs.h header to abstract over the which
>    headers are needed for struct statfs

I've suggested in some other mail that you introduce a QEMU type
for struct statfs with per-OS helpers. I guess this would cover
this part as well.

>  - Define `ENOATTR` only if not only defined
>    (it's defined in system headers on Darwin).
> 

The ENOATTR part is independent from the others. Please move it
to its own patch.

> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
>  fsdev/file-op-9p.h          |  2 +-
>  fsdev/virtfs-proxy-helper.c |  4 +++-
>  hw/9pfs/9p-local.c          |  2 ++
>  include/qemu/statfs.h       | 19 +++++++++++++++++++
>  include/qemu/xattr.h        |  4 +++-
>  5 files changed, 28 insertions(+), 3 deletions(-)
>  create mode 100644 include/qemu/statfs.h
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 3fa062b..111f804 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -16,7 +16,7 @@
>  
>  #include <dirent.h>
>  #include <utime.h>
> -#include <sys/vfs.h>
> +#include "qemu/statfs.h"
>  #include "qemu-fsdev-throttle.h"
>  
>  #define SM_LOCAL_MODE_BITS    0600
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index 6f132c5..94fb069 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -13,17 +13,19 @@
>  #include <sys/resource.h>
>  #include <getopt.h>
>  #include <syslog.h>
> +#ifdef CONFIG_LINUX
>  #include <sys/capability.h>

I'd rather leave this to the other patch where you implement the
setuid logic for Darwin.

>  #include <sys/fsuid.h>

From the setfsuid(2) manual page:

Thus, setfsuid() is nowadays unneeded and should be avoided in new
applications (likewise for setfsgid(2)).

So this header can be safely dropped, along with the big "from man7
capabilities" which is basically explaining why we don't use setfsuid(),
and should rather appear in a commit message, than in the code.

> -#include <sys/vfs.h>
>  #include <sys/ioctl.h>

This is needed for ioctl(), which is used for FS_IOC_GETVERSION,
so this should rather be guarded by FS_IOC_GETVERSION than by
CONFIG_LINUX... or maybe not guarded at all if Darwin has this
header, for the sake of simplicity.

>  #include <linux/fs.h>
>  #ifdef CONFIG_LINUX_MAGIC_H
>  #include <linux/magic.h>
>  #endif
> +#endif
>  #include "qemu-common.h"
>  #include "qemu/sockets.h"
>  #include "qemu/xattr.h"
> +#include "qemu/statfs.h"
>  #include "9p-iov-marshal.h"
>  #include "hw/9pfs/9p-proxy.h"
>  #include "fsdev/9p-iov-marshal.h"
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 828e8d6..d713983 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -27,10 +27,12 @@
>  #include "qemu/error-report.h"
>  #include "qemu/option.h"
>  #include <libgen.h>
> +#ifdef CONFIG_LINUX
>  #include <linux/fs.h>
>  #ifdef CONFIG_LINUX_MAGIC_H
>  #include <linux/magic.h>
>  #endif
> +#endif
>  #include <sys/ioctl.h>
>  
>  #ifndef XFS_SUPER_MAGIC
> diff --git a/include/qemu/statfs.h b/include/qemu/statfs.h
> new file mode 100644
> index 0000000..dde289f
> --- /dev/null
> +++ b/include/qemu/statfs.h
> @@ -0,0 +1,19 @@
> +/*
> + * Host statfs header abstraction
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2, or any
> + * later version.  See the COPYING file in the top-level directory.
> + *
> + */
> +#ifndef QEMU_STATFS_H
> +#define QEMU_STATFS_H
> +
> +#ifdef CONFIG_LINUX
> +# include <sys/vfs.h>
> +#endif
> +#ifdef CONFIG_DARWIN
> +# include <sys/param.h>
> +# include <sys/mount.h>
> +#endif
> +
> +#endif
> diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h
> index a83fe8e..f1d0f7b 100644
> --- a/include/qemu/xattr.h
> +++ b/include/qemu/xattr.h
> @@ -22,7 +22,9 @@
>  #ifdef CONFIG_LIBATTR
>  #  include <attr/xattr.h>
>  #else
> -#  define ENOATTR ENODATA
> +#  if !defined(ENOATTR)
> +#    define ENOATTR ENODATA
> +#  endif
>  #  include <sys/xattr.h>
>  #endif
>  

[Qemu-devel] [PATCH v3 02/13] 9p: Rename 9p-util -> 9p-util-linux

[Keno Fischer]

The current file only has the Linux versions of these functions.
Rename the file accordingly and update the Makefile to only build
it on Linux. A Darwin version of these will follow later in the
series.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p-util-linux.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p-util.c       | 59 -------------------------------------------------
 hw/9pfs/Makefile.objs   |  3 ++-
 3 files changed, 61 insertions(+), 60 deletions(-)
 create mode 100644 hw/9pfs/9p-util-linux.c
 delete mode 100644 hw/9pfs/9p-util.c

diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
new file mode 100644
index 0000000..defa3a4
--- /dev/null
+++ b/hw/9pfs/9p-util-linux.c
@@ -0,0 +1,59 @@
+/*
+ * 9p utilities (Linux Implementation)
+ *
+ * Copyright IBM, Corp. 2017
+ *
+ * Authors:
+ *  Greg Kurz <groug@kaod.org>
+ *
+ * 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 "qemu/osdep.h"
+#include "qemu/xattr.h"
+#include "9p-util.h"
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                             void *value, size_t size)
+{
+    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+    int ret;
+
+    ret = lgetxattr(proc_path, name, value, size);
+    g_free(proc_path);
+    return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+                              char *list, size_t size)
+{
+    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+    int ret;
+
+    ret = llistxattr(proc_path, list, size);
+    g_free(proc_path);
+    return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+                                const char *name)
+{
+    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+    int ret;
+
+    ret = lremovexattr(proc_path, name);
+    g_free(proc_path);
+    return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                         void *value, size_t size, int flags)
+{
+    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+    int ret;
+
+    ret = lsetxattr(proc_path, name, value, size, flags);
+    g_free(proc_path);
+    return ret;
+}
diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c
deleted file mode 100644
index 614b7fc..0000000
--- a/hw/9pfs/9p-util.c
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * 9p utilities
- *
- * Copyright IBM, Corp. 2017
- *
- * Authors:
- *  Greg Kurz <groug@kaod.org>
- *
- * 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 "qemu/osdep.h"
-#include "qemu/xattr.h"
-#include "9p-util.h"
-
-ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
-                             void *value, size_t size)
-{
-    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
-    int ret;
-
-    ret = lgetxattr(proc_path, name, value, size);
-    g_free(proc_path);
-    return ret;
-}
-
-ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
-                              char *list, size_t size)
-{
-    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
-    int ret;
-
-    ret = llistxattr(proc_path, list, size);
-    g_free(proc_path);
-    return ret;
-}
-
-ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
-                                const char *name)
-{
-    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
-    int ret;
-
-    ret = lremovexattr(proc_path, name);
-    g_free(proc_path);
-    return ret;
-}
-
-int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
-                         void *value, size_t size, int flags)
-{
-    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
-    int ret;
-
-    ret = lsetxattr(proc_path, name, value, size, flags);
-    g_free(proc_path);
-    return ret;
-}
diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index e3fa673..95e3bc0 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -1,5 +1,6 @@
 ifeq ($(call lor,$(CONFIG_VIRTIO_9P),$(CONFIG_XEN)),y)
-common-obj-y  = 9p.o 9p-util.o
+common-obj-y  = 9p.o
+common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
 common-obj-y += 9p-local.o 9p-xattr.o
 common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
 common-obj-y += coth.o cofs.o codir.o cofile.o
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 02/13] 9p: Rename 9p-util -> 9p-util-linux

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:46 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> The current file only has the Linux versions of these functions.
> Rename the file accordingly and update the Makefile to only build
> it on Linux. A Darwin version of these will follow later in the
> series.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---

I reviewed this patch during v2:

https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00110.html

Please remember to add any Reviewed-by, Acked-by, Tested-by tags that were
posted. This speeds up the reviewing process when you post a new version.

>  hw/9pfs/9p-util-linux.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/9p-util.c       | 59 -------------------------------------------------
>  hw/9pfs/Makefile.objs   |  3 ++-
>  3 files changed, 61 insertions(+), 60 deletions(-)
>  create mode 100644 hw/9pfs/9p-util-linux.c
>  delete mode 100644 hw/9pfs/9p-util.c
> 
> diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
> new file mode 100644
> index 0000000..defa3a4
> --- /dev/null
> +++ b/hw/9pfs/9p-util-linux.c
> @@ -0,0 +1,59 @@
> +/*
> + * 9p utilities (Linux Implementation)
> + *
> + * Copyright IBM, Corp. 2017
> + *
> + * Authors:
> + *  Greg Kurz <groug@kaod.org>
> + *
> + * 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 "qemu/osdep.h"
> +#include "qemu/xattr.h"
> +#include "9p-util.h"
> +
> +ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                             void *value, size_t size)
> +{
> +    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> +    int ret;
> +
> +    ret = lgetxattr(proc_path, name, value, size);
> +    g_free(proc_path);
> +    return ret;
> +}
> +
> +ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> +                              char *list, size_t size)
> +{
> +    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> +    int ret;
> +
> +    ret = llistxattr(proc_path, list, size);
> +    g_free(proc_path);
> +    return ret;
> +}
> +
> +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> +                                const char *name)
> +{
> +    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> +    int ret;
> +
> +    ret = lremovexattr(proc_path, name);
> +    g_free(proc_path);
> +    return ret;
> +}
> +
> +int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                         void *value, size_t size, int flags)
> +{
> +    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> +    int ret;
> +
> +    ret = lsetxattr(proc_path, name, value, size, flags);
> +    g_free(proc_path);
> +    return ret;
> +}
> diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c
> deleted file mode 100644
> index 614b7fc..0000000
> --- a/hw/9pfs/9p-util.c
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/*
> - * 9p utilities
> - *
> - * Copyright IBM, Corp. 2017
> - *
> - * Authors:
> - *  Greg Kurz <groug@kaod.org>
> - *
> - * 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 "qemu/osdep.h"
> -#include "qemu/xattr.h"
> -#include "9p-util.h"
> -
> -ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> -                             void *value, size_t size)
> -{
> -    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> -    int ret;
> -
> -    ret = lgetxattr(proc_path, name, value, size);
> -    g_free(proc_path);
> -    return ret;
> -}
> -
> -ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> -                              char *list, size_t size)
> -{
> -    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> -    int ret;
> -
> -    ret = llistxattr(proc_path, list, size);
> -    g_free(proc_path);
> -    return ret;
> -}
> -
> -ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> -                                const char *name)
> -{
> -    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> -    int ret;
> -
> -    ret = lremovexattr(proc_path, name);
> -    g_free(proc_path);
> -    return ret;
> -}
> -
> -int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> -                         void *value, size_t size, int flags)
> -{
> -    char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
> -    int ret;
> -
> -    ret = lsetxattr(proc_path, name, value, size, flags);
> -    g_free(proc_path);
> -    return ret;
> -}
> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index e3fa673..95e3bc0 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -1,5 +1,6 @@
>  ifeq ($(call lor,$(CONFIG_VIRTIO_9P),$(CONFIG_XEN)),y)
> -common-obj-y  = 9p.o 9p-util.o
> +common-obj-y  = 9p.o
> +common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
>  common-obj-y += 9p-local.o 9p-xattr.o
>  common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
>  common-obj-y += coth.o cofs.o codir.o cofile.o

[Qemu-devel] [PATCH v3 03/13] 9p: darwin: Handle struct stat(fs) differences

[Keno Fischer]

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 fsdev/virtfs-proxy-helper.c | 14 +++++++++++---
 hw/9pfs/9p-proxy.c          | 17 ++++++++++++++---
 hw/9pfs/9p-synth.c          |  2 ++
 hw/9pfs/9p.c                | 16 ++++++++++++++--
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 94fb069..3bc1269 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -506,12 +506,15 @@ static void stat_to_prstat(ProxyStat *pr_stat, struct stat *stat)
     pr_stat->st_size = stat->st_size;
     pr_stat->st_blksize = stat->st_blksize;
     pr_stat->st_blocks = stat->st_blocks;
+#ifdef CONFIG_DARWIN
+    pr_stat->st_atim_nsec = stat->st_atimespec.tv_nsec;
+    pr_stat->st_mtim_nsec = stat->st_mtimespec.tv_nsec;
+    pr_stat->st_ctim_nsec = stat->st_ctimespec.tv_nsec;
+#else
     pr_stat->st_atim_sec = stat->st_atim.tv_sec;
-    pr_stat->st_atim_nsec = stat->st_atim.tv_nsec;
     pr_stat->st_mtim_sec = stat->st_mtim.tv_sec;
-    pr_stat->st_mtim_nsec = stat->st_mtim.tv_nsec;
     pr_stat->st_ctim_sec = stat->st_ctim.tv_sec;
-    pr_stat->st_ctim_nsec = stat->st_ctim.tv_nsec;
+#endif
 }
 
 static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct statfs *stfs)
@@ -524,10 +527,15 @@ static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct statfs *stfs)
     pr_stfs->f_bavail = stfs->f_bavail;
     pr_stfs->f_files = stfs->f_files;
     pr_stfs->f_ffree = stfs->f_ffree;
+#ifdef CONFIG_DARWIN
+    pr_stfs->f_fsid[0] = stfs->f_fsid.val[0];
+    pr_stfs->f_fsid[1] = stfs->f_fsid.val[1];
+#else
     pr_stfs->f_fsid[0] = stfs->f_fsid.__val[0];
     pr_stfs->f_fsid[1] = stfs->f_fsid.__val[1];
     pr_stfs->f_namelen = stfs->f_namelen;
     pr_stfs->f_frsize = stfs->f_frsize;
+#endif
 }
 
 /*
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 47a94e0..8a2c174 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -117,10 +117,15 @@ static void prstatfs_to_statfs(struct statfs *stfs, ProxyStatFS *prstfs)
     stfs->f_bavail = prstfs->f_bavail;
     stfs->f_files = prstfs->f_files;
     stfs->f_ffree = prstfs->f_ffree;
+#ifdef CONFIG_DARWIN
+    stfs->f_fsid.val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
+    stfs->f_fsid.val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
+#else
     stfs->f_fsid.__val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
     stfs->f_fsid.__val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
     stfs->f_namelen = prstfs->f_namelen;
     stfs->f_frsize = prstfs->f_frsize;
+#endif
 }
 
 /* Converts proxy_stat structure to VFS stat structure */
@@ -137,12 +142,18 @@ static void prstat_to_stat(struct stat *stbuf, ProxyStat *prstat)
    stbuf->st_size = prstat->st_size;
    stbuf->st_blksize = prstat->st_blksize;
    stbuf->st_blocks = prstat->st_blocks;
-   stbuf->st_atim.tv_sec = prstat->st_atim_sec;
-   stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
+   stbuf->st_atime = prstat->st_atim_sec;
    stbuf->st_mtime = prstat->st_mtim_sec;
-   stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
    stbuf->st_ctime = prstat->st_ctim_sec;
+#ifdef CONFIG_DARWIN
+   stbuf->st_atimespec.tv_nsec = prstat->st_atim_nsec;
+   stbuf->st_mtimespec.tv_nsec = prstat->st_mtim_nsec;
+   stbuf->st_ctimespec.tv_nsec = prstat->st_ctim_nsec;
+#else
+   stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
+   stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
    stbuf->st_ctim.tv_nsec = prstat->st_ctim_nsec;
+#endif
 }
 
 /*
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 54239c9..eb68b42 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -426,7 +426,9 @@ static int synth_statfs(FsContext *s, V9fsPath *fs_path,
     stbuf->f_bsize = 512;
     stbuf->f_blocks = 0;
     stbuf->f_files = synth_node_count;
+#ifndef CONFIG_DARWIN
     stbuf->f_namelen = NAME_MAX;
+#endif
     return 0;
 }
 
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index eef289e..8e6b908 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -905,11 +905,17 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
     v9lstat->st_blksize = stbuf->st_blksize;
     v9lstat->st_blocks = stbuf->st_blocks;
     v9lstat->st_atime_sec = stbuf->st_atime;
-    v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
     v9lstat->st_mtime_sec = stbuf->st_mtime;
-    v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
     v9lstat->st_ctime_sec = stbuf->st_ctime;
+#ifdef CONFIG_DARWIN
+    v9lstat->st_atime_nsec = stbuf->st_atimespec.tv_nsec;
+    v9lstat->st_mtime_nsec = stbuf->st_mtimespec.tv_nsec;
+    v9lstat->st_ctime_nsec = stbuf->st_ctimespec.tv_nsec;
+#else
+    v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
+    v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
     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;
 
@@ -2959,9 +2965,15 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
     f_bavail = stbuf->f_bavail/bsize_factor;
     f_files  = stbuf->f_files;
     f_ffree  = stbuf->f_ffree;
+#ifdef CONFIG_DARWIN
+    fsid_val = (unsigned int)stbuf->f_fsid.val[0] |
+               (unsigned long long)stbuf->f_fsid.val[1] << 32;
+    f_namelen = MAXNAMLEN;
+#else
     fsid_val = (unsigned int) stbuf->f_fsid.__val[0] |
                (unsigned long long)stbuf->f_fsid.__val[1] << 32;
     f_namelen = stbuf->f_namelen;
+#endif
 
     return pdu_marshal(pdu, offset, "ddqqqqqqd",
                        f_type, f_bsize, f_blocks, f_bfree,
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 03/13] 9p: darwin: Handle struct stat(fs) differences

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:47 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---

The problem with 'struct stat' and 'struct statfs' is that they're not only
system types, but also part of the FileOperations API in fsdev/file-op-9p.h:

    int (*lstat)(FsContext *, V9fsPath *, struct stat *);
    int (*fstat)(FsContext *, int, V9fsFidOpenState *, struct stat *);
    int (*statfs)(FsContext *s, V9fsPath *path, struct statfs *stbuf);

and these shouldn't depend on the host OS. I guess it's time to introduce our
own types, based on the Linux ones, as well as qemu_stat*() per-OS helpers.

This would allow to get rid of many ifdefs in this patch.

>  fsdev/virtfs-proxy-helper.c | 14 +++++++++++---
>  hw/9pfs/9p-proxy.c          | 17 ++++++++++++++---
>  hw/9pfs/9p-synth.c          |  2 ++
>  hw/9pfs/9p.c                | 16 ++++++++++++++--
>  4 files changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index 94fb069..3bc1269 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -506,12 +506,15 @@ static void stat_to_prstat(ProxyStat *pr_stat, struct stat *stat)
>      pr_stat->st_size = stat->st_size;
>      pr_stat->st_blksize = stat->st_blksize;
>      pr_stat->st_blocks = stat->st_blocks;
> +#ifdef CONFIG_DARWIN
> +    pr_stat->st_atim_nsec = stat->st_atimespec.tv_nsec;
> +    pr_stat->st_mtim_nsec = stat->st_mtimespec.tv_nsec;
> +    pr_stat->st_ctim_nsec = stat->st_ctimespec.tv_nsec;
> +#else
>      pr_stat->st_atim_sec = stat->st_atim.tv_sec;
> -    pr_stat->st_atim_nsec = stat->st_atim.tv_nsec;
>      pr_stat->st_mtim_sec = stat->st_mtim.tv_sec;
> -    pr_stat->st_mtim_nsec = stat->st_mtim.tv_nsec;
>      pr_stat->st_ctim_sec = stat->st_ctim.tv_sec;
> -    pr_stat->st_ctim_nsec = stat->st_ctim.tv_nsec;

Hmm... this is wrong. It translates tv_nsec only with Darwin and
tv_sec only with Linux.

> +#endif
>  }
>  
>  static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct statfs *stfs)
> @@ -524,10 +527,15 @@ static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct statfs *stfs)
>      pr_stfs->f_bavail = stfs->f_bavail;
>      pr_stfs->f_files = stfs->f_files;
>      pr_stfs->f_ffree = stfs->f_ffree;
> +#ifdef CONFIG_DARWIN
> +    pr_stfs->f_fsid[0] = stfs->f_fsid.val[0];
> +    pr_stfs->f_fsid[1] = stfs->f_fsid.val[1];
> +#else
>      pr_stfs->f_fsid[0] = stfs->f_fsid.__val[0];
>      pr_stfs->f_fsid[1] = stfs->f_fsid.__val[1];
>      pr_stfs->f_namelen = stfs->f_namelen;
>      pr_stfs->f_frsize = stfs->f_frsize;



Hmm... the ProxyStatFS type is part of the protocol used between
QEMU and virtfs-proxy-helper. It isn't acceptable to send a partly
initialized structure back to QEMU. If something is missing on
Darwin, please provide a fallback in Darwin's implementation for
qemu_statfs().

> +#endif
>  }
>  
>  /*
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index 47a94e0..8a2c174 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -117,10 +117,15 @@ static void prstatfs_to_statfs(struct statfs *stfs, ProxyStatFS *prstfs)
>      stfs->f_bavail = prstfs->f_bavail;
>      stfs->f_files = prstfs->f_files;
>      stfs->f_ffree = prstfs->f_ffree;
> +#ifdef CONFIG_DARWIN
> +    stfs->f_fsid.val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
> +    stfs->f_fsid.val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
> +#else
>      stfs->f_fsid.__val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFU;
>      stfs->f_fsid.__val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFU;
>      stfs->f_namelen = prstfs->f_namelen;
>      stfs->f_frsize = prstfs->f_frsize;

Same remark.

> +#endif
>  }
>  
>  /* Converts proxy_stat structure to VFS stat structure */
> @@ -137,12 +142,18 @@ static void prstat_to_stat(struct stat *stbuf, ProxyStat *prstat)
>     stbuf->st_size = prstat->st_size;
>     stbuf->st_blksize = prstat->st_blksize;
>     stbuf->st_blocks = prstat->st_blocks;
> -   stbuf->st_atim.tv_sec = prstat->st_atim_sec;
> -   stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
> +   stbuf->st_atime = prstat->st_atim_sec;
>     stbuf->st_mtime = prstat->st_mtim_sec;
> -   stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
>     stbuf->st_ctime = prstat->st_ctim_sec;
> +#ifdef CONFIG_DARWIN
> +   stbuf->st_atimespec.tv_nsec = prstat->st_atim_nsec;
> +   stbuf->st_mtimespec.tv_nsec = prstat->st_mtim_nsec;
> +   stbuf->st_ctimespec.tv_nsec = prstat->st_ctim_nsec;
> +#else
> +   stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
> +   stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
>     stbuf->st_ctim.tv_nsec = prstat->st_ctim_nsec;
> +#endif
>  }
>  
>  /*
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index 54239c9..eb68b42 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -426,7 +426,9 @@ static int synth_statfs(FsContext *s, V9fsPath *fs_path,
>      stbuf->f_bsize = 512;
>      stbuf->f_blocks = 0;
>      stbuf->f_files = synth_node_count;
> +#ifndef CONFIG_DARWIN
>      stbuf->f_namelen = NAME_MAX;
> +#endif
>      return 0;
>  }
>  
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index eef289e..8e6b908 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -905,11 +905,17 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
>      v9lstat->st_blksize = stbuf->st_blksize;
>      v9lstat->st_blocks = stbuf->st_blocks;
>      v9lstat->st_atime_sec = stbuf->st_atime;
> -    v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
>      v9lstat->st_mtime_sec = stbuf->st_mtime;
> -    v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
>      v9lstat->st_ctime_sec = stbuf->st_ctime;
> +#ifdef CONFIG_DARWIN
> +    v9lstat->st_atime_nsec = stbuf->st_atimespec.tv_nsec;
> +    v9lstat->st_mtime_nsec = stbuf->st_mtimespec.tv_nsec;
> +    v9lstat->st_ctime_nsec = stbuf->st_ctimespec.tv_nsec;
> +#else
> +    v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
> +    v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec;
>      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;
>  
> @@ -2959,9 +2965,15 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
>      f_bavail = stbuf->f_bavail/bsize_factor;
>      f_files  = stbuf->f_files;
>      f_ffree  = stbuf->f_ffree;
> +#ifdef CONFIG_DARWIN
> +    fsid_val = (unsigned int)stbuf->f_fsid.val[0] |
> +               (unsigned long long)stbuf->f_fsid.val[1] << 32;
> +    f_namelen = MAXNAMLEN;

Like said above, I'd prefer f_namelen being set to NAME_MAX, which is
the POSIX equivalent of MAXNAMLEN, in the backends (like you did for
9p synth).

> +#else
>      fsid_val = (unsigned int) stbuf->f_fsid.__val[0] |
>                 (unsigned long long)stbuf->f_fsid.__val[1] << 32;
>      f_namelen = stbuf->f_namelen;
> +#endif
>  
>      return pdu_marshal(pdu, offset, "ddqqqqqqd",
>                         f_type, f_bsize, f_blocks, f_bfree,

[Qemu-devel] [PATCH v3 04/13] 9p: darwin: Handle struct dirent differences

[Keno Fischer]

On darwin d_seekoff exists, but is optional and does not seem to
be commonly used by file systems. Use `telldir` instead to obtain
the seek offset.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p-synth.c |  2 ++
 hw/9pfs/9p.c       | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index eb68b42..a312f8c 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -221,7 +221,9 @@ static void synth_direntry(V9fsSynthNode *node,
 {
     strcpy(entry->d_name, node->name);
     entry->d_ino = node->attr->inode;
+#ifndef CONFIG_DARWIN
     entry->d_off = off + 1;
+#endif
 }
 
 static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 8e6b908..06139c9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1738,6 +1738,25 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
     return offset;
 }
 
+/**
+ * Get the seek offset of a dirent. If not available from the structure itself,
+ * obtain it by calling telldir.
+ */
+static int v9fs_dent_telldir(V9fsPDU *pdu, V9fsFidState *fidp,
+                             struct dirent *dent)
+{
+#ifdef CONFIG_DARWIN
+    /*
+     * Darwin has d_seekoff, which appears to function similarly to d_off.
+     * However, it does not appear to be supported on all file systems,
+     * so use telldir for correctness.
+     */
+    return v9fs_co_telldir(pdu, fidp);
+#else
+    return dent->d_off;
+#endif
+}
+
 static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
                                                   V9fsFidState *fidp,
                                                   uint32_t max_count)
@@ -1801,7 +1820,11 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
         count += len;
         v9fs_stat_free(&v9stat);
         v9fs_path_free(&path);
-        saved_dir_pos = dent->d_off;
+        saved_dir_pos = v9fs_dent_telldir(pdu, fidp, dent);
+        if (saved_dir_pos < 0) {
+            err = saved_dir_pos;
+            break;
+        }
     }
 
     v9fs_readdir_unlock(&fidp->fs.dir);
@@ -1915,7 +1938,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
     V9fsString name;
     int len, err = 0;
     int32_t count = 0;
-    off_t saved_dir_pos;
+    off_t saved_dir_pos, off;
     struct dirent *dent;
 
     /* save the directory position */
@@ -1951,10 +1974,15 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
         /* Fill the other fields with dummy values */
         qid.type = 0;
         qid.version = 0;
+        off = v9fs_dent_telldir(pdu, fidp, dent);
+        if (off < 0) {
+            err = off;
+            break;
+        }
 
         /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
         len = pdu_marshal(pdu, 11 + count, "Qqbs",
-                          &qid, dent->d_off,
+                          &qid, off,
                           dent->d_type, &name);
 
         v9fs_readdir_unlock(&fidp->fs.dir);
@@ -1966,7 +1994,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
         }
         count += len;
         v9fs_string_free(&name);
-        saved_dir_pos = dent->d_off;
+        saved_dir_pos = off;
     }
 
     v9fs_readdir_unlock(&fidp->fs.dir);
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 04/13] 9p: darwin: Handle struct dirent differences

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:48 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> On darwin d_seekoff exists, but is optional and does not seem to
> be commonly used by file systems. Use `telldir` instead to obtain
> the seek offset.
> 

Thinking again about this one, I guess we also need our own
type for dirent and per-OS qemu_readdir(), and have the Darwin
version to call telldir() directly.

> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
>  hw/9pfs/9p-synth.c |  2 ++
>  hw/9pfs/9p.c       | 36 ++++++++++++++++++++++++++++++++----
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index eb68b42..a312f8c 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -221,7 +221,9 @@ static void synth_direntry(V9fsSynthNode *node,
>  {
>      strcpy(entry->d_name, node->name);
>      entry->d_ino = node->attr->inode;
> +#ifndef CONFIG_DARWIN
>      entry->d_off = off + 1;
> +#endif
>  }
>  
>  static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 8e6b908..06139c9 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1738,6 +1738,25 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>      return offset;
>  }
>  
> +/**
> + * Get the seek offset of a dirent. If not available from the structure itself,
> + * obtain it by calling telldir.
> + */
> +static int v9fs_dent_telldir(V9fsPDU *pdu, V9fsFidState *fidp,
> +                             struct dirent *dent)
> +{
> +#ifdef CONFIG_DARWIN
> +    /*
> +     * Darwin has d_seekoff, which appears to function similarly to d_off.
> +     * However, it does not appear to be supported on all file systems,
> +     * so use telldir for correctness.
> +     */
> +    return v9fs_co_telldir(pdu, fidp);
> +#else
> +    return dent->d_off;
> +#endif
> +}
> +
>  static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>                                                    V9fsFidState *fidp,
>                                                    uint32_t max_count)
> @@ -1801,7 +1820,11 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          count += len;
>          v9fs_stat_free(&v9stat);
>          v9fs_path_free(&path);
> -        saved_dir_pos = dent->d_off;
> +        saved_dir_pos = v9fs_dent_telldir(pdu, fidp, dent);
> +        if (saved_dir_pos < 0) {
> +            err = saved_dir_pos;
> +            break;
> +        }
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -1915,7 +1938,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>      V9fsString name;
>      int len, err = 0;
>      int32_t count = 0;
> -    off_t saved_dir_pos;
> +    off_t saved_dir_pos, off;
>      struct dirent *dent;
>  
>      /* save the directory position */
> @@ -1951,10 +1974,15 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>          /* Fill the other fields with dummy values */
>          qid.type = 0;
>          qid.version = 0;
> +        off = v9fs_dent_telldir(pdu, fidp, dent);
> +        if (off < 0) {
> +            err = off;
> +            break;
> +        }
>  
>          /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
>          len = pdu_marshal(pdu, 11 + count, "Qqbs",
> -                          &qid, dent->d_off,
> +                          &qid, off,
>                            dent->d_type, &name);
>  
>          v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -1966,7 +1994,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
>          }
>          count += len;
>          v9fs_string_free(&name);
> -        saved_dir_pos = dent->d_off;
> +        saved_dir_pos = off;
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);

[Qemu-devel] [PATCH v3 05/13] 9p: darwin: Explicitly cast comparisons of mode_t with -1

[Keno Fischer]

Comparisons of mode_t with -1 require an explicit cast, since mode_t
is unsigned on Darwin.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p-local.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index d713983..98d4073 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -310,7 +310,7 @@ update_map_file:
     if (credp->fc_gid != -1) {
         gid = credp->fc_gid;
     }
-    if (credp->fc_mode != -1) {
+    if (credp->fc_mode != (mode_t)-1) {
         mode = credp->fc_mode;
     }
     if (credp->fc_rdev != -1) {
@@ -416,7 +416,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
             return err;
         }
     }
-    if (credp->fc_mode != -1) {
+    if (credp->fc_mode != (mode_t)-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);
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 05/13] 9p: darwin: Explicitly cast comparisons of mode_t with -1

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:49 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> Comparisons of mode_t with -1 require an explicit cast, since mode_t
> is unsigned on Darwin.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---

Applied to 9p-next.

>  hw/9pfs/9p-local.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index d713983..98d4073 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -310,7 +310,7 @@ update_map_file:
>      if (credp->fc_gid != -1) {
>          gid = credp->fc_gid;
>      }
> -    if (credp->fc_mode != -1) {
> +    if (credp->fc_mode != (mode_t)-1) {
>          mode = credp->fc_mode;
>      }
>      if (credp->fc_rdev != -1) {
> @@ -416,7 +416,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
>              return err;
>          }
>      }
> -    if (credp->fc_mode != -1) {
> +    if (credp->fc_mode != (mode_t)-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);

[Qemu-devel] [PATCH v3 06/13] 9p: darwin: Ignore O_{NOATIME, DIRECT}

[Keno Fischer]

Darwin doesn't have either of these flags. Darwin does have
F_NOCACHE, which is similar to O_DIRECT, but has different
enough semantics that other projects don't generally map
them automatically. In any case, we don't support O_DIRECT
on Linux at the moment either.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 06139c9..e650459 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -123,11 +123,18 @@ static int dotl_to_open_flags(int flags)
         { P9_DOTL_NONBLOCK, O_NONBLOCK } ,
         { P9_DOTL_DSYNC, O_DSYNC },
         { P9_DOTL_FASYNC, FASYNC },
+#ifndef CONFIG_DARWIN
+        { P9_DOTL_NOATIME, O_NOATIME },
+        /* On Darwin, we could map to F_NOCACHE, which is
+           similar, but doesn't quite have the same
+           semantics. However, we don't support O_DIRECT
+           even on linux at the moment, so we just ignore
+           it here. */
         { P9_DOTL_DIRECT, O_DIRECT },
+#endif
         { P9_DOTL_LARGEFILE, O_LARGEFILE },
         { P9_DOTL_DIRECTORY, O_DIRECTORY },
         { P9_DOTL_NOFOLLOW, O_NOFOLLOW },
-        { P9_DOTL_NOATIME, O_NOATIME },
         { P9_DOTL_SYNC, O_SYNC },
     };
 
@@ -156,10 +163,12 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
      */
     flags = dotl_to_open_flags(oflags);
     flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
+#ifndef CONFIG_DARWIN
     /*
      * Ignore direct disk access hint until the server supports it.
      */
     flags &= ~O_DIRECT;
+#endif
     return flags;
 }
 
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 06/13] 9p: darwin: Ignore O_{NOATIME, DIRECT}

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:50 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> Darwin doesn't have either of these flags. Darwin does have
> F_NOCACHE, which is similar to O_DIRECT, but has different
> enough semantics that other projects don't generally map
> them automatically. In any case, we don't support O_DIRECT
> on Linux at the moment either.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/9pfs/9p.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 06139c9..e650459 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -123,11 +123,18 @@ static int dotl_to_open_flags(int flags)
>          { P9_DOTL_NONBLOCK, O_NONBLOCK } ,
>          { P9_DOTL_DSYNC, O_DSYNC },
>          { P9_DOTL_FASYNC, FASYNC },
> +#ifndef CONFIG_DARWIN
> +        { P9_DOTL_NOATIME, O_NOATIME },
> +        /* On Darwin, we could map to F_NOCACHE, which is
> +           similar, but doesn't quite have the same
> +           semantics. However, we don't support O_DIRECT
> +           even on linux at the moment, so we just ignore
> +           it here. */
>          { P9_DOTL_DIRECT, O_DIRECT },
> +#endif
>          { P9_DOTL_LARGEFILE, O_LARGEFILE },
>          { P9_DOTL_DIRECTORY, O_DIRECTORY },
>          { P9_DOTL_NOFOLLOW, O_NOFOLLOW },
> -        { P9_DOTL_NOATIME, O_NOATIME },
>          { P9_DOTL_SYNC, O_SYNC },
>      };
>  
> @@ -156,10 +163,12 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
>       */
>      flags = dotl_to_open_flags(oflags);
>      flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
> +#ifndef CONFIG_DARWIN
>      /*
>       * Ignore direct disk access hint until the server supports it.
>       */
>      flags &= ~O_DIRECT;
> +#endif
>      return flags;
>  }
>  

[Qemu-devel] [PATCH v3 07/13] 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX

[Keno Fischer]

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e650459..abfb8dc 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3374,6 +3374,13 @@ out_nofid:
     v9fs_string_free(&name);
 }
 
+#if defined(CONFIG_DARWIN) && !defined(XATTR_SIZE_MAX)
+/* Darwin doesn't seem to define a maximum xattr size in its user
+   user space header, but looking at the kernel source, HFS supports
+   up to INT32_MAX, so use that as the maximum.
+*/
+#define XATTR_SIZE_MAX INT32_MAX
+#endif
 static void coroutine_fn v9fs_xattrcreate(void *opaque)
 {
     int flags, rflags = 0;
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 07/13] 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:51 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
>  hw/9pfs/9p.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index e650459..abfb8dc 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3374,6 +3374,13 @@ out_nofid:
>      v9fs_string_free(&name);
>  }
>  
> +#if defined(CONFIG_DARWIN) && !defined(XATTR_SIZE_MAX)
> +/* Darwin doesn't seem to define a maximum xattr size in its user
> +   user space header, but looking at the kernel source, HFS supports
> +   up to INT32_MAX, so use that as the maximum.
> +*/
> +#define XATTR_SIZE_MAX INT32_MAX

XATTR_SIZE_MAX is used to cap the size that was received from the client,
and then we do:

    xattr_fidp->fs.xattr.value = g_malloc0(size);

so I'd rather have something very much smaller than 4 gigs.

I now think that having an explicit dependency on XATTR_SIZE_MAX was a poor
choice actually. The problem isn't related to what the host supports, but
only to set a reasonable limit on what we accept from guests. We don't want
a buggy 9p client to crash QEMU.

TXATTRCREATE is only present in the 9p2000.L protocol, ie, linux guests,
which I don't expect to create extended attributes greater than 64k (ie,
XATTR_SIZE_MAX in the guest). We've been using this value for nearly two
years and nobody complained.

No need for a define. I guess you can open code the limit at the only
location where it is needed, with a short comment.

> +#endif
>  static void coroutine_fn v9fs_xattrcreate(void *opaque)
>  {
>      int flags, rflags = 0;

[Qemu-devel] [PATCH v3 08/13] 9p: darwin: *xattr_nofollow implementations

[Keno Fischer]

This implements the darwin equivalent of the functions that were
moved to 9p-util(-linux) earlier in this series in the new
9p-util-darwin file.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 hw/9pfs/9p-util-darwin.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/Makefile.objs    |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 hw/9pfs/9p-util-darwin.c

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
new file mode 100644
index 0000000..cdb4c9e
--- /dev/null
+++ b/hw/9pfs/9p-util-darwin.c
@@ -0,0 +1,64 @@
+/*
+ * 9p utilities (Darwin Implementation)
+ *
+ * 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 "qemu/osdep.h"
+#include "qemu/xattr.h"
+#include "9p-util.h"
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                             void *value, size_t size)
+{
+    int ret;
+    int fd = openat_file(dirfd, filename,
+                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    ret = fgetxattr(fd, name, value, size, 0, 0);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+                              char *list, size_t size)
+{
+    int ret;
+    int fd = openat_file(dirfd, filename,
+                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    ret = flistxattr(fd, list, size, 0);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+                                const char *name)
+{
+    int ret;
+    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    ret = fremovexattr(fd, name, 0);
+    close_preserve_errno(fd);
+    return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+                         void *value, size_t size, int flags)
+{
+    int ret;
+    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+    ret = fsetxattr(fd, name, value, size, 0, flags);
+    close_preserve_errno(fd);
+    return ret;
+}
diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index 95e3bc0..0de39af 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -1,6 +1,7 @@
 ifeq ($(call lor,$(CONFIG_VIRTIO_9P),$(CONFIG_XEN)),y)
 common-obj-y  = 9p.o
 common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
+common-obj-$(CONFIG_DARWIN) += 9p-util-darwin.o
 common-obj-y += 9p-local.o 9p-xattr.o
 common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
 common-obj-y += coth.o cofs.o codir.o cofile.o
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 08/13] 9p: darwin: *xattr_nofollow implementations

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:52 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> This implements the darwin equivalent of the functions that were
> moved to 9p-util(-linux) earlier in this series in the new
> 9p-util-darwin file.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
>  hw/9pfs/9p-util-darwin.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/Makefile.objs    |  1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 hw/9pfs/9p-util-darwin.c
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> new file mode 100644
> index 0000000..cdb4c9e
> --- /dev/null
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -0,0 +1,64 @@
> +/*
> + * 9p utilities (Darwin Implementation)
> + *
> + * 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 "qemu/osdep.h"
> +#include "qemu/xattr.h"
> +#include "9p-util.h"
> +
> +ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                             void *value, size_t size)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename,
> +                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fgetxattr(fd, name, value, size, 0, 0);

Hmm... I had said it looked good, but not that good actually. This isn't
strictly equivalent of what we have with Linux, because it requires the
file to have appropriate access rights.

Why not just use the magic sycall from patch 11 and call getxattr() ?

> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> +                              char *list, size_t size)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename,
> +                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = flistxattr(fd, list, size, 0);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> +                                const char *name)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fremovexattr(fd, name, 0);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                         void *value, size_t size, int flags)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fsetxattr(fd, name, value, size, 0, flags);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index 95e3bc0..0de39af 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -1,6 +1,7 @@
>  ifeq ($(call lor,$(CONFIG_VIRTIO_9P),$(CONFIG_XEN)),y)
>  common-obj-y  = 9p.o
>  common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
> +common-obj-$(CONFIG_DARWIN) += 9p-util-darwin.o
>  common-obj-y += 9p-local.o 9p-xattr.o
>  common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
>  common-obj-y += coth.o cofs.o codir.o cofile.o

[Qemu-devel] [PATCH v3 09/13] 9p: darwin: Compatibility for f/l*xattr

[Keno Fischer]

On darwin `fgetxattr` takes two extra optional arguments,
and the l* variants are not defined (in favor of an extra
flag to the regular variants.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 Makefile                    |  6 ++++++
 fsdev/virtfs-proxy-helper.c |  9 +++++----
 hw/9pfs/9p-local.c          | 12 ++++++++----
 hw/9pfs/9p-util.h           | 17 +++++++++++++++++
 4 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index e46f2b6..046e553 100644
--- a/Makefile
+++ b/Makefile
@@ -545,7 +545,13 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
 qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS)
 
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
+ifdef CONFIG_DARWIN
+fsdev/virtfs-proxy-helper$(EXESUF): hw/9pfs/9p-util-darwin.o
+endif
+ifdef CONFIG_LINUX
+fsdev/virtfs-proxy-helper$(EXESUF): hw/9pfs/9p-util-linux.o
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
+endif
 
 scsi/qemu-pr-helper$(EXESUF): scsi/qemu-pr-helper.o scsi/utils.o $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
 ifdef CONFIG_MPATH
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 3bc1269..a26f8b8 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -28,6 +28,7 @@
 #include "qemu/statfs.h"
 #include "9p-iov-marshal.h"
 #include "hw/9pfs/9p-proxy.h"
+#include "hw/9pfs/9p-util.h"
 #include "fsdev/9p-iov-marshal.h"
 
 #define PROGNAME "virtfs-proxy-helper"
@@ -459,7 +460,7 @@ static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
         v9fs_string_init(&name);
         retval = proxy_unmarshal(iovec, offset, "s", &name);
         if (retval > 0) {
-            retval = lgetxattr(path.data, name.data, xattr.data, size);
+            retval = qemu_lgetxattr(path.data, name.data, xattr.data, size);
             if (retval < 0) {
                 retval = -errno;
             } else {
@@ -469,7 +470,7 @@ static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
         v9fs_string_free(&name);
         break;
     case T_LLISTXATTR:
-        retval = llistxattr(path.data, xattr.data, size);
+        retval = qemu_llistxattr(path.data, xattr.data, size);
         if (retval < 0) {
             retval = -errno;
         } else {
@@ -1000,7 +1001,7 @@ static int process_requests(int sock)
             retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sssdd", &path,
                                      &name, &value, &size, &flags);
             if (retval > 0) {
-                retval = lsetxattr(path.data,
+                retval = qemu_lsetxattr(path.data,
                                    name.data, value.data, size, flags);
                 if (retval < 0) {
                     retval = -errno;
@@ -1016,7 +1017,7 @@ static int process_requests(int sock)
             retval = proxy_unmarshal(&in_iovec,
                                      PROXY_HDR_SZ, "ss", &path, &name);
             if (retval > 0) {
-                retval = lremovexattr(path.data, name.data);
+                retval = qemu_lremovexattr(path.data, name.data);
                 if (retval < 0) {
                     retval = -errno;
                 }
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 98d4073..768ef6f 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -776,16 +776,20 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
         mode_t tmp_mode;
         dev_t tmp_dev;
 
-        if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+        if (qemu_fgetxattr(fd, "user.virtfs.uid",
+                           &tmp_uid, sizeof(uid_t)) > 0) {
             stbuf->st_uid = le32_to_cpu(tmp_uid);
         }
-        if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+        if (qemu_fgetxattr(fd, "user.virtfs.gid",
+                           &tmp_gid, sizeof(gid_t)) > 0) {
             stbuf->st_gid = le32_to_cpu(tmp_gid);
         }
-        if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
+        if (qemu_fgetxattr(fd, "user.virtfs.mode",
+                           &tmp_mode, sizeof(mode_t)) > 0) {
             stbuf->st_mode = le32_to_cpu(tmp_mode);
         }
-        if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
+        if (qemu_fgetxattr(fd, "user.virtfs.rdev",
+                           &tmp_dev, sizeof(dev_t)) > 0) {
             stbuf->st_rdev = le64_to_cpu(tmp_dev);
         }
     } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 79ed6b2..50a03c7 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -19,6 +19,23 @@
 #define O_PATH_9P_UTIL 0
 #endif
 
+#ifdef CONFIG_DARWIN
+#define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
+#define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
+#define qemu_llistxattr(...) listxattr(__VA_ARGS__, XATTR_NOFOLLOW)
+#define qemu_lremovexattr(...) removexattr(__VA_ARGS__, XATTR_NOFOLLOW)
+static inline int qemu_lsetxattr(const char *path, const char *name,
+                                 const void *value, size_t size, int flags) {
+    return setxattr(path, name, value, size, 0, flags | XATTR_NOFOLLOW);
+}
+#else
+#define qemu_fgetxattr fgetxattr
+#define qemu_lgetxattr lgetxattr
+#define qemu_llistxattr llistxattr
+#define qemu_lremovexattr lremovexattr
+#define qemu_lsetxattr lsetxattr
+#endif
+
 static inline void close_preserve_errno(int fd)
 {
     int serrno = errno;
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 09/13] 9p: darwin: Compatibility for f/l*xattr

[Greg Kurz]

On Sat, 16 Jun 2018 20:56:53 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> On darwin `fgetxattr` takes two extra optional arguments,
> and the l* variants are not defined (in favor of an extra
> flag to the regular variants.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  Makefile                    |  6 ++++++
>  fsdev/virtfs-proxy-helper.c |  9 +++++----
>  hw/9pfs/9p-local.c          | 12 ++++++++----
>  hw/9pfs/9p-util.h           | 17 +++++++++++++++++
>  4 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e46f2b6..046e553 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -545,7 +545,13 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
>  qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS)
>  
>  fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
> +ifdef CONFIG_DARWIN
> +fsdev/virtfs-proxy-helper$(EXESUF): hw/9pfs/9p-util-darwin.o
> +endif
> +ifdef CONFIG_LINUX
> +fsdev/virtfs-proxy-helper$(EXESUF): hw/9pfs/9p-util-linux.o
>  fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
> +endif
>  
>  scsi/qemu-pr-helper$(EXESUF): scsi/qemu-pr-helper.o scsi/utils.o $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
>  ifdef CONFIG_MPATH
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index 3bc1269..a26f8b8 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -28,6 +28,7 @@
>  #include "qemu/statfs.h"
>  #include "9p-iov-marshal.h"
>  #include "hw/9pfs/9p-proxy.h"
> +#include "hw/9pfs/9p-util.h"
>  #include "fsdev/9p-iov-marshal.h"
>  
>  #define PROGNAME "virtfs-proxy-helper"
> @@ -459,7 +460,7 @@ static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
>          v9fs_string_init(&name);
>          retval = proxy_unmarshal(iovec, offset, "s", &name);
>          if (retval > 0) {
> -            retval = lgetxattr(path.data, name.data, xattr.data, size);
> +            retval = qemu_lgetxattr(path.data, name.data, xattr.data, size);
>              if (retval < 0) {
>                  retval = -errno;
>              } else {
> @@ -469,7 +470,7 @@ static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
>          v9fs_string_free(&name);
>          break;
>      case T_LLISTXATTR:
> -        retval = llistxattr(path.data, xattr.data, size);
> +        retval = qemu_llistxattr(path.data, xattr.data, size);
>          if (retval < 0) {
>              retval = -errno;
>          } else {
> @@ -1000,7 +1001,7 @@ static int process_requests(int sock)
>              retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sssdd", &path,
>                                       &name, &value, &size, &flags);
>              if (retval > 0) {
> -                retval = lsetxattr(path.data,
> +                retval = qemu_lsetxattr(path.data,
>                                     name.data, value.data, size, flags);
>                  if (retval < 0) {
>                      retval = -errno;
> @@ -1016,7 +1017,7 @@ static int process_requests(int sock)
>              retval = proxy_unmarshal(&in_iovec,
>                                       PROXY_HDR_SZ, "ss", &path, &name);
>              if (retval > 0) {
> -                retval = lremovexattr(path.data, name.data);
> +                retval = qemu_lremovexattr(path.data, name.data);
>                  if (retval < 0) {
>                      retval = -errno;
>                  }
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 98d4073..768ef6f 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -776,16 +776,20 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
>          mode_t tmp_mode;
>          dev_t tmp_dev;
>  
> -        if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
> +        if (qemu_fgetxattr(fd, "user.virtfs.uid",
> +                           &tmp_uid, sizeof(uid_t)) > 0) {
>              stbuf->st_uid = le32_to_cpu(tmp_uid);
>          }
> -        if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
> +        if (qemu_fgetxattr(fd, "user.virtfs.gid",
> +                           &tmp_gid, sizeof(gid_t)) > 0) {
>              stbuf->st_gid = le32_to_cpu(tmp_gid);
>          }
> -        if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
> +        if (qemu_fgetxattr(fd, "user.virtfs.mode",
> +                           &tmp_mode, sizeof(mode_t)) > 0) {
>              stbuf->st_mode = le32_to_cpu(tmp_mode);
>          }
> -        if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
> +        if (qemu_fgetxattr(fd, "user.virtfs.rdev",
> +                           &tmp_dev, sizeof(dev_t)) > 0) {
>              stbuf->st_rdev = le64_to_cpu(tmp_dev);
>          }
>      } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 79ed6b2..50a03c7 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -19,6 +19,23 @@
>  #define O_PATH_9P_UTIL 0
>  #endif
>  
> +#ifdef CONFIG_DARWIN
> +#define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
> +#define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> +#define qemu_llistxattr(...) listxattr(__VA_ARGS__, XATTR_NOFOLLOW)
> +#define qemu_lremovexattr(...) removexattr(__VA_ARGS__, XATTR_NOFOLLOW)
> +static inline int qemu_lsetxattr(const char *path, const char *name,
> +                                 const void *value, size_t size, int flags) {
> +    return setxattr(path, name, value, size, 0, flags | XATTR_NOFOLLOW);
> +}
> +#else
> +#define qemu_fgetxattr fgetxattr
> +#define qemu_lgetxattr lgetxattr
> +#define qemu_llistxattr llistxattr
> +#define qemu_lremovexattr lremovexattr
> +#define qemu_lsetxattr lsetxattr
> +#endif
> +
>  static inline void close_preserve_errno(int fd)
>  {
>      int serrno = errno;

[Qemu-devel] [PATCH v3 10/13] 9p: darwin: Provide a fallback implementation for utimensat

[Keno Fischer]

This function is new in Mac OS 10.13. Provide a fallback implementation
when building against older SDKs. The complication in the definition comes
having to separately handle the used SDK version and the target OS version.

- If the SDK version is too low (__MAC_10_13 not defined), utimensat is not
  defined in the header, so we must not try to use it (doing so would error).
- Otherwise, if the targetted OS version is at least 10.13, we know this
  function is available, so we can unconditionally call it.
- Lastly, we check for the availability of the __builtin_available macro to
  potentially insert a dynamic check for this OS version. However, __builtin_available
  is only available with sufficiently recent versions of clang and while all
  Apple clang versions that ship with Xcode versions that support the 10.13
  SDK support with builtin, we want to allow building with compilers other
  than Apple clang that may not support this builtin.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 fsdev/virtfs-proxy-helper.c |  3 +-
 hw/9pfs/9p-local.c          |  2 +-
 hw/9pfs/9p-util-darwin.c    | 96 +++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p-util-linux.c     |  6 +++
 hw/9pfs/9p-util.h           |  8 ++++
 hw/9pfs/9p.c                |  1 +
 6 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index a26f8b8..d8dd3f5 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -957,8 +957,7 @@ static int process_requests(int sock)
                                      &spec[0].tv_sec, &spec[0].tv_nsec,
                                      &spec[1].tv_sec, &spec[1].tv_nsec);
             if (retval > 0) {
-                retval = utimensat(AT_FDCWD, path.data, spec,
-                                   AT_SYMLINK_NOFOLLOW);
+                retval = utimensat_nofollow(AT_FDCWD, path.data, spec);
                 if (retval < 0) {
                     retval = -errno;
                 }
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 768ef6f..56bcabf 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1071,7 +1071,7 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path,
         goto out;
     }
 
-    ret = utimensat(dirfd, name, buf, AT_SYMLINK_NOFOLLOW);
+    ret = utimensat_nofollow(dirfd, name, buf);
     close_preserve_errno(dirfd);
 out:
     g_free(dirpath);
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index cdb4c9e..ac414bc 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -62,3 +62,99 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
     close_preserve_errno(fd);
     return ret;
 }
+
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+static int update_times_from_stat(int fd, struct timespec times[2],
+                                  int update0, int update1)
+{
+    struct stat buf;
+    int ret = fstat(fd, &buf);
+    if (ret == -1) {
+        return ret;
+    }
+    if (update0) {
+        times[0] = buf.st_atimespec;
+    }
+    if (update1) {
+        times[1] = buf.st_mtimespec;
+    }
+    return 0;
+}
+
+int utimensat_nofollow(int dirfd, const char *filename,
+                       const struct timespec times_in[2])
+{
+    int ret, fd;
+    int special0, special1;
+    struct timeval futimes_buf[2];
+    struct timespec times[2];
+    memcpy(times, times_in, 2 * sizeof(struct timespec));
+
+/* Check whether we have an SDK version that defines utimensat */
+#if defined(__MAC_10_13)
+# if __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_10_13
+#  define UTIMENSAT_AVAILABLE 1
+# elif __has_builtin(__builtin_available)
+#  define UTIMENSAT_AVAILABLE __builtin_available(macos 10.13, *)
+# else
+#  define UTIMENSAT_AVAILABLE 0
+# endif
+    if (UTIMENSAT_AVAILABLE) {
+        return utimensat(dirfd, filename, times, AT_SYMLINK_NOFOLLOW);
+    }
+#endif
+
+    /* utimensat not available. Use futimes. */
+    fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+    if (fd == -1) {
+        return -1;
+    }
+
+    special0 = times[0].tv_nsec == UTIME_OMIT;
+    special1 = times[1].tv_nsec == UTIME_OMIT;
+    if (special0 || special1) {
+        /* If both are set, nothing to do */
+        if (special0 && special1) {
+            ret = 0;
+            goto done;
+        }
+
+        ret = update_times_from_stat(fd, times, special0, special1);
+        if (ret < 0) {
+            goto done;
+        }
+    }
+
+    special0 = times[0].tv_nsec == UTIME_NOW;
+    special1 = times[1].tv_nsec == UTIME_NOW;
+    if (special0 || special1) {
+        ret = futimes(fd, NULL);
+        if (ret < 0) {
+            goto done;
+        }
+
+        /* If both are set, we are done */
+        if (special0 && special1) {
+            ret = 0;
+            goto done;
+        }
+
+        ret = update_times_from_stat(fd, times, special0, special1);
+        if (ret < 0) {
+            goto done;
+        }
+    }
+
+    futimes_buf[0].tv_sec = times[0].tv_sec;
+    futimes_buf[0].tv_usec = times[0].tv_nsec / 1000;
+    futimes_buf[1].tv_sec = times[1].tv_sec;
+    futimes_buf[1].tv_usec = times[1].tv_nsec / 1000;
+    ret = futimes(fd, futimes_buf);
+
+done:
+    close_preserve_errno(fd);
+    return ret;
+}
diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
index defa3a4..3902378 100644
--- a/hw/9pfs/9p-util-linux.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -57,3 +57,9 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
     g_free(proc_path);
     return ret;
 }
+
+int utimensat_nofollow(int dirfd, const char *filename,
+                       const struct timespec times[2])
+{
+    return utimensat(dirfd, filename, times, AT_SYMLINK_NOFOLLOW);
+}
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 50a03c7..b1dc08a 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -36,6 +36,12 @@ static inline int qemu_lsetxattr(const char *path, const char *name,
 #define qemu_lsetxattr lsetxattr
 #endif
 
+/* Compatibility with old SDK Versions for Darwin */
+#if defined(CONFIG_DARWIN) && !defined(UTIME_NOW)
+#define UTIME_NOW -1
+#define UTIME_OMIT -2
+#endif
+
 static inline void close_preserve_errno(int fd)
 {
     int serrno = errno;
@@ -81,5 +87,7 @@ ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
                               char *list, size_t size);
 ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
                                 const char *name);
+int utimensat_nofollow(int dirfd, const char *filename,
+                       const struct timespec times[2]);
 
 #endif
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index abfb8dc..dc53691 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -21,6 +21,7 @@
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "9p-xattr.h"
+#include "9p-util.h"
 #include "coth.h"
 #include "trace.h"
 #include "migration/blocker.h"
-- 
2.8.1

[Qemu-devel] [PATCH v3 11/13] 9p: darwin: Implement compatibility for mknodat

[Keno Fischer]

Darwin does not support mknodat. However, to avoid race conditions
with later setting the permissions, we must avoid using mknod on
the full path instead. We could try to fchdir, but that would cause
problems if multiple threads try to call mknodat at the same time.
However, luckily there is a solution: Darwin as an (unexposed in the
C library) system call that sets the cwd for the current thread only.
This should suffice to use mknod safely.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---

Changes since v2:
 - Silence clang warning for deprecated uses of `syscall`. It is
  unforunate that we have to use this depreacted interface, but
  there does not seem to be a better option.

 hw/9pfs/9p-local.c       |  5 +++--
 hw/9pfs/9p-util-darwin.c | 31 +++++++++++++++++++++++++++++++
 hw/9pfs/9p-util-linux.c  |  5 +++++
 hw/9pfs/9p-util.h        |  2 ++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 56bcabf..450f31c 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -668,7 +668,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
 
     if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
         fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
-        err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
+        err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
         if (err == -1) {
             goto out;
         }
@@ -683,7 +683,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         }
     } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
                fs_ctx->export_flags & V9FS_SM_NONE) {
-        err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
+        err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
         if (err == -1) {
             goto out;
         }
@@ -696,6 +696,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
 
 err_end:
     unlinkat_preserve_errno(dirfd, name, 0);
+
 out:
     close_preserve_errno(dirfd);
     return err;
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index ac414bc..194f068 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -158,3 +158,34 @@ done:
     close_preserve_errno(fd);
     return ret;
 }
+
+#ifndef SYS___pthread_fchdir
+# define SYS___pthread_fchdir 349
+#endif
+
+// This is an undocumented OS X syscall. It would be best to avoid it,
+// but there doesn't seem to be another safe way to implement mknodat.
+// Dear Apple, please implement mknodat before you remove this syscall.
+static int fchdir_thread_local(int fd)
+{
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+    return syscall(SYS___pthread_fchdir, fd);
+#pragma clang diagnostic pop
+}
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+    int preserved_errno, err;
+    if (fchdir_thread_local(dirfd) < 0) {
+        return -1;
+    }
+    err = mknod(filename, mode, dev);
+    preserved_errno = errno;
+    /* Stop using the thread-local cwd */
+    fchdir_thread_local(-1);
+    if (err < 0) {
+        errno = preserved_errno;
+    }
+    return err;
+}
diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
index 3902378..06399c5 100644
--- a/hw/9pfs/9p-util-linux.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -63,3 +63,8 @@ int utimensat_nofollow(int dirfd, const char *filename,
 {
     return utimensat(dirfd, filename, times, AT_SYMLINK_NOFOLLOW);
 }
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+    return mknodat(dirfd, filename, mode, dev);
+}
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index b1dc08a..127564d 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -90,4 +90,6 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
 int utimensat_nofollow(int dirfd, const char *filename,
                        const struct timespec times[2]);
 
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
+
 #endif
-- 
2.8.1

[Qemu-devel] [PATCH v3 12/13] 9p: darwin: virtfs-proxy: Implement setuid code for darwin

[Keno Fischer]

Darwin does not have linux capabilities, so make that code linux-only.
Darwin also does not have setresuid/gid. The correct way to temporarily
drop capabilities is to call seteuid/gid.

Also factor out the code that acquires acquire_dac_override into a separate
function in the linux implementation. I had originally done this when
I thought it made sense to have only one `setugid` function, but I retained
this because it seems clearer this way.

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 fsdev/virtfs-proxy-helper.c | 200 +++++++++++++++++++++++++++-----------------
 1 file changed, 125 insertions(+), 75 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index d8dd3f5..6baf2a6 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -82,6 +82,7 @@ static void do_perror(const char *string)
     }
 }
 
+#ifdef CONFIG_LINUX
 static int do_cap_set(cap_value_t *cap_value, int size, int reset)
 {
     cap_t caps;
@@ -121,6 +122,85 @@ error:
     return -1;
 }
 
+static int acquire_dac_override(void)
+{
+    cap_value_t cap_list[] = {
+        CAP_DAC_OVERRIDE,
+    };
+    return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0);
+}
+
+/*
+ * from man 7 capabilities, section
+ * Effect of User ID Changes on Capabilities:
+ * If the effective user ID is changed from nonzero to 0, then the permitted
+ * set is copied to the effective set.  If the effective user ID is changed
+ * from 0 to nonzero, then all capabilities are are cleared from the effective
+ * set.
+ *
+ * The setfsuid/setfsgid man pages warn that changing the effective user ID may
+ * expose the program to unwanted signals, but this is not true anymore: for an
+ * unprivileged (without CAP_KILL) program to send a signal, the real or
+ * effective user ID of the sending process must equal the real or saved user
+ * ID of the target process.  Even when dropping privileges, it is enough to
+ * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
+ * be exposed to signals.  So just use setresuid/setresgid.
+ */
+static int setugid(int uid, int gid, int *suid, int *sgid)
+{
+    int retval;
+
+    *suid = geteuid();
+    *sgid = getegid();
+
+    if (setresgid(-1, gid, *sgid) == -1) {
+        retval = -errno;
+        goto err_out;
+    }
+
+    if (setresuid(-1, uid, *suid) == -1) {
+        retval = -errno;
+        goto err_sgid;
+    }
+
+    if (uid != 0 || gid != 0) {
+        /*
+        * We still need DAC_OVERRIDE because we don't change
+        * supplementary group ids, and hence may be subjected DAC rules
+        */
+        if (acquire_dac_override() < 0) {
+            retval = -errno;
+            goto err_suid;
+        }
+    }
+    return 0;
+
+err_suid:
+    if (setresuid(-1, *suid, *suid) == -1) {
+        abort();
+    }
+err_sgid:
+    if (setresgid(-1, *sgid, *sgid) == -1) {
+        abort();
+    }
+err_out:
+    return retval;
+}
+
+/*
+ * This is used to reset the ugid back with the saved values
+ * There is nothing much we can do checking error values here.
+ */
+static void resetugid(int suid, int sgid)
+{
+    if (setresgid(-1, sgid, sgid) == -1) {
+        abort();
+    }
+    if (setresuid(-1, suid, suid) == -1) {
+        abort();
+    }
+}
+
 static int init_capabilities(void)
 {
     /* helper needs following capabilities only */
@@ -135,6 +215,51 @@ static int init_capabilities(void)
     };
     return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 1);
 }
+#else
+static int setugid(int uid, int gid, int *suid, int *sgid)
+{
+    int retval;
+
+    *suid = geteuid();
+    *sgid = getegid();
+
+    if (setegid(gid) == -1) {
+        retval = -errno;
+        goto err_out;
+    }
+
+    if (seteuid(uid) == -1) {
+        retval = -errno;
+        goto err_sgid;
+    }
+
+err_sgid:
+    if (setgid(*sgid) == -1) {
+        abort();
+    }
+err_out:
+    return retval;
+}
+
+/*
+ * This is used to reset the ugid back with the saved values
+ * There is nothing much we can do checking error values here.
+ */
+static void resetugid(int suid, int sgid)
+{
+    if (setegid(sgid) == -1) {
+        abort();
+    }
+    if (seteuid(suid) == -1) {
+        abort();
+    }
+}
+
+static int init_capabilities(void)
+{
+    return 0;
+}
+#endif
 
 static int socket_read(int sockfd, void *buff, ssize_t size)
 {
@@ -279,81 +404,6 @@ static int send_status(int sockfd, struct iovec *iovec, int status)
 }
 
 /*
- * from man 7 capabilities, section
- * Effect of User ID Changes on Capabilities:
- * If the effective user ID is changed from nonzero to 0, then the permitted
- * set is copied to the effective set.  If the effective user ID is changed
- * from 0 to nonzero, then all capabilities are are cleared from the effective
- * set.
- *
- * The setfsuid/setfsgid man pages warn that changing the effective user ID may
- * expose the program to unwanted signals, but this is not true anymore: for an
- * unprivileged (without CAP_KILL) program to send a signal, the real or
- * effective user ID of the sending process must equal the real or saved user
- * ID of the target process.  Even when dropping privileges, it is enough to
- * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
- * be exposed to signals.  So just use setresuid/setresgid.
- */
-static int setugid(int uid, int gid, int *suid, int *sgid)
-{
-    int retval;
-
-    /*
-     * We still need DAC_OVERRIDE because we don't change
-     * supplementary group ids, and hence may be subjected DAC rules
-     */
-    cap_value_t cap_list[] = {
-        CAP_DAC_OVERRIDE,
-    };
-
-    *suid = geteuid();
-    *sgid = getegid();
-
-    if (setresgid(-1, gid, *sgid) == -1) {
-        retval = -errno;
-        goto err_out;
-    }
-
-    if (setresuid(-1, uid, *suid) == -1) {
-        retval = -errno;
-        goto err_sgid;
-    }
-
-    if (uid != 0 || gid != 0) {
-        if (do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0) < 0) {
-            retval = -errno;
-            goto err_suid;
-        }
-    }
-    return 0;
-
-err_suid:
-    if (setresuid(-1, *suid, *suid) == -1) {
-        abort();
-    }
-err_sgid:
-    if (setresgid(-1, *sgid, *sgid) == -1) {
-        abort();
-    }
-err_out:
-    return retval;
-}
-
-/*
- * This is used to reset the ugid back with the saved values
- * There is nothing much we can do checking error values here.
- */
-static void resetugid(int suid, int sgid)
-{
-    if (setresgid(-1, sgid, sgid) == -1) {
-        abort();
-    }
-    if (setresuid(-1, suid, suid) == -1) {
-        abort();
-    }
-}
-
-/*
  * send response in two parts
  * 1) ProxyHeader
  * 2) Response or error status
-- 
2.8.1

[Qemu-devel] [PATCH v3 13/13] 9p: darwin: configure: Allow VirtFS on Darwin

[Keno Fischer]

Signed-off-by: Keno Fischer <keno@juliacomputing.com>
---
 Makefile.objs |  1 +
 configure     | 22 +++++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 7a9828d..c968a9a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -104,6 +104,7 @@ common-obj-$(CONFIG_WIN32) += os-win32.o
 common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
+common-obj-$(CONFIG_DARWIN) += fsdev/
 
 common-obj-y += migration/
 
diff --git a/configure b/configure
index 195c9bd..74f593a 100755
--- a/configure
+++ b/configure
@@ -5568,16 +5568,28 @@ 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 under Linux"
+        fi
+        virtfs=no
+      fi
+    elif test "$darwin" = 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 supported only on Linux and Darwin"
       fi
       virtfs=no
     fi
+  fi
+  if test "$linux" = yes; then
     if test "$mpath" != no && test "$mpathpersist" = yes ; then
       mpath=yes
     else
@@ -5588,10 +5600,6 @@ if test "$softmmu" = yes ; then
     fi
     tools="$tools scsi/qemu-pr-helper\$(EXESUF)"
   else
-    if test "$virtfs" = yes; then
-      error_exit "VirtFS is supported only on Linux"
-    fi
-    virtfs=no
     if test "$mpath" = yes; then
       error_exit "Multipath is supported only on Linux"
     fi
-- 
2.8.1

Re: [Qemu-devel] [PATCH v3 00/13] 9p: Add support for Darwin

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: cover.1529196703.git.keno@juliacomputing.com
Subject: [Qemu-devel] [PATCH v3 00/13] 9p: Add support for Darwin

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20180615194705.28019-1-alex.bennee@linaro.org -> patchew/20180615194705.28019-1-alex.bennee@linaro.org
 * [new tag]               patchew/cover.1529196703.git.keno@juliacomputing.com -> patchew/cover.1529196703.git.keno@juliacomputing.com
Switched to a new branch 'test'
4b8587ff41 9p: darwin: configure: Allow VirtFS on Darwin
6fca82963b 9p: darwin: virtfs-proxy: Implement setuid code for darwin
da47c1cf4a 9p: darwin: Implement compatibility for mknodat
52eb0c76e0 9p: darwin: Provide a fallback implementation for utimensat
3d55eb845d 9p: darwin: Compatibility for f/l*xattr
c1c235d3c0 9p: darwin: *xattr_nofollow implementations
28b7d5a3d9 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX
100a9f6570 9p: darwin: Ignore O_{NOATIME, DIRECT}
8d93cfca8b 9p: darwin: Explicitly cast comparisons of mode_t with -1
6d83d8da90 9p: darwin: Handle struct dirent differences
e5419933d4 9p: darwin: Handle struct stat(fs) differences
2c193d5f06 9p: Rename 9p-util -> 9p-util-linux
8fb0b6857d 9p: linux: Fix a couple Linux assumptions

=== OUTPUT BEGIN ===
Checking PATCH 1/13: 9p: linux: Fix a couple Linux assumptions...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#71: 
new file mode 100644

total: 0 errors, 1 warnings, 69 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/13: 9p: Rename 9p-util -> 9p-util-linux...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#16: 
rename from hw/9pfs/9p-util.c

total: 0 errors, 1 warnings, 13 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/13: 9p: darwin: Handle struct stat(fs) differences...
Checking PATCH 4/13: 9p: darwin: Handle struct dirent differences...
Checking PATCH 5/13: 9p: darwin: Explicitly cast comparisons of mode_t with -1...
Checking PATCH 6/13: 9p: darwin: Ignore O_{NOATIME, DIRECT}...
Checking PATCH 7/13: 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX...
Checking PATCH 8/13: 9p: darwin: *xattr_nofollow implementations...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
new file mode 100644

total: 0 errors, 1 warnings, 71 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/13: 9p: darwin: Compatibility for f/l*xattr...
Checking PATCH 10/13: 9p: darwin: Provide a fallback implementation for utimensat...
WARNING: architecture specific defines should be avoided
#60: FILE: hw/9pfs/9p-util-darwin.c:66:
+#ifndef __has_builtin

WARNING: architecture specific defines should be avoided
#91: FILE: hw/9pfs/9p-util-darwin.c:97:
+#if defined(__MAC_10_13)

WARNING: architecture specific defines should be avoided
#92: FILE: hw/9pfs/9p-util-darwin.c:98:
+# if __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_10_13

total: 0 errors, 3 warnings, 151 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/13: 9p: darwin: Implement compatibility for mknodat...
ERROR: do not use C99 // comments
#60: FILE: hw/9pfs/9p-util-darwin.c:166:
+// This is an undocumented OS X syscall. It would be best to avoid it,

ERROR: do not use C99 // comments
#61: FILE: hw/9pfs/9p-util-darwin.c:167:
+// but there doesn't seem to be another safe way to implement mknodat.

ERROR: do not use C99 // comments
#62: FILE: hw/9pfs/9p-util-darwin.c:168:
+// Dear Apple, please implement mknodat before you remove this syscall.

total: 3 errors, 0 warnings, 71 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 12/13: 9p: darwin: virtfs-proxy: Implement setuid code for darwin...
Checking PATCH 13/13: 9p: darwin: configure: Allow VirtFS on Darwin...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com