Bug#397973: parted: Fix mac partition table corruption

2007-03-04 Thread David Härdeman

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

2007-03-04 Thread David Härdeman

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

2007-03-03 Thread Steve Langasek
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

2007-03-03 Thread Steve Langasek
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]

2007-03-03 Thread Steve Langasek
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

2007-03-02 Thread David Härdeman
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

2007-03-02 Thread Steve Langasek
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

2007-03-02 Thread David Härdeman

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