This might be a bit more controversial than my last linux-swap patch. :-)
I just noticed due to a comment from Otavio Salvador that linux-swap had its name changed to "linux-swap(new)" a while back. I know this was ages ago (January 2007!), but I'd like to suggest that this was poorly designed, for the following reasons: Firstly, "old" and "new" is never a good way to name anything. When a third format comes along, you either have to rename things or you have to cope with "new" not being current any more. I used to live in a place called "New Court" which was built in the early 19th century. :-) There are perfectly good version numbers for the swap formats, 0 and 1 - why not use them? Secondly, according to mkswap(8) swap v0 has not been supported since Linux 2.5.22, and swap v1 has been supported since 2.1.117. In other words, just about the entire Linux planet is using swap v1 now. It seems wrong to make them all type "(new)" as a sort of code for "the one that actually works". Thirdly, even if you want to call the current swap format something other than "linux-swap", it would be most convenient if "linux-swap" were supported for input. I suppose this means that people using libparted still have to cope with the new name on output (?), but even so it would be useful. Fourthly, linux_swap.c calls the formats "swap_v1" and "swap_v2" in its function names, but mkswap uses the options -v0 and -v1 and calls the versions thus. This seems eccentric. I think this should be fixed before a 1.9.0 release, before people start depending on the new behaviour too much. How about the attached patch? Supporting alternative names for file systems is a little bit tricky, because you don't want to probe the file system once for each name (not only is it inefficient, but it confuses _best_match something rotten). I added a new PedFileSystemAlias structure so that we can register alternative names outside of the main list of file system types. Thanks, -- Colin Watson [[email protected]]
>From 791126aa0e2e7bda55ff3610eb02dce326079bfb Mon Sep 17 00:00:00 2001 From: Colin Watson <[email protected]> Date: Thu, 18 Jun 2009 13:50:47 +0100 Subject: [PATCH] Rationalise linux-swap fs names, and add a "linux-swap" alias * libparted/filesys.c (ped_file_system_alias_register, ped_file_system_alias_unregister, ped_file_system_alias_get_next): New functions. (ped_file_system_type_get): Walk aliases as well. * include/parted/filesys.h (struct _PedFileSystemAlias): New structure. (ped_file_system_alias_register, ped_file_system_alias_unregister, ped_file_system_alias_get_next): Add prototypes. * parted/parted.c (_init_messages): Walk file system aliases as well as types. * parted/ui.c (init_fs_type_str): Likewise. * libparted/fs/linux_swap/linux_swap.c (_swap_v1_type, _swap_v1_open, _swap_v1_probe, _swap_v1_clobber, _swap_v1_ops): Rename to _swap_v0_type etc. to match version number used in mkswap. Update all users. (_swap_v2_type, _swap_v2_open, _swap_v2_probe, _swap_v2_clobber, _swap_v2_ops): Rename to _swap_v1_type etc. to match version number used in mkswap. Update all users. (_swap_v0_type): Rename type from "linux-swap(old)" to "linux-swap(v0)". (_swap_v1_type): Rename type from "linux-swap(new)" to "linux-swap(v1)". (ped_file_system_linux_swap_init, ped_file_system_linux_swap_done): Register/unregister a "linux-swap" alias for "linux-swap(v1)". * libparted/labels/misc.h (is_linux_swap): Update comment. * tests/t2100-mkswap.sh: Refer to "linux-swap(v1)" rather than "linux-swap(new)". Test creation via the new alias. --- include/parted/filesys.h | 20 ++++++++++ libparted/filesys.c | 67 +++++++++++++++++++++++++++++++++- libparted/fs/linux_swap/linux_swap.c | 64 +++++++++++++++++--------------- libparted/labels/misc.h | 2 +- parted/parted.c | 37 +++++++++++++++++-- parted/ui.c | 9 +++++ tests/t2100-mkswap.sh | 11 ++++-- 7 files changed, 172 insertions(+), 38 deletions(-) diff --git a/include/parted/filesys.h b/include/parted/filesys.h index 644b13a..22061a3 100644 --- a/include/parted/filesys.h +++ b/include/parted/filesys.h @@ -28,6 +28,7 @@ typedef struct _PedFileSystem PedFileSystem; typedef struct _PedFileSystemType PedFileSystemType; +typedef struct _PedFileSystemAlias PedFileSystemAlias; typedef const struct _PedFileSystemOps PedFileSystemOps; #include <parted/geom.h> @@ -62,6 +63,17 @@ struct _PedFileSystemType { PedFileSystemOps* const ops; }; +/** + * Structure describing a file system alias. This is separate from + * PedFileSystemType because probing only looks through the list of types, + * and does not probe aliases separately. + */ +struct _PedFileSystemAlias { + PedFileSystemAlias* next; + PedFileSystemType* fs_type; + const char* alias; +}; + /** * Structure describing file system @@ -79,10 +91,18 @@ struct _PedFileSystem { extern void ped_file_system_type_register (PedFileSystemType* type); extern void ped_file_system_type_unregister (PedFileSystemType* type); +extern void ped_file_system_alias_register (PedFileSystemType* type, + const char* alias); +extern void ped_file_system_alias_unregister (PedFileSystemType* type, + const char* alias); + extern PedFileSystemType* ped_file_system_type_get (const char* name); extern PedFileSystemType* ped_file_system_type_get_next (const PedFileSystemType* fs_type); +extern PedFileSystemAlias* +ped_file_system_alias_get_next (const PedFileSystemAlias* fs_alias); + extern PedFileSystemType* ped_file_system_probe (PedGeometry* geom); extern PedGeometry* ped_file_system_probe_specific ( const PedFileSystemType* fs_type, diff --git a/libparted/filesys.c b/libparted/filesys.c index ba48a79..d8505f1 100644 --- a/libparted/filesys.c +++ b/libparted/filesys.c @@ -41,6 +41,7 @@ #define BUFFER_SIZE 4096 /* in sectors */ static PedFileSystemType* fs_types = NULL; +static PedFileSystemAlias* fs_aliases = NULL; void ped_file_system_type_register (PedFileSystemType* fs_type) @@ -72,6 +73,47 @@ ped_file_system_type_unregister (PedFileSystemType* fs_type) fs_types = fs_type->next; } +void +ped_file_system_alias_register (PedFileSystemType* fs_type, const char* alias) +{ + PedFileSystemAlias* fs_alias; + + PED_ASSERT (fs_type != NULL, return); + PED_ASSERT (alias != NULL, return); + + fs_alias = ped_malloc (sizeof (PedFileSystemAlias)); + if (!fs_alias) + return; + + fs_alias->next = fs_aliases; + fs_alias->fs_type = fs_type; + fs_alias->alias = alias; + fs_aliases = fs_alias; +} + +void +ped_file_system_alias_unregister (PedFileSystemType* fs_type, + const char* alias) +{ + PedFileSystemAlias* walk; + PedFileSystemAlias* last = NULL; + + PED_ASSERT (fs_aliases != NULL, return); + PED_ASSERT (fs_type != NULL, return); + PED_ASSERT (alias != NULL, return); + + for (walk = fs_aliases; walk; last = walk, walk = walk->next) { + if (walk->fs_type == fs_type && !strcmp (walk->alias, alias)) + break; + } + + PED_ASSERT (walk != NULL, return); + if (last) + last->next = walk->next; + else + fs_aliases = walk->next; +} + /** * Get a PedFileSystemType by its @p name. * @@ -81,6 +123,7 @@ PedFileSystemType* ped_file_system_type_get (const char* name) { PedFileSystemType* walk; + PedFileSystemAlias* alias_walk; PED_ASSERT (name != NULL, return NULL); @@ -88,7 +131,15 @@ ped_file_system_type_get (const char* name) if (!strcasecmp (walk->name, name)) break; } - return walk; + if (walk != NULL) + return walk; + + for (alias_walk = fs_aliases; alias_walk != NULL; + alias_walk = alias_walk->next) { + if (!strcasecmp (alias_walk->alias, name)) + break; + } + return alias_walk->fs_type; } /** @@ -106,6 +157,20 @@ ped_file_system_type_get_next (const PedFileSystemType* fs_type) } /** + * Get the next PedFileSystemAlias after @p fs_alias. + * + * @return @c NULL if @p fs_alias is the last item in the list. + */ +PedFileSystemAlias* +ped_file_system_alias_get_next (const PedFileSystemAlias* fs_alias) +{ + if (fs_alias) + return fs_alias->next; + else + return fs_aliases; +} + +/** * Attempt to find a file system and return the region it occupies. * * @param fs_type The file system type to probe for. diff --git a/libparted/fs/linux_swap/linux_swap.c b/libparted/fs/linux_swap/linux_swap.c index 62bb121..fd589b7 100644 --- a/libparted/fs/linux_swap/linux_swap.c +++ b/libparted/fs/linux_swap/linux_swap.c @@ -69,12 +69,12 @@ typedef struct { unsigned int max_bad_pages; } SwapSpecific; +static PedFileSystemType _swap_v0_type; static PedFileSystemType _swap_v1_type; -static PedFileSystemType _swap_v2_type; static PedFileSystemType _swap_swsusp_type; +static PedFileSystem* _swap_v0_open (PedGeometry* geom); static PedFileSystem* _swap_v1_open (PedGeometry* geom); -static PedFileSystem* _swap_v2_open (PedGeometry* geom); static PedFileSystem* _swap_swsusp_open (PedGeometry* geom); static int swap_close (PedFileSystem* fs); @@ -89,11 +89,11 @@ _generic_swap_probe (PedGeometry* geom, int kind) switch (kind) { /* Check for old style swap partitions. */ case 0: - fs = _swap_v1_open(geom); + fs = _swap_v0_open(geom); break; /* Check for new style swap partitions. */ case 1: - fs = _swap_v2_open(geom); + fs = _swap_v1_open(geom); break; /* Check for swap partitions containing swsusp data. */ case -1: @@ -135,11 +135,11 @@ _generic_swap_clobber (PedGeometry* geom, int kind) switch (kind) { /* Check for old style swap partitions. */ case 0: - fs = _swap_v1_open(geom); + fs = _swap_v0_open(geom); break; /* Check for new style swap partitions. */ case 1: - fs = _swap_v2_open(geom); + fs = _swap_v1_open(geom); break; /* Check for swap partitions containing swsusp data. */ case -1: @@ -222,7 +222,7 @@ swap_alloc (PedGeometry* geom) fs->geom = ped_geometry_duplicate (geom); if (!fs->geom) goto error_free_buffer; - fs->type = &_swap_v2_type; + fs->type = &_swap_v1_type; return fs; error_free_buffer: @@ -251,7 +251,7 @@ swap_free (PedFileSystem* fs) } static PedFileSystem* -_swap_v1_open (PedGeometry* geom) +_swap_v0_open (PedGeometry* geom) { PedFileSystem* fs; SwapSpecific* fs_info; @@ -293,7 +293,7 @@ error: } static PedFileSystem* -_swap_v2_open (PedGeometry* geom) +_swap_v1_open (PedGeometry* geom) { PedFileSystem* fs; SwapSpecific* fs_info; @@ -565,7 +565,7 @@ error: static PedFileSystem* swap_copy (const PedFileSystem* fs, PedGeometry* geom, PedTimer* timer) { - return ped_file_system_create (geom, &_swap_v2_type, timer); + return ped_file_system_create (geom, &_swap_v1_type, timer); } static int @@ -602,12 +602,12 @@ swap_get_copy_constraint (const PedFileSystem* fs, const PedDevice* dev) #endif /* !DISCOVER_ONLY */ static PedGeometry* -_swap_v1_probe (PedGeometry* geom) { +_swap_v0_probe (PedGeometry* geom) { return _generic_swap_probe (geom, 0); } static PedGeometry* -_swap_v2_probe (PedGeometry* geom) { +_swap_v1_probe (PedGeometry* geom) { return _generic_swap_probe (geom, 1); } @@ -617,12 +617,12 @@ _swap_swsusp_probe (PedGeometry* geom) { } static int -_swap_v1_clobber (PedGeometry* geom) { +_swap_v0_clobber (PedGeometry* geom) { return _generic_swap_clobber (geom, 0); } static int -_swap_v2_clobber (PedGeometry* geom) { +_swap_v1_clobber (PedGeometry* geom) { return _generic_swap_clobber (geom, 1); } @@ -631,11 +631,11 @@ _swap_swsusp_clobber (PedGeometry* geom) { return _generic_swap_clobber (geom, -1); } -static PedFileSystemOps _swap_v1_ops = { - probe: _swap_v1_probe, +static PedFileSystemOps _swap_v0_ops = { + probe: _swap_v0_probe, #ifndef DISCOVER_ONLY - clobber: _swap_v1_clobber, - open: _swap_v1_open, + clobber: _swap_v0_clobber, + open: _swap_v0_open, create: swap_create, close: swap_close, check: swap_check, @@ -658,11 +658,11 @@ static PedFileSystemOps _swap_v1_ops = { #endif /* !DISCOVER_ONLY */ }; -static PedFileSystemOps _swap_v2_ops = { - probe: _swap_v2_probe, +static PedFileSystemOps _swap_v1_ops = { + probe: _swap_v1_probe, #ifndef DISCOVER_ONLY - clobber: _swap_v2_clobber, - open: _swap_v2_open, + clobber: _swap_v1_clobber, + open: _swap_v1_open, create: swap_create, close: swap_close, check: swap_check, @@ -712,17 +712,17 @@ static PedFileSystemOps _swap_swsusp_ops = { #endif /* !DISCOVER_ONLY */ }; -static PedFileSystemType _swap_v1_type = { +static PedFileSystemType _swap_v0_type = { next: NULL, - ops: &_swap_v1_ops, - name: "linux-swap(old)", + ops: &_swap_v0_ops, + name: "linux-swap(v0)", block_sizes: LINUXSWAP_BLOCK_SIZES }; -static PedFileSystemType _swap_v2_type = { +static PedFileSystemType _swap_v1_type = { next: NULL, - ops: &_swap_v2_ops, - name: "linux-swap(new)", + ops: &_swap_v1_ops, + name: "linux-swap(v1)", block_sizes: LINUXSWAP_BLOCK_SIZES }; @@ -736,15 +736,19 @@ static PedFileSystemType _swap_swsusp_type = { void ped_file_system_linux_swap_init () { + ped_file_system_type_register (&_swap_v0_type); ped_file_system_type_register (&_swap_v1_type); - ped_file_system_type_register (&_swap_v2_type); ped_file_system_type_register (&_swap_swsusp_type); + + ped_file_system_alias_register (&_swap_v1_type, "linux-swap"); } void ped_file_system_linux_swap_done () { + ped_file_system_alias_unregister (&_swap_v1_type, "linux-swap"); + + ped_file_system_type_unregister (&_swap_v0_type); ped_file_system_type_unregister (&_swap_v1_type); - ped_file_system_type_unregister (&_swap_v2_type); ped_file_system_type_unregister (&_swap_swsusp_type); } diff --git a/libparted/labels/misc.h b/libparted/labels/misc.h index a086e88..e69d518 100644 --- a/libparted/labels/misc.h +++ b/libparted/labels/misc.h @@ -18,7 +18,7 @@ /* Return nonzero if FS_TYPE_NAME starts with "linux-swap". This must match the NUL-terminated "linux-swap" as well - as "linux-swap(old)" and "linux-swap(new)". */ + as "linux-swap(v0)" and "linux-swap(v1)". */ static inline int is_linux_swap (char const *fs_type_name) { diff --git a/parted/parted.c b/parted/parted.c index e6364bf..17b2b6c 100644 --- a/parted/parted.c +++ b/parted/parted.c @@ -1983,6 +1983,7 @@ _init_messages () StrList* list; int first; PedFileSystemType* fs_type; + PedFileSystemAlias* fs_alias; PedDiskType* disk_type; PedPartitionFlag part_flag; PedUnit unit; @@ -2039,7 +2040,7 @@ _init_messages () label_type_msg = str_list_convert (list); str_list_destroy (list); -/* mkfs - file system types */ +/* mkfs - file system types and aliases */ list = str_list_create (_(fs_type_msg_start), NULL); first = 1; @@ -2054,12 +2055,23 @@ _init_messages () str_list_append (list, ", "); str_list_append (list, fs_type->name); } + for (fs_alias = ped_file_system_alias_get_next (NULL); + fs_alias; fs_alias = ped_file_system_alias_get_next (fs_alias)) { + if (fs_alias->fs_type->ops->create == NULL) + continue; + + if (first) + first = 0; + else + str_list_append (list, ", "); + str_list_append (list, fs_alias->alias); + } str_list_append (list, "\n"); mkfs_fs_type_msg = str_list_convert (list); str_list_destroy (list); -/* mkpart - file system types */ +/* mkpart - file system types and aliases */ list = str_list_create (_(fs_type_msg_start), NULL); first = 1; @@ -2071,12 +2083,20 @@ _init_messages () str_list_append (list, ", "); str_list_append (list, fs_type->name); } + for (fs_alias = ped_file_system_alias_get_next (NULL); + fs_alias; fs_alias = ped_file_system_alias_get_next (fs_alias)) { + if (first) + first = 0; + else + str_list_append (list, ", "); + str_list_append (list, fs_alias->alias); + } str_list_append (list, "\n"); mkpart_fs_type_msg = str_list_convert (list); str_list_destroy (list); -/* resize - file system types */ +/* resize - file system types and aliases */ list = str_list_create (_(resize_msg_start), NULL); first = 1; @@ -2091,6 +2111,17 @@ _init_messages () str_list_append (list, ", "); str_list_append (list, fs_type->name); } + for (fs_alias = ped_file_system_alias_get_next (NULL); + fs_alias; fs_alias = ped_file_system_alias_get_next (fs_alias)) { + if (fs_alias->fs_type->ops->resize == NULL) + continue; + + if (first) + first = 0; + else + str_list_append (list, ", "); + str_list_append (list, fs_alias->alias); + } str_list_append (list, "\n"); resize_fs_type_msg = str_list_convert (list); diff --git a/parted/ui.c b/parted/ui.c index 7b2a248..f50536c 100644 --- a/parted/ui.c +++ b/parted/ui.c @@ -1347,6 +1347,7 @@ static int init_fs_type_str () { PedFileSystemType* walk; + PedFileSystemAlias* alias_walk; fs_type_list = NULL; @@ -1357,6 +1358,14 @@ init_fs_type_str () if (!fs_type_list) return 0; } + for (alias_walk = ped_file_system_alias_get_next (NULL); alias_walk; + alias_walk = ped_file_system_alias_get_next (alias_walk)) + { + fs_type_list = str_list_insert (fs_type_list, + alias_walk->alias); + if (!fs_type_list) + return 0; + } return 1; } diff --git a/tests/t2100-mkswap.sh b/tests/t2100-mkswap.sh index cde1639..af8a15b 100755 --- a/tests/t2100-mkswap.sh +++ b/tests/t2100-mkswap.sh @@ -21,7 +21,7 @@ test_description='create linux-swap partitions' . $srcdir/test-lib.sh ###################################################################### -# When creating a partition of type linux-swap(new) in a DOS partition +# When creating a partition of type linux-swap(v1) in a DOS partition # table, ensure that the proper file system type (0x82) is used. # Some releases, e.g. parted-1.8.8 would mistakenly use 0x83. ###################################################################### @@ -44,7 +44,7 @@ test_expect_success 'expect no output' 'compare out /dev/null' test_expect_success \ 'create a linux-swap file system' \ - 'parted -s $dev mkfs 1 "linux-swap(new)" > out 2>&1' + 'parted -s $dev mkfs 1 "linux-swap(v1)" > out 2>&1' test_expect_success 'expect no output' 'compare out /dev/null' # Extract the byte at offset 451. It must be 0x82, not 0x83. @@ -69,7 +69,7 @@ test_expect_success 'expect no output' 'compare out /dev/null' test_expect_success \ 'create another linux-swap file system' \ - 'parted -s $dev2 mkfs 1 "linux-swap(new)" > out 2>&1' + 'parted -s $dev2 mkfs 1 "linux-swap(v1)" > out 2>&1' test_expect_success 'expect no output' 'compare out /dev/null' # partition starts at offset 16384; swap UUID is 1036 bytes in @@ -95,4 +95,9 @@ test_expect_success \ 'check preserves linux-swap UUID' \ 'compare uuid2 uuid2-new' +test_expect_success \ + 'create a linux-swap file system via alias' \ + 'parted -s $dev mkfs 1 linux-swap > out 2>&1' +test_expect_success 'expect no output' 'compare out /dev/null' + test_done -- 1.6.3.1
_______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
