[parted-devel] [PATCH 1/2] libparted: copy flags when duplicating GPT partitions

Jim Meyering jim at meyering.net
Wed Nov 2 17:37:13 UTC 2011


Brian C. Lane wrote:
> On Wed, Nov 02, 2011 at 05:09:32PM +0100, Jim Meyering wrote:
>> Hi Brian,
>>
>> Thanks for that fix, and especially for the added test case (which I'm
>> about to look at).  Does this have an associated bugzilla number?
>> If so, let me know and I'll mention it in the log message before I push.
>
> Yes, it is 747497
>
> I had a particularly hard time getting that test case working, so if you
> see anything wrong with it let me know. The creation of the example
> partition was harder than I expected it to be. It doesn't work with all
> disk labels as I was hoping, but msdos, gpt and bsd should cover the
> majority of uses.

Thanks again for writing it.

I agree that testing only msdos+gpt+bsd is good enough.
It's not worth spending much time on other partition table types,
so I'm happy to let that slide.

I have changed two things in the test case c-set:
  - use path_prepend_ to specify the additional directory (".")
    rather than appending to PATH manually.
  - split some longer-than-80-columns lines in duplicate.c.

Here's the former, that I'm folding in.

diff --git a/tests/t0501-duplicate.sh b/tests/t0501-duplicate.sh
index 0585a95..3d0fac4 100644
--- a/tests/t0501-duplicate.sh
+++ b/tests/t0501-duplicate.sh
@@ -16,10 +16,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

-. "${srcdir=.}/init.sh"; path_prepend_ ../parted
-
-PATH="..:$PATH"
-export PATH
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted .

 for t in msdos gpt bsd; do
     duplicate $t || fail=1

I ran the new test via valgrind and was pleased to see that it
reported no memory or fd leaks.

Here's what I've pushed:

>From febeedd8d3dbddcc6e831591b05f590eaca97b12 Mon Sep 17 00:00:00 2001
From: "Brian C. Lane" <bcl at redhat.com>
Date: Mon, 31 Oct 2011 16:35:16 -0700
Subject: [PATCH 1/2] libparted: copy flags when duplicating GPT partitions

* libparted/labels/gpt.c (gpt_partition_duplicate): Copy flags to new
partition.
* NEWS: Mention this fix.
Reported by Chris Murphy in https://bugzilla.redhat.com/747497.
---
 NEWS                   |    3 +++
 libparted/labels/gpt.c |    4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index bc5152b..af1d957 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,9 @@ GNU parted NEWS                                    -*- outline -*-

 ** Bug fixes

+  libparted: gpt_disk_duplicate now copies the flags over to the new
+  disk object. Previously the flags would be undefined.
+
   libparted: no longer aborts (failed assertion) due to a nilfs2_probe bug
   [bug introduced in parted-2.4 with the addition of nilfs2 support]

diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index 8c9816f..f2bda41 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -1362,9 +1362,7 @@ gpt_partition_duplicate (const PedPartition *part)
   if (!result_data)
     goto error_free_part;

-  result_data->type = part_data->type;
-  result_data->uuid = part_data->uuid;
-  strcpy (result_data->name, part_data->name);
+  *result_data = *part_data;
   return result;

 error_free_part:
--
1.7.8.rc0.32.g87bf9


>From deb572c298c5b09cb768de8bd3d045df427f1e8f Mon Sep 17 00:00:00 2001
From: "Brian C. Lane" <bcl at redhat.com>
Date: Mon, 31 Oct 2011 16:35:17 -0700
Subject: [PATCH 2/2] tests: add new test to check ped_disk_duplicate

* tests/duplicate.c: New test.
* tests/t0501-duplicate.sh: New test program.
* tests/Makefile.am (TEST): Add new test.
(check_PROGRAMS): Add new test program.
---
 tests/Makefile.am        |    3 +-
 tests/duplicate.c        |  134 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/t0501-duplicate.sh |   25 +++++++++
 3 files changed, 161 insertions(+), 1 deletions(-)
 create mode 100644 tests/duplicate.c
 create mode 100644 tests/t0501-duplicate.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 63bcd31..9638248 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -25,6 +25,7 @@ TESTS = \
   t0300-dos-on-gpt.sh \
   t0400-loop-clobber-infloop.sh \
   t0500-dup-clobber.sh \
+  t0501-duplicate.sh \
   t1100-busy-label.sh \
   t1101-busy-partition.sh \
   t1700-probe-fs.sh \
@@ -63,7 +64,7 @@ EXTRA_DIST = \
   $(TESTS) lvm-utils.sh t-local.sh t-lvm.sh \
   init.cfg init.sh t-lib-helpers.sh

-check_PROGRAMS = print-align print-max dup-clobber
+check_PROGRAMS = print-align print-max dup-clobber duplicate
 LDADD = \
   $(top_builddir)/libparted/libparted.la
 AM_CPPFLAGS = \
