Re: [PATCH 1/3] block: Return error in rescan_partitions if revalidating disk failed

2015-04-21 Thread James Bottomley 
On Tue, 2015-03-24 at 18:16 +0800, Fam Zheng wrote:
 If the disk can't read capacity, we should return an error.

I'm not sure I buy this: you need to justify why.

For instance removable media return an error in revalidate that causes
the medium not present flag to be set ... do we really want to propagate
the error in that case?

 Signed-off-by: Fam Zheng f...@redhat.com
 ---
  block/partition-generic.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/block/partition-generic.c b/block/partition-generic.c
 index 0d9e5f9..1e60d7d 100644
 --- a/block/partition-generic.c
 +++ b/block/partition-generic.c
 @@ -427,11 +427,11 @@ rescan:
   return res;
  
   if (disk-fops-revalidate_disk)
 - disk-fops-revalidate_disk(disk);
 + res = disk-fops-revalidate_disk(disk);

You're assuming here that a negative error code is being returned, but
some (cciss) seem to be returning 1 on error.

You'll need to assure us that you've checked all the consumers before
making this change.

James

   check_disk_size_change(disk, bdev);
   bdev-bd_invalidated = 0;
 - if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
 - return 0;
 + if (res || !get_capacity(disk) || !(state = check_partition(disk, 
 bdev)))
 + return res;
   if (IS_ERR(state)) {
   /*
* I/O error reading the partition table.  If any



--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] block: Return error in rescan_partitions if revalidating disk failed

2015-04-21 Thread Fam Zheng 
On Tue, 04/21 19:58, James Bottomley wrote:
 On Tue, 2015-03-24 at 18:16 +0800, Fam Zheng wrote:
  If the disk can't read capacity, we should return an error.
 
 I'm not sure I buy this: you need to justify why.
 
 For instance removable media return an error in revalidate that causes
 the medium not present flag to be set ... do we really want to propagate
 the error in that case?

I think we should. There is a command failed, and the user won't get a valid
disk size.

 
  Signed-off-by: Fam Zheng f...@redhat.com
  ---
   block/partition-generic.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/block/partition-generic.c b/block/partition-generic.c
  index 0d9e5f9..1e60d7d 100644
  --- a/block/partition-generic.c
  +++ b/block/partition-generic.c
  @@ -427,11 +427,11 @@ rescan:
  return res;
   
  if (disk-fops-revalidate_disk)
  -   disk-fops-revalidate_disk(disk);
  +   res = disk-fops-revalidate_disk(disk);
 
 You're assuming here that a negative error code is being returned, but
 some (cciss) seem to be returning 1 on error.
 
 You'll need to assure us that you've checked all the consumers before
 making this change.

I'll look at the return values.

Fam

 
 James
 
  check_disk_size_change(disk, bdev);
  bdev-bd_invalidated = 0;
  -   if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
  -   return 0;
  +   if (res || !get_capacity(disk) || !(state = check_partition(disk, 
  bdev)))
  +   return res;
  if (IS_ERR(state)) {
  /*
   * I/O error reading the partition table.  If any
 
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] block: Return error in rescan_partitions if revalidating disk failed

2015-03-24 Thread Fam Zheng 
If the disk can't read capacity, we should return an error.

Signed-off-by: Fam Zheng f...@redhat.com
---
 block/partition-generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/partition-generic.c b/block/partition-generic.c
index 0d9e5f9..1e60d7d 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -427,11 +427,11 @@ rescan:
return res;
 
if (disk-fops-revalidate_disk)
-   disk-fops-revalidate_disk(disk);
+   res = disk-fops-revalidate_disk(disk);
check_disk_size_change(disk, bdev);
bdev-bd_invalidated = 0;
-   if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
-   return 0;
+   if (res || !get_capacity(disk) || !(state = check_partition(disk, 
bdev)))
+   return res;
if (IS_ERR(state)) {
/*
 * I/O error reading the partition table.  If any
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html