[PATCH 1/4] virtio: Add a little more debugging
From: Simon Glass <sjg@chromium.org> Add some debugging to virtiofs in the directory-handling area. Drop a stray, blank line while here. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/virtio/fs.c | 1 + drivers/virtio/fs_dir.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/fs.c b/drivers/virtio/fs.c index 9437f31bdba..f1065fe96d7 100644 --- a/drivers/virtio/fs.c +++ b/drivers/virtio/fs.c @@ -224,6 +224,7 @@ int virtio_fs_readdir(struct udevice *dev, u64 nodeid, u64 fh, u64 offset, in.size = size; in.offset = offset; ret = virtio_fs_xfer(dev, &inhdr, &in, sizeof(in), &outhdr, buf, size); + log_debug("fh %llx offset %llx\n", in.fh, in.offset); log_debug("len %x error %x unique %llx\n", outhdr.len, outhdr.error, outhdr.unique); if (ret) diff --git a/drivers/virtio/fs_dir.c b/drivers/virtio/fs_dir.c index b346ded8a09..72ab9a087a8 100644 --- a/drivers/virtio/fs_dir.c +++ b/drivers/virtio/fs_dir.c @@ -31,12 +31,12 @@ static int virtio_fs_dir_open(struct udevice *dev, struct fs_dir_stream *strm) log_debug("opening inode %lld\n", dir_priv->inode); ret = virtio_fs_opendir(fs, dir_priv->inode, &strm->fh); + log_debug("2 open ret %d strm %p fh %llx\n", ret, strm, strm->fh); if (ret) { log_err("Failed to open directory: %d\n", ret); return ret; } strm->dev = dev; - strm->offset = 0; return 0; @@ -53,7 +53,8 @@ int virtio_fs_dir_read(struct udevice *dev, struct fs_dir_stream *strm, char buf[0x200]; int ret, size; - log_debug("start %lld strm %p\n", dir_priv->inode, strm); + log_debug("start %lld strm %p fh %llx\n", dir_priv->inode, strm, + strm->fh); log_debug("offset %lld\n", strm->offset); ret = virtio_fs_readdir(fs, dir_priv->inode, strm->fh, strm->offset, buf, sizeof(buf), &size); -- 2.43.0 base-commit: 41d10e0f8b88ed7fabd6c95cc129bf52fed080a7 branch: virt
From: Simon Glass <sjg@chromium.org> Rather than creating a new stream, which gets lost, virtio_fs_dir_open() should use the existing one. Fix it, so that the fh member is preserved for future use. This fixes a problem where the fh value is not preserved, thus causing a subsequent 'ls' on the same directory to fail. Signed-off-by: Simon Glass <sjg@chromium.org> Fixes: #15 --- drivers/virtio/fs_dir.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/virtio/fs_dir.c b/drivers/virtio/fs_dir.c index 72ab9a087a8..4ca320175c5 100644 --- a/drivers/virtio/fs_dir.c +++ b/drivers/virtio/fs_dir.c @@ -25,10 +25,6 @@ static int virtio_fs_dir_open(struct udevice *dev, struct fs_dir_stream *strm) struct udevice *fs = dev_get_parent(dev); int ret; - strm = malloc(sizeof(struct fs_dir_stream)); - if (!strm) - return log_msg_ret("vso", -ENOMEM); - log_debug("opening inode %lld\n", dir_priv->inode); ret = virtio_fs_opendir(fs, dir_priv->inode, &strm->fh); log_debug("2 open ret %d strm %p fh %llx\n", ret, strm, strm->fh); -- 2.43.0 base-commit: 41d10e0f8b88ed7fabd6c95cc129bf52fed080a7 branch: virt
From: Simon Glass <sjg@chromium.org> It is easier to use an empty string when the root directory is intended. Adjust the code to drop use of "/" and NULL and just use and empty string. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/virtio/fs_dir.c | 6 ++---- drivers/virtio/fs_internal.h | 4 ++-- fs/dir-uclass.c | 2 +- fs/sandbox/sandboxfs.c | 8 +++++--- include/dir.h | 5 +++-- include/fs.h | 9 +++++++-- test/dm/fs.c | 4 ++-- 7 files changed, 22 insertions(+), 16 deletions(-) diff --git a/drivers/virtio/fs_dir.c b/drivers/virtio/fs_dir.c index 4ca320175c5..cfd9573250a 100644 --- a/drivers/virtio/fs_dir.c +++ b/drivers/virtio/fs_dir.c @@ -160,14 +160,12 @@ int virtio_fs_setup_dir(struct udevice *fsdev, const char *path, { struct virtio_fs_dir_priv *dir_priv; struct udevice *dir; - bool has_path; u64 inode; int ret; log_debug("looking up path '%s'\n", path); inode = FUSE_ROOT_ID; - has_path = path && strcmp("/", path); - if (has_path) { + if (*path) { ret = virtio_fs_lookup(fsdev, path, &inode); if (ret) { log_err("Failed to lookup directory '%s': %d\n", path, @@ -190,7 +188,7 @@ int virtio_fs_setup_dir(struct udevice *fsdev, const char *path, return 0; no_add: - if (has_path) + if (*path) ret = virtio_fs_forget(fsdev, inode); return ret; diff --git a/drivers/virtio/fs_internal.h b/drivers/virtio/fs_internal.h index 460a79d9854..c620e0f40e4 100644 --- a/drivers/virtio/fs_internal.h +++ b/drivers/virtio/fs_internal.h @@ -152,10 +152,10 @@ long virtio_fs_read(struct udevice *dev, u64 nodeid, u64 fh, u64 offset, * Looks up a path to find the corresponding inode in the virtio-fs filesystem, * then creates and probes a new 'directory' device to represent it. * - * If the path is the root (NULL or "/"), it uses the root inode directly. + * If the path is the root (@path is ""), it uses the root inode directly. * * @fsdev: The virtio-fs filesystem device - * @path: The path of the directory to set up (e.g., "/boot" or "/") + * @path: The path of the directory to set up (e.g., "/boot", or "" for root) * @devp: On success, returns a pointer to the newly created directory device * Return: 0 on success, -ve on error */ diff --git a/fs/dir-uclass.c b/fs/dir-uclass.c index f2c3a7c3806..c5d7ef45d3b 100644 --- a/fs/dir-uclass.c +++ b/fs/dir-uclass.c @@ -26,7 +26,7 @@ int dir_add_probe(struct udevice *fsdev, struct driver *drv, const char *path, str = strdup(dev_name); if (!str) goto no_dev_name; - dup_path = strdup(path && strcmp("/", path) ? path : ""); + dup_path = strdup(path); if (!str) goto no_dev_path; diff --git a/fs/sandbox/sandboxfs.c b/fs/sandbox/sandboxfs.c index adf852ee53d..5b8f4f50a8e 100644 --- a/fs/sandbox/sandboxfs.c +++ b/fs/sandbox/sandboxfs.c @@ -206,7 +206,8 @@ static int sandbox_dir_open(struct udevice *dev, struct fs_dir_stream *strm) struct dir_uc_priv *dir_uc_priv = dev_get_uclass_priv(dev); int ret; - ret = os_dirent_ls(dir_uc_priv->path, &priv->head); + ret = os_dirent_ls(*dir_uc_priv->path ? dir_uc_priv->path : ".", + &priv->head); if (ret) { log_err("Failed to open directory: %d\n", ret); return ret; @@ -300,7 +301,8 @@ static int sandbox_dir_open_file(struct udevice *dir, const char *leaf, struct udevice *dev; off_t size; - snprintf(pathname, sizeof(pathname), "%s/%s", uc_priv->path, leaf); + snprintf(pathname, sizeof(pathname), "%s/%s", + *uc_priv->path ? uc_priv->path: ".", leaf); ftype = os_get_filetype(pathname); if (ftype < 0) return log_msg_ret("soF", ftype); @@ -367,7 +369,7 @@ static int sandbox_fs_lookup_dir(struct udevice *dev, const char *path, int ftype; int ret; - ftype = os_get_filetype(path ?: "/"); + ftype = os_get_filetype(*path ? path : "/"); if (ftype < 0) return ftype; if (ftype != OS_FILET_DIR) diff --git a/include/dir.h b/include/dir.h index f948843c6b3..c25fac4fcd3 100644 --- a/include/dir.h +++ b/include/dir.h @@ -117,10 +117,11 @@ int dir_close(struct udevice *dev, struct fs_dir_stream *strm); /** * dir_add_probe() - Add a new directory and probe it * + * This sets up the uclass-private data for the new directory + * * @fsdev: Filesystem containing the directory * @drv: Driver to use - * @path Absolute path to directory (within the filesystem), or NULL/"/" for - * root + * @path Absolute path to directory (within the filesystem), or "" for root * @devp: Returns the new device, probed ready for use * */ int dir_add_probe(struct udevice *fsdev, struct driver *drv, const char *path, diff --git a/include/fs.h b/include/fs.h index 5321d339b07..6696b9e26c7 100644 --- a/include/fs.h +++ b/include/fs.h @@ -58,8 +58,11 @@ struct fs_ops { /** * lookup_dir() - Look up a directory on a filesystem * + * This should not set up the uclass-private data; this is done by + * fs_lookup_dir() + * * @dev: Filesystem device - * @path: Path to look up, empty or "/" for the root + * @path: Path to look up, "" for the root * @dirp: Returns associated directory device, creating if necessary * Return 0 if OK, -ENOENT, other -ve on error */ @@ -89,8 +92,10 @@ int fs_unmount(struct udevice *dev); /** * fs_lookup_dir() - Look up a directory on a filesystem * + * If a new directory-device is created, its uclass data is set up also + * * @dev: Filesystem device - * @path: Path to look up, empty or "/" for the root + * @path: Path to look up, "" or "/" for the root * @dirp: Returns associated directory device, creating if necessary * Return 0 if OK, -ENOENT, other -ve on error */ diff --git a/test/dm/fs.c b/test/dm/fs.c index e94b7854184..31712617d09 100644 --- a/test/dm/fs.c +++ b/test/dm/fs.c @@ -44,7 +44,7 @@ static int dm_test_fs_dir(struct unit_test_state *uts) ut_assertok(fs_mount(fsdev)); ut_asserteq(-ENOENT, fs_lookup_dir(fsdev, "does-not-exit", &dir)); - ut_assertok(fs_lookup_dir(fsdev, ".", &dir)); + ut_assertok(fs_lookup_dir(fsdev, "", &dir)); ut_assertnonnull(dir); ut_asserteq_str("fs.dir", dir->name); @@ -79,7 +79,7 @@ static int dm_test_fs_file(struct unit_test_state *uts) ut_assertok(fs_mount(fsdev)); - ut_assertok(fs_lookup_dir(fsdev, ".", &dir)); + ut_assertok(fs_lookup_dir(fsdev, "", &dir)); ut_assertnonnull(dir); ut_asserteq_str("fs.dir", dir->name); -- 2.43.0 base-commit: 41d10e0f8b88ed7fabd6c95cc129bf52fed080a7 branch: virt
From: Simon Glass <sjg@chromium.org> At present a new directory device is created for every access to a directory. This means that a new device is always created when 'ls' is used. Where the directory being accessed matches an existing device, this is not necessary. Check that path against existing directories and reuse the device if possible. Signed-off-by: Simon Glass <sjg@chromium.org> --- fs/fs-uclass.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/fs/fs-uclass.c b/fs/fs-uclass.c index 96fb04a106c..a335391bd74 100644 --- a/fs/fs-uclass.c +++ b/fs/fs-uclass.c @@ -9,6 +9,7 @@ #include <bootdev.h> #include <bootmeth.h> +#include <dir.h> #include <dm.h> #include <fs.h> #include <dm/device-internal.h> @@ -41,8 +42,35 @@ int fs_split_path(const char *fname, char **subdirp, const char **leafp) int fs_lookup_dir(struct udevice *dev, const char *path, struct udevice **dirp) { struct fs_ops *ops = fs_get_ops(dev); + struct udevice *dir; + int ret; + + if (!path || !strcmp("/", path)) + path = ""; + + /* see if we already have this directory */ + device_foreach_child(dir, dev) { + struct dir_uc_priv *priv; + + if (!device_active(dir)) + continue; + + priv = dev_get_uclass_priv(dir); + log_debug("dir %s '%s' '%s'\n", dir->name, path, priv->path); + if (!strcmp(path, priv->path)) { + *dirp = dir; + log_debug("found: dev '%s'\n", dir->name); + return 0; + } + } + + ret = ops->lookup_dir(dev, path, &dir); + if (ret) + return log_msg_ret("fld", ret); - return ops->lookup_dir(dev, path, dirp); + *dirp = dir; + + return 0; } int fs_mount(struct udevice *dev) -- 2.43.0 base-commit: 41d10e0f8b88ed7fabd6c95cc129bf52fed080a7 branch: virt
participants (1)
-
Simon Glass