| Message ID | e47c58ca95c93503a9d85dc6d749acf9fe9c6e76.1527310210.git.keno@alumni.harvard.edu |
|---|---|
| State | New |
| Headers | show |
| Series | 9p: Add support for Darwin | expand |
On Sat, 26 May 2018 01:23:13 -0400 keno@juliacomputing.com wrote: > From: Keno Fischer <keno@alumni.harvard.edu> > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > --- > hw/9pfs/9p-local.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index c55ea25..3e358b7 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -669,6 +669,13 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > return -1; > } > > +#ifdef CONFIG_DARWIN > + /* Darwin doesn't have mknodat and it's unlikely to work anyway, What's unlikely to work ? > + so let's just mark it as unsupported */ > + err = -1; > + errno = EOPNOTSUPP; > + goto out; > +#else Please introduce qemu_mknodat() with distinct implementations for linux and darwin. > 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); > @@ -699,6 +706,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > > err_end: > unlinkat_preserve_errno(dirfd, name, 0); > +#endif > + > out: > close_preserve_errno(dirfd); > return err;
>> +#ifdef CONFIG_DARWIN >> + /* Darwin doesn't have mknodat and it's unlikely to work anyway, > > What's unlikely to work ? > My concern was that allowing this would cause unexpected behavior, since the device numbers will differ between OS X and Linux. Though maybe this isn't the place to worry about that.
On Thu, 31 May 2018 12:37:56 -0400 Keno Fischer <keno@juliacomputing.com> wrote: > >> +#ifdef CONFIG_DARWIN > >> + /* Darwin doesn't have mknodat and it's unlikely to work anyway, > > > > What's unlikely to work ? > > > > My concern was that allowing this would cause unexpected > behavior, since the device numbers will differ between OS X > and Linux. Though maybe this isn't the place to worry about > that. The numbers may differ indeed but we don't really care since the server never opens device files. This is just a directory entry.
>> My concern was that allowing this would cause unexpected >> behavior, since the device numbers will differ between OS X >> and Linux. Though maybe this isn't the place to worry about >> that. > > The numbers may differ indeed but we don't really care since the > server never opens device files. This is just a directory entry. Ok, let me try to implement it. However, I don't think it is possible to implement mknodat (or at least I can't think of how) on Darwin directly. I could use regular mknod, but presumably, this is used to avoid a race condition between creating the device and setting the permissions. Can you think of a good way to resolve that?
On Thu, May 31, 2018 at 6:56 PM, Keno Fischer <keno@juliacomputing.com> wrote: >>> My concern was that allowing this would cause unexpected >>> behavior, since the device numbers will differ between OS X >>> and Linux. Though maybe this isn't the place to worry about >>> that. >> >> The numbers may differ indeed but we don't really care since the >> server never opens device files. This is just a directory entry. > > Ok, let me try to implement it. However, I don't think it is possible > to implement mknodat (or at least I can't think of how) on Darwin > directly. I could use regular mknod, but presumably, this is used > to avoid a race condition between creating the device and setting > the permissions. Can you think of a good way to resolve that? Would it work to fchdir in to the directory and use a cwd-relative mknod, then fchdir back? Or do we need to maintain the cwd while in this code?
On Thu, May 31, 2018 at 7:06 PM, Keno Fischer <keno@juliacomputing.com> wrote: > On Thu, May 31, 2018 at 6:56 PM, Keno Fischer <keno@juliacomputing.com> wrote: >>>> My concern was that allowing this would cause unexpected >>>> behavior, since the device numbers will differ between OS X >>>> and Linux. Though maybe this isn't the place to worry about >>>> that. >>> >>> The numbers may differ indeed but we don't really care since the >>> server never opens device files. This is just a directory entry. >> >> Ok, let me try to implement it. However, I don't think it is possible >> to implement mknodat (or at least I can't think of how) on Darwin >> directly. I could use regular mknod, but presumably, this is used >> to avoid a race condition between creating the device and setting >> the permissions. Can you think of a good way to resolve that? > > Would it work to fchdir in to the directory and use a cwd-relative > mknod, then fchdir back? Or do we need to maintain the cwd > while in this code? Sorry for the triple-self-post here, but I took a look at the Darwin kernel source and there's an unexposed (from the Darwin C library) syscall that only changes the current thread's cwd. That seems like it should be safe, so I'll go ahead and use that to implement mknodat. I'll also submit a feature request to apple to implement mknodat.
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index c55ea25..3e358b7 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -669,6 +669,13 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, return -1; } +#ifdef CONFIG_DARWIN + /* Darwin doesn't have mknodat and it's unlikely to work anyway, + so let's just mark it as unsupported */ + err = -1; + errno = EOPNOTSUPP; + goto out; +#else 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); @@ -699,6 +706,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, err_end: unlinkat_preserve_errno(dirfd, name, 0); +#endif + out: close_preserve_errno(dirfd); return err;