[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#380226: NTFS (partition) not recreated correctly after resize:incorrect start sector



On Thu, 2007-03-01 at 01:04 +0100, Frans Pop wrote:
> On Wednesday 28 February 2007 01:52, Ben Hutchings wrote:
> > This patch might fix parted_server, but I don't know how to test it.
> 
> I've done some extensive testing repeatedly resizing (both down and up) an 
> NTFS Vista partition, and the patch works well. Vista boots correctly 
> after the resize operations.

Well that was lucky!

> It turns out that a minor additional patch in partman-partitioning is 
> necessary (a short sleep to allow the system to settle before running the 
> final 'ntfsresize -f' command).

I hate having to do that...

> I would still very much like to hear the rationale behind the patch and 
> especially an analysis of how/why it will (not) affect resizing other 
> filesystems. This will help to decide how much additional testing is 
> needed.

Here's the patch again with line numbering for reference:

 1:diff -u parted_server.c~ parted_server.
 2:--- parted_server.c~	2007-01-11 13:57:27.000000000 +0000
 3:+++ parted_server.c	2007-02-27 23:50:18.000000000 +0000
 4:@@ -712,8 +712,6 @@
 5:                         ped_file_system_close(fs);
 6:                         fs = NULL;
 7:                 }
 8:-                if (NULL == fs && NULL != ped_file_system_probe(&(part->geom)))
 9:-                        return false;
10:         } else {
11:                 fs = NULL;
12:         }

I moved line 8 down to line 27 where we use the result of the probe to
construct a constraint.  If the probe fails this cascades down to a
constraint of NULL which is detected as an error at line 58.

13:@@ -727,10 +725,50 @@
14:         log("successfully checked");
15:         if (part->type & PED_PARTITION_LOGICAL)
16:                 maximize_extended_partition(disk);
17:-        if (NULL != fs)
18:+        if (NULL != fs) {
19:                 constraint = ped_file_system_get_resize_constraint(fs);
20:-        else
21:-                constraint = ped_constraint_any(disk->dev);

The constraint ped_constraint_any(disk->dev) was being applied to
filesystems that parted can't open, which include NTFS.

22:+        } else {

This "else" probably should be "else if (open_filesystem)".  What
exactly is a "virtual partition" and why don't we want to look at the
filesystem on it?

23:+                PedFileSystemType *fs_type;
24:+                PedGeometry *fs_geom;
25:+                PedAlignment start_align;
26:+                PedGeometry full_dev;
27:+                fs_type = ped_file_system_probe(&(part->geom));
28:+                log("probed file system: %s", NULL != fs_type ? "yes" : "no");
29:+                if (NULL != fs_type) {
30:+                        fs_geom = ped_file_system_probe_specific(fs_type,
31:+                                                                 &part->geom);
32:+                } else {
33:+                        fs_geom = NULL;
34:+                }
35:+                if (NULL != fs_geom) {

Should be self-explanatory.

36:+                        /* We cannot resize or move the fs but we can move the
37:+                         * end of the partition so long as it contains the
38:+                         * whole fs.
39:+                         */
40:+                        if (ped_alignment_init(&start_align,
41:+                                                fs_geom->start, 0)
42:+                            && ped_geometry_init(&full_dev, disk->dev,
43:+                                                 0, disk->dev->length - 1)) {
44:+                                constraint = ped_constraint_new(
45:+                                        &start_align,
46:+                                        ped_alignment_any,
47:+                                        &full_dev, &full_dev,
48:+                                        fs_geom->length,
49:+                                        disk->dev->length);

I think the comment explains this adequately.

50:+                        } else {
51:+                                constraint = NULL;
52:+                        }
53:+                        ped_geometry_destroy(fs_geom);
54:+                } else {
55:+                        constraint = NULL;
56:+                }
57:+        }
58:+        if (open_filesystem && NULL == constraint) {
59:+                log("failed to get resize constraint");
60:+                if (NULL != fs)
61:+                        ped_file_system_close(fs);
62:+                return false;
63:+        }

If we care about the filesystem on the partition, we do not proceed
unless we know what constraint it imposes on resizing.

This can fail due to:
1. Failure to get the constraint from an open filesystem (line 19).
This is a new possibility for failure; currently we would continue at
the risk of data loss.
2. Failure to get the filesystem type (line 27).  This already results
in failure (line 8).
3. Failure to get the filesystem geometry (line 30).  This is a new
possibility for failure; currently we would continue at the risk of data
loss.
4. Out of memory (lines 27, 30, 44).  There is a (very) marginally
higher memory requirement.

64:         if (!ped_disk_set_partition_geom(disk, part, constraint, start, end))
65:                 result = false;
66:         else if (NULL == fs)

> I have not yet tested with other filesystems, but will try to do so 
> tomorrow.

All filesystems that parted knows how to probe but not open will be
affected by the change: UFS, ReiserFS, JFS, XFS, APFS and AFFS.

Ben.

-- 
Ben Hutchings
Quantity is no substitute for quality, but it's the only one we've got.

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: