Hi all, On Fri, Mar 26, 2010 at 11:19:37AM -0400, Phillip Susi wrote: > Attached are the two patches I have come up with. They are self > described. I have already been asked by Colin Watson to clean it up a > bit so the coding style conforms ot the surrounding code, which I will > be doing tomorrow. Please let me know what you think about them. [...] > From: Phillip Susi <[email protected]> > Description: In the event that some partitions are in use, > they can not be modified in the running kernel usign BLKPG. > Warn the user that this is the case and advise them to reboot, > just like we do when BLKRRPART fails for the same reason, unless > the partition in question is unchanged. > Index: parted-2.2/libparted/arch/linux.c > =================================================================== > --- parted-2.2.orig/libparted/arch/linux.c 2010-03-25 20:19:29.407917597 > -0400 > +++ parted-2.2/libparted/arch/linux.c 2010-03-25 20:22:08.487917004 -0400 > @@ -2411,7 +2411,9 @@ > * Sync the partition table in two step process: > * 1. Remove all of the partitions from the kernel's tables, but do not > attempt > * removal of any partition for which the corresponding ioctl call fails. > - * 2. Add all the partitions that we hold in disk. > + * 2. Add all the partitions that we hold in disk, throwing a warning > + * if we can not beacuse step 1 failed to remove it and it is not being > + * added back with the same start and length. > * > * To achieve this two step process we must calculate the minimum number of > * maximum possible partitions between what linux supports and what the label > @@ -2444,24 +2446,46 @@ > int i; > > for (i = 1; i <= lpn; i++) { > - rets[i - 1] = _blkpg_remove_partition (disk, i); > - errnums[i - 1] = errno; > + rets[i - 1] = _blkpg_remove_partition (disk, i); > + errnums[i - 1] = errno;
This would reintroduce problems described in this thread: http://lists.alioth.debian.org/pipermail/parted-devel/2010-January/003355.html I'm not sure the solution is possible, but I'd suggest to do something like the following: - if the BLKPG_DEL_PART ioctl returns EBUSY, then - check whether the partition is busy (linux_partition_is_busy() ) - if it is not busy, then wait for some specific time and retry the BLKPG_DEL_PART ioctl() The idea behind this is not to fail if something (like HAL) opens the device for a short period of time, but do not unnecessarily wait if the partition is mounted. > } > > for (i = 1; i <= lpn; i++) { > - const PedPartition *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; > - } > + const PedPartition *part = ped_disk_get_partition (disk, i); > + if (part) { > + if (!rets[i - 1] && errnums[i - 1] == EBUSY) > + { > + struct hd_geometry geom; > + unsigned long long length; > + int fd; > + char *dev_name; > + > + memset( &geom, 0, sizeof( geom ) ); > + length = 0; > + /* get start and length of existing partition */ > + dev_name = _device_get_part_path (disk->dev, i); > + fd = open( dev_name, O_RDONLY ); > + ioctl( fd, HDIO_GETGEO, &geom ); > + ioctl( fd, BLKGETSIZE64, &length ); > + length /= disk->dev->sector_size; Why not use _device_get_length() here? > + close( fd ); I think there should be some checks of return values - if it is not possible to open the device for whatever reason, or issue the ioctl()s, then the code should jump directly to ped_exception_throw(). > + if( geom.start != part->geom.start || > + length != part->geom.length ) > + ped_exception_throw( > + PED_EXCEPTION_WARNING, > + PED_EXCEPTION_IGNORE, > + _("WARNING: partition %d on %s could > not be modified, probably " > + "because it is in use. As a result, > the old partition " > + "will remain in use until after > reboot. You should reboot " > + "now before making further changes."), > + i, disk->dev->path); > + continue; > + } > + > + /* add the (possibly modified or new) partition */ > + if (!_blkpg_add_partition (disk, part)) > + ret = 0; > + } > } > > free (rets); Petr -- Petr Uzel, openSUSE Boosters Team IRC: ptr_uzl @ freenode
pgpTINd70ClEy.pgp
Description: PGP signature
_______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
