From 085c2af234c5571f372466801837fa26f03feae1 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 3 Apr 2003 20:09:19 -0500 Subject: [PATCH] Import bug fixes from EVMS 2.0 tree. Fixed possible hangs caused by bugs in calling waitpid, and not setting the pipe to non-blocking mode. Also fixed a file descriptor leak. Made sure all functions call log entry/exit functions. --- lib/evms/ChangeLog | 39 +++++++++++++++++++++++++++++++ lib/evms/fs_ext2.c | 61 ++++++++++++++++++++++++++++-------------------- lib/evms/fsimext2.c | 67 +++++++++++++++++++++++++++++++++++++++++++---------- lib/evms/fsimext2.h | 2 +- 4 files changed, 131 insertions(+), 38 deletions(-) diff --git a/lib/evms/ChangeLog b/lib/evms/ChangeLog index 73ef768..0a9d659 100644 --- a/lib/evms/ChangeLog +++ b/lib/evms/ChangeLog @@ -1,3 +1,42 @@ +2003-04-03 Theodore Ts'o + + * fs_ext2.c (fs_setup): During FSIM setup, when the FSIM cannot be + loaded due to incompatible tool versions, just print a + message to the log instead of displaying a message to the + user. (EVMS 2.0, rev 1.3, change to commented out code only) + (ext2_plugrec): fix long plug-in name to indicate support + for ext3 (EVMS 2.0, rev 1.4) + (fs_expand, fs_shrink): Don't save the return value from + waitpid(). Especially don't save it back to the same + variable that will be used to call waitpid() again. + (EVMS 2.0, rev 1.12) + (fs_mkfs, fs_init_task, fs_set_option): Make sure most + functions have entry/exit log macros. (EVMS 2.0 rev 1.14) + (fs_expand, fs_shrink): Check for errors from + fsim_get_ext2_superblock(). (EVMS 2.0 rev 1.15) + + * fsimext2.c (set_mkfs_options): Fix compile warnings. + gcc 3.2.1 doesn't like empty "default:" cases at the end + of "switch" statements (EVMS 2.0 rev 1.2) + (fsim_get_volume_limits): Remove unused varable (EVMS 2.0 + rev 1.4) + (ext2fs_swap_super): Remove unnecessary "return" (EVMS 2.0 + rev 1.7) + Make sure most functions have entry/exit log macros (EVMS + 2.0 rev 1.8, 1.9) + (fsim_fsck, fsim_mkfs): Don't save the return value from + waitpid(). Especially don't save it back to the same + variable that will be used to call waitpid() again. (EVMS + 2.0 rev 1.5/partial). + (fsim_fsck): In fsim_fsck(), set the read pipe to + non-blocking before going into the loop waiting for data. + If there are no more data left, the function hangs waiting + for the read() to complete. (EVMS 2.0 rev 1.5/partial) + (fsim_fsck): Fix file descriptor leak. + + * fsimext2.h (LOGEXITRC): Synchronize text for exit macro with + EVMS 2.0 (rev 1.2). + 2002-11-09 Theodore Ts'o * Release of E2fsprogs 1.32 diff --git a/lib/evms/fs_ext2.c b/lib/evms/fs_ext2.c index 17c1c88..27341a6 100644 --- a/lib/evms/fs_ext2.c +++ b/lib/evms/fs_ext2.c @@ -46,22 +46,22 @@ static int fs_setup( engine_functions_t *engine_function_table) static int fs_setup( engine_mode_t mode, engine_functions_t *engine_function_table) #endif { - int rc = 0; + int rc = 0; EngFncs = engine_function_table; LOGENTRY(); - /* - * We don't really care about the e2fsprogs version, but we leave - * this here in case we do at a later date.... - */ - rc = fsim_test_version(); + /* + * We don't really care about the e2fsprogs version, but we leave + * this here in case we do at a later date.... + */ + rc = fsim_test_version(); #if 0 - if ( rc ) { - MESSAGE( "e2fsprogs must be version 1.XXX or later to function properly with this FSIM." ); - MESSAGE( "Please get the current version of e2fsprogs from http://e2fsprogs.sourceforge.net" ); - rc = ENOSYS; - } + if ( rc ) { + LOG_WARNING( "e2fsprogs must be version 1.XXX or later to function properly with this FSIM.\n" ); + LOG_WARNING( "Please get the current version of e2fsprogs from http://e2fsprogs.sourceforge.net\n" ); + rc = ENOSYS; + } #endif LOGEXIT(); return rc; @@ -280,6 +280,9 @@ static int fs_expand( logical_volume_t * volume, /* get and validate current ext2/3 superblock */ sb = (struct ext2_super_block *) volume->private_data; rc = fsim_get_ext2_superblock( volume, sb ); + if (rc) { + goto errout; + } if ((sb->s_lastcheck < sb->s_mtime) || (sb->s_state & EXT2_ERROR_FS) || ((sb->s_state & EXT2_VALID_FS) == 0)) { @@ -352,7 +355,7 @@ static int fs_expand( logical_volume_t * volume, /* wait for child to complete */ fcntl(fds2[0], F_SETFL, fcntl(fds2[0], F_GETFL,0) | O_NONBLOCK); - while (!(pidf = waitpid( pidf, &status, WNOHANG ))) { + while (!(waitpid( pidf, &status, WNOHANG ))) { bytes_read = read(fds2[0],buffer,MAX_USER_MESSAGE_LEN); if (bytes_read > 0) { if (!banner) @@ -445,6 +448,10 @@ static int fs_shrink( logical_volume_t * volume, /* get and validate current ext2/3 superblock */ sb = (struct ext2_super_block *) volume->private_data; rc = fsim_get_ext2_superblock( volume, sb ); + if (rc) { + goto errout; + } + requested_size = requested_size >> (1 + sb->s_log_block_size); if ((sb->s_lastcheck < sb->s_mtime) || (sb->s_state & EXT2_ERROR_FS) || @@ -515,7 +522,7 @@ static int fs_shrink( logical_volume_t * volume, fcntl(fds2[0], F_SETFL, fcntl(fds2[0], F_GETFL,0) | O_NONBLOCK); /* wait for child to complete */ - while (!(pidf = waitpid( pidf, &status, WNOHANG ))) { + while (!(waitpid( pidf, &status, WNOHANG ))) { bytes_read = read(fds2[0],buffer,MAX_USER_MESSAGE_LEN); if (bytes_read > 0) { if (!banner) @@ -567,18 +574,20 @@ static int fs_mkfs(logical_volume_t * volume, option_array_t * options ) LOGENTRY(); - /* don't format if mounted */ + /* don't format if mounted */ if (EVMS_IS_MOUNTED(volume)) { - return EBUSY; + rc = EBUSY; + goto errout; } - rc = fsim_mkfs(volume, options); + rc = fsim_mkfs(volume, options); - /* probe to set up private data */ - if ( !rc ) { - rc = fs_probe(volume); - } + /* probe to set up private data */ + if (!rc) { + rc = fs_probe(volume); + } +errout: LOGEXITRC(); return rc; } @@ -670,7 +679,8 @@ static int fs_init_task( task_context_t * context ) /* Parameter check */ if (!context) { - return EFAULT; + rc = EFAULT; + goto errout; } rc = EngFncs->get_volume_list(NULL, &global_volumes); @@ -877,10 +887,9 @@ static int fs_init_task( task_context_t * context ) break; } +errout: LOGEXITRC(); - return rc; - } @@ -901,7 +910,8 @@ static int fs_set_option( task_context_t * context, /* Parameter check */ if (!context || !value || !effect) { - return EFAULT; + rc = EFAULT; + goto errout; } *effect = 0; @@ -1004,6 +1014,7 @@ static int fs_set_option( task_context_t * context, break; } +errout: LOGEXITRC(); return rc; } @@ -1453,7 +1464,7 @@ plugin_record_t ext2_plugrec = { ENGINE_FSIM_API_PATCH_LEVEL} }, #endif short_name: "Ext2/3", - long_name: "Ext2 File System Interface Module", + long_name: "Ext2/3 File System Interface Module", oem_name: "IBM", functions: {fsim: &fsim_ops}, container_functions: NULL diff --git a/lib/evms/fsimext2.c b/lib/evms/fsimext2.c index 344364b..1cd6fae 100644 --- a/lib/evms/fsimext2.c +++ b/lib/evms/fsimext2.c @@ -60,15 +60,14 @@ int fsim_get_volume_limits( struct ext2_super_block * sb, sector_count_t * vol_max_size) { int rc = 0; - sector_count_t fs_size; int blk_to_sect; blk_to_sect = (1 + sb->s_log_block_size); - fs_size = sb->s_blocks_count << blk_to_sect; *fs_min_size = (sb->s_blocks_count - sb->s_free_blocks_count) << blk_to_sect; *fs_max_size = (sector_count_t) 1 << (32+blk_to_sect); *vol_max_size = 0xffffffffff; + LOGEXITRC(); return rc; } @@ -81,6 +80,8 @@ int fsim_unmkfs( logical_volume_t * volume ) int fd; int rc = 0; + LOGENTRY(); + fd = open(EVMS_GET_DEVNAME(volume), O_RDWR|O_EXCL, 0); if (fd < 0) return -1; @@ -96,6 +97,7 @@ int fsim_unmkfs( logical_volume_t * volume ) fd = close(fd); + LOGEXITRC(); return rc; } @@ -111,12 +113,14 @@ int fsim_mkfs(logical_volume_t * volume, option_array_t * options ) pid_t pidm; int status; + LOGENTRY(); + /* Fork and execute the correct program. */ switch (pidm = fork()) { /* error */ case -1: - return EIO; + rc = EIO; /* child */ case 0: @@ -135,7 +139,16 @@ int fsim_mkfs(logical_volume_t * volume, option_array_t * options ) /* parent */ default: /* wait for child to complete */ - pidm = waitpid( pidm, &status, 0 ); + while (1) { + rc = waitpid( pidm, &status, 0 ); + if (rc == -1) { + if (errno == EINTR) + continue; + rc = errno; + goto reterr; + } else + break; + } if ( WIFEXITED(status) ) { /* get mke2fs exit code */ rc = WEXITSTATUS(status); @@ -149,6 +162,8 @@ int fsim_mkfs(logical_volume_t * volume, option_array_t * options ) } } +reterr: + LOGEXITRC(); return rc; } @@ -172,6 +187,8 @@ void set_mkfs_options( option_array_t * options, int i, bufsize, opt_count = 2; char *buf; + LOGENTRY(); + argv[0] = "mke2fs"; /* 'quiet' option */ @@ -194,6 +211,7 @@ void set_mkfs_options( option_array_t * options, break; default: /* not one we expect, just skip it */ + break; } } @@ -287,6 +305,7 @@ void set_mkfs_options( option_array_t * options, "mke2fs command: %s\n", buf); free(buf); + LOGEXIT(); return; } @@ -305,6 +324,8 @@ int fsim_fsck(logical_volume_t * volume, option_array_t * options, int fds2[2]; int banner = 0; + LOGENTRY(); + /* open pipe, alloc buffer for collecting fsck.jfs output */ rc = pipe(fds2); if (rc) { @@ -319,7 +340,7 @@ int fsim_fsck(logical_volume_t * volume, option_array_t * options, /* error */ case -1: - return EIO; + rc = EIO; /* child */ case 0: @@ -339,7 +360,8 @@ int fsim_fsck(logical_volume_t * volume, option_array_t * options, close(fds2[1]); /* wait for child to complete */ - while (!(pidf = waitpid( pidf, &status, WNOHANG ))) { + fcntl(fds2[0], F_SETFL, fcntl(fds2[0], F_GETFL,0) | O_NONBLOCK); + while (!(waitpid( pidf, &status, WNOHANG ))) { /* read e2fsck output */ bytes_read = read(fds2[0],buffer,MAX_USER_MESSAGE_LEN); if (bytes_read > 0) { @@ -378,7 +400,10 @@ int fsim_fsck(logical_volume_t * volume, option_array_t * options, EngFncs->engine_free(buffer); } - return rc; + close(fds2[0]); + + LOGEXITRC(); + return rc; } @@ -399,6 +424,8 @@ void set_fsck_options( option_array_t * options, char ** argv, logical_volume_t int do_preen = 1; char *buf; + LOGENTRY(); + argv[0] = "e2fsck"; if (options) @@ -510,6 +537,7 @@ void set_fsck_options( option_array_t * options, char ** argv, logical_volume_t "fsck command: %s\n", buf); free(buf); + LOGEXIT(); return; } /* @@ -525,6 +553,7 @@ void set_fsck_options( option_array_t * options, char ** argv, logical_volume_t */ static void ext2fs_swap_super(struct ext2_super_block * sb) { + LOGENTRY(); sb->s_inodes_count = DISK_TO_CPU32(sb->s_inodes_count); sb->s_blocks_count = DISK_TO_CPU32(sb->s_blocks_count); sb->s_r_blocks_count = DISK_TO_CPU32(sb->s_r_blocks_count); @@ -560,8 +589,7 @@ static void ext2fs_swap_super(struct ext2_super_block * sb) sb->s_journal_inum = DISK_TO_CPU32(sb->s_journal_inum); sb->s_journal_dev = DISK_TO_CPU32(sb->s_journal_dev); sb->s_last_orphan = DISK_TO_CPU32(sb->s_last_orphan); - - return; + LOGEXIT(); } @@ -584,8 +612,14 @@ int fsim_get_ext2_superblock( logical_volume_t *volume, struct ext2_super_block int fd; int rc = 0; + LOGENTRY(); + fd = open(EVMS_GET_DEVNAME(volume), O_RDONLY, 0); - if (fd < 0) return EIO; + if (fd < 0) { + rc = EIO; + LOGEXITRC(); + return rc; + } /* get primary superblock */ rc = fsim_rw_diskblocks( fd, EXT2_SUPER_LOC, SIZE_OF_SUPER, sb_ptr, GET ); @@ -600,6 +634,7 @@ int fsim_get_ext2_superblock( logical_volume_t *volume, struct ext2_super_block close(fd); + LOGEXITRC(); return rc; } @@ -633,7 +668,10 @@ int fsim_rw_diskblocks( int dev_ptr, { off_t Actual_Location; size_t Bytes_Transferred; + int rc = 0; + LOGENTRY(); + Actual_Location = lseek(dev_ptr,disk_offset, SEEK_SET); if ( ( Actual_Location < 0 ) || ( Actual_Location != disk_offset ) ) return ERROR; @@ -646,14 +684,19 @@ int fsim_rw_diskblocks( int dev_ptr, Bytes_Transferred = write(dev_ptr,data_buffer,disk_count); break; default: - return EINVAL; + rc = EINVAL; + LOGEXITRC(); + return rc; break; } if ( Bytes_Transferred != disk_count ) { - return EIO; + rc = EIO; + LOGEXITRC(); + return rc; } + LOGEXIT(); return FSIM_SUCCESS; } diff --git a/lib/evms/fsimext2.h b/lib/evms/fsimext2.h index 178fc19..91ff81c 100644 --- a/lib/evms/fsimext2.h +++ b/lib/evms/fsimext2.h @@ -54,7 +54,7 @@ engine_functions_t *EngFncs; #define MESSAGE(msg, args...) EngFncs->user_message(pMyPluginRecord, NULL, NULL, msg, ##args) #define LOGENTRY() EngFncs->write_log_entry(ENTRY_EXIT, pMyPluginRecord, "%s: Enter.\n", __FUNCTION__ ) #define LOGEXIT() EngFncs->write_log_entry(ENTRY_EXIT, pMyPluginRecord, "%s: Exit.\n", __FUNCTION__ ) -#define LOGEXITRC() EngFncs->write_log_entry(ENTRY_EXIT, pMyPluginRecord, "%s: Exit. RC= %d.\n", __FUNCTION__ , rc) +#define LOGEXITRC() EngFncs->write_log_entry(ENTRY_EXIT, pMyPluginRecord, "%s: Exit. rc = %d.\n", __FUNCTION__ , rc) #define LOG_CRITICAL(msg, args...) EngFncs->write_log_entry(CRITICAL, pMyPluginRecord, "%s: " msg, __FUNCTION__ , ## args) #define LOG_SERIOUS(msg, args...) EngFncs->write_log_entry(SERIOUS, pMyPluginRecord, "%s: " msg, __FUNCTION__ , ## args) #define LOG_ERROR(msg, args...) EngFncs->write_log_entry(ERROR, pMyPluginRecord, "%s: " msg, __FUNCTION__ , ## args) -- 1.8.3.1