[parted-devel] [PATCH] Always allow at least two sectors for extended boot record

[Jim Meyering]

Phillip Susi wrote:

> On 4/21/2010 4:50 PM, Jim Meyering wrote:
>> IMHO, parted should err on the side of safety.
>> Imagine a script getting the warning.  It should not
>> have to parse stderr to determine that it must not proceed.
>>
>>> Maybe something other than PED_EXCEPTION_IGNORE
>>> should be used?  Perhaps ignore or fail?
>
> I think you will satisfy both by using PED_EXCEPTION_IGNORE_CANCEL (
> there doesn't seem to be an ignore or fail ).  That way the message is
> printed and you have to either choose to ignore and parted will exit
> with a status of 0, or you choose to cancel and it exits with a failure
> code.
>
>> BTW, wouldn't this diagnostic do better to say how we've failed
>> to notify the kernel of changes that we've written to disk?
>>
>> +        if (parts[0]) {
>> +                parts[strlen (parts) - 2] = 0;
>> +                ped_exception_throw (
>> +                        PED_EXCEPTION_WARNING,
>> +                        PED_EXCEPTION_IGNORE,
>> +                        _("Partition(s) %s on %s could not be modified, probably "
>> +                          "because it/they is/are in use.  As a result, the old partition(s) "
>> +                          "will remain in use until after reboot. You should reboot "
>> +                          "now before making further changes."),
>> +                        parts, disk->dev->path);
>> +        }
>
> I'm confused now.  I thought "say how we've failed to notify the kernel
> of changes we've written to disk" is exactly what it does.

The message quoted above doesn't mention "kernel" at all.
That is why I find it misleading.
I prefer this wording.  This patch also makes an interactive
"ignore" let parted exit successfully.

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index f32bd21..988fde7 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2531,15 +2531,16 @@ _disk_sync_part_table (PedDisk* disk)
 		ret = 1;
 	else {
                 bad_part_list[strlen (bad_part_list) - 2] = 0;
-                ped_exception_throw (
+                if (ped_exception_throw (
                         PED_EXCEPTION_WARNING,
-                        PED_EXCEPTION_IGNORE,
-                        _("Partition(s) %s on %s could not be modified, probably "
-                          "because it/they are in use.  As a result, "
-                          "the old partition(s) will remain in use until after "
-                          "reboot. You should reboot "
-                          "now before making further changes."),
-                        bad_part_list, disk->dev->path);
+                        PED_EXCEPTION_IGNORE_CANCEL,
+                        _("Partition(s) %s on %s have been written, but we have "
+			  "been unable to inform the kernel of the change, "
+			  "probably because it/they are in use.  As a result, "
+                          "the old partition(s) will remain in use.  You "
+                          "should reboot now before making further changes."),
+                        bad_part_list, disk->dev->path) == PED_EXCEPTION_IGNORE)
+                        ret = 1;
 		free (bad_part_list);
         }
  free_errnums: