On Thu, 2007-03-01 at 17:29 +0100, Frans Pop wrote:
> On Thursday 01 March 2007 06:11, Ben Hutchings wrote:
> > On further thinking, I realise there are several problems with the
> > patch:
> > 1. It tries to probe even if open_filesystem is false (already
> > identified).
> > 2. It can return after maximize_extended_partition() without rolling
> > that change back.
> > 3. It doesn't check whether the existing invalid filesystem geometry
> > returned by the probe is valid.
> >
> > This should fix those problems by combining all the work that's
> > conditional on the open_filesystem flag:
>
> This patch again results in the start of the ntfs partition being moved
> to sector 63.
>
> Here's the relevant bit from the partman log:
> <snip>
> parted_server: main_loop: iteration 41
> parted_server: Opening infifo
> /lib/partman/active_partition/70resize/do_option: IN:
> VIRTUAL_RESIZE_PARTITION =dev=scsi=host0=bus0=target0=lun0=disc
> 1048576-20972568575 15000000000
Ah, so it is using VIRTUAL_RESIZE_PARTITION. I should have read the
script you pointed me at last time.
What is the intended difference in semantics between RESIZE_PARTITION
and VIRTUAL_RESIZE_PARTITION? In the resize_partition() function these
are distinguished by the open_filesystem flag which implied to me that
in the latter case we wouldn't expect to find a filesystem in the
partition at all. Clearly that's not the case here.
This patch reduces the difference that the open_filesystem flag makes.
When the flag is true (for RESIZE_PARTITION) the only change is to check
construction of the constraint. When the flag is false, the behaviour
will differ as follows:
1. Probing the file-system
- If true, use ped_file_system_open()
- If false, use ped_file_system_probe_specific()
(In any case, this should succeed and produce a file-system
geometry within the partition geometry, or ped_file_system_probe()
should fail indicating that no file-system is present. If neither
is true, the resize fails.)
2. Resizing the file-system and partition (if found)
- If true, check the integrity of any file-system, get its resize
constraint, and resize it.
- If false, construct a generic constraint for the partition that
does not require resizing the file-system itself.
I hope these are the correct semantics.
diff -u parted_server.c~ parted_server.c
--- parted_server.c~ 2007-01-11 13:57:27.000000000 +0000
+++ parted_server.c 2007-03-02 01:29:07.000000000 +0000
@@ -714,8 +714,58 @@
}
if (NULL == fs && NULL != ped_file_system_probe(&(part->geom)))
return false;
+ if (NULL != fs)
+ constraint = ped_file_system_get_resize_constraint(fs);
+ else
+ constraint = ped_constraint_any(disk->dev);
} else {
+ PedFileSystemType *fs_type;
+ PedGeometry *fs_geom;
+ PedAlignment start_align;
+ PedGeometry full_dev;
fs = NULL;
+ fs_type = ped_file_system_probe(&(part->geom));
+ log("probed file system: %s", NULL != fs_type ? "yes" : "no");
+ if (NULL != fs_type)
+ fs_geom = ped_file_system_probe_specific(fs_type,
+ &part->geom);
+ else
+ fs_geom = NULL;
+ if (NULL != fs_geom && (fs_geom->start < (part->geom).start
+ || fs_geom->end > (part->geom).end)) {
+ ped_geometry_destroy(fs_geom);
+ fs_geom = NULL;
+ }
+ if (NULL == fs_geom && NULL != fs_type)
+ return false;
+ if (NULL != fs_geom) {
+ /* We cannot resize or move the fs but we can
+ * move the end of the partition so long as it
+ * contains the whole fs.
+ */
+ if (ped_alignment_init(&start_align,
+ fs_geom->start, 0)
+ && ped_geometry_init(&full_dev, disk->dev,
+ 0, disk->dev->length - 1)) {
+ constraint = ped_constraint_new(
+ &start_align,
+ ped_alignment_any,
+ &full_dev, &full_dev,
+ fs_geom->length,
+ disk->dev->length);
+ } else {
+ constraint = NULL;
+ }
+ ped_geometry_destroy(fs_geom);
+ } else {
+ constraint = ped_constraint_any(disk->dev);
+ }
+ }
+ if (NULL == constraint) {
+ log("failed to get resize constraint");
+ if (NULL != fs)
+ ped_file_system_close(fs);
+ return false;
}
log("try to check the file system for errors");
if (NULL != fs && !timered_file_system_check(fs)) {
@@ -727,10 +777,6 @@
log("successfully checked");
if (part->type & PED_PARTITION_LOGICAL)
maximize_extended_partition(disk);
- if (NULL != fs)
- constraint = ped_file_system_get_resize_constraint(fs);
- else
- constraint = ped_constraint_any(disk->dev);
if (!ped_disk_set_partition_geom(disk, part, constraint, start, end))
result = false;
else if (NULL == fs)
-- END --
It should be clear that all the new code (aside from the error check)
only runs in the currently broken case, so this does not affect resizing
ext2 etc. And none of it is running below
maximize_extended_partition().
One risk it introduces is that a partition which contains the broken
remnants of a filesystem may now be recognised and the sanity check on
geometry could prevent it from being resized with
VIRTUAL_RESIZE_PARTITION (this problem already applies to
RESIZE_PARTITION). It should be possible for the user to work around
that by deleting and recreating the partition rather than resizing it.
However there's no obvious error message when this happens.
Ben.
--
Ben Hutchings
Gates has joked that everything goes on and off unexepectedly in the house,
which is run by a high-end PC network built on Windows NT. - Seattle Times
Attachment:
signature.asc
Description: This is a digitally signed message part