Jim Meyering <[email protected]> wrote:
...
> In the process, I discovered a GPT-related bug that
> would provoke a failed assertion.  So I fixed that.  Patch below.
> Would you like to write the test case for *it*?
> If so, please name it t0201-gpt.sh, since it'll be
> very similar to the one below (which has a FIXME
> comment I plan to address before pushing).
>
> My plan for tomorrow:
>
>   - apply your patch, and test a little
>   - address FIXME comment in the test script below
>   - ensure that the test passes
>   - push your patch and the test-adding one

Petr,

Here are the results of the above.

Changes I've made:
  - adjusted your commit log comments (it's not specific to
      interactive use)
  - added a "default: break;" case, to make that choice explicit,
      and to avoid compile-time warnings
  - addressed the FIXME comment in my test, made quoting more
      consistent, and tweaked a string or two

I'll wait an hour or two, in case you have comments.

> If you write the test to exercise my patch,
> (or tell me you'd rather not, in which case I'll do it),
> I'll push those two change sets, too.


>From 74ade0e93ee7884d9da12fafb9454456bbfa736f Mon Sep 17 00:00:00 2001
From: Petr Uzel <[email protected]>
Date: Fri, 13 Feb 2009 13:27:55 +0100
Subject: [PATCH 1/2] gpt: do not automatically "correct" a suspicious GPT 
partition table

Previously, when parted was invoked on a disk with a GPT partition table
and the backup GPT was not in the last sector of the disk, and even if
the requested operation was just to print the partition table, parted
would "repair" this automatically. This behavior is undesirable in the
following situation:

dm-raid on top of block device. The dm-raid is partitioned with GPT. If
the dm-raid starts on the first block of underlying device (AFAIK this is
the case with FastTrack controllers) and the user runs parted on the
dm-raid, it will identify the physical device as being partitioned with
GPT and see the backup GPT table not to be in the last sector of the
physical device and thus move it to this location (which may lead to
destruction of dm-raid metadata in case they are located at the end of
physical device).

This patch modifies parted's behavior to ignore fixing of backup GPT
position by default.
---
 libparted/labels/gpt.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index 13d2e88..eea562d 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -831,21 +831,26 @@ gpt_read (PedDisk * disk)
                        char* zeros = ped_malloc (pth_get_size (disk->dev));

 #ifndef DISCOVER_ONLY
-                       if (ped_exception_throw (
+                       switch (ped_exception_throw (
                                PED_EXCEPTION_ERROR,
-                               PED_EXCEPTION_FIX | PED_EXCEPTION_CANCEL,
+                               PED_EXCEPTION_FIX | PED_EXCEPTION_CANCEL | 
PED_EXCEPTION_IGNORE,
                _("The backup GPT table is not at the end of the disk, as it "
                  "should be.  This might mean that another operating system "
                  "believes the disk is smaller.  Fix, by moving the backup "
-                 "to the end (and removing the old backup)?"))
-                                       == PED_EXCEPTION_CANCEL)
-                               goto error_free_gpt;
+                 "to the end (and removing the old backup)?"))) {
+                               case PED_EXCEPTION_CANCEL:
+                                       goto error_free_gpt;
+                               case PED_EXCEPTION_FIX:
+                                       write_back = 1;
+                                       memset (zeros, 0, 
disk->dev->sector_size);
+                                       ped_device_write (disk->dev, zeros,
+                                                         PED_LE64_TO_CPU 
(gpt->AlternateLBA),
+                                                         1);
+                                       break;
+                               default:
+                                       break;
+                       }

-                       write_back = 1;
-                       memset (zeros, 0, disk->dev->sector_size);
-                       ped_device_write (disk->dev, zeros,
-                                         PED_LE64_TO_CPU (gpt->AlternateLBA),
-                                         1);
 #endif /* !DISCOVER_ONLY */
                }
        } else { /* primary GPT *not* ok */
--
1.6.2.rc1.175.g96b8a


>From 5bbcffae45afd08dac41d96dc94b235439a67a61 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Mon, 16 Feb 2009 17:54:57 +0100
Subject: [PATCH 2/2] gpt: add a test: printing a partition table must not 
modify it

* tests/t0200-gpt.sh: New file.
* tests/Makefile.am (TESTS): Add the new test.
---
 tests/Makefile.am  |    1 +
 tests/t0200-gpt.sh |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100755 tests/t0200-gpt.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1214f9c..14e4862 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,6 +1,7 @@
 TESTS = \
   t0000-basic.sh \
   t0100-print.sh \
+  t0200-gpt.sh \
   t1000-mkpartfs.sh \
   t1100-busy-label.sh \
   t1500-small-ext2.sh \
diff --git a/tests/t0200-gpt.sh b/tests/t0200-gpt.sh
new file mode 100755
index 0000000..633d580
--- /dev/null
+++ b/tests/t0200-gpt.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+test_description='printing a GPT partition table must not modify it'
+
+: ${srcdir=.}
+. $srcdir/test-lib.sh
+
+N=2M
+dev=loop-file
+test_expect_success \
+    'create a file large enough to hold a GPT partition table' \
+    'dd if=/dev/null of=$dev bs=1 seek=$N 2> /dev/null'
+
+test_expect_success \
+    'create a GPT partition table' \
+    'parted -s $dev mklabel gpt > out 2>&1'
+test_expect_success 'expect no output' 'compare out /dev/null'
+
+test_expect_success \
+    'save a copy of the original primary GPT table' \
+    'dd if=$dev of=before count=1 skip=1'
+
+test_expect_success \
+    'extend the backing file by 1 byte' \
+    'printf x >> $dev'
+
+test_expect_success \
+    'use parted simply to print the partition table' \
+    'parted -m -s $dev u s p > out 2> err'
+# don't bother comparing stdout
+test_expect_success 'expect no stderr' 'compare err /dev/null'
+
+test_expect_success \
+    'extract the primary GPT table again' \
+    'dd if=$dev of=after count=1 skip=1'
+
+test_expect_success \
+    'compare partition tables (they had better be identical)' \
+    'compare before after'
+
+test_done
--
1.6.2.rc1.175.g96b8a

_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel

Reply via email to