diff --git a/tests/duplicate.c b/tests/duplicate.c
new file mode 100644
index 0000000..129537c
--- /dev/null
+++ b/tests/duplicate.c
@@ -0,0 +1,134 @@
+/* Demonstrate that ped_disk_duplicate is working correctly.
+*/
+#include <config.h>
+#include <parted/parted.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+
+#include "closeout.h"
+#include "progname.h"
+
+int
+main (int argc, char **argv)
+{
+  atexit (close_stdout);
+  set_program_name (argv[0]);
+
+  if (argc != 2)
+    return EXIT_FAILURE;
+
+  char const *dev_name = "dev-file";
+
+  /* Create a file.  */
+  int fd = open (dev_name, O_CREAT|O_TRUNC|O_WRONLY, 0644);
+  assert (0 <= fd);
+  off_t size = 8 * 1024 * 1024;
+  assert (ftruncate (fd, size) == 0);
+  assert (close (fd) == 0);
+
+  PedDevice *dev = ped_device_get (dev_name);
+  assert (dev);
+
+  PedDisk *disk = ped_disk_new_fresh (dev, ped_disk_type_get (argv[1]));
+  assert (disk);
+  assert (ped_disk_commit(disk));
+  ped_disk_destroy (disk);
+
+  /* re-open the disk */
+  disk = ped_disk_new (dev);
+  assert (disk);
+
+  /* Create a partition */
+  const PedFileSystemType *fs_type = ped_file_system_type_get ("ext2");
+  assert (fs_type);
+  PedPartitionType part_type = PED_PARTITION_NORMAL;
+  const PedGeometry *geometry = ped_geometry_new (dev, 34, 1024);
+  assert (geometry);
+  PedPartition *part = ped_partition_new (disk, part_type, fs_type,
+					  geometry->start, geometry->end);
+  assert (part);
+  PedConstraint *constraint = ped_constraint_exact (geometry);
+  assert (constraint);
+
+  if (ped_partition_is_flag_available (part, PED_PARTITION_BOOT))
+    assert (ped_partition_set_flag (part, PED_PARTITION_BOOT, 1));
+
+  assert (ped_disk_add_partition (disk, part, constraint));
+  ped_constraint_destroy (constraint);
+
+  assert (ped_partition_set_system (part, fs_type));
+  if (ped_partition_is_flag_available (part, PED_PARTITION_LBA))
+    ped_partition_set_flag (part, PED_PARTITION_LBA, 1);
+
+  assert (ped_disk_commit(disk));
+
+  /* Duplicate it */
+  PedDisk *copy = ped_disk_duplicate (disk);
+  assert (ped_disk_commit(copy));
+
+  /* Compare the two copies */
+
+  /* Check the device */
+  assert (strcmp (disk->dev->model, copy->dev->model) == 0);
+  assert (strcmp (disk->dev->path, copy->dev->path) == 0);
+  assert (disk->dev->sector_size == copy->dev->sector_size);
+  assert (disk->dev->phys_sector_size == copy->dev->phys_sector_size);
+  assert (disk->dev->length == copy->dev->length);
+
+  /* Check the type */
+  assert (strcmp (disk->type->name, copy->type->name) == 0);
+  assert (disk->type->features == copy->type->features);
+
+  /* Check the flags */
+  for (PedDiskFlag flag = PED_DISK_FIRST_FLAG; flag <= PED_DISK_LAST_FLAG;
+       flag++) {
+    if (!ped_disk_is_flag_available(disk, flag))
+      continue;
+    assert (ped_disk_get_flag (disk, flag) == ped_disk_get_flag (copy, flag));
+  }
+
+  /* Check the partitions */
+  PedPartition *disk_part, *copy_part;
+  for ( disk_part = disk->part_list, copy_part = copy->part_list;
+        disk_part && copy_part;
+        disk_part = disk_part->next, copy_part = copy_part->next)
+  {
+    /* Only active partitions are duplicated */
+    if (!ped_partition_is_active (disk_part))
+      continue;
+
+    assert (disk_part->geom.start == copy_part->geom.start);
+    assert (disk_part->geom.end == copy_part->geom.end);
+    assert (disk_part->geom.length == copy_part->geom.length);
+    assert (disk_part->num == copy_part->num);
+    assert (disk_part->type == copy_part->type);
+
+    if (disk_part->fs_type && disk_part->fs_type->name) {
+      assert (strcmp (disk_part->fs_type->name, copy_part->fs_type->name) == 0);
+    }
+
+    /* Check the flags */
+    for (PedPartitionFlag flag = PED_PARTITION_FIRST_FLAG;
+	 flag <= PED_PARTITION_LAST_FLAG; flag++) {
+      if (!ped_partition_is_flag_available(disk_part, flag))
+        continue;
+      fprintf (stderr, "Checking partition flag %d\n", flag);
+      fprintf (stderr, "%d ? %d\n", ped_partition_get_flag (disk_part, flag),
+	       ped_partition_get_flag (copy_part, flag));
+      assert (ped_partition_get_flag (disk_part, flag)
+	      == ped_partition_get_flag (copy_part, flag));
+    }
+  }
+
+  /* Cleanup the mess */
+  ped_disk_destroy (copy);
+  ped_disk_destroy (disk);
+  ped_device_destroy (dev);
+
+  return EXIT_SUCCESS;
+}
diff --git a/tests/t0501-duplicate.sh b/tests/t0501-duplicate.sh
new file mode 100644
index 0000000..3d0fac4
--- /dev/null
+++ b/tests/t0501-duplicate.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+# Drive the dup-clobber program.
+
+# Copyright (C) 2011 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/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted .
+
+for t in msdos gpt bsd; do
+    duplicate $t || fail=1
+done
+
+Exit $fail
--
1.7.8.rc0.32.g87bf9



More information about the parted-devel mailing list