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

[Curtis Gedak]

Hi Jim,

The patch as packaged with description looks good to me.  :-)

Curtis

Jim Meyering wrote:
> Curtis Gedak wrote:
>   
>> Curtis Gedak wrote:
>>     
>>> Jim Meyering wrote:
>>>       
>>>> Curtis Gedak wrote:
>>>>
>>>>         
>>>>> This patch is ready for your review.  Perhaps you might consider
>>>>> including this patch in the parted-2.2 release.
>>>>>
>>>>>           
>>>> However, would you please write a high-level summary that I can add to
>>>> the "Bug fixes" section of the NEWS file, since this does indeed solve
>>>> a problem people have encountered.
>>>>
>>>>         
>> Hi Jim,
>>
>> Following is an improved bug fix description for NEWS.  This version
>> is less ambiguous in my opinion.  :)
>>     
>
> Thank you.
> I'll push this shortly.
>
> From 0a21f0b7ed7ff0e536a5c30dfe1910c33d2ca243 Mon Sep 17 00:00:00 2001
> From: Curtis Gedak <gedakc at gmail.com>
> Date: Mon, 22 Feb 2010 09:13:53 +0100
> Subject: [PATCH] linux: add wait time and retries to kernel partition reread
>
> Occasionally when using parted with newer GNU/Linux kernels (2.6.31)
> and udev (145), the kernel would fail to reread the partition table.
> This could lead to problems with subsequent actions such as formatting
> the partition.
>
> Basically this patch increases the retry_count, and adds one sleep(1)
> function call prior to the the last few ioctl() calls.
>
> This patch might not be a perfect solution to this problem of "failure
> to inform kernel of partition changes", but it does significantly
> reduce the likelihood of encountering the problem.
>
> Details on the testing results and procedures used can be found at
> the following link:
> https://bugzilla.gnome.org/show_bug.cgi?id=604298#c9
>
> * libparted/arch/linux.c (_kernel_reread_part_table): Sleep for a
> full second if retry_count makes it down to "3".  Start it at 9
> rather than at 5.
> * NEWS (Bug fixes):
> ---
>  NEWS                   |    9 +++++++++
>  libparted/arch/linux.c |    7 +++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 28f87de..8e80746 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -29,6 +29,15 @@ GNU parted NEWS                                    -*- outline -*-
>
>    "make install" no longer installs tests programs named disk and label
>
> +  libparted: try harder to inform kernel of partition changes.
> +  Previously when editing partitions, occasionally the kernel would
> +  fail to be informed of partition changes.  When this happened future
> +  problems would occur because the kernel had incorrect information.
> +  For example, if this problem arose when resizing or creating a
> +  new partition, then an incorrect partition size might be displayed
> +  or a user might encounter a failure to format or delete a newly
> +  created partition, respectively.
> +
>
>  * Noteworthy changes in release 2.1 (2009-12-20) [stable]
>
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index a083028..49ab08c 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -1,5 +1,5 @@
>  /* libparted - a library for manipulating disk partitions
> -    Copyright (C) 1999 - 2005, 2007, 2008, 2009 Free Software Foundation, Inc.
> +    Copyright (C) 1999-2010 Free Software Foundation, Inc.
>
>      This program is free software; you can redistribute it and/or modify
>      it under the terms of the GNU General Public License as published by
> @@ -2521,12 +2521,15 @@ 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)
> +                        sleep(1); /* Pause to allow system to settle */
> +
>                  if (!retry_count) {
>                          ped_exception_throw (
>                                  PED_EXCEPTION_WARNING,
> --
> 1.7.0.262.gef208
>