From: Simon Glass <simon.glass@canonical.com> Implement ext4l_unlink() to delete files from ext4 filesystems. This enables the 'rm' command to work with ext4l. The implementation: - Resolves the parent directory and target file - Verifies the target is not a directory (use rmdir for that) - Calls ext4_unlink() to remove the directory entry - Uses journal transactions for crash safety Add ext4l_op_ptr() macro to select between ext4l_unlink() and the fallback based on CONFIG_EXT4_WRITE Call ext4_commit_super() to ensure the changes are written to disk. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- (no changes since v1) fs/ext4l/interface.c | 251 +++++++++++++++++----------- fs/fs_legacy.c | 2 +- include/ext4l.h | 9 + test/fs/ext4l.c | 41 +++++ test/py/tests/test_fs/test_ext4l.py | 5 + 5 files changed, 212 insertions(+), 96 deletions(-) diff --git a/fs/ext4l/interface.c b/fs/ext4l/interface.c index 94bfd71bfb5..442617a0d3e 100644 --- a/fs/ext4l/interface.c +++ b/fs/ext4l/interface.c @@ -874,75 +874,131 @@ int ext4l_read(const char *filename, void *buf, loff_t offset, loff_t len, return 0; } -static int ext4l_write_file(struct inode *dir, const char *filename, void *buf, - loff_t offset, loff_t len, loff_t *actwrite) +/** + * ext4l_resolve_file() - Resolve a file path for write operations + * @path: Path to process + * @dir_dentryp: Returns parent directory dentry (caller must kfree) + * @dentryp: Returns file dentry after lookup (caller must kfree) + * @path_copyp: Returns path copy (caller must free) + * + * Common setup for write operations. Validates inputs, checks read-write + * mount, parses path, resolves parent directory, creates dentries, and + * performs lookup. + * + * Return: 0 on success, negative on error + */ +static int ext4l_resolve_file(const char *path, struct dentry **dir_dentryp, + struct dentry **dentryp, char **path_copyp) { + char *path_copy, *dir_path, *last_slash; struct dentry *dir_dentry, *dentry, *result; - handle_t *handle = NULL; - struct buffer_head *bh; - struct inode *inode; - loff_t pos, end; - umode_t mode; + struct inode *dir_inode; + const char *basename; int ret; - /* Create dentry for the parent directory */ + if (!ext4l_sb) + return -ENODEV; + + if (!path) + return -EINVAL; + + /* Check if filesystem is mounted read-write */ + if (ext4l_sb->s_flags & SB_RDONLY) + return -EROFS; + + /* Parse path to get parent directory and basename */ + path_copy = strdup(path); + if (!path_copy) + return -ENOMEM; + + last_slash = strrchr(path_copy, '/'); + if (last_slash) { + *last_slash = '\0'; + dir_path = path_copy; + basename = last_slash + 1; + if (*dir_path == '\0') + dir_path = "/"; + } else { + dir_path = "/"; + basename = path; + } + + /* Resolve parent directory */ + ret = ext4l_resolve_path(dir_path, &dir_inode); + if (ret) { + free(path_copy); + return ret; + } + + if (!S_ISDIR(dir_inode->i_mode)) { + free(path_copy); + return -ENOTDIR; + } + + /* Create dentry for parent directory */ dir_dentry = kzalloc(sizeof(struct dentry), GFP_KERNEL); - if (!dir_dentry) + if (!dir_dentry) { + free(path_copy); return -ENOMEM; - dir_dentry->d_inode = dir; - dir_dentry->d_sb = dir->i_sb; + } + dir_dentry->d_inode = dir_inode; + dir_dentry->d_sb = dir_inode->i_sb; - /* Create dentry for the filename */ + /* Create dentry for the file */ dentry = kzalloc(sizeof(struct dentry), GFP_KERNEL); if (!dentry) { kfree(dir_dentry); + free(path_copy); return -ENOMEM; } - - /* Initialize dentry (kzalloc already zeros memory) */ - dentry->d_name.name = filename; - dentry->d_name.len = strlen(filename); - dentry->d_sb = dir->i_sb; + dentry->d_name.name = basename; + dentry->d_name.len = strlen(basename); + dentry->d_sb = dir_inode->i_sb; dentry->d_parent = dir_dentry; - /* Lookup file */ - result = ext4_lookup(dir, dentry, 0); - + /* Look up the file */ + result = ext4_lookup(dir_inode, dentry, 0); if (IS_ERR(result)) { - ret = PTR_ERR(result); - goto out_dentry; + kfree(dentry); + kfree(dir_dentry); + free(path_copy); + return PTR_ERR(result); } - if (result && result->d_inode) { + *dir_dentryp = dir_dentry; + *dentryp = dentry; + *path_copyp = path_copy; + + return 0; +} + +static int ext4l_write_file(struct dentry *dir_dentry, struct dentry *dentry, + void *buf, loff_t offset, loff_t len, + loff_t *actwrite) +{ + struct inode *dir = dir_dentry->d_inode; + handle_t *handle = NULL; + struct buffer_head *bh; + struct inode *inode; + loff_t pos, end; + umode_t mode; + int ret; + + if (dentry->d_inode) { /* File exists - use the existing inode for overwrite */ - inode = result->d_inode; - if (result != dentry) { - /* Use the result dentry instead */ - kfree(dentry); - dentry = result; - } + inode = dentry->d_inode; } else { - /* ext4_lookup returned NULL or a dentry with NULL inode */ - if (result && result != dentry) { - /* Free the result dentry since it doesn't have an inode */ - kfree(result); - } - /* Keep using our original dentry */ - dentry->d_inode = NULL; - /* File does not exist, create it */ /* Mode: 0644 (rw-r--r--) | S_IFREG */ mode = S_IFREG | 0644; ret = ext4_create(&nop_mnt_idmap, dir, dentry, mode, true); if (ret) - goto out_dentry; + return ret; inode = dentry->d_inode; } - if (!inode) { - ret = -EIO; - goto out_dentry; - } + if (!inode) + return -EIO; /* * Attach jinode for journaling if needed (like ext4_file_open does). @@ -950,7 +1006,7 @@ static int ext4l_write_file(struct inode *dir, const char *filename, void *buf, */ ret = ext4_inode_attach_jinode(inode); if (ret < 0) - goto out_dentry; + return ret; /* * Start a journal handle for the write operation. @@ -959,11 +1015,8 @@ static int ext4l_write_file(struct inode *dir, const char *filename, void *buf, */ handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, EXT4_DATA_TRANS_BLOCKS(inode->i_sb)); - if (IS_ERR(handle)) { - ret = PTR_ERR(handle); - handle = NULL; - goto out_dentry; - } + if (IS_ERR(handle)) + return PTR_ERR(handle); /* Write data to file */ pos = offset; @@ -1063,84 +1116,92 @@ out_handle: if (handle) { int stop_ret = ext4_journal_stop(handle); - if (stop_ret) { - if (!ret) - ret = stop_ret; - } + if (stop_ret && !ret) + ret = stop_ret; } -out_dentry: - /* - * Free our manually allocated dentries. In U-Boot's minimal dcache, - * these won't be cached elsewhere. - */ - kfree(dir_dentry); - kfree(dentry); return ret; } int ext4l_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite) { - struct inode *parent_inode = NULL; - char *parent_path = NULL; - const char *basename; + struct dentry *dir_dentry, *dentry; char *path_copy; - char *last_slash; int ret; - if (!ext4l_sb) - return -ENODEV; - - if (!filename || !buf || !actwrite) + if (!buf || !actwrite) return -EINVAL; - /* Check if filesystem is mounted read-write */ - if (ext4l_sb->s_flags & SB_RDONLY) - return -EROFS; + ret = ext4l_resolve_file(filename, &dir_dentry, &dentry, &path_copy); + if (ret) + return ret; - /* Parse filename to get parent directory and basename */ - path_copy = strdup(filename); - if (!path_copy) - return -ENOMEM; + /* Call write implementation */ + ret = ext4l_write_file(dir_dentry, dentry, buf, offset, len, actwrite); - last_slash = strrchr(path_copy, '/'); + /* Sync all dirty buffers - U-Boot has no journal thread */ + if (!ret) { + int sync_ret = bh_cache_sync(); - if (last_slash) { - *last_slash = '\0'; - parent_path = path_copy; - basename = last_slash + 1; - if (*parent_path == '\0') /* Root directory */ - parent_path = "/"; - } else { - parent_path = "/"; - basename = filename; + if (sync_ret) + ret = sync_ret; } - /* Resolve parent directory inode */ - ret = ext4l_resolve_path(parent_path, &parent_inode); - if (ret) { - free(path_copy); + kfree(dentry); + kfree(dir_dentry); + free(path_copy); + return ret; +} + +int ext4l_unlink(const char *filename) +{ + struct dentry *dentry, *dir_dentry; + char *path_copy; + int ret; + + ret = ext4l_resolve_file(filename, &dir_dentry, &dentry, &path_copy); + if (ret) return ret; + + /* Check if file exists */ + if (!dentry->d_inode) { + ret = -ENOENT; + goto out; } - if (!S_ISDIR(parent_inode->i_mode)) { - free(path_copy); - return -ENOTDIR; + /* Cannot unlink directories with unlink - use rmdir */ + if (S_ISDIR(dentry->d_inode->i_mode)) { + ret = -EISDIR; + goto out; } - /* Call write implementation */ - ret = ext4l_write_file(parent_inode, basename, buf, offset, len, - actwrite); + /* Unlink the file */ + ret = __ext4_unlink(dir_dentry->d_inode, &dentry->d_name, + dentry->d_inode, dentry); - /* Sync all dirty buffers - U-Boot has no journal thread */ + /* + * Release inode - this triggers ext4_evict_inode for nlink=0 inodes, + * which frees the data blocks and inode. + */ + if (dentry->d_inode) { + iput(dentry->d_inode); + dentry->d_inode = NULL; + } + + /* Sync all dirty buffers after inode eviction */ if (!ret) { int sync_ret = bh_cache_sync(); if (sync_ret) ret = sync_ret; + /* Commit superblock with updated free counts */ + ext4_commit_super(ext4l_sb); } +out: + kfree(dentry); + kfree(dir_dentry); free(path_copy); return ret; } diff --git a/fs/fs_legacy.c b/fs/fs_legacy.c index 66545834928..1eb79b338ee 100644 --- a/fs/fs_legacy.c +++ b/fs/fs_legacy.c @@ -288,7 +288,7 @@ static struct fstype_info fstypes[] = { .opendir = ext4l_opendir, .readdir = ext4l_readdir, .closedir = ext4l_closedir, - .unlink = fs_unlink_unsupported, + .unlink = ext4l_op_ptr(ext4l_unlink, fs_unlink_unsupported), .mkdir = fs_mkdir_unsupported, .ln = fs_ln_unsupported, .rename = fs_rename_unsupported, diff --git a/include/ext4l.h b/include/ext4l.h index 5dfb671690c..abcf17f99ad 100644 --- a/include/ext4l.h +++ b/include/ext4l.h @@ -92,6 +92,15 @@ int ext4l_read(const char *filename, void *buf, loff_t offset, loff_t len, int ext4l_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite); +/** + * ext4l_unlink() - Delete a file + * + * @filename: Path to file to delete + * Return: 0 on success, -ENOENT if file not found, -EISDIR if path is a + * directory, -EROFS if read-only, negative on other errors + */ +int ext4l_unlink(const char *filename); + /** * ext4l_get_uuid() - Get the filesystem UUID * diff --git a/test/fs/ext4l.c b/test/fs/ext4l.c index 82587a87894..dec6e3340bb 100644 --- a/test/fs/ext4l.c +++ b/test/fs/ext4l.c @@ -438,3 +438,44 @@ static int fs_test_ext4l_write_norun(struct unit_test_state *uts) } FS_TEST_ARGS(fs_test_ext4l_write_norun, UTF_SCAN_FDT | UTF_CONSOLE | UTF_MANUAL, { "fs_image", UT_ARG_STR }); + +/** + * fs_test_ext4l_unlink_norun() - Test ext4l_unlink function + * + * Verifies that ext4l can delete files from the filesystem. + * + * Arguments: + * fs_image: Path to the ext4 filesystem image + */ +static int fs_test_ext4l_unlink_norun(struct unit_test_state *uts) +{ + const char *fs_image = ut_str(EXT4L_ARG_IMAGE); + const char *test_data = "unlink test\n"; + size_t test_len = strlen(test_data); + loff_t actwrite; + + ut_assertnonnull(fs_image); + ut_assertok(run_commandf("host bind 0 %s", fs_image)); + ut_assertok(fs_set_blk_dev("host", "0", FS_TYPE_ANY)); + + /* Create a new file to unlink */ + ut_assertok(ext4l_write("/unlinkme.txt", (void *)test_data, 0, + test_len, &actwrite)); + ut_asserteq(test_len, actwrite); + + /* Verify file exists (same mount) */ + ut_asserteq(1, ext4l_exists("/unlinkme.txt")); + + /* Unlink the file */ + ut_assertok(ext4l_unlink("/unlinkme.txt")); + + /* Verify file no longer exists */ + ut_asserteq(0, ext4l_exists("/unlinkme.txt")); + + /* Verify unlinking non-existent file returns -ENOENT */ + ut_asserteq(-ENOENT, ext4l_unlink("/nonexistent")); + + return 0; +} +FS_TEST_ARGS(fs_test_ext4l_unlink_norun, UTF_SCAN_FDT | UTF_CONSOLE | UTF_MANUAL, + { "fs_image", UT_ARG_STR }); diff --git a/test/py/tests/test_fs/test_ext4l.py b/test/py/tests/test_fs/test_ext4l.py index e953be379eb..edea38f7faf 100644 --- a/test/py/tests/test_fs/test_ext4l.py +++ b/test/py/tests/test_fs/test_ext4l.py @@ -123,3 +123,8 @@ class TestExt4l: """Test that ext4l can write file contents.""" with ubman.log.section('Test ext4l write'): ubman.run_ut('fs', 'fs_test_ext4l_write', fs_image=ext4_image) + + def test_unlink(self, ubman, ext4_image): + """Test that ext4l can delete files.""" + with ubman.log.section('Test ext4l unlink'): + ubman.run_ut('fs', 'fs_test_ext4l_unlink', fs_image=ext4_image) -- 2.43.0