Re: [PATCH 6/9] s390/block/dasd_genhd: add error handling support for add_disk()
From: Luis Chamberlain <mcgrof@kernel.org>
Date: 2021-09-13 16:51:54
Also in:
ceph-devel, linux-mmc, linux-s390, lkml
On Mon, Sep 13, 2021 at 02:19:38PM +0200, Jan Höppner wrote:
quoted hunk ↗ jump to hunk
On 13/09/2021 10:17, Jan Höppner wrote:quoted
On 02/09/2021 19:41, Luis Chamberlain wrote:quoted
We never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling. Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- drivers/s390/block/dasd_genhd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c index fa966e0db6ca..ba07022283bc 100644 --- a/drivers/s390/block/dasd_genhd.c +++ b/drivers/s390/block/dasd_genhd.c@@ -33,7 +33,7 @@ int dasd_gendisk_alloc(struct dasd_block *block) { struct gendisk *gdp; struct dasd_device *base; - int len; + int len, rc; /* Make sure the minor for this device exists. */ base = block->base;@@ -79,7 +79,11 @@ int dasd_gendisk_alloc(struct dasd_block *block) dasd_add_link_to_gendisk(gdp, base); block->gdp = gdp; set_capacity(block->gdp, 0); - device_add_disk(&base->cdev->dev, block->gdp, NULL); + + rc = device_add_disk(&base->cdev->dev, block->gdp, NULL); + if (rc) + return rc; +I think, just like with some of the other changes, there is some cleanup required before returning. I'll prepare a patch and come back to you.It's actually just one call that is required. The patch should look like this:diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c index fa966e0db6ca..80673dbfb1f9 100644 --- a/drivers/s390/block/dasd_genhd.c +++ b/drivers/s390/block/dasd_genhd.c@@ -33,7 +33,7 @@ int dasd_gendisk_alloc(struct dasd_block *block) { struct gendisk *gdp; struct dasd_device *base; - int len; + int len, rc; /* Make sure the minor for this device exists. */ base = block->base;@@ -79,7 +79,13 @@ int dasd_gendisk_alloc(struct dasd_block *block) dasd_add_link_to_gendisk(gdp, base); block->gdp = gdp; set_capacity(block->gdp, 0); - device_add_disk(&base->cdev->dev, block->gdp, NULL); + + rc = device_add_disk(&base->cdev->dev, block->gdp, NULL); + if (rc) { + dasd_gendisk_free(block); + return rc; + } +
Thanks! Would you like to to fold this fix into my patch and resend eventually? Or will you send a replacement? Luis