[parted-devel] Bug: Removal of BLKPG causes regression of ability to manipulate disks with other partitions in use

[Phillip Susi]

On 3/29/2010 10:16 AM, Petr Uzel wrote:
> This would reintroduce problems described in this thread:
> http://lists.alioth.debian.org/pipermail/parted-devel/2010-January/003355.html

I'm not sure I understand this problem.  Transient opens from
udev/hal/whatever should only happen right after we change the partition
table, so this should only ever be a problem if we try to change it,
then immediately alter it again.

It looks like Ubuntu has a patch to invoke udevadm settle.  Perhaps this
is another way of solving that problem?  If this is a good solution then
maybe it should be applied upstream?

> 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()

If we got to this point then linux_partition_is_busy() already checked
and thought that the partition was not mounted.  If it does, then parted
denies the request to remove the partition  before ever trying to sync
the partition table.  If it actually turns out that it is busy, then
perhaps a 1 second sleep followed by a retry might help, though again, I
don't see why this would happen unless we sync the partition table twice
in rapid succession without a call to udevadm settle.

>> +				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?

I suppose I could.  Looking at that it looks like I could also use
LINUX_SPECIFIC(dev)->fd instead of building the name and calling open()
myself.

>> +				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().

I didn't bother checking since I initialize the size and geometry to
zero, so if any of the open or ioctl calls fail, they will still be
zero, which will not match the current start and length, so we throw the
warning anyhow.