mbox series

[00/13] 9p: Add support for Darwin

Message ID cover.1527310210.git.keno@alumni.harvard.edu
Headers show
Series 9p: Add support for Darwin | expand

Message

Keno Fischer May 26, 2018, 5:23 a.m. UTC 
From: Keno Fischer <keno@alumni.harvard.edu>

Hi Greg,

this series adds support for building the 9p code on Mac OS X.
It seems to work decently well (tested by booting up a linux
guest and building a copy of qemu on a 9p mount in the guest),
but there are probably corner cases I got wrong (particular
in the xattr support). Is there a stress test you recommend
running for those corner cases?

I've split the commits rather finely to hopefully ease review,
of each individual concern I ran into while porting. Happy to
merge commits back together if you would prefer.

Lastly, I should remark that I'm not super familiar with the qemu
code base, so please let me know if there's a better place for
some of the code (particularly some of the compatibility code).

Keno Fischer (13):
  9p: linux: Fix a couple Linux assumptions
  9p: Avoid warning if FS_IOC_GETVERSION is not defined
  9p: Move a couple xattr functions to 9p-util
  9p: darwin: Handle struct stat(fs) differences
  9p: darwin: Handle struct dirent differences
  9p: darwin: Address minor differences
  9p: darwin: Properly translate AT_REMOVEDIR flag
  9p: darwin: Ignore O_{NOATIME, DIRECT}
  9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX
  9p: darwin: *xattr_nofollow implementations
  9p: darwin: Mark mknod as unsupported
  9p: darwin: Provide a fallback implementation for utimensat
  9p: darwin: configure: Allow VirtFS on Darwin

 Makefile.objs        |   1 +
 configure            |  23 ++++++----
 fsdev/file-op-9p.h   |   6 +++
 hw/9pfs/9p-local.c   |  84 +++++++++++++++++++++++-------------
 hw/9pfs/9p-proxy.c   |  17 ++++++--
 hw/9pfs/9p-synth.c   |   6 +++
 hw/9pfs/9p-util.c    | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/9pfs/9p-util.h    |  13 ++++++
 hw/9pfs/9p-xattr.c   |  33 --------------
 hw/9pfs/9p.c         |  79 +++++++++++++++++++++++++--------
 include/qemu/xattr.h |   4 +-
 11 files changed, 293 insertions(+), 93 deletions(-)

Comments

no-reply@patchew.org May 26, 2018, 5:37 a.m. UTC | #1 
Hi,

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

