[dm-devel] [PATCH] fix mirror device creation with lvcreate failed

[NeilBrown]

On Thu, 18 Sep 2014 00:56:33 -0600 "Liuhua Wang" <lwang at suse.com> wrote:

> Hi, Alasdair
> Hi, Neil
> 
> >>> On 9/17/2014 at 07:41 PM, in message
> <20140917114151.GF2407 at agk-dp.fab.redhat.com>, Alasdair G Kergon
> <agk at redhat.com> wrote: 
> > On Tue, Sep 16, 2014 at 11:11:35PM -0600, Liuhua Wang wrote:
> > > There are error messages:
> > > ---------------------
> > > [22798.077877] end_request: I/O error, dev dasdd, sector 2240
> > > [22798.077902] device-mapper: raid: Failed to read superblock of device at 
> > position 1
> > > [23087.875420] type=1006 audit(1404313201.346:28): pid=1596 uid=0 old 
> > auid=4294967295 new auid=0 old ses=4294967295 new ses=27 res=1
> > > [23127.707296] dasd-eckd.d2bf98: 0.0.141d: ERP 0000000036ab1e40 has run out 
> > of retries and failed
> > > ----------------------
> >  
> > OK, so it sounds like Neil's patch will be enough to fix this.
> > Can you test that and confirm it solves the problem?
> > 
> 
> I tested Neil's patch, lvcreate succeed, the lv created can display,
> be written successfully, but only there still are error messages:
> --------------------------------
> 2014-09-18T06:39:52.653239+02:00 s390vsl158 kernel: device-mapper: raid: Superblocks created for new array
> 2014-09-18T06:39:52.663361+02:00 s390vsl158 kernel: md/raid1:mdX: not clean -- starting background reconstruction
> 2014-09-18T06:39:52.663366+02:00 s390vsl158 kernel: md/raid1:mdX: active with 2 out of 2 mirrors
> 2014-09-18T06:39:52.663366+02:00 s390vsl158 kernel: Choosing daemon_sleep default (5 sec)
> 2014-09-18T06:39:52.663367+02:00 s390vsl158 kernel: created bitmap (1 pages) for device mdX
> 2014-09-18T06:39:52.663368+02:00 s390vsl158 kernel: end_request: I/O error, dev dasdb, sector 19202144
> 2014-09-18T06:39:52.663369+02:00 s390vsl158 kernel: md: super_written gets error=-5, uptodate=0
> 2014-09-18T06:39:52.663369+02:00 s390vsl158 kernel: md/raid1:mdX: Disk failure on dm-3, disabling device.
> -------------------------
> 
> So I add also roundup to md_super_write() as following,
> then the above error messages disappears.
> --------------------------
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 1294238..18cd2e7 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -754,6 +754,8 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
>  	struct bio *bio = bio_alloc_mddev(GFP_NOIO, 1, mddev);
>  
>  	bio->bi_bdev = rdev->meta_bdev ? rdev->meta_bdev : rdev->bdev;
> +        size = roundup(size, bdev_logical_block_size(bio->bi_bdev));
> +	
>  	bio->bi_iter.bi_sector = sector;
>  	bio_add_page(bio, page, size, 0);
>  	bio->bi_private = rdev;

I'm not really comfortable with this change.

It is always safe to read more than requested.  Writing more than requested
is quite different.

I think I would rather that rdev->sb_size were rounded up by the code that
sets it.  That would make it clear to anyone reading the code that a full
block was always written.

Jon: how does lvm allocate space for the dm-raid metadata?  Does it always
allocate some minimum?  Is the allocation aware of underlying block size?
Is it reasonable to round up rdev->sb_size in load_super() ??


Thanks,
NeilBrown


> @@ -784,6 +786,10 @@ int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
>  
>  	bio->bi_bdev = (metadata_op && rdev->meta_bdev) ?
>  		rdev->meta_bdev : rdev->bdev;
> +        size = roundup(size, bdev_logical_block_size(bio->bi_bdev));
> +        if (size > PAGE_SIZE)
> +                return -EINVAL;
> +
>  	if (metadata_op)
>  		bio->bi_iter.bi_sector = sector + rdev->sb_start;
>  	else if (rdev->mddev->reshape_position != MaxSector &&

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20140922/04059fc0/attachment.sig>