Skip to content

Conversation

@HotMercury
Copy link
Collaborator

This PR addressed issue #50 , but I don't think I fully understand the reason why that if we not initial the disk to zero, it will cause error happended.

mkfs.c Outdated
stat_buf.st_size = blk_size;
}

/* Initialize the file system image with zeros */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should clear a root index block instead. That is, calloc(SIMPLEFS_BLOCK_SIZE) and write(fd, block, SIMPLEFS_BLOCK_SIZE). In addition, do verify the result of write system call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jserv, I think this issue is related to #20. The problem arises from incorrect reading of simplefs_file_ei_block. If the values inside blockdata are random or not the correct format, then the resulting simplefs_file_ei_block will be incorrect, causing the initial nr_files and simplefs_extent to have meaningless values. However, this leads to an sb_bread error during simplefs_lookup and other directory related problems.

To address this issue, the solution is to initialize the file system image with zeros instead of just clearing a root index block. This ensures that the values within blockdata are consistent and prevent the occurrence of random values.
Alternatively, should I approach this issue differently to find a solution?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To address this issue, the solution is to initialize the file system image with zeros instead of just clearing a root index block. This ensures that the values within blockdata are consistent and prevent the occurrence of random values. Alternatively, should I approach this issue differently to find a solution?

I would expect the minimal operations against the raw file system image. That is, to clear a root index block instead of zeroing the whole file system image. You should investigate the necessary changes required to make the file system consistent.

jserv

This comment was marked as outdated.

HotMercury added a commit to HotMercury/simplefs that referenced this pull request May 18, 2024
If we use `dd if=/dev/random of=test.img bs=1M count=32` rather
then `if=/dev/zero` and mount it, it will cause EIO error and
stuck in for loop.

This commit addresses an issue where `ls` commands would read random
, meaningless values from `simplefs_file_ei_block` for uninitialized
blocks. This occurred because the blocks contained residual data,
leading to erroneous outputs.

Changes:
1. Clear the root extent block during the `mkfs` process to prevent
error `simplefs_file_ei_block` data.
2. Ensured that file data blocks are cleared immediately upon
allocation to prevent error `simplefs_file` data.

Impact:
By clearing these blocks, we prevent the mistake during `ls`
command and resolve Issue sysprog21#51 and sysprog21#20.
bitmap.h Outdated
struct simplefs_sb_info *sbi = SIMPLEFS_SB(sb);
uint32_t ret = get_first_free_bits(sbi->bfree_bitmap, sbi->nr_blocks, len);
if (ret)
if (ret) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with the following:

    if (!ret) /* Add proper comments here */
        return 0;

    sbi->nr_free_blocks -= len;
    ...

mkfs.c Outdated
static int write_data_blocks(int fd, struct superblock *sb)
{
/* FIXME: unimplemented */
char *buffer = (char *) calloc(1, SIMPLEFS_BLOCK_SIZE);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove (char *) here.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Append Close #20 at the end of git commit message.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use backtick characters in the git commit message. Instead, use quote marks.

If we use 'dd if=/dev/random of=test.img bs=1M count=32' rather
then 'if=/dev/zero' and mount it, it will cause EIO error and
stuck in for loop.

This commit addresses an issue where 'ls' commands would read random
, meaningless values from 'simplefs_file_ei_block' for uninitialized
blocks. This occurred because the blocks contained residual data,
leading to erroneous outputs.

Changes:
1. Clear the root extent block during the 'mkfs' process to prevent
error 'simplefs_file_ei_block' data.
2. Ensured that file data blocks are cleared immediately upon
allocation to prevent error 'simplefs_file' data.

Impact:
By clearing these blocks, we prevent the mistake during 'ls'
command and resolve Issue sysprog21#51 and sysprog21#20.

Close sysprog21#20 and sysprog21#51
@jserv jserv merged commit 855c339 into sysprog21:master May 18, 2024
@jserv jserv mentioned this pull request May 18, 2024
@jserv
Copy link
Collaborator

jserv commented May 18, 2024

Thank @HotMercury for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants