2011-08-17 08:04:19

by Kazuya Mio

[permalink] [raw]
Subject: [PATCH v2 11/12] e4defrag: Fix error messages more clearly

There are some error messages we don't really understand what happens.
This patch fixes those messages.

Signed-off-by: Kazuya Mio <[email protected]>
---
misc/e4defrag.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index ec1b15a..df532d3 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -323,6 +323,7 @@ static int is_ext4(const char *file)

mnt_type = realloc(mnt_type, strlen(mnt->mnt_type) + 1);
if (mnt_type == NULL) {
+ perror("Failed to allocate memory");
endmntent(fp);
return -1;
}
@@ -727,11 +728,9 @@ static int get_file_extents(int fd, struct fiemap_extent_list **ext_list_head)
!(ext_buf[EXTENT_MAX_COUNT-1].fe_flags
& FIEMAP_EXTENT_LAST));

- FREE(fiemap_buf);
- return 0;
out:
FREE(fiemap_buf);
- return -1;
+ return errno ? -1 : 0;
}

/*
@@ -873,7 +872,7 @@ static int call_defrag(int fd, int donor_fd, const char *file,
if (mode_flag & DETAIL) {
printf("\n");
PRINT_ERR_MSG_WITH_ERRNO(
- "Failed to free page");
+ "Failed to synchronize the file");
} else {
printf("\t[ NG ]\n");
}
@@ -971,15 +970,6 @@ static int file_defrag(const char *file, const struct stat64 *buf,
return 0;
}

- /* Has no blocks */
- if (buf->st_blocks == 0) {
- if (mode_flag & DETAIL) {
- PRINT_FILE_NAME(file);
- IN_FTW_PRINT_ERR_MSG("File has no blocks");
- }
- return 0;
- }
-
fd = open64(file, O_RDWR);
if (fd < 0) {
if (mode_flag & DETAIL) {
@@ -997,6 +987,12 @@ static int file_defrag(const char *file, const struct stat64 *buf,
PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
}
goto out;
+ } else if (orig_list == NULL) {
+ if (mode_flag & DETAIL) {
+ PRINT_FILE_NAME(file);
+ IN_FTW_PRINT_ERR_MSG("File has no blocks");
+ }
+ goto out;
}

/* Count file fragments before defrag */
@@ -1089,6 +1085,12 @@ static int file_defrag(const char *file, const struct stat64 *buf,
PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
}
goto out;
+ } else if (donor_list == NULL) {
+ if (mode_flag & DETAIL) {
+ PRINT_FILE_NAME(file);
+ IN_FTW_PRINT_ERR_MSG("Temporary file was removed");
+ }
+ goto out;
}

donor_score = e2p_get_fragscore(donor_fd, threshold,
@@ -1136,7 +1138,7 @@ check_improvement:
file_frags_end = file_frag_count(fd);
if (file_frags_end < 0) {
printf("\n");
- PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_INFO);
+ PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
goto out;
}

@@ -1282,10 +1284,7 @@ int main(int argc, char *argv[])
}

switch (arg_type) {
- case DIRNAME:
- printf("ext4 defragmentation for directory(%s)\n",
- argv[i]);


2011-08-17 16:36:39

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] e4defrag: Fix error messages more clearly

On 2011-08-17, at 1:48 AM, Kazuya Mio <[email protected]> wrote:
> There are some error messages we don't really understand what happens.
> This patch fixes those messages.
>
> Signed-off-by: Kazuya Mio <[email protected]>
> ---
> misc/e4defrag.c | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
> index ec1b15a..df532d3 100644
> --- a/misc/e4defrag.c
> +++ b/misc/e4defrag.c
> @@ -323,6 +323,7 @@ static int is_ext4(const char *file)
>
> mnt_type = realloc(mnt_type, strlen(mnt->mnt_type) + 1);
> if (mnt_type == NULL) {
> + perror("Failed to allocate memory");
> endmntent(fp);
> return -1;
> }
> @@ -727,11 +728,9 @@ static int get_file_extents(int fd, struct fiemap_extent_list **ext_list_head)
> !(ext_buf[EXTENT_MAX_COUNT-1].fe_flags
> & FIEMAP_EXTENT_LAST));
>
> - FREE(fiemap_buf);
> - return 0;
> out:
> FREE(fiemap_buf);
> - return -1;
> + return errno ? -1 : 0;
> }
>
> /*
> @@ -873,7 +872,7 @@ static int call_defrag(int fd, int donor_fd, const char *file,
> if (mode_flag & DETAIL) {
> printf("\n");
> PRINT_ERR_MSG_WITH_ERRNO(
> - "Failed to free page");
> + "Failed to synchronize the file");
> } else {
> printf("\t[ NG ]\n");
> }
> @@ -971,15 +970,6 @@ static int file_defrag(const char *file, const struct stat64 *buf,
> return 0;
> }
>
> - /* Has no blocks */
> - if (buf->st_blocks == 0) {
> - if (mode_flag & DETAIL) {
> - PRINT_FILE_NAME(file);
> - IN_FTW_PRINT_ERR_MSG("File has no blocks");
> - }
> - return 0;
> - }

I think it still makes sense to skip defrag if the file has no blocks. Depending on FIEMAP is dangerous if the file is being modified, and just adds overhead.

> fd = open64(file, O_RDWR);
> if (fd < 0) {
> if (mode_flag & DETAIL) {
> @@ -997,6 +987,12 @@ static int file_defrag(const char *file, const struct stat64 *buf,
> PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
> }
> goto out;
> + } else if (orig_list == NULL) {
> + if (mode_flag & DETAIL) {
> + PRINT_FILE_NAME(file);
> + IN_FTW_PRINT_ERR_MSG("File has no blocks");
> + }
> + goto out;
> }
>
> /* Count file fragments before defrag */
> @@ -1089,6 +1085,12 @@ static int file_defrag(const char *file, const struct stat64 *buf,
> PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
> }
> goto out;
> + } else if (donor_list == NULL) {
> + if (mode_flag & DETAIL) {
> + PRINT_FILE_NAME(file);
> + IN_FTW_PRINT_ERR_MSG("Temporary file was removed");
> + }
> + goto out;
> }
>
> donor_score = e2p_get_fragscore(donor_fd, threshold,
> @@ -1136,7 +1138,7 @@ check_improvement:
> file_frags_end = file_frag_count(fd);
> if (file_frags_end < 0) {
> printf("\n");
> - PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_INFO);
> + PRINT_ERR_MSG_WITH_ERRNO(NGMSG_FILE_EXTENT);
> goto out;
> }
>
> @@ -1282,10 +1284,7 @@ int main(int argc, char *argv[])
> }
>
> switch (arg_type) {
> - case DIRNAME:
> - printf("ext4 defragmentation for directory(%s)\n",
> - argv[i]);
> -
> + case DIRNAME: {
> int mount_dir_len = 0;
> mount_dir_len = strnlen(lost_found_dir, PATH_MAX);
>
> @@ -1313,6 +1312,10 @@ int main(int argc, char *argv[])
> /* "e4defrag mount_piont_dir/else_dir" */
> memset(lost_found_dir, 0, PATH_MAX + 1);
> }
> +
> + printf("ext4 defragmentation for directory(%s)\n",
> + argv[i]);
> + }
> case DEVNAME:
> if (arg_type == DEVNAME) {
> strncpy(lost_found_dir, dir_name,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html