[PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[Josh Hunt]

We found with newer kernels we started seeing the cdrom device showing
up in /proc/partitions, but it was not there before. Looking into this I found
that commit d27769ec... block: add GENHD_FL_NO_PART_SCAN introduces this change
in behavior. It's not clear to me from the commit's changelog if this change was
intentional or not. This comment still remains:
/* Don't show non-partitionable removeable devices or empty devices */
so I've decided to send a patch to restore the behavior of not printing
unpartitionable removable devices.

Thanks
Josh
---

After commit d27769ec... block: add GENHD_FL_NO_PART_SCAN, /proc/partitions
started printing removable devices with only one partition. This is different
than prior to the commit. This restores /proc/partitions to behave as it did before.

Signed-off-by: Josh Hunt <johunt@akamai.com>
---
 block/genhd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 6cace66..6bfeb2a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -851,7 +851,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	char buf[BDEVNAME_SIZE];
 
 	/* Don't show non-partitionable removeable devices or empty devices */
-	if (!get_capacity(sgp) || (!disk_max_parts(sgp) &&
+	if (!get_capacity(sgp) || (!(disk_max_parts(sgp) > 1) &&
 				   (sgp->flags & GENHD_FL_REMOVABLE)))
 		return 0;
 	if (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
-- 
1.7.0.4

Re: [PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[Josh Hunt]

ping?

On 11/19/2012 08:56 PM, Josh Hunt wrote:
> We found with newer kernels we started seeing the cdrom device showing
> up in /proc/partitions, but it was not there before. Looking into this I found
> that commit d27769ec... block: add GENHD_FL_NO_PART_SCAN introduces this change
> in behavior. It's not clear to me from the commit's changelog if this change was
> intentional or not. This comment still remains:
> /* Don't show non-partitionable removeable devices or empty devices */
> so I've decided to send a patch to restore the behavior of not printing
> unpartitionable removable devices.
>
> Thanks
> Josh
> ---
>
> After commit d27769ec... block: add GENHD_FL_NO_PART_SCAN, /proc/partitions
> started printing removable devices with only one partition. This is different
> than prior to the commit. This restores /proc/partitions to behave as it did before.
>
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> ---
>   block/genhd.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/block/genhd.c b/block/genhd.c
> index 6cace66..6bfeb2a 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -851,7 +851,7 @@ static int show_partition(struct seq_file *seqf, void *v)
>   	char buf[BDEVNAME_SIZE];
>
>   	/* Don't show non-partitionable removeable devices or empty devices */
> -	if (!get_capacity(sgp) || (!disk_max_parts(sgp) &&
> +	if (!get_capacity(sgp) || (!(disk_max_parts(sgp) > 1) &&
>   				   (sgp->flags & GENHD_FL_REMOVABLE)))
>   		return 0;
>   	if (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
>

Re: [PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[Andrew Morton]

On Mon, 19 Nov 2012 18:56:49 -0800
Josh Hunt <johunt@akamai.com> wrote:

> We found with newer kernels we started seeing the cdrom device showing
> up in /proc/partitions, but it was not there before. Looking into this I found
> that commit d27769ec... block: add GENHD_FL_NO_PART_SCAN introduces this change
> in behavior. It's not clear to me from the commit's changelog if this change was
> intentional or not. This comment still remains:
> /* Don't show non-partitionable removeable devices or empty devices */
> so I've decided to send a patch to restore the behavior of not printing
> unpartitionable removable devices.

d27769ec was merged in August 2011, so I after all this time, your fix
could be viewed as "changing existing behaviour".

So perhaps it would be best to leave things alone.  Is there any
particular problem with the post-Aug, 2011 behaviour?

Re: [PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[Josh Hunt]

On 12/03/2012 06:06 PM, Andrew Morton wrote:
> On Mon, 19 Nov 2012 18:56:49 -0800
> Josh Hunt <johunt@akamai.com> wrote:
>
>> We found with newer kernels we started seeing the cdrom device showing
>> up in /proc/partitions, but it was not there before. Looking into this I found
>> that commit d27769ec... block: add GENHD_FL_NO_PART_SCAN introduces this change
>> in behavior. It's not clear to me from the commit's changelog if this change was
>> intentional or not. This comment still remains:
>> /* Don't show non-partitionable removeable devices or empty devices */
>> so I've decided to send a patch to restore the behavior of not printing
>> unpartitionable removable devices.
>
> d27769ec was merged in August 2011, so I after all this time, your fix
> could be viewed as "changing existing behaviour".
>
> So perhaps it would be best to leave things alone.  Is there any
> particular problem with the post-Aug, 2011 behaviour?
>

We caught this by a script that parses /proc/partitions and made some 
assumptions about the contents therein. It had worked fine up until when 
this behavior changed. We were able to modify our script to get what we 
needed.

The patch was meant to do two things: 1) understand if this was an 
unintended change and 2) if so, propose a solution to resolve it. Since 
the comment was left in the source I believe either a) my patch should 
be applied or b) a new patch with the comment removed should be put in 
since it's no longer correct. I did not think this type of change to 
kernel abi was generally acceptable.

While the commit is over a year old, it changes behavior which had been 
in tact for a while (years?) from what I can tell. We were running 3.0 
with stable updates until we upgraded to 3.2 and hit this. Neither of 
these are what I would consider "old" kernels.

Thanks for looking at this.

Josh

Re: [PATCH] block: Restore /proc/partitions to not display non-partitionable removable devices

[Andrew Morton]

On Mon, 03 Dec 2012 18:40:32 -0600
Josh Hunt <johunt@akamai.com> wrote:

> On 12/03/2012 06:06 PM, Andrew Morton wrote:
> > On Mon, 19 Nov 2012 18:56:49 -0800
> > Josh Hunt <johunt@akamai.com> wrote:
> >
> >> We found with newer kernels we started seeing the cdrom device showing
> >> up in /proc/partitions, but it was not there before. Looking into this I found
> >> that commit d27769ec... block: add GENHD_FL_NO_PART_SCAN introduces this change
> >> in behavior. It's not clear to me from the commit's changelog if this change was
> >> intentional or not. This comment still remains:
> >> /* Don't show non-partitionable removeable devices or empty devices */
> >> so I've decided to send a patch to restore the behavior of not printing
> >> unpartitionable removable devices.
> >
> > d27769ec was merged in August 2011, so I after all this time, your fix
> > could be viewed as "changing existing behaviour".
> >
> > So perhaps it would be best to leave things alone.  Is there any
> > particular problem with the post-Aug, 2011 behaviour?
> >
> 
> We caught this by a script that parses /proc/partitions and made some 
> assumptions about the contents therein. It had worked fine up until when 
> this behavior changed. We were able to modify our script to get what we 
> needed.
> 
> The patch was meant to do two things: 1) understand if this was an 
> unintended change and 2) if so, propose a solution to resolve it. Since 
> the comment was left in the source I believe either a) my patch should 
> be applied or b) a new patch with the comment removed should be put in 
> since it's no longer correct. I did not think this type of change to 
> kernel abi was generally acceptable.
> 
> While the commit is over a year old, it changes behavior which had been 
> in tact for a while (years?) from what I can tell. We were running 3.0 
> with stable updates until we upgraded to 3.2 and hit this. Neither of 
> these are what I would consider "old" kernels.
> 

Yes, this is difficult.  Removing existing entries is more likely to
cause damage than adding new ones, so I suspect the safest approach is
to just leave things as they now are.

In which case yes, we should repair that comment.  ie: change it to a
comment which explains *why* we display removable devices.  Unlike the
existing comment which tells us "what" but not "why", when "why" is
what we wanted to know, sigh.