[Parted-maintainers] Bug#380226: NTFS (partition) not recreated correctly after resize:incorrect start sector

[Ben Hutchings]

Since this bug report is very long, I was not able to understand what
was really going wrong until I met Frans on Friday and discussed it in
person.  Let me state the problem as I understand it, both to confirm
that I understand it correctly and to serve as a quick summary for
anyone else coming newly to the bug report.

Summary of the bug

Parted is not capable of resizing NTFS.  Therefore, when resizing a
partition containing an NTFS, it should never move the start of the
partition and it should only move the end of the partition so long as it
will still include the whole of the filesystem.  However, if the
partition is the first partition on a DOS-partitioned disk it may move
the start of the partition.

Investigation and hypothesis

I looked through the code used for resizing and I believe I can see the
source of this bug.

do_resize() gets two constraints: the bounds of the neighbouring
partitions, generated by snap_to_boundaries(), and one from the
filesystem code, generated by ped_file_system_get_resize_constraint().
Then it combines them using ped_constraint_intersect() and passes the
combined constraint into ped_disk_set_partition_geom().

If a filesystem is not resizable (as NTFS is not) then there is no
function for getting the resize constraint and
ped_file_system_get_resize_constraint() returns NULL.  Here NULL seems
to be an error value, because the resize is not meant to be
unconstrained.

ped_constraint_intersect() returns NULL if either parameter is NULL.
Here again NULL is treated as an error value - if it meant "no
constraint" then when one parameter is NULL the function should return
the other parameter value.

However, ped_disk_set_partition_geom() will quite happily continue if
the constraint parameter is NULL.  Here NULL is treated as "no
constraint" and the error has been silently ignored!

I don't think alignment restrictions have anyting to do with the bug.

Proposed solution

If I understood this correctly, the first step to fixing this would be
to make do_resize() or ped_disk_set_partition_geom() fail and report an
error if the constraint is NULL.  However, this will disable resizing of
any partition if the filesystem resizing is not supported.  So the
second step would be to change ped_file_system_get_resize_constraint()
to return the correct constraint where the filesystem does not support
resizing, as stated in the problem summary.

Status

I have not been able to test this as I do not have a suitable virtual
machine or virtual disk system available.  (Is there a way to make the
kernel look for partitions on loopback devices?)  The following patch
might help to confirm my hypothesis:

diff -Nru /tmp/5n58ke8AFZ/parted-1.7.1/parted/parted.c /tmp/oqAdmNqKfm/parted-1.7.1/parted/parted.c
--- /tmp/5n58ke8AFZ/parted-1.7.1/parted/parted.c	2007-01-19 20:29:41.000000000 +0100
+++ /tmp/oqAdmNqKfm/parted-1.7.1/parted/parted.c	2007-02-24 18:25:12.000000000 +0100
@@ -1568,6 +1568,53 @@
         return 0;
 }
 
+static void
+print_alignment (const PedAlignment* alignment)
+{
+	if (alignment == NULL) {
+		printf ("NULL");
+	} else {
+		printf ("& (PedAlignment) { offset: %lld, grain_size: %lld }",
+			alignment->offset, alignment->grain_size);
+	}
+}
+
+static void
+print_geometry (const PedGeometry* geom)
+{
+	if (geom == NULL) {
+		printf ("NULL");
+	} else {
+		printf ("& (PedGeometry) { dev: & (PedDevice) { path: \"%s\", .... }, start: %lld, length: %lld, end: %lld }",
+			geom->dev->path, geom->start, geom->length, geom->end);
+	}
+}
+
+static void
+print_constraint (const PedConstraint* constraint)
+{
+	if (constraint == NULL) {
+		printf ("NULL\n");
+	} else {
+		printf ("& (PedConstraint) {\n\tstart_align: ");
+		print_alignment (constraint->start_align);
+		printf (",\n"
+			"\tend_align: ");
+		print_alignment (constraint->end_align);
+		printf (",\n"
+			"\tstart_range: ");
+		print_geometry (constraint->start_range);
+		printf (",\n"
+			"\tend range: ");
+		print_geometry (constraint->end_range);
+		printf (",\n"
+			"\tmin_size: %lld,\n"
+			"\tmax_size: %lld"
+			"\n}",
+			constraint->min_size, constraint->max_size);
+	}
+}
+
 static int
 do_resize (PedDevice** dev)
 {
@@ -1610,13 +1657,22 @@
                         goto error_destroy_constraint;
                 ped_partition_set_system (part, NULL);
         } else {
+		PedConstraint *fs_constraint, *part_constraint;
+
                 fs = ped_file_system_open (&part->geom);
                 if (!fs)
                         goto error_destroy_disk;
+		fs_constraint = ped_file_system_get_resize_constraint (fs);
+		printf ("fs_constraint = ");
+		print_constraint (fs_constraint);
+		part_constraint = constraint_from_start_end (*dev, range_start, range_end));
+		printf ("part_constraint = ");
+		print_constraint (part_constraint);
                 constraint = constraint_intersect_and_destroy (
-                                ped_file_system_get_resize_constraint (fs),
-                                constraint_from_start_end (
-                                        *dev, range_start, range_end));
+			fs_constraint, part_constraint);
+		printf ("constraint = ");
+		print_constraint (constraint);
+
                 if (!ped_disk_set_partition_geom (disk, part, constraint,
                                                   new_geom.start, new_geom.end))
                         goto error_close_fs;
-- END --

Ben.

-- 
Ben Hutchings
When you say `I wrote a program that crashed Windows', people just stare ...
and say `Hey, I got those with the system, *for free*'. - Linus Torvalds
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/parted-maintainers/attachments/20070225/66276bb8/attachment.pgp