[parted-devel] Possible Race Condition using test code, libparted, and Fedora 12

[Curtis Gedak]

Hi Petr,

Petr Uzel wrote:
> On Wed, Jan 27, 2010 at 01:51:11PM -0700, Curtis Gedak wrote:
>   
>> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
>> index aefe788..1147b1e 100644
>> --- a/libparted/arch/linux.c
>> +++ b/libparted/arch/linux.c
>> @@ -2518,12 +2518,14 @@ static int
>> _kernel_reread_part_table (PedDevice* dev)
>> {
>>         LinuxSpecific*  arch_specific = LINUX_SPECIFIC (dev);
>> -        int             retry_count = 5;
>> +        int             retry_count = 9;
>>
>>         sync();
>>         while (ioctl (arch_specific->fd, BLKRRPART)) {
>>                 retry_count--;
>>                 sync();
>> +                if (retry_count == 3)
>>     
>
>                    ^^^^^^^^^^^^^^^^^^^^^
> Don't you mean something like 'if (retry_count % 3 == 0)' 
> instead? That would pause on the 3rd, 6th and 9th (last) iteration.
>   

My original patch did try to do something like you suggest.  After 
analyzing the patch, I realized that it only paused once in all of the 
10 iterations.  This single pause was sufficient to catch all of the 
"failed to inform kernel of partition changes" problems that occurred 
during my testing.

Hence this time I did intend that the code only pause once for 1 
second.  :-)

My thoughts are that 1 second is a very long time for a computer.  If 
more than an single 1 second pause is used, then in my opinion this 
would unnecessarily increase the amount of time required until the call 
would finally fail.  For example this function call would still fail 
when used on a device with at least one partition mounted.  I did not 
want this to creep up to 2 or 3 seconds without significant benefits to 
the user.

In many ways this patch is a balancing act between having the system do 
the right thing, and not making a user wait unnecessarily.

>   
>> +                        sleep(1); /* Pause to allow system to settle */
>>                 if (!retry_count) {
>>                         ped_exception_throw (
>>                                 PED_EXCEPTION_WARNING,
>>
>>     
Regards,
Curtis Gedak