Bug#397973: parted: Fix mac partition table corruption
On Sat, Mar 03, 2007 at 10:05:39PM -0800, Steve Langasek wrote: A further concern on this patch: On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote: diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c 2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) + mac_partition_set_system (part, part-fs_type); + } return 1; So in this case, if (!state), mac_data-is_lvm is never un-set. Is that not an issue? The mac_data-is_lvm = state; could probably be kept. I'm not sure why that isn't done in the current upstream version (it does the state change for the other flags). It needs to be moved to the last line though (just before return 1). It's also interesting to note that some flags (e.g. PED_PARTITION_SWAP) clears other flags when set (PED_PARTITION_ROOT in that case) but there are still many nonsensical combinations that *are* allowed (PED_PARTITION_SWAP PED_PARTITION_BOOT...eeh?) Also, the mac_partition_set_system will change the system_name which in effect means that the other flags are no longer valid (did I mention that the mac partitioning code was confusing yet?). In summary, I think the mac code needs some more work, and that the current patch + the mac_data-is_something flag clearing is the best we can do ATM. -- David Härdeman
Bug#397973: parted: Fix mac partition table corruption
On Sat, Mar 03, 2007 at 10:01:00PM -0800, Steve Langasek wrote: On Sat, Mar 03, 2007 at 03:25:20AM +0100, David Härdeman wrote: So clearing the flag will only clear it in the internal mac_data structure, it won't cause the system name of the partition to be reset? Or is this handled by mac_partition_set_system? Yes, the system name will not be reset. clearing the flag implies nothing else than that it doesn't apply, it doesn't say what type the partition is after the flag is removed. Which means, AIUI, that clearing the flag is not sufficient to clear the flag on disk, so if a user clears the flag, saves changes, closes parted (or similar), and restarts parted, the flag will show up again, correct? That seems suboptimal to me. Sorry, I was confused...the code with the patch applied is: case PED_PARTITION_LVM: if (state) { strcpy (mac_data-system_name, Linux_LVM); mac_data-is_lvm = state; } else { if (mac_data-is_lvm) mac_partition_set_system (part, part-fs_type); } return 1; So, if you clear the LVM flag, state is 0, and mac_data-is_lvm is 1, therefore mac_partition_set_system() is called which sets the system_name to Apple_UNIX_SVR2, so the flag is really cleared on disk (and other flags are cleared on-disk as well since the system_name is changed...this is not reflected in the state of mac_data-is_*whatever*. If you clear a non-set LVM flag, state is 0 and mac_data-is_lvm is 0, so mac_partition_set_system is not called. -- David Härdeman
Bug#397973: parted: Fix mac partition table corruption
On Sat, Mar 03, 2007 at 03:25:20AM +0100, David Härdeman wrote: So clearing the flag will only clear it in the internal mac_data structure, it won't cause the system name of the partition to be reset? Or is this handled by mac_partition_set_system? Yes, the system name will not be reset. clearing the flag implies nothing else than that it doesn't apply, it doesn't say what type the partition is after the flag is removed. Which means, AIUI, that clearing the flag is not sufficient to clear the flag on disk, so if a user clears the flag, saves changes, closes parted (or similar), and restarts parted, the flag will show up again, correct? That seems suboptimal to me. If we would set a default, then a partition of type foobar (without the lvm flag set) would get its system type changed if you executed set partnr lvm off in parted. Yes, I understand that's a deficiency of how raid flag support is implemented on mac partition tables. Also, I believe this is the same approach that has been taken in the later versions of the upstream package (see the source package in experimental). Ah well, it's a minor point anyway, compared with the bits that are wholly breaking partman, so I won't insist on it (at least, not here and now). Maintainers, please advise whether you're planning a maintainer upload of parted to fix this for etch, otherwise I'll probably NMU this weekend. Thanks, -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. [EMAIL PROTECTED] http://www.debian.org/
Bug#397973: parted: Fix mac partition table corruption
A further concern on this patch: On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote: diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) + mac_partition_set_system (part, part-fs_type); + } return 1; So in this case, if (!state), mac_data-is_lvm is never un-set. Is that not an issue? -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. [EMAIL PROTECTED] http://www.debian.org/
Bug#413184: [EMAIL PROTECTED]: Re: Bug#397973: parted: Fix mac partition table corruption]
Right, forwarding to the right bug, since the main bug moved out from under me. - Forwarded message from Steve Langasek [EMAIL PROTECTED] - From: Steve Langasek [EMAIL PROTECTED] To: David Härdeman [EMAIL PROTECTED] Cc: [EMAIL PROTECTED], [EMAIL PROTECTED] Subject: Re: Bug#397973: parted: Fix mac partition table corruption Date: Sat, 3 Mar 2007 22:00:59 -0800 On Sat, Mar 03, 2007 at 03:25:20AM +0100, David Härdeman wrote: So clearing the flag will only clear it in the internal mac_data structure, it won't cause the system name of the partition to be reset? Or is this handled by mac_partition_set_system? Yes, the system name will not be reset. clearing the flag implies nothing else than that it doesn't apply, it doesn't say what type the partition is after the flag is removed. Which means, AIUI, that clearing the flag is not sufficient to clear the flag on disk, so if a user clears the flag, saves changes, closes parted (or similar), and restarts parted, the flag will show up again, correct? That seems suboptimal to me. If we would set a default, then a partition of type foobar (without the lvm flag set) would get its system type changed if you executed set partnr lvm off in parted. Yes, I understand that's a deficiency of how raid flag support is implemented on mac partition tables. Also, I believe this is the same approach that has been taken in the later versions of the upstream package (see the source package in experimental). Ah well, it's a minor point anyway, compared with the bits that are wholly breaking partman, so I won't insist on it (at least, not here and now). Maintainers, please advise whether you're planning a maintainer upload of parted to fix this for etch, otherwise I'll probably NMU this weekend. Thanks, -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. [EMAIL PROTECTED] http://www.debian.org/ - End forwarded message - - Forwarded message from Steve Langasek [EMAIL PROTECTED] - From: Steve Langasek [EMAIL PROTECTED] To: David Härdeman [EMAIL PROTECTED] Cc: [EMAIL PROTECTED], [EMAIL PROTECTED] Subject: Re: parted: Fix mac partition table corruption Date: Sat, 3 Mar 2007 22:05:38 -0800 A further concern on this patch: On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote: diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) + mac_partition_set_system (part, part-fs_type); + } return 1; So in this case, if (!state), mac_data-is_lvm is never un-set. Is that not an issue? -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. [EMAIL PROTECTED] http://www.debian.org/ - End forwarded message -
Bug#397973: parted: Fix mac partition table corruption
The attached patch changes libparted so that it doesn't corrupt the mac parition table when flags that alter the system_name entry are set/unset. This fixes the error for me (using a mac partition table on a i386 machine). Some minor fixes remain in partman-md and partman-lvm (lvm, swap and raid flags are mutually exclusive) -- David Härdeman diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c 2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) +mac_partition_set_system (part, part-fs_type); + } return 1; case PED_PARTITION_RAID: - mac_data-is_raid = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_RAID); - else + mac_data-is_raid = state; + } else { + if (mac_data-is_raid) mac_partition_set_system (part, part-fs_type); + } return 1; default:
Bug#397973: parted: Fix mac partition table corruption
On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote: diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) + mac_partition_set_system (part, part-fs_type); + } return 1; So clearing the flag will only clear it in the internal mac_data structure, it won't cause the system name of the partition to be reset? Or is this handled by mac_partition_set_system? Thanks, -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. [EMAIL PROTECTED] http://www.debian.org/
Bug#397973: parted: Fix mac partition table corruption
On Fri, Mar 02, 2007 at 06:14:19PM -0800, Steve Langasek wrote: On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote: diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c 2006-05-25 19:28:55.0 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.0 +0100 @@ -1260,19 +1260,23 @@ return 1; case PED_PARTITION_LVM: - mac_data-is_lvm = state; - if (state) + if (state) { strcpy (mac_data-system_name, Linux_LVM); - else - mac_partition_set_system (part, part-fs_type); + mac_data-is_lvm = state; + } else { + if (mac_data-is_lvm) + mac_partition_set_system (part, part-fs_type); + } return 1; So clearing the flag will only clear it in the internal mac_data structure, it won't cause the system name of the partition to be reset? Or is this handled by mac_partition_set_system? Yes, the system name will not be reset. clearing the flag implies nothing else than that it doesn't apply, it doesn't say what type the partition is after the flag is removed. If we would set a default, then a partition of type foobar (without the lvm flag set) would get its system type changed if you executed set partnr lvm off in parted. Also, I believe this is the same approach that has been taken in the later versions of the upstream package (see the source package in experimental). -- David Härdeman