[parted-devel] [PATCH] Properly sync partitions with operating system
Joel Granados
jgranado at redhat.com
Thu Feb 19 12:55:33 UTC 2009
On Thu, Feb 19, 2009 at 12:28:14PM +0100, Petr Uzel wrote:
> Hi,
>
> please see the inlined comments.
>
>
> On Wed, Feb 18, 2009 at 10:01:11PM +0100, Joel Granados Moreno wrote:
> > ---
> > include/parted/disk.h | 3 +-
> > libparted/arch/linux.c | 113 +++++++++++++++++++++++++++++++++-------------
> > libparted/disk.c | 14 +++++-
> > libparted/labels/aix.c | 8 +++
> > libparted/labels/bsd.c | 2 +
> > libparted/labels/dasd.c | 1 +
> > libparted/labels/dos.c | 10 ++++-
> > libparted/labels/dvh.c | 3 +-
> > libparted/labels/gpt.c | 9 +++-
> > libparted/labels/loop.c | 2 +
> > libparted/labels/mac.c | 10 ++++-
> > libparted/labels/pc98.c | 2 +
> > libparted/labels/rdb.c | 2 +
> > libparted/labels/sun.c | 2 +
> > 14 files changed, 143 insertions(+), 38 deletions(-)
> >
> > diff --git a/include/parted/disk.h b/include/parted/disk.h
> > index dfcf39e..9e50716 100644
> > --- a/include/parted/disk.h
> > +++ b/include/parted/disk.h
> > @@ -210,6 +210,7 @@ struct _PedDiskOps {
> > /* other */
> > int (*alloc_metadata) (PedDisk* disk);
> > int (*get_max_primary_partition_count) (const PedDisk* disk);
> > + int (*get_max_partition_count) (const PedDisk* disk);
> > };
> >
> > struct _PedDiskType {
> > @@ -322,7 +323,7 @@ extern PedPartition* ped_disk_extended_partition (const PedDisk* disk);
> > /* internal functions */
> > extern PedDisk* _ped_disk_alloc (const PedDevice* dev, const PedDiskType* type);
> > extern void _ped_disk_free (PedDisk* disk);
> > -
> > +extern int ped_disk_get_max_partition_num(const PedDisk* disk);
> >
> > /** @} */
> >
> > diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> > index 4a8c9e7..35ef1a5 100644
> > --- a/libparted/arch/linux.c
> > +++ b/libparted/arch/linux.c
> > @@ -279,6 +279,9 @@ struct blkdev_ioctl_param {
> > || (M) == SCSI_CDROM_MAJOR \
> > || ((M) >= SCSI_DISK1_MAJOR && (M) <= SCSI_DISK7_MAJOR))
> >
> > +/* Maximum number of partitions supported by linux. */
> > +#define MAX_NUM_PARTS 16
>
> I'd suggest to increase this to 64
Yes. I'm sorry, you had already suggested this and I forgot :)
>
> > +
> > static char* _device_get_part_path (PedDevice* dev, int num);
> > static int _partition_is_mounted_by_path (const char* path);
> >
> > @@ -2261,45 +2264,91 @@ _blkpg_remove_partition (PedDisk* disk, int n)
> > BLKPG_DEL_PARTITION);
> > }
> >
> > +/*
> > + * The number of partitions that a device can have depends on the kernel.
> > + * If we don't find this value in /sys/block/DEV/range, we will use our own
> > + * value.
> > + */
> > +static int
> > +_device_get_partition_range(PedDevice* dev)
> > +{
> > + int range;
> > + char path[512];
> > + FILE* fd;
> > +
> > + if(snprintf(path, sizeof(path), "/sys/block/%s/range", dev->path) < 0)
> > + return MAX_NUM_PARTS;
> > +
> > + fd = fopen(path, "r");
> > + if(!fd)
> > + return MAX_NUM_PARTS;
> > +
> > + if(fscanf(fd, "%d", range) == EOF)
> > + return MAX_NUM_PARTS;
> > +
> > + return range;
> > +}
> > +
> > +/*
> > + * We sync the partition table in two step process:
> > + * 1. We remove all the partitions from the kernel's tables. The partitions
> > + * will not be removed if the ioctl call fails.
> > + * 2. We add all the partitions that we hold in disk.
> > + *
> > + * To achieve this two step process we must calculate the minimum number of
> > + * maximum possible partitions between what linux supports and what the label
> > + * type supports. EX:
> > + *
> > + * number=MIN(max_parts_supported_in_linux,max_parts_supported_in_msdos_tables)
> > + *
> > + * For now we will hard code the maximum number of partitions for linux to 16.
>
> s/16/MAX_NUM_PARTS
Yep changed.
>
> > + * Might want to search for a way to include this from somewhere.
> > + */
> > static int
> > _disk_sync_part_table (PedDisk* disk)
> > {
> > - int largest_partnum = ped_disk_get_last_partition_num (disk);
> > - if (largest_partnum <= 0)
> > - return 1;
> > + PED_ASSERT(disk != NULL, return 0);
> > + PED_ASSERT(disk->dev != NULL, return 0);
> >
> > - int last = 16;
> > - int* rets = ped_malloc(sizeof(int) * last);
> > - int* errnums = ped_malloc(sizeof(int) * last);
> > - int ret = 1;
> > - int i;
> > + /* lpn = largest partition number. */
> > + int lpn = PED_MIN(_device_get_partition_range(disk->dev),
> > + ped_disk_get_max_partition_num(disk));
> >
> > - for (i = 1; i <= last; i++) {
> > - rets[i - 1] = _blkpg_remove_partition (disk, i);
> > - errnums[i - 1] = errno;
> > - }
> > + /* Its not possible to support largets_partnum <= 0. */
>
> s/largets/largest
You also commented on this mistak before. changed. now for real
>
> > + if(lpn <= 0)
> > + return 0;
> >
> > - for (i = 1; i <= last; i++) {
> > - const PedPartition *part;
> > + int* rets = ped_malloc(sizeof(int) * lpn);
> > + int* errnums = ped_malloc(sizeof(int) * lpn);
> > + int ret = 1;
> > + int i;
> >
> > - part = ped_disk_get_partition (disk, i);
> > - if (part) {
> > - /* busy... so we won't (can't!) disturb ;) Prolly
> > - * doesn't matter anyway, because users shouldn't be
> > - * changing mounted partitions anyway...
> > - */
> > - if (!rets[i - 1] && errnums[i - 1] == EBUSY)
> > - continue;
> > -
> > - /* add the (possibly modified or new) partition */
> > - if (!_blkpg_add_partition (disk, part))
> > - ret = 0;
> > - }
> > - }
> > + for(i = 1; i <= lpn; i++) {
> > + rets[i - 1] = _blkpg_remove_partition (disk, i);
> > + errnums[i - 1] = errno;
> > + }
> > +
> > + for(i = 1; i <= lpn; i++) {
> > + const PedPartition *part;
> > +
> > + part = ped_disk_get_partition (disk, i);
> > + if (part) {
> > + /* busy... so we won't (can't!) disturb ;) Prolly
> > + * doesn't matter anyway, because users shouldn't be
> > + * changing mounted partitions anyway...
> > + */
> > + if (!rets[i - 1] && errnums[i - 1] == EBUSY)
> > + continue;
> > +
> > + /* add the (possibly modified or new) partition */
> > + if (!_blkpg_add_partition (disk, part))
> > + ret = 0;
> > + }
> > + }
> >
> > - free (rets);
> > - free (errnums);
> > - return ret;
> > + free (rets);
> > + free (errnums);
> > + return ret;
> > }
> >
> > #ifdef ENABLE_DEVICE_MAPPER
> > diff --git a/libparted/disk.c b/libparted/disk.c
> > index 93929b2..f259a9a 100644
> > --- a/libparted/disk.c
> > +++ b/libparted/disk.c
> > @@ -628,7 +628,7 @@ ped_disk_get_primary_partition_count (const PedDisk* disk)
> > }
> >
> > /**
> > - * Get the highest partition number on \p disk.
> > + * Get the highest available partition number on \p disk.
> > */
> > int
> > ped_disk_get_last_partition_num (const PedDisk* disk)
> > @@ -648,6 +648,18 @@ ped_disk_get_last_partition_num (const PedDisk* disk)
> > }
> >
> > /**
> > + * Get the highest suppoerted partition number on \p disk.
>
> s/suppoerted/supported
>
> > + */
> > +int
> > +ped_disk_get_max_partition_num(const PedDisk* disk)
> > +{
> > + PED_ASSERT(disk != NULL, return 0);
> > + PED_ASSERT(disk->type->ops->get_max_partition_count != NULL, return 0);
> > +
> > + return disk->type->ops->get_max_partition_count(disk);
> > +}
> > +
> > +/**
> > * Get the maximum number of (primary) partitions the disk label supports.
> > *
> > * For example, MacIntosh partition maps can have different sizes,
> > diff --git a/libparted/labels/aix.c b/libparted/labels/aix.c
> > index b3dd176..c5da1fd 100644
> > --- a/libparted/labels/aix.c
> > +++ b/libparted/labels/aix.c
> > @@ -222,6 +222,12 @@ aix_get_max_primary_partition_count (const PedDisk* disk)
> > }
> >
> > static int
> > +aix_get_max_partitoin_count (const PedDisk* disk)
> > +{
> > + return 16;
> > +}
> > +
> > +static int
> > aix_partition_align (PedPartition* part, const PedConstraint* constraint)
> > {
> > PED_ASSERT (part != NULL, return 0);
> > @@ -270,6 +276,8 @@ static PedDiskOps aix_disk_ops = {
> > alloc_metadata: aix_alloc_metadata,
> > get_max_primary_partition_count:
> > aix_get_max_primary_partition_count,
> > + get_max_partition_count:
> > + aix_get_max_partitoin_count,
> >
> > partition_set_name: NULL,
> > partition_get_name: NULL,
> > diff --git a/libparted/labels/bsd.c b/libparted/labels/bsd.c
> > index 3eb5363..a1c75b8 100644
> > --- a/libparted/labels/bsd.c
> > +++ b/libparted/labels/bsd.c
> > @@ -656,6 +656,8 @@ static PedDiskOps bsd_disk_ops = {
> >
> > alloc_metadata: bsd_alloc_metadata,
> > get_max_primary_partition_count:
> > + bsd_get_max_primary_partition_count,
> > + get_max_partition_count:
> > bsd_get_max_primary_partition_count
> > };
> >
> > diff --git a/libparted/labels/dasd.c b/libparted/labels/dasd.c
> > index 9edab40..af35707 100644
> > --- a/libparted/labels/dasd.c
> > +++ b/libparted/labels/dasd.c
> > @@ -128,6 +128,7 @@ static PedDiskOps dasd_disk_ops = {
> >
> > alloc_metadata: dasd_alloc_metadata,
> > get_max_primary_partition_count: dasd_get_max_primary_partition_count,
> > + get_max_partition_count: dasd_get_max_primary_partition_count,
> >
> > partition_duplicate: NULL
> > };
> > diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
> > index cb7e45e..3193069 100644
> > --- a/libparted/labels/dos.c
> > +++ b/libparted/labels/dos.c
> > @@ -2213,6 +2213,12 @@ msdos_get_max_primary_partition_count (const PedDisk* disk)
> > return 4;
> > }
> >
> > +static int
> > +msdos_get_max_partition_count(const PedDisk* disk)
> > +{
> > + return 16;
> > +}
> > +
> > static PedDiskOps msdos_disk_ops = {
> > probe: msdos_probe,
> > #ifndef DISCOVER_ONLY
> > @@ -2244,7 +2250,9 @@ static PedDiskOps msdos_disk_ops = {
> >
> > alloc_metadata: msdos_alloc_metadata,
> > get_max_primary_partition_count:
> > - msdos_get_max_primary_partition_count
> > + msdos_get_max_primary_partition_count,
> > + get_max_partition_count:
> > + msdos_get_max_partition_count
> > };
> >
> > static PedDiskType msdos_disk_type = {
> > diff --git a/libparted/labels/dvh.c b/libparted/labels/dvh.c
> > index 8d8ba76..bdea1c7 100644
> > --- a/libparted/labels/dvh.c
> > +++ b/libparted/labels/dvh.c
> > @@ -888,7 +888,8 @@ static PedDiskOps dvh_disk_ops = {
> >
> > alloc_metadata: dvh_alloc_metadata,
> > get_max_primary_partition_count:
> > - dvh_get_max_primary_partition_count
> > + dvh_get_max_primary_partition_count,
> > + get_max_partition_count: dvh_get_max_primary_partition_count
> > };
> >
> > static PedDiskType dvh_disk_type = {
> > diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
> > index eea562d..6c9929b 100644
> > --- a/libparted/labels/gpt.c
> > +++ b/libparted/labels/gpt.c
> > @@ -1470,6 +1470,12 @@ gpt_get_max_primary_partition_count (const PedDisk *disk)
> > return gpt_disk_data->entry_count;
> > }
> >
> > +static int
> > +gpt_get_max_partitoin_count(const PedDisk *disk)
> > +{
> > + return 16;
> > +}
> > +
>
> Are you sure about this? I think GPT has no such on the number of
> partitions.
>
To be honest, have to do a background check on all the hardcoded number
that represent maximum amount of partitions. If someone has a better
value, pls be my guest.
>
> > static PedConstraint*
> > _non_metadata_constraint (const PedDisk* disk)
> > {
> > @@ -1524,7 +1530,8 @@ static PedDiskOps gpt_disk_ops = {
> > partition_align: gpt_partition_align,
> > partition_enumerate: gpt_partition_enumerate,
> > alloc_metadata: gpt_alloc_metadata,
> > - get_max_primary_partition_count: gpt_get_max_primary_partition_count
> > + get_max_primary_partition_count: gpt_get_max_primary_partition_count,
> > + get_max_partition_count: gpt_get_max_partitoin_count
> > };
> >
> > static PedDiskType gpt_disk_type = {
> > diff --git a/libparted/labels/loop.c b/libparted/labels/loop.c
> > index a6a2057..19181ee 100644
> > --- a/libparted/labels/loop.c
> > +++ b/libparted/labels/loop.c
> > @@ -311,6 +311,8 @@ static PedDiskOps loop_disk_ops = {
> >
> > alloc_metadata: loop_alloc_metadata,
> > get_max_primary_partition_count:
> > + loop_get_max_primary_partition_count,
> > + get_max_partition_count:
> > loop_get_max_primary_partition_count
> > };
> >
> > diff --git a/libparted/labels/mac.c b/libparted/labels/mac.c
> > index 3ec8390..8f17e8a 100644
> > --- a/libparted/labels/mac.c
> > +++ b/libparted/labels/mac.c
> > @@ -1563,6 +1563,12 @@ mac_get_max_primary_partition_count (const PedDisk* disk)
> > - mac_disk_data->free_part_entry_count + 1;
> > }
> >
> > +static int
> > +mac_get_max_partitoin_count (const PedDisk* disk)
> > +{
> > + return 65536;
> > +}
> > +
> > static PedDiskOps mac_disk_ops = {
> > probe: mac_probe,
> > #ifndef DISCOVER_ONLY
> > @@ -1596,7 +1602,9 @@ static PedDiskOps mac_disk_ops = {
> >
> > alloc_metadata: mac_alloc_metadata,
> > get_max_primary_partition_count:
> > - mac_get_max_primary_partition_count
> > + mac_get_max_primary_partition_count,
> > + get_max_partition_count:
> > + mac_get_max_partitoin_count
> > };
> >
> > static PedDiskType mac_disk_type = {
> > diff --git a/libparted/labels/pc98.c b/libparted/labels/pc98.c
> > index a7b7e57..05e1e45 100644
> > --- a/libparted/labels/pc98.c
> > +++ b/libparted/labels/pc98.c
> > @@ -863,6 +863,8 @@ static PedDiskOps pc98_disk_ops = {
> >
> > alloc_metadata: pc98_alloc_metadata,
> > get_max_primary_partition_count:
> > + pc98_get_max_primary_partition_count,
> > + get_max_partition_count:
> > pc98_get_max_primary_partition_count
> > };
> >
> > diff --git a/libparted/labels/rdb.c b/libparted/labels/rdb.c
> > index 386b2bd..093d48f 100644
> > --- a/libparted/labels/rdb.c
> > +++ b/libparted/labels/rdb.c
> > @@ -1157,6 +1157,8 @@ static PedDiskOps amiga_disk_ops = {
> >
> > alloc_metadata: amiga_alloc_metadata,
> > get_max_primary_partition_count:
> > + amiga_get_max_primary_partition_count,
> > + get_max_partition_count:
> > amiga_get_max_primary_partition_count
> > };
> >
> > diff --git a/libparted/labels/sun.c b/libparted/labels/sun.c
> > index 76f2b78..befc2fc 100644
> > --- a/libparted/labels/sun.c
> > +++ b/libparted/labels/sun.c
> > @@ -858,6 +858,8 @@ static PedDiskOps sun_disk_ops = {
> > alloc_metadata: sun_alloc_metadata,
> > get_max_primary_partition_count:
> > sun_get_max_primary_partition_count,
> > + get_max_partition_count:
> > + sun_get_max_primary_partition_count,
> >
> > partition_set_name: NULL,
> > partition_get_name: NULL,
> > --
> > 1.6.0.6
> >
> >
> > _______________________________________________
> > parted-devel mailing list
> > parted-devel at lists.alioth.debian.org
> > http://lists.alioth.debian.org/mailman/listinfo/parted-devel
>
> --
> Best regards / s pozdravem
>
> Petr Uzel, Packages maintainer
> ---------------------------------------------------------------------
> SUSE LINUX, s.r.o. e-mail: puzel at suse.cz
> Lihovarská 1060/12 tel: +420 284 028 964
> 190 00 Prague 9 fax: +420 284 028 951
> Czech Republic http://www.suse.cz
>
> _______________________________________________
> parted-devel mailing list
> parted-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/parted-devel
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
More information about the parted-devel
mailing list