Re: dm: Fix report zone remapping

[Mike Snitzer <snitzer@redhat.com>]

On Fri, Oct 05 2018 at  9:24pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> Mike,
> 
> On 2018/10/06 3:29, Mike Snitzer wrote:
> > On Fri, Oct 05 2018 at  6:34am -0400,
> > Damien Le Moal <damien.lemoal@wdc.com> wrote:
> > 
> >> If dm-linear or dm-flakey have targets on top of a partition of a zoned
> >> block device, remapping of the start sector and write pointer position
> >> of the zones reported by a report zones BIO must be modified to not
> >> only account for the target table entry mapping, but also to account
> >> for the partition first sector. This start sector must be substracted
> >> to the start sector of all zones reported. The write pointer position
> >> of sequential zones must also be reduced by this offset.
> >>
> >> Since there is no easy way to access the underlying bdev of the target
> >> table entry from the dm_target or dm_target_io pointers, modify the
> >> interface of the function dm_remap_zone_report() to allow a target to
> >> pass the partition block device starting sector (offset).
> >>
> >> Fixes: 10999307c14e ("dm: introduce dm_remap_zone_report()")
> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >> Cc: <stable@vger.kernel.org>
> > 
> > [dropping the actual stable cc from the mail, please don't cc stable in
> > the mail header, could be that git-send-email just pulled it in but...]
> 
> My apologies about that. It was indeed git-send-email. Will fix that.
> 
> > This needs a different fix.  There should be absolutely no need to pass
> > in a start offset for a device that is already supposed to be described
> > within the 'struct bio' passed to dm_remap_zone_report().
> > 
> > Why can't you just use the bio->bi_bdev ?  Err, since commit
> > 74d46992e0d9d, I mean bio->bi_disk to get the start sector?
> > 
> > You should be able to get it from:
> > 
> > struct hd_struct *part = disk_get_part(bio->bi_disk, bio->bi_partno);
> > part->start_sect;
> > disk_put_part(part);
> > 
> > or:
> > 
> > struct block_device *bdev = bdget_disk(bio->bi_disk, bio->bi_partno);
> > get_start_sect(fc->dev->bdev);
> > bdput(bdev);
> 
> I am afraid this does not work. The original bio disk is of course /dev/dm-xx so
> not the partition (partno == 0), and the mapped bio passed as argument to the
> remap zone function (clone of the orig bio) is pointing to the entire underlying
> disk, not the partition disk used in dmsetup. bi_partno is 0 for both BIOs.
> Which seems weird, __bio_clone_fast() does copy the disk and part number as is.
> The disk is of course correctly changed from the orig dm disk to the actual
> physical disk in the clone, but the partition number stays the same. In the end,
> only the target has the correct bdev (disk+partno) information it seems.

OK, that is a bug that must to be found then.

> It looks like the partno gets lost, which does not seem correct. I must be
> missing something, but I fail to see what it is. Any suggestion ?

Not at the moment but I'll have a closer look early next week.

Thanks,
Mike