| Message ID | 20220220165056.72289-5-wwcohen@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9p: Add support for darwin | expand |
On Sonntag, 20. Februar 2022 17:50:49 CET Will Cohen wrote: > From: Keno Fischer <keno@juliacomputing.com> > > 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 and inject it into d_seekoff, and create a > qemu_dirent_off helper to call it appropriately when appropriate. > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > [Michael Roitzsch: - Rebase for NixOS] > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com> > [Will Cohen: - Adjust to pass testing > - Ensure that d_seekoff is filled using telldir > on darwin, and create qemu_dirent_off helper > to decide which to access] > [Fabian Franz: - Add telldir error handling for darwin] > Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com> > [Will Cohen: - Ensure that telldir error handling uses > signed int > - Cleanup of telldir error handling > - Remove superfluous error handling for > qemu_dirent_off > - Adjust formatting > - Use qemu_dirent_off in codir.c] > Signed-off-by: Will Cohen <wwcohen@gmail.com> > --- This patch does not compile on Linux ... > hw/9pfs/9p-local.c | 9 +++++++++ > hw/9pfs/9p-proxy.c | 16 +++++++++++++++- > hw/9pfs/9p-synth.c | 4 ++++ > hw/9pfs/9p-util.h | 16 ++++++++++++++++ > hw/9pfs/9p.c | 7 +++++-- > hw/9pfs/codir.c | 4 +++- > 6 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index 1a5e3eed73..f3272f0b43 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -562,6 +562,15 @@ again: > if (!entry) { > return NULL; > } > +#ifdef CONFIG_DARWIN > + int off; > + off = telldir(fs->dir.stream); > + /* If telldir fails, fail the entire readdir call */ > + if (off < 0) { > + return NULL; > + } > + entry->d_seekoff = off; > +#endif > > if (ctx->export_flags & V9FS_SM_MAPPED) { > entry->d_type = DT_UNKNOWN; > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index b1664080d8..8b4b5cf7dc 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx, > V9fsFidOpenState *fs) > > static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs) > { > - return readdir(fs->dir.stream); > + struct dirent *entry; > + entry = readdir(fs->dir.stream); > +#ifdef CONFIG_DARWIN > + if (!entry) { > + return NULL; > + } > + int td; > + td = telldir(fs->dir.stream); > + /* If telldir fails, fail the entire readdir call */ > + if (td < 0) { > + return NULL; > + } > + entry->d_seekoff = td; > +#endif > + return entry; > } > > static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off) > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c > index bf9b0c5ddd..b3080e415b 100644 > --- a/hw/9pfs/9p-synth.c > +++ b/hw/9pfs/9p-synth.c > @@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node, > offsetof(struct dirent, d_name) + sz); > memcpy(entry->d_name, node->name, sz); > entry->d_ino = node->attr->inode; > +#ifdef CONFIG_DARWIN > + entry->d_seekoff = off + 1; > +#else > entry->d_off = off + 1; > +#endif > } > > static struct dirent *synth_get_dentry(V9fsSynthNode *dir, > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h > index 546f46dc7d..d41f37f085 100644 > --- a/hw/9pfs/9p-util.h > +++ b/hw/9pfs/9p-util.h > @@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char > *filename, const char *name); > > #endif ... ^- this is the end of file #endif, so qemu_dirent_off() should be above that #endif, and ... > + > + > +/** > + * 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 ensure it is manually injected earlier and call here when > + * needed. > + */ > +inline off_t qemu_dirent_off(struct dirent *dent) ... this function declaration misses the 'static' keyword, which is mandatory to prevent a linker error. Best regards, Christian Schoenebeck > +{ > +#ifdef CONFIG_DARWIN > + return dent->d_seekoff; > +#else > + return dent->d_off; > +#endif > +} > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 1563d7b7c6..caf3b240fe 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -27,6 +27,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" > @@ -2281,7 +2282,7 @@ 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 = qemu_dirent_off(dent); > } > > v9fs_readdir_unlock(&fidp->fs.dir); > @@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, > V9fsFidState *fidp, V9fsString name; > int len, err = 0; > int32_t count = 0; > + off_t off; > struct dirent *dent; > struct stat *st; > struct V9fsDirEnt *entries = NULL; > @@ -2480,12 +2482,13 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU > *pdu, V9fsFidState *fidp, qid.version = 0; > } > > + off = qemu_dirent_off(dent); > v9fs_string_init(&name); > v9fs_string_sprintf(&name, "%s", dent->d_name); > > /* 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_string_free(&name); > diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c > index c0873bde16..f96d8ac4e6 100644 > --- a/hw/9pfs/codir.c > +++ b/hw/9pfs/codir.c > @@ -22,6 +22,8 @@ > #include "qemu/coroutine.h" > #include "qemu/main-loop.h" > #include "coth.h" > +#include "9p-xattr.h" > +#include "9p-util.h" > > /* > * Intended to be called from bottom-half (e.g. background I/O thread) > @@ -166,7 +168,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState > *fidp, } > > size += len; > - saved_dir_pos = dent->d_off; > + saved_dir_pos = qemu_dirent_off(dent); > } > > /* restore (last) saved position */
Apologies — I tested on Mac but should have done Linux too. Will revise. On Sun, Feb 20, 2022 at 4:28 PM Christian Schoenebeck < qemu_oss@crudebyte.com> wrote: > On Sonntag, 20. Februar 2022 17:50:49 CET Will Cohen wrote: > > From: Keno Fischer <keno@juliacomputing.com> > > > > 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 and inject it into d_seekoff, and create a > > qemu_dirent_off helper to call it appropriately when appropriate. > > > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > > [Michael Roitzsch: - Rebase for NixOS] > > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com> > > [Will Cohen: - Adjust to pass testing > > - Ensure that d_seekoff is filled using telldir > > on darwin, and create qemu_dirent_off helper > > to decide which to access] > > [Fabian Franz: - Add telldir error handling for darwin] > > Signed-off-by: Fabian Franz <fabianfranz.oss@gmail.com> > > [Will Cohen: - Ensure that telldir error handling uses > > signed int > > - Cleanup of telldir error handling > > - Remove superfluous error handling for > > qemu_dirent_off > > - Adjust formatting > > - Use qemu_dirent_off in codir.c] > > Signed-off-by: Will Cohen <wwcohen@gmail.com> > > --- > > This patch does not compile on Linux ... > > > hw/9pfs/9p-local.c | 9 +++++++++ > > hw/9pfs/9p-proxy.c | 16 +++++++++++++++- > > hw/9pfs/9p-synth.c | 4 ++++ > > hw/9pfs/9p-util.h | 16 ++++++++++++++++ > > hw/9pfs/9p.c | 7 +++++-- > > hw/9pfs/codir.c | 4 +++- > > 6 files changed, 52 insertions(+), 4 deletions(-) > > > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > > index 1a5e3eed73..f3272f0b43 100644 > > --- a/hw/9pfs/9p-local.c > > +++ b/hw/9pfs/9p-local.c > > @@ -562,6 +562,15 @@ again: > > if (!entry) { > > return NULL; > > } > > +#ifdef CONFIG_DARWIN > > + int off; > > + off = telldir(fs->dir.stream); > > + /* If telldir fails, fail the entire readdir call */ > > + if (off < 0) { > > + return NULL; > > + } > > + entry->d_seekoff = off; > > +#endif > > > > if (ctx->export_flags & V9FS_SM_MAPPED) { > > entry->d_type = DT_UNKNOWN; > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > > index b1664080d8..8b4b5cf7dc 100644 > > --- a/hw/9pfs/9p-proxy.c > > +++ b/hw/9pfs/9p-proxy.c > > @@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx, > > V9fsFidOpenState *fs) > > > > static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState > *fs) > > { > > - return readdir(fs->dir.stream); > > + struct dirent *entry; > > + entry = readdir(fs->dir.stream); > > +#ifdef CONFIG_DARWIN > > + if (!entry) { > > + return NULL; > > + } > > + int td; > > + td = telldir(fs->dir.stream); > > + /* If telldir fails, fail the entire readdir call */ > > + if (td < 0) { > > + return NULL; > > + } > > + entry->d_seekoff = td; > > +#endif > > + return entry; > > } > > > > static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t > off) > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c > > index bf9b0c5ddd..b3080e415b 100644 > > --- a/hw/9pfs/9p-synth.c > > +++ b/hw/9pfs/9p-synth.c > > @@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node, > > offsetof(struct dirent, d_name) + sz); > > memcpy(entry->d_name, node->name, sz); > > entry->d_ino = node->attr->inode; > > +#ifdef CONFIG_DARWIN > > + entry->d_seekoff = off + 1; > > +#else > > entry->d_off = off + 1; > > +#endif > > } > > > > static struct dirent *synth_get_dentry(V9fsSynthNode *dir, > > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h > > index 546f46dc7d..d41f37f085 100644 > > --- a/hw/9pfs/9p-util.h > > +++ b/hw/9pfs/9p-util.h > > @@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char > > *filename, const char *name); > > > > #endif > > ... ^- this is the end of file #endif, so qemu_dirent_off() should be > above > that #endif, and ... > > > + > > + > > +/** > > + * 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 ensure it is manually injected earlier and call here when > > + * needed. > > + */ > > +inline off_t qemu_dirent_off(struct dirent *dent) > > ... this function declaration misses the 'static' keyword, which is > mandatory > to prevent a linker error. > > Best regards, > Christian Schoenebeck > > > +{ > > +#ifdef CONFIG_DARWIN > > + return dent->d_seekoff; > > +#else > > + return dent->d_off; > > +#endif > > +} > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index 1563d7b7c6..caf3b240fe 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -27,6 +27,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" > > @@ -2281,7 +2282,7 @@ 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 = qemu_dirent_off(dent); > > } > > > > v9fs_readdir_unlock(&fidp->fs.dir); > > @@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU > *pdu, > > V9fsFidState *fidp, V9fsString name; > > int len, err = 0; > > int32_t count = 0; > > + off_t off; > > struct dirent *dent; > > struct stat *st; > > struct V9fsDirEnt *entries = NULL; > > @@ -2480,12 +2482,13 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU > > *pdu, V9fsFidState *fidp, qid.version = 0; > > } > > > > + off = qemu_dirent_off(dent); > > v9fs_string_init(&name); > > v9fs_string_sprintf(&name, "%s", dent->d_name); > > > > /* 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_string_free(&name); > > diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c > > index c0873bde16..f96d8ac4e6 100644 > > --- a/hw/9pfs/codir.c > > +++ b/hw/9pfs/codir.c > > @@ -22,6 +22,8 @@ > > #include "qemu/coroutine.h" > > #include "qemu/main-loop.h" > > #include "coth.h" > > +#include "9p-xattr.h" > > +#include "9p-util.h" > > > > /* > > * Intended to be called from bottom-half (e.g. background I/O thread) > > @@ -166,7 +168,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState > > *fidp, } > > > > size += len; > > - saved_dir_pos = dent->d_off; > > + saved_dir_pos = qemu_dirent_off(dent); > > } > > > > /* restore (last) saved position */ > > >
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 1a5e3eed73..f3272f0b43 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -562,6 +562,15 @@ again: if (!entry) { return NULL; } +#ifdef CONFIG_DARWIN + int off; + off = telldir(fs->dir.stream); + /* If telldir fails, fail the entire readdir call */ + if (off < 0) { + return NULL; + } + entry->d_seekoff = off; +#endif if (ctx->export_flags & V9FS_SM_MAPPED) { entry->d_type = DT_UNKNOWN; diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index b1664080d8..8b4b5cf7dc 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx, V9fsFidOpenState *fs) static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs) { - return readdir(fs->dir.stream); + struct dirent *entry; + entry = readdir(fs->dir.stream); +#ifdef CONFIG_DARWIN + if (!entry) { + return NULL; + } + int td; + td = telldir(fs->dir.stream); + /* If telldir fails, fail the entire readdir call */ + if (td < 0) { + return NULL; + } + entry->d_seekoff = td; +#endif + return entry; } static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off) diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c index bf9b0c5ddd..b3080e415b 100644 --- a/hw/9pfs/9p-synth.c +++ b/hw/9pfs/9p-synth.c @@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node, offsetof(struct dirent, d_name) + sz); memcpy(entry->d_name, node->name, sz); entry->d_ino = node->attr->inode; +#ifdef CONFIG_DARWIN + entry->d_seekoff = off + 1; +#else entry->d_off = off + 1; +#endif } static struct dirent *synth_get_dentry(V9fsSynthNode *dir, diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index 546f46dc7d..d41f37f085 100644 --- a/hw/9pfs/9p-util.h +++ b/hw/9pfs/9p-util.h @@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, const char *name); #endif + + +/** + * 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 ensure it is manually injected earlier and call here when + * needed. + */ +inline off_t qemu_dirent_off(struct dirent *dent) +{ +#ifdef CONFIG_DARWIN + return dent->d_seekoff; +#else + return dent->d_off; +#endif +} diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 1563d7b7c6..caf3b240fe 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -27,6 +27,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" @@ -2281,7 +2282,7 @@ 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 = qemu_dirent_off(dent); } v9fs_readdir_unlock(&fidp->fs.dir); @@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString name; int len, err = 0; int32_t count = 0; + off_t off; struct dirent *dent; struct stat *st; struct V9fsDirEnt *entries = NULL; @@ -2480,12 +2482,13 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp, qid.version = 0; } + off = qemu_dirent_off(dent); v9fs_string_init(&name); v9fs_string_sprintf(&name, "%s", dent->d_name); /* 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_string_free(&name); diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c index c0873bde16..f96d8ac4e6 100644 --- a/hw/9pfs/codir.c +++ b/hw/9pfs/codir.c @@ -22,6 +22,8 @@ #include "qemu/coroutine.h" #include "qemu/main-loop.h" #include "coth.h" +#include "9p-xattr.h" +#include "9p-util.h" /* * Intended to be called from bottom-half (e.g. background I/O thread) @@ -166,7 +168,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, } size += len; - saved_dir_pos = dent->d_off; + saved_dir_pos = qemu_dirent_off(dent); } /* restore (last) saved position */