Type: series
Message-id: cover.1527310210.git.keno@alumni.harvard.edu
Subject: [Qemu-devel] [PATCH 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/1527266793-301361-1-git-send-email-mst@redhat.com -> patchew/1527266793-301361-1-git-send-email-mst@redhat.com
 * [new tag]               patchew/cover.1527310210.git.keno@alumni.harvard.edu -> patchew/cover.1527310210.git.keno@alumni.harvard.edu
Switched to a new branch 'test'
d5026b2480 9p: darwin: configure: Allow VirtFS on Darwin
e375fe9d7a 9p: darwin: Provide a fallback implementation for utimensat
7e0b2ee759 9p: darwin: Mark mknod as unsupported
e78d656057 9p: darwin: *xattr_nofollow implementations
dff94417c5 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX
2630a2f4ea 9p: darwin: Ignore O_{NOATIME, DIRECT}
7277fdd263 9p: darwin: Properly translate AT_REMOVEDIR flag
1ee1435360 9p: darwin: Address minor differences
a03d05d35a 9p: darwin: Handle struct dirent differences
160cefc589 9p: darwin: Handle struct stat(fs) differences
f920617dc0 9p: Move a couple xattr functions to 9p-util
3b4130c0b6 9p: Avoid warning if FS_IOC_GETVERSION is not defined
46cf5c5c4d 9p: linux: Fix a couple Linux assumptions

=== OUTPUT BEGIN ===
Checking PATCH 1/13: 9p: linux: Fix a couple Linux assumptions...
Checking PATCH 2/13: 9p: Avoid warning if FS_IOC_GETVERSION is not defined...
Checking PATCH 3/13: 9p: Move a couple xattr functions to 9p-util...
WARNING: line over 80 characters
#26: FILE: hw/9pfs/9p-local.c:779:
+        if (fgetxattr_follow(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {

WARNING: line over 80 characters
#30: FILE: hw/9pfs/9p-local.c:782:
+        if (fgetxattr_follow(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0)

WARNING: line over 80 characters
#35: FILE: hw/9pfs/9p-local.c:786:
+        if (fgetxattr_follow(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0)

WARNING: line over 80 characters
#40: FILE: hw/9pfs/9p-local.c:790:
+        if (fgetxattr_follow(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0)

total: 0 errors, 4 warnings, 129 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 4/13: 9p: darwin: Handle struct stat(fs) differences...
Checking PATCH 5/13: 9p: darwin: Handle struct dirent differences...
ERROR: do not use C99 // comments
#46: FILE: hw/9pfs/9p.c:1960:
+        // Darwin has d_seekoff, which appears to function

ERROR: do not use C99 // comments
#47: FILE: hw/9pfs/9p.c:1961:
+        // analogously to d_off. However, it does not appear

ERROR: do not use C99 // comments
#48: FILE: hw/9pfs/9p.c:1962:
+        // to be supported on all file systems, so use

ERROR: do not use C99 // comments
#49: FILE: hw/9pfs/9p.c:1963:
+        // telldir for correctness.

total: 4 errors, 0 warnings, 50 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 6/13: 9p: darwin: Address minor differences...
Checking PATCH 7/13: 9p: darwin: Properly translate AT_REMOVEDIR flag...
Checking PATCH 8/13: 9p: darwin: Ignore O_{NOATIME, DIRECT}...
Checking PATCH 9/13: 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX...
Checking PATCH 10/13: 9p: darwin: *xattr_nofollow implementations...
WARNING: line over 80 characters
#20: FILE: hw/9pfs/9p-util.c:22:
+    int fd = openat_file(dirfd, filename, O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);

ERROR: braces {} are necessary for all arms of this statement
#21: FILE: hw/9pfs/9p-util.c:23:
+    if (fd == -1)
[...]

WARNING: line over 80 characters
#51: FILE: hw/9pfs/9p-util.c:52:
+    int fd = openat_file(dirfd, filename, O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);

ERROR: braces {} are necessary for all arms of this statement
#52: FILE: hw/9pfs/9p-util.c:53:
+    if (fd == -1)
[...]

ERROR: braces {} are necessary for all arms of this statement
#73: FILE: hw/9pfs/9p-util.c:73:
+    if (fd == -1)
[...]

ERROR: braces {} are necessary for all arms of this statement
#95: FILE: hw/9pfs/9p-util.c:94:
+    if (fd == -1)
[...]

total: 4 errors, 2 warnings, 94 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: Mark mknod as unsupported...
Checking PATCH 12/13: 9p: darwin: Provide a fallback implementation for utimensat...
WARNING: architecture specific defines should be avoided
#34: FILE: hw/9pfs/9p-util.c:108:
+#ifndef __has_builtin

WARNING: line over 80 characters
#38: FILE: hw/9pfs/9p-util.c:112:
+int utimensat_nofollow(int dirfd, const char *filename, const struct timespec times[2])

WARNING: line over 80 characters
#41: FILE: hw/9pfs/9p-util.c:115:
+#if defined(__MAC_10_13) /* Check whether we have an SDK version that defines utimensat */

WARNING: architecture specific defines should be avoided
#41: FILE: hw/9pfs/9p-util.c:115:
+#if defined(__MAC_10_13) /* Check whether we have an SDK version that defines utimensat */

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

ERROR: that open brace { should be on the previous line
#49: FILE: hw/9pfs/9p-util.c:123:
+    if (UTIMENSAT_AVAILABLE)
+    {

ERROR: do not use C99 // comments
#54: FILE: hw/9pfs/9p-util.c:128:
+    // utimensat not available. Use futimes.

ERROR: braces {} are necessary for all arms of this statement
#56: FILE: hw/9pfs/9p-util.c:130:
+    if (fd == -1)
[...]

WARNING: line over 80 characters
#92: FILE: hw/9pfs/9p-util.h:75:
+int utimensat_nofollow(int dirfd, const char *filename, const struct timespec times[2]);

total: 3 errors, 6 warnings, 74 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 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