Re: 9p
On 2014 Jun 04 (Wed) at 02:19:49 -0500 (-0500), strake...@gmail.com wrote: :On Tue, Jun 03, 2014 at 10:44:57PM -0700, Philip Guenther wrote: : Yes, some code I copied verbatim from plan9port or earlier work of mine, : so that's fully in plan9 or my habitual style. : : IF YOU COPIED MORE THAN TRIVIAL CODE FROM plan9port YOU MUST ACKNOWLEDGE : IT BY INCLUDING THEIR COPYRIGHT ON IT AND ABIDING BY THEIR LICENSE : RESTRICTIONS. : :Erratum: I copied part of 9p.h from libixp, not plan9port. The license seems equivalent to ISC. I include it in latest version below. : :Too, I copied some functions from userspace libc to libkern. : :The rest is mine. : : In case you are able to resolve the copyright and license issues with : this, here are some comments on the synchronization logic in it: : :Thanks. I have to sleep myself soon, so I shall leave this until after that. : You need to add the (correct) copyright statements to EACH AND EVERY FILE. Period. For files that have copied lines, you need to also copy their entire copyright statement from the file. For lines that you wrote, you need to add your own copyright statement. -- Children are unpredictable. You never know what inconsistency they're going to catch you in next. -- Franklin P. Jones
Re: 9p
On 04/06/2014, Philip Guenther guent...@gmail.com wrote: The only reason to care whether another kernel thread had made it far enough into tlseep as to be on a sleep queue is if it's calling wakeup() on that thread's wait channel, but you MUST use some sort of lock to protect that shared condition that the tsleeping thread is waiting for, period. For tsleep(), the involved lock is the kernel lock (aka big lock), while for msleep() it's the passed mutex. Would a rwlock do? The sender and recver operate asynchronously, so the sender needs to hold a lock while sending and release it when asleep, but it can't be a mutex as the send operation may sleep, so I used requ.ready as the lock, but a rwlock seems appropriate.
Re: 9p
On Wed, Jun 4, 2014 at 2:00 PM, M Farkas-Dyck strake...@gmail.com wrote: Would a rwlock do? The sender and recver operate asynchronously, so the sender needs to hold a lock while sending and release it when asleep, but it can't be a mutex as the send operation may sleep, so I used requ.ready as the lock, but a rwlock seems appropriate. msleep() unlocks the mutex after the process is placed on the sleep queue, and then relocks the mutex after being woken up. I would expect that should be sufficient for your use case, but I haven't looked close enough. Style nits: Please keep source files ASCII; i.e., Copyright (C) instead of the Unicode copyright character. Also, look at man 9 style; in particular, OpenBSD doesn't put spaces around - and we keep lines under 80 characters long. Oh, and no static functions in the kernel. They cause problems with ddb.
Re: 9p
Matthew, you obviously did not spot the evil 9p/util.h yet. This file ought to be named ozymandias.h. Also, I am vetoing the addition of -fms-extensions to the kernel build options. Whatever files require this option to build needs to be fixed to be proper, unambiguous, C99, instead.
Re: 9p
Matthew Dempsky matt...@dempsky.org wrote: Also, look at man 9 style; in particular, OpenBSD doesn't put spaces around - Not in cited document, but noted. Oh, and no static functions in the kernel. They cause problems with ddb. Even inline functions? Miod Vallat m...@online.fr wrote: Matthew, you obviously did not spot the evil 9p/util.h yet. This file ought to be named ozymandias.h. I could do that ☺ I copied that file from earlier work of mine. I only actually need a few macros in it, so I shall strip it down for the next version. Also, I am vetoing the addition of -fms-extensions to the kernel build options. Whatever files require this option to build needs to be fixed to be proper, unambiguous, C99, instead. All I need are anonymous struct or union members, also legal under gnu99 and plan9. Alas, gcc seems to lack an option to enable these alone. Next version awaits pronunciation on rwlocked sleep.
Re: 9p
On Wed, Jun 4, 2014 at 4:19 PM, M Farkas-Dyck strake...@gmail.com wrote: Matthew Dempsky matt...@dempsky.org wrote: Also, look at man 9 style; in particular, OpenBSD doesn't put spaces around - Not in cited document, but noted. Read more of the existing kernel code. Oh, and no static functions in the kernel. They cause problems with ddb. Even inline functions? Miod Vallat m...@online.fr wrote: Matthew, you obviously did not spot the evil 9p/util.h yet. This file ought to be named ozymandias.h. I could do that ☺ I copied that file from earlier work of mine. I only actually need a few macros in it, so I shall strip it down for the next version. Also, I am vetoing the addition of -fms-extensions to the kernel build options. Whatever files require this option to build needs to be fixed to be proper, unambiguous, C99, instead. All I need are anonymous struct or union members, also legal under gnu99 and plan9. Alas, gcc seems to lack an option to enable these alone. Next version awaits pronunciation on rwlocked sleep.
Re: 9p
On Wed, 4 Jun 2014, Philip Guenther wrote: On Wed, Jun 4, 2014 at 4:19 PM, M Farkas-Dyck strake...@gmail.com wrote: Matthew Dempsky matt...@dempsky.org wrote: Also, look at man 9 style; in particular, OpenBSD doesn't put spaces around - Not in cited document, but noted. Read more of the existing kernel code. (stupid gmail control-enter binding) Your code should be styled on the existing kernel code. All I need are anonymous struct or union members, also legal under gnu99 and plan9. Alas, gcc seems to lack an option to enable these alone. Then do without them and use _foo and such as the structure members, as seen in structures like struct sigaction and siginfo_t. Next version awaits pronunciation on rwlocked sleep. Use tsleep(). Given your first attempt at writing a new synchronization primitive, I'm not interested in reading another without demonstration of greater understanding of the existing primitives. Philip Guenther
Re: 9p
Matthew, you obviously did not spot the evil 9p/util.h yet. This file ought to be named ozymandias.h. Also, I am vetoing the addition of -fms-extensions to the kernel build options. Whatever files require this option to build needs to be fixed to be proper, unambiguous, C99, instead. I feel I need to go further with commentary. This will strike some as mean. I believe I found one line in the diff which didn't violate community standards, and it was a blank line. The rest... sorry, but it is off our map. This entire diff feels like it is being thrown at us to distract some developer from more important work. Congratulations, it has worked! For a bit... and I hope at this point it ends.
Re: 9p
the style(9) ... b :-)
On Fri, May 30, 2014 at 08:48:11PM -0500, strake...@gmail.com wrote:
One can actually read a file now! Yay!
I have another patch to allocate each message buffer from a pool as needed
rather than share one among all requests, but I'm not sure that it makes
sense to do so, as all requests go thru the same channel anyhow.
diff --git a/sbin/mount_9p/mount_9p.c b/sbin/mount_9p/mount_9p.c
new file mode 100644
index 000..418f1f7
--- /dev/null
+++ b/sbin/mount_9p/mount_9p.c
@@ -0,0 +1,18 @@
+#include sys/param.h
+#include sys/mount.h
+#include stdio.h
+#include err.h
+
+int
+main (int argc, char *argu[])
+{
+ struct p9p_args args;
+ int c;
+
+ if (argc 3) errx (1, usage: %s fd path, argu[0]);
+ args.fd = strtol (argu[1], 0, 10);
+ args.msize = 1 20;
+ c = mount (MOUNT_P9P, argu[2], 0, args);
+ if (c) err (1, failed to mount);
+ return 0;
+}
diff --git a/sys/9p/9p.c b/sys/9p/9p.c
new file mode 100644
index 000..37c5ebd
--- /dev/null
+++ b/sys/9p/9p.c
@@ -0,0 +1,251 @@
+#include sys/param.h
+#include sys/malloc.h
+#include sys/systm.h
+#include sys/file.h
+#include 9p.h
+#include util.h
+
+int read9pmsg (struct file *fp, uint8_t *msg, size_t n) {
+ ssize_t size;
+ int c;
+
+ if (n 7) return EMSGSIZE;
+ c = read (fp, msg, 7);
+ if (c) return c;
+ LOAD32LE(size, msg);
+ if (size 7) return EIO;
+ switch (msg[4] /* type */) {
+ // Special case for TWrite and RRead, lest we copy huge data
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ if (n size) return EMSGSIZE;
+ return read (fp, msg + 7, size - 7);
+}
+
+int write9pmsg (struct file *fp, uint8_t *msg) {
+ size_t size;
+
+ LOAD32LE(size, msg);
+ switch (msg[4] /* type */) {
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ return write (fp, msg, size);
+}
+
+size_t loadQid (Qid *p_qid, uint8_t *msg) {
+ p_qid - type = msg[0];
+ LOAD32LE(p_qid - vers, msg + 1);
+ LOAD64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+size_t storQid (Qid *p_qid, uint8_t *msg) {
+ msg[0] = p_qid - type;
+ STOR32LE(p_qid - vers, msg + 1);
+ STOR64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+static size_t vsscan9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ switch (fmt) {
+ case '0':
+ *(uint8_t *)p = msg[0];
+ return 1;
+ case '1':
+ LOAD16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ LOAD32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ LOAD64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return loadDir (p, msg);
+ case 'q':
+ return loadQid (p, msg);
+ case 's':
+ LOAD16LE(l, msg);
+ *(char **)p = malloc (l + 1, M_TEMP, M_WAITOK);
+ strlcpy (*(char **)p, msg + 2, l + 1);
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+static size_t vsprint9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ char *xs;
+ switch (fmt) {
+ case '0':
+ if (msg) msg[0] = *(uint8_t *)p;
+ return 1;
+ case '1':
+ if (msg) STOR16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ if (msg) STOR32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ if (msg) STOR64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return storDir (p, msg);
+ case 'q':
+ return (msg ? storQid (p, msg) : 13);
+ case 's':
+ xs = *(char **)p;
+ l = xs ? strlen (xs) : 0;
+ if (msg) {
+ STOR16LE(l, msg);
+ if (xs) memcpy (msg + 2, xs, l);
+ }
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+#define ATOFFSET(t, p, r) (*(t *)((uint8_t *)(p) + r))
+
+static size_t vsscan9p (uint8_t *msg, char *fmt, void *p, size_t rs[]) {
+ size_t n;
+ uint8_t *q, *msg0;
+ msg0 = msg;
+ for (; fmt[0]; fmt++, rs++) switch (fmt[0]) {
+ case 'l':
+ msg += 2;
+ rs--;
+ break;
+ case 'n':
+ LOAD16LE(n, msg);
+ msg += 2;
+ ATOFFSET(uint16_t, p, rs[0]) = n;
+ fmt++;
+ rs++;
+ q = malloc (sizeof (void *)*n, M_TEMP, M_WAITOK);
+ ATOFFSET(void *, p, rs[0]) = q;
+ for (size_t ii = 0; ii n; ii++) {
+ size_t m
Re: 9p
On 03/06/2014, Gilles Chehade gil...@poolp.org wrote: the style(9) ... b :-) Yes, some code I copied verbatim from plan9port or earlier work of mine, so that's fully in plan9 or my habitual style. Other code I wrote afresh in a bastard hybrid of KNF and my habitual style ☺ I'll KNFalize it later when it's mostly written so I can more easily automate the deed. Mind, some rules seem customarily broken in filesystem code, for example to not define type names ending in _t, and some are insane, for example to sort local variables by size, which in general must be done per-architecture.
Re: 9p
On Tue, 3 Jun 2014, strake...@gmail.com wrote: Latest version, much unbroken. Mounted filesystem seems fully readable now, but creation fails queerly: the file name is as long as the requested name but garbage. I shall hack further... First off, a utter blocker to this code being included in OpenBSD. In a earlier message you wrote this: Yes, some code I copied verbatim from plan9port or earlier work of mine, so that's fully in plan9 or my habitual style. IF YOU COPIED MORE THAN TRIVIAL CODE FROM plan9port YOU MUST ACKNOWLEDGE IT BY INCLUDING THEIR COPYRIGHT ON IT AND ABIDING BY THEIR LICENSE RESTRICTIONS. If that license places more restrictions than the modern 3-clause BSD license, then it will NOT be included in the OpenBSD kernel distribution. Currently your code has *no* copyright statement on it, which means that you've retained all rights to the code and no one can do anything with it. In case you are able to resolve the copyright and license issues with this, here are some comments on the synchronization logic in it: +.Ft int +.Fn tsleep_inc_int void *ident unsigned int *flagp int priority const char *wmesg int timo That raised my monobrow... +.Fn tsleep_inc_int +is the same as +.Fn tsleep , +but takes another argument: +.Bl -tag -width priority +.It Fa flagp +A pointer to a variable that will be incremented when the process is safely on the sleep queue. The ...and then this made me wonder if it was actually possible to use safely. The only reason to care whether another kernel thread had made it far enough into tlseep as to be on a sleep queue is if it's calling wakeup() on that thread's wait channel, but you MUST use some sort of lock to protect that shared condition that the tsleeping thread is waiting for, period. For tsleep(), the involved lock is the kernel lock (aka big lock), while for msleep() it's the passed mutex. At that point, the int being incremented is just a counter and the thread could increment it itself before calling tsleep(). No need for a new function. +.Dv PNORELOCK +flag must be set in the +.Fa priority +argument. +.El This *IS* unsafe. PNORELOCK cannot be used safely with tsleep() because it will do unsafe calls if ktraced. It also is a good sign that your sleep condition is wrong. Back to how you're using tsleep_inc_int(): in this case, the problem is that you're trying to use the int itself to wakeup another thread that spins, but that CANNOT WORK RELIABLY. If you want another thread to block until this one has done something, then it should have its own condition and tsleep() on it. This thread would then wake it after changing the state to one where the thread can make progress. I.e., this code: ... + if (c = tsleep_inc_int (requ, requ.ready, PNORELOCK | curproc - p_priority, awaiting response, 0)) goto end; ... ...combined with this... + /* Lest we and sender race */ + while (!requp - ready); + *requp - fcallp = fcall; + wakeup (requp); ...must change, period. Maybe it's just a matter of changing that while() loop to while (!requp - ready) tsleep(requp-ready, PSOCK, 9Precv, 0); and then having p9p_require() call wakeup(requ.ready) before it calls tsleep(). Side note: the priority passed to tsleep() should be a P* constant from sys/param.h, like PSOCK, or maybe a small adjustment from one of those, so that it's priority is raised when it has something useful to do that may be blocking userspace or other threads. I will comment no further until the copyright and license situation on this is believably resolved, as it's a waste of time if that can't happen. Philip Guenther
Re: 9p
One can actually read a file now! Yay!
I have another patch to allocate each message buffer from a pool as needed
rather than share one among all requests, but I'm not sure that it makes sense
to do so, as all requests go thru the same channel anyhow.
diff --git a/sbin/mount_9p/mount_9p.c b/sbin/mount_9p/mount_9p.c
new file mode 100644
index 000..418f1f7
--- /dev/null
+++ b/sbin/mount_9p/mount_9p.c
@@ -0,0 +1,18 @@
+#include sys/param.h
+#include sys/mount.h
+#include stdio.h
+#include err.h
+
+int
+main (int argc, char *argu[])
+{
+ struct p9p_args args;
+ int c;
+
+ if (argc 3) errx (1, usage: %s fd path, argu[0]);
+ args.fd = strtol (argu[1], 0, 10);
+ args.msize = 1 20;
+ c = mount (MOUNT_P9P, argu[2], 0, args);
+ if (c) err (1, failed to mount);
+ return 0;
+}
diff --git a/sys/9p/9p.c b/sys/9p/9p.c
new file mode 100644
index 000..37c5ebd
--- /dev/null
+++ b/sys/9p/9p.c
@@ -0,0 +1,251 @@
+#include sys/param.h
+#include sys/malloc.h
+#include sys/systm.h
+#include sys/file.h
+#include 9p.h
+#include util.h
+
+int read9pmsg (struct file *fp, uint8_t *msg, size_t n) {
+ ssize_t size;
+ int c;
+
+ if (n 7) return EMSGSIZE;
+ c = read (fp, msg, 7);
+ if (c) return c;
+ LOAD32LE(size, msg);
+ if (size 7) return EIO;
+ switch (msg[4] /* type */) {
+ // Special case for TWrite and RRead, lest we copy huge data
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ if (n size) return EMSGSIZE;
+ return read (fp, msg + 7, size - 7);
+}
+
+int write9pmsg (struct file *fp, uint8_t *msg) {
+ size_t size;
+
+ LOAD32LE(size, msg);
+ switch (msg[4] /* type */) {
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ return write (fp, msg, size);
+}
+
+size_t loadQid (Qid *p_qid, uint8_t *msg) {
+ p_qid - type = msg[0];
+ LOAD32LE(p_qid - vers, msg + 1);
+ LOAD64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+size_t storQid (Qid *p_qid, uint8_t *msg) {
+ msg[0] = p_qid - type;
+ STOR32LE(p_qid - vers, msg + 1);
+ STOR64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+static size_t vsscan9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ switch (fmt) {
+ case '0':
+ *(uint8_t *)p = msg[0];
+ return 1;
+ case '1':
+ LOAD16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ LOAD32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ LOAD64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return loadDir (p, msg);
+ case 'q':
+ return loadQid (p, msg);
+ case 's':
+ LOAD16LE(l, msg);
+ *(char **)p = malloc (l + 1, M_TEMP, M_WAITOK);
+ strlcpy (*(char **)p, msg + 2, l + 1);
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+static size_t vsprint9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ char *xs;
+ switch (fmt) {
+ case '0':
+ if (msg) msg[0] = *(uint8_t *)p;
+ return 1;
+ case '1':
+ if (msg) STOR16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ if (msg) STOR32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ if (msg) STOR64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return storDir (p, msg);
+ case 'q':
+ return (msg ? storQid (p, msg) : 13);
+ case 's':
+ xs = *(char **)p;
+ l = xs ? strlen (xs) : 0;
+ if (msg) {
+ STOR16LE(l, msg);
+ if (xs) memcpy (msg + 2, xs, l);
+ }
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+#define ATOFFSET(t, p, r) (*(t *)((uint8_t *)(p) + r))
+
+static size_t vsscan9p (uint8_t *msg, char *fmt, void *p, size_t rs[]) {
+ size_t n;
+ uint8_t *q, *msg0;
+ msg0 = msg;
+ for (; fmt[0]; fmt++, rs++) switch (fmt[0]) {
+ case 'l':
+ msg += 2;
+ rs--;
+ break;
+ case 'n':
+ LOAD16LE(n, msg);
+ msg += 2;
+ ATOFFSET(uint16_t, p, rs[0]) = n;
+ fmt++;
+ rs++;
+ q = malloc (sizeof (void *)*n, M_TEMP, M_WAITOK);
+ ATOFFSET(void *, p, rs[0]) = q;
+ for (size_t ii = 0; ii n; ii++) {
+ size_t m = vsscan9p1 (msg, fmt[0], q
Re: 9p
On Fri, May 30, 2014 at 10:42 PM, M Farkas-Dyck strake...@gmail.com wrote: Ls seems to stat the directory and allocate a large enough dent buffer. I couldn't find what ls calls to do so, but I assume that it's a common function and other programs use it too. opendir() and readdir() are the functions you're looking for. They're the ones that open a directory and call getdents() to read the entries from it. They're both standardized and used *everywhere* for reading directories, so if you want this file system to work at all, it must work for those functions. Note that getdents() is used by some ports, so it must work correctly too; you can't just hack around problems by changing readdir(). ls invokes those via fts_open() and related functions. It's used by basically all BSD programs that might need to traverse a directory tree. Problem is that directories in 9p customarily have length 0, so ls says every such directory empty. Getdents itself works. It returns dirent structures, as defined in sys/dirent.h, with non-zero d_fileno, with d_off values that are seek offsets to the succeeding entry in the directory, with d_reclen of the padded and 8-byte aligned size of the directory entry, with d_type of either DT_UNKNOWN or the appropriate DT_* value, with d_name of the desired NUL-terminated name, and with d_namlen the length of the name? 3 options immediately come to mind: 1. Modify said function to not care how long stat says the directory is, choose an arbitrary dent buffer size, and iterate if need be 2. Read full directory on every stat call, blah 3. Lie that every directory length is (off_t)(-1) and hope that programs know to not try to allocate so much... opendir/readdir don't look at the st_size member, so I don't think any of those will help. You haven't found the real problem yet. Philip Guenther
Re: 9p
On 31/05/2014, Philip Guenther guent...@gmail.com wrote: opendir/readdir don't look at the st_size member, so I don't think any of those will help. You haven't found the real problem yet. Yes, I missed this in getdents docs: nbytes must be greater than or equal to the block size associated with the file. With block size great enough to hold a full 9p stat structure, it now works. Thanks for the help.
Re: 9p
On Thursday 29 May 2014 19:17:25, M Farkas-Dyck wrote: On 29/05/2014, Ted Unangst t...@tedunangst.com wrote: The first question is why not use fuse? I think it's better to have one userland filesystem interface than two. We already have 2: fuse, nfs. • 9p can operate over arbitrary transports, such as virtio and tcp. Fuse can't. • To my knowledge one can't have root as fuse. • Fuse requires a copy to/from a memory buffer on read/write. I have usage cases in which fuse is the performance-limiting factor. I agree that 9p support would be nice, especially with virtio support. I don't think I will have time to help with it in the near future, though. I have noticed that there is also o9fs [1]. I haven't tried it, and I don't know what the plans of its author are. But in case you haven't, you may want to look at it and decide if you want to merge your projects, cherry-pick a few things, or just ignore it. Cheers, Stefan [1] https://bitbucket.org/iru/o9fs/overview
Re: 9p
I actually agree that it might not be a bad thing. However, as we've seen with lots of things that touch vfs it's pretty easy to get to 80 or 90 percent functionality and then the last 10% is a royal red pain in the butt, with possibly awful crashing bugs. So I'm certainly not averse to someone working on it, but it would have to be solid and with people to love it before it ever made it into the tree - which might not ever happen without that last 10%. I do not want anything else messing around with the midlayer in the tree that is mostly complete - because that's partly wrong - and the partly wrongs tend to become Vietnam/Iraq/Afghanistan issues[1] by those who imported the code in the first place. So, with that in mind, if you're committed, please keep hacking on it. I might even have time to look occasionally iif you send me a diff. -Bob [1] Withdraw and leave the mess for the natives to deal with. On Fri, May 30, 2014 at 12:59 PM, Stefan Fritsch s...@sfritsch.de wrote: On Thursday 29 May 2014 19:17:25, M Farkas-Dyck wrote: On 29/05/2014, Ted Unangst t...@tedunangst.com wrote: The first question is why not use fuse? I think it's better to have one userland filesystem interface than two. We already have 2: fuse, nfs. • 9p can operate over arbitrary transports, such as virtio and tcp. Fuse can't. • To my knowledge one can't have root as fuse. • Fuse requires a copy to/from a memory buffer on read/write. I have usage cases in which fuse is the performance-limiting factor. I agree that 9p support would be nice, especially with virtio support. I don't think I will have time to help with it in the near future, though. I have noticed that there is also o9fs [1]. I haven't tried it, and I don't know what the plans of its author are. But in case you haven't, you may want to look at it and decide if you want to merge your projects, cherry-pick a few things, or just ignore it. Cheers, Stefan [1] https://bitbucket.org/iru/o9fs/overview
Re: 9p
However, as we've seen with lots of things that touch vfs it's pretty easy to get to 80 or 90 percent functionality and then the last 10% is a royal red pain in the butt, with possibly awful crashing bugs. The word possibly makes that sentence too optimistic.
Re: 9p
Yes, that's true. you *WILL* have awful crashing or hanging bugs to chase ;) Welcome to the midlayer. Wine bottles are optional but highly recommended. On Fri, May 30, 2014 at 2:55 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: However, as we've seen with lots of things that touch vfs it's pretty easy to get to 80 or 90 percent functionality and then the last 10% is a royal red pain in the butt, with possibly awful crashing bugs. The word possibly makes that sentence too optimistic.
Re: 9p
Yes, that's true. you *WILL* have awful crashing or hanging bugs to chase ;) Welcome to the midlayer. Wine bottles are optional but highly recommended. And dual purpose. 1) drink it with pleasure in the company of a VFS hacker 2) when the midlayer breaks, beat the VFS hacker over the head
Re: 9p
Most VFS hackers would say there is a third purpose. but don't scare him away yet... On Fri, May 30, 2014 at 3:01 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: Yes, that's true. you *WILL* have awful crashing or hanging bugs to chase ;) Welcome to the midlayer. Wine bottles are optional but highly recommended. And dual purpose. 1) drink it with pleasure in the company of a VFS hacker 2) when the midlayer breaks, beat the VFS hacker over the head
Re: 9p
Stefan Fritsch s...@sfritsch.de wrote: [1] https://bitbucket.org/iru/o9fs/overview Thanks for the link; this could be useful. Bob Beck b...@obtuse.com wrote: So I'm certainly not averse to someone working on it, but it would have to be solid and with people to love it before it ever made it into the tree How would it be distributed until then? Another repository? Patches posted to this list? So, with that in mind, if you're committed, please keep hacking on it. Now that I know that it has at least a snowball's chance in Hell to be in stock OpenBSD, yes ☺ I might even have time to look occasionally iif you send me a diff. Shall I send it to you alone, or the list? I have a newer version ready to send now.
Re: 9p
You pick. But before you do think about how to test it. On 30 May 2014 19:19, M Farkas-Dyck strake...@gmail.com wrote: Stefan Fritsch s...@sfritsch.de wrote: [1] https://bitbucket.org/iru/o9fs/overview Thanks for the link; this could be useful. Bob Beck b...@obtuse.com wrote: So I'm certainly not averse to someone working on it, but it would have to be solid and with people to love it before it ever made it into the tree How would it be distributed until then? Another repository? Patches posted to this list? So, with that in mind, if you're committed, please keep hacking on it. Now that I know that it has at least a snowball's chance in Hell to be in stock OpenBSD, yes ☺ I might even have time to look occasionally iif you send me a diff. Shall I send it to you alone, or the list? I have a newer version ready to send now.
Re: 9p
Ls seems to stat the directory and allocate a large enough dent buffer. I couldn't find what ls calls to do so, but I assume that it's a common function and other programs use it too. Problem is that directories in 9p customarily have length 0, so ls says every such directory empty. Getdents itself works. 3 options immediately come to mind: 1. Modify said function to not care how long stat says the directory is, choose an arbitrary dent buffer size, and iterate if need be 2. Read full directory on every stat call, blah 3. Lie that every directory length is (off_t)(-1) and hope that programs know to not try to allocate so much... Are any others? Which is the least evil?
9p
I have been writing a 9p vfs interface. Ultimately I hope to have this in stock
OpenBSD. So far it's incomplete and experimental and often faulty; I shall hack
it further when I have time, but meanwhile I post the diff here in case someone
else wishes to do so. This is my first vfs code, and I've been learning as I
go, in part from tmpfs code. Questions and criticism welcome.
diff --git a/sbin/mount_9p/mount_9p.c b/sbin/mount_9p/mount_9p.c
new file mode 100644
index 000..418f1f7
--- /dev/null
+++ b/sbin/mount_9p/mount_9p.c
@@ -0,0 +1,18 @@
+#include sys/param.h
+#include sys/mount.h
+#include stdio.h
+#include err.h
+
+int
+main (int argc, char *argu[])
+{
+ struct p9p_args args;
+ int c;
+
+ if (argc 3) errx (1, usage: %s fd path, argu[0]);
+ args.fd = strtol (argu[1], 0, 10);
+ args.msize = 1 20;
+ c = mount (MOUNT_P9P, argu[2], 0, args);
+ if (c) err (1, failed to mount);
+ return 0;
+}
diff --git a/sys/9p/9p.c b/sys/9p/9p.c
new file mode 100644
index 000..37c5ebd
--- /dev/null
+++ b/sys/9p/9p.c
@@ -0,0 +1,251 @@
+#include sys/param.h
+#include sys/malloc.h
+#include sys/systm.h
+#include sys/file.h
+#include 9p.h
+#include util.h
+
+int read9pmsg (struct file *fp, uint8_t *msg, size_t n) {
+ ssize_t size;
+ int c;
+
+ if (n 7) return EMSGSIZE;
+ c = read (fp, msg, 7);
+ if (c) return c;
+ LOAD32LE(size, msg);
+ if (size 7) return EIO;
+ switch (msg[4] /* type */) {
+ // Special case for TWrite and RRead, lest we copy huge data
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ if (n size) return EMSGSIZE;
+ return read (fp, msg + 7, size - 7);
+}
+
+int write9pmsg (struct file *fp, uint8_t *msg) {
+ size_t size;
+
+ LOAD32LE(size, msg);
+ switch (msg[4] /* type */) {
+ case RRead:
+ size = 11;
+ break;
+ case TWrite:
+ size = 23;
+ break;
+ }
+ return write (fp, msg, size);
+}
+
+size_t loadQid (Qid *p_qid, uint8_t *msg) {
+ p_qid - type = msg[0];
+ LOAD32LE(p_qid - vers, msg + 1);
+ LOAD64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+size_t storQid (Qid *p_qid, uint8_t *msg) {
+ msg[0] = p_qid - type;
+ STOR32LE(p_qid - vers, msg + 1);
+ STOR64LE(p_qid - path, msg + 5);
+ return 13;
+}
+
+static size_t vsscan9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ switch (fmt) {
+ case '0':
+ *(uint8_t *)p = msg[0];
+ return 1;
+ case '1':
+ LOAD16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ LOAD32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ LOAD64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return loadDir (p, msg);
+ case 'q':
+ return loadQid (p, msg);
+ case 's':
+ LOAD16LE(l, msg);
+ *(char **)p = malloc (l + 1, M_TEMP, M_WAITOK);
+ strlcpy (*(char **)p, msg + 2, l + 1);
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+static size_t vsprint9p1 (uint8_t *msg, char fmt, void *p) {
+ size_t l;
+ char *xs;
+ switch (fmt) {
+ case '0':
+ if (msg) msg[0] = *(uint8_t *)p;
+ return 1;
+ case '1':
+ if (msg) STOR16LE(*(uint16_t *)p, msg);
+ return 2;
+ case '2':
+ if (msg) STOR32LE(*(uint32_t *)p, msg);
+ return 4;
+ case '3':
+ if (msg) STOR64LE(*(uint64_t *)p, msg);
+ return 8;
+ case 'd':
+ return storDir (p, msg);
+ case 'q':
+ return (msg ? storQid (p, msg) : 13);
+ case 's':
+ xs = *(char **)p;
+ l = xs ? strlen (xs) : 0;
+ if (msg) {
+ STOR16LE(l, msg);
+ if (xs) memcpy (msg + 2, xs, l);
+ }
+ return (2 + l);
+ default:
+ panic (bad 9p message format spec);
+ }
+}
+
+#define ATOFFSET(t, p, r) (*(t *)((uint8_t *)(p) + r))
+
+static size_t vsscan9p (uint8_t *msg, char *fmt, void *p, size_t rs[]) {
+ size_t n;
+ uint8_t *q, *msg0;
+ msg0 = msg;
+ for (; fmt[0]; fmt++, rs++) switch (fmt[0]) {
+ case 'l':
+ msg += 2;
+ rs--;
+ break;
+ case 'n':
+ LOAD16LE(n, msg);
+ msg += 2;
+ ATOFFSET(uint16_t, p, rs[0]) = n;
+ fmt++;
+ rs++;
+ q = malloc (sizeof (void *)*n, M_TEMP, M_WAITOK);
+ ATOFFSET(void *, p, rs[0]) = q
Re: 9p
If any interest, please tell me before using earlier code, as I now have a later version, which I would post in that case.
Re: 9p
On Thu, May 29, 2014 at 03:12, strake...@gmail.com wrote: I have been writing a 9p vfs interface. Ultimately I hope to have this in stock OpenBSD. So far it's incomplete and experimental and often faulty; I shall hack it further when I have time, but meanwhile I post the diff here in case someone else wishes to do so. This is my first vfs code, and I've been learning as I go, in part from tmpfs code. Questions and criticism welcome. The first question is why not use fuse? I think it's better to have one userland filesystem interface than two.
Re: 9p
On 29/05/2014, Ted Unangst t...@tedunangst.com wrote: The first question is why not use fuse? I think it's better to have one userland filesystem interface than two. We already have 2: fuse, nfs. • 9p can operate over arbitrary transports, such as virtio and tcp. Fuse can't. • To my knowledge one can't have root as fuse. • Fuse requires a copy to/from a memory buffer on read/write. I have usage cases in which fuse is the performance-limiting factor.
