Re: [PATCH 1/2] dm: update table type check for dax

[Kani, Toshimitsu]

On Tue, 2016-06-28 at 20:40 -0400, Mike Snitzer wrote:
> On Tue, Jun 28 2016 atÂÂ4:23pm -0400,
> Kani, Toshimitsu <toshi.kani@xxxxxxx> wrote:
> 
> > On Tue, 2016-06-28 at 16:07 -0400, Mike Snitzer wrote:
> > > 
> > > On Tue, Jun 28 2016 atÂÂ3:37pm -0400,
> > > Toshi Kani <toshi.kani@xxxxxxx> wrote:
> > Â:
> > > 
> > > You said in the 0th header: "Patch 1 solves an error when lvremove is
> > > made to a snapshot device."
> > > 
> > > I'm not seeing why this patch 1 fixes anything specific to snapshot
> > > device removal (but I can see why patch 2 makes snapshot creation
> > > "work").ÂÂI'll apply your 2nd patch and see if I can see what you mean.
> > > 
> > > I actually see this error, without either of your 2 proposed patches
> > > applied, when I try to create a snapshot of a DAX capable LV:
> > > 
> > > # lvcreate -s -n snap -L 100M pmem/lv
> > > Â device-mapper: reload ioctl on (253:7) failed: Invalid argument
> > > Â Failed to lock logical volume pmem/lv.
> > > Â Aborting. Manual intervention required.
> > > Jun 28 15:57:28 rhel-storage-02 kernel: device-mapper: ioctl: can't
> > > change device type after initial table load.
> >
> > Yes, patch 2 fixes this error.
> > 
> > I have not looked into why lvremove does this, but lvremove to a snapshot
> > device fails to reload DM table of "<dev>-lvsnap" device (which is marked
> > as DM_TYPE_BIO_BASED) with DM_TYPE_DAX_BIO_BASED. ÂPatch 1 fixes this
> > error.
>
> It looks like a strange intermediate state that lvm2 uses during
> snapshot removal.
> 
> Full listing of snapshot related DM tables (before lvremove):
> 
> pmem-lv-real: 0 6086656 linear 259:0 2048
> pmem-lv: 0 6086656 snapshot-origin 253:5
> pmem-snap-cow: 0 204800 linear 259:0 6088704
> pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8
> 
> When removing this snapshot we're wanting to be left with:
> 
> pmem-lv: 0 6086656 linear 259:0 2048
> 
> I augmented the DM core error to be more expressive, resulting in:
> Â device-mapper: ioctl: 253:7: can't change device type (from 1 to 4) after
> initial table load.
> 
> 1 is DM_TYPE_BIO_BASED and 4 is DM_TYPE_DAX_BIO_BASED -- which makes
> sense given the linear target is DM_TYPE_DAX_BIO_BASED.
> 
> The previous DM table for 253:7 was:
> Â pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8
> 
> The intermediate table that lvm2 is trying to load for 253:7 is:
> Â 0 204800 linear 259:0 6088704
> 
> (this linear target was previously pmem-snap-cow)

Yes, that is consistent with what I saw.

> > 
> > I think it also generally makes sense to allow this case.
>
> You're probably right but I need to think about it a little bit more.

Sounds good.

Thanks!
-Toshi