From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ipmail05.adl6.internode.on.net ([150.101.137.143]:54187 "EHLO ipmail05.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751143AbcGTXyg (ORCPT ); Wed, 20 Jul 2016 19:54:36 -0400 Date: Thu, 21 Jul 2016 09:54:33 +1000 From: Dave Chinner Subject: Re: [PATCH 6/6] xfstests: Add mkfs input validation tests Message-ID: <20160720235433.GA12670@dastard> References: <1468500214-6237-1-git-send-email-jtulak@redhat.com> <1468500214-6237-7-git-send-email-jtulak@redhat.com> <20160716093358.GL2432@eguan.usersys.redhat.com> <20160717233003.GX1922@dastard> <20160718114723.GG27776@eguan.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: fstests-owner@vger.kernel.org Content-Transfer-Encoding: quoted-printable To: Jan Tulak Cc: Eryu Guan , fstests@vger.kernel.org, xfs-oss List-ID: On Mon, Jul 18, 2016 at 02:33:29PM +0200, Jan Tulak wrote: > On Mon, Jul 18, 2016 at 1:54 PM, Jan Tulak wrote: > > On Mon, Jul 18, 2016 at 1:47 PM, Eryu Guan wrote: > >> On Mon, Jul 18, 2016 at 01:29:47PM +0200, Jan Tulak wrote: > >>> On Mon, Jul 18, 2016 at 1:30 AM, Dave Chinner = wrote: > >>> > On Sat, Jul 16, 2016 at 05:33:58PM +0800, Eryu Guan wrote: > >>> >> On Thu, Jul 14, 2016 at 02:43:34PM +0200, Jan Tulak wrote: > >>> >> > +do_mkfs_fail -l lazy-count=3D1garbage $SCRATCH_DEV > >>> >> > +do_mkfs_fail -l lazy-count=3D2 $SCRATCH_DEV > >>> >> > +do_mkfs_fail -l lazy-count=3D0 -m crc=3D1 $SCRATCH_DEV > >>> >> > +do_mkfs_fail -l version=3D1 -m crc=3D1 $SCRATCH_DEV > >>> >> > >>> >> This test fails in my DAX testing, where SCRATCH_DEV is ramdisk.= The > >>> >> mkfs itself should fail, but it passed. Log version 2 was used > >>> >> automatically, instead of prompting "V2 logs always enabled for = CRC > >>> >> enabled filesytems" > >>> >> > >>> >> [root@dhcp-66-86-11 xfstests]# mkfs -t xfs -f -l version=3D1 -m = crc=3D1 /dev/ram0 > >>> >> meta-data=3D/dev/ram0 isize=3D512 agcount=3D1, a= gsize=3D4096 blks > >>> >> =3D sectsz=3D4096 attr=3D2, proj= id32bit=3D1 > >>> >> =3D crc=3D1 finobt=3D1, sp= arse=3D0 > >>> >> data =3D bsize=3D4096 blocks=3D4096,= imaxpct=3D25 > >>> >> =3D sunit=3D0 swidth=3D0 blk= s > >>> >> naming =3Dversion 2 bsize=3D4096 ascii-ci=3D0 f= type=3D1 > >>> >> log =3Dinternal log bsize=3D4096 blocks=3D1605,= version=3D2 > >>> >> =3D sectsz=3D4096 sunit=3D1 blks= , lazy-count=3D1 > >>> >> realtime =3Dnone extsz=3D4096 blocks=3D0, rt= extents=3D0 > >>> >> > >>> >> Is it a mkfs.xfs bug or the test case should handle the special = case? > >>> > > >>> > Looks like it might be a side effect of using a 4k sector size. v= 1 > >>> > logs only supported 512 byte sectors, so it's entirely possible t= hat > >>> > the sector size is silently overriding the log version > >>> > specification. Probably should be fixed in mkfs. > >>> > > >>> > > >>> > >>> I tried to duplicate this, but in my config it didn't failed - how = did > >>> you create the ramdisk? > >> > >> I think you need to test on a 4k sector size disk. I use scsi_debug = to > >> simulate physical 4k sector disk to reproduce this: > >> > >> [root@dhcp-66-86-11 xfsprogs-dev]# modprobe -r scsi_debug > >> [root@dhcp-66-86-11 xfsprogs-dev]# modprobe scsi_debug dev_size_mb=3D= 128 physblk_exp=3D3 > >> [root@dhcp-66-86-11 xfsprogs-dev]# blockdev --getbsz --getpbsz --get= ss /dev/sdc > >> 4096 > >> 4096 > >> 512 > >> [root@dhcp-66-86-11 xfsprogs-dev]# mkfs -t xfs -l version=3D1 -m crc= =3D1 /dev/sdc So this is an invalid filesystem configuration. It should be detected as such during command line parsing and rejected before we get anywhere near checking topology constraints. In mkfs terms, it's a conflicting option set. > And the culprit is in mkfs, some forty lines before the crc & log versi= on check: >=20 > 2026 =E2=87=A5 } else if (lsectorsize > XFS_MIN_SECTORSIZE && !ls= u && !lsunit) { > 2027 =E2=87=A5 =E2=87=A5 lsu =3D blocksize; > 2028 =E2=87=A5 =E2=87=A5 sb_feat.log_version =3D 2; > 2029 =E2=87=A5 } >=20 > The possible solutions I can think of are: None of which really appeal because, IMO, they are trying to solve the wrong problem. The whole point of moving to table based command line option parsing is that we can encode these sorts of conflicts into the option table. The conflict resolution in the option table is currently not complete - it can only encode and detect conflicts within a suboption type, but not across suboption types (e.g. within -d suboptions, but not between -d and -l suboptions). This is simply because I never got as far as implementing this level of conflict encoding/resolution. In essence, the conflict array needs to define the sub option type, the suboption that is in conflict and the value that it conflicts against. Hence the conflicts table can then encode such things as "version 1 logs are invalid for CRC enabled filesystems" and vice versa. Cheers, Dave. --=20 Dave Chinner david@fromorbit.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay3.corp.sgi.com [198.149.34.15]) by oss.sgi.com (Postfix) with ESMTP id 371E17CA0 for ; Wed, 20 Jul 2016 18:54:38 -0500 (CDT) Received: from cuda.sgi.com (cuda1.sgi.com [192.48.157.11]) by relay3.corp.sgi.com (Postfix) with ESMTP id BE16EAC003 for ; Wed, 20 Jul 2016 16:54:37 -0700 (PDT) Received: from ipmail05.adl6.internode.on.net (ipmail05.adl6.internode.on.net [150.101.137.143]) by cuda.sgi.com with ESMTP id hEZVFaxLtiiFZKYL for ; Wed, 20 Jul 2016 16:54:35 -0700 (PDT) Date: Thu, 21 Jul 2016 09:54:33 +1000 From: Dave Chinner Subject: Re: [PATCH 6/6] xfstests: Add mkfs input validation tests Message-ID: <20160720235433.GA12670@dastard> References: <1468500214-6237-1-git-send-email-jtulak@redhat.com> <1468500214-6237-7-git-send-email-jtulak@redhat.com> <20160716093358.GL2432@eguan.usersys.redhat.com> <20160717233003.GX1922@dastard> <20160718114723.GG27776@eguan.usersys.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Jan Tulak Cc: Eryu Guan , fstests@vger.kernel.org, xfs-oss T24gTW9uLCBKdWwgMTgsIDIwMTYgYXQgMDI6MzM6MjlQTSArMDIwMCwgSmFuIFR1bGFrIHdyb3Rl Ogo+IE9uIE1vbiwgSnVsIDE4LCAyMDE2IGF0IDE6NTQgUE0sIEphbiBUdWxhayA8anR1bGFrQHJl ZGhhdC5jb20+IHdyb3RlOgo+ID4gT24gTW9uLCBKdWwgMTgsIDIwMTYgYXQgMTo0NyBQTSwgRXJ5 dSBHdWFuIDxlZ3VhbkByZWRoYXQuY29tPiB3cm90ZToKPiA+PiBPbiBNb24sIEp1bCAxOCwgMjAx NiBhdCAwMToyOTo0N1BNICswMjAwLCBKYW4gVHVsYWsgd3JvdGU6Cj4gPj4+IE9uIE1vbiwgSnVs IDE4LCAyMDE2IGF0IDE6MzAgQU0sIERhdmUgQ2hpbm5lciA8ZGF2aWRAZnJvbW9yYml0LmNvbT4g d3JvdGU6Cj4gPj4+ID4gT24gU2F0LCBKdWwgMTYsIDIwMTYgYXQgMDU6MzM6NThQTSArMDgwMCwg RXJ5dSBHdWFuIHdyb3RlOgo+ID4+PiA+PiBPbiBUaHUsIEp1bCAxNCwgMjAxNiBhdCAwMjo0Mzoz NFBNICswMjAwLCBKYW4gVHVsYWsgd3JvdGU6Cj4gPj4+ID4+ID4gK2RvX21rZnNfZmFpbCAtbCBs YXp5LWNvdW50PTFnYXJiYWdlICRTQ1JBVENIX0RFVgo+ID4+PiA+PiA+ICtkb19ta2ZzX2ZhaWwg LWwgbGF6eS1jb3VudD0yICRTQ1JBVENIX0RFVgo+ID4+PiA+PiA+ICtkb19ta2ZzX2ZhaWwgLWwg bGF6eS1jb3VudD0wIC1tIGNyYz0xICRTQ1JBVENIX0RFVgo+ID4+PiA+PiA+ICtkb19ta2ZzX2Zh aWwgLWwgdmVyc2lvbj0xIC1tIGNyYz0xICRTQ1JBVENIX0RFVgo+ID4+PiA+Pgo+ID4+PiA+PiBU aGlzIHRlc3QgZmFpbHMgaW4gbXkgREFYIHRlc3RpbmcsIHdoZXJlIFNDUkFUQ0hfREVWIGlzIHJh bWRpc2suIFRoZQo+ID4+PiA+PiBta2ZzIGl0c2VsZiBzaG91bGQgZmFpbCwgYnV0IGl0IHBhc3Nl ZC4gTG9nIHZlcnNpb24gMiB3YXMgdXNlZAo+ID4+PiA+PiBhdXRvbWF0aWNhbGx5LCBpbnN0ZWFk IG9mIHByb21wdGluZyAiVjIgbG9ncyBhbHdheXMgZW5hYmxlZCBmb3IgQ1JDCj4gPj4+ID4+IGVu YWJsZWQgZmlsZXN5dGVtcyIKPiA+Pj4gPj4KPiA+Pj4gPj4gW3Jvb3RAZGhjcC02Ni04Ni0xMSB4 ZnN0ZXN0c10jIG1rZnMgLXQgeGZzIC1mIC1sIHZlcnNpb249MSAtbSBjcmM9MSAvZGV2L3JhbTAK PiA+Pj4gPj4gbWV0YS1kYXRhPS9kZXYvcmFtMCAgICAgICAgICAgICAgaXNpemU9NTEyICAgIGFn Y291bnQ9MSwgYWdzaXplPTQwOTYgYmxrcwo+ID4+PiA+PiAgICAgICAgICA9ICAgICAgICAgICAg ICAgICAgICAgICBzZWN0c3o9NDA5NiAgYXR0cj0yLCBwcm9qaWQzMmJpdD0xCj4gPj4+ID4+ICAg ICAgICAgID0gICAgICAgICAgICAgICAgICAgICAgIGNyYz0xICAgICAgICBmaW5vYnQ9MSwgc3Bh cnNlPTAKPiA+Pj4gPj4gZGF0YSAgICAgPSAgICAgICAgICAgICAgICAgICAgICAgYnNpemU9NDA5 NiAgIGJsb2Nrcz00MDk2LCBpbWF4cGN0PTI1Cj4gPj4+ID4+ICAgICAgICAgID0gICAgICAgICAg ICAgICAgICAgICAgIHN1bml0PTAgICAgICBzd2lkdGg9MCBibGtzCj4gPj4+ID4+IG5hbWluZyAg ID12ZXJzaW9uIDIgICAgICAgICAgICAgIGJzaXplPTQwOTYgICBhc2NpaS1jaT0wIGZ0eXBlPTEK PiA+Pj4gPj4gbG9nICAgICAgPWludGVybmFsIGxvZyAgICAgICAgICAgYnNpemU9NDA5NiAgIGJs b2Nrcz0xNjA1LCB2ZXJzaW9uPTIKPiA+Pj4gPj4gICAgICAgICAgPSAgICAgICAgICAgICAgICAg ICAgICAgc2VjdHN6PTQwOTYgIHN1bml0PTEgYmxrcywgbGF6eS1jb3VudD0xCj4gPj4+ID4+IHJl YWx0aW1lID1ub25lICAgICAgICAgICAgICAgICAgIGV4dHN6PTQwOTYgICBibG9ja3M9MCwgcnRl eHRlbnRzPTAKPiA+Pj4gPj4KPiA+Pj4gPj4gSXMgaXQgYSBta2ZzLnhmcyBidWcgb3IgdGhlIHRl c3QgY2FzZSBzaG91bGQgaGFuZGxlIHRoZSBzcGVjaWFsIGNhc2U/Cj4gPj4+ID4KPiA+Pj4gPiBM b29rcyBsaWtlIGl0IG1pZ2h0IGJlIGEgc2lkZSBlZmZlY3Qgb2YgdXNpbmcgYSA0ayBzZWN0b3Ig c2l6ZS4gdjEKPiA+Pj4gPiBsb2dzIG9ubHkgc3VwcG9ydGVkIDUxMiBieXRlIHNlY3RvcnMsIHNv IGl0J3MgZW50aXJlbHkgcG9zc2libGUgdGhhdAo+ID4+PiA+IHRoZSBzZWN0b3Igc2l6ZSBpcyBz aWxlbnRseSBvdmVycmlkaW5nIHRoZSBsb2cgdmVyc2lvbgo+ID4+PiA+IHNwZWNpZmljYXRpb24u IFByb2JhYmx5IHNob3VsZCBiZSBmaXhlZCBpbiBta2ZzLgo+ID4+PiA+Cj4gPj4+ID4KPiA+Pj4K PiA+Pj4gSSB0cmllZCB0byBkdXBsaWNhdGUgdGhpcywgYnV0IGluIG15IGNvbmZpZyBpdCBkaWRu J3QgZmFpbGVkIC0gaG93IGRpZAo+ID4+PiB5b3UgY3JlYXRlIHRoZSByYW1kaXNrPwo+ID4+Cj4g Pj4gSSB0aGluayB5b3UgbmVlZCB0byB0ZXN0IG9uIGEgNGsgc2VjdG9yIHNpemUgZGlzay4gSSB1 c2Ugc2NzaV9kZWJ1ZyB0bwo+ID4+IHNpbXVsYXRlIHBoeXNpY2FsIDRrIHNlY3RvciBkaXNrIHRv IHJlcHJvZHVjZSB0aGlzOgo+ID4+Cj4gPj4gW3Jvb3RAZGhjcC02Ni04Ni0xMSB4ZnNwcm9ncy1k ZXZdIyBtb2Rwcm9iZSAtciBzY3NpX2RlYnVnCj4gPj4gW3Jvb3RAZGhjcC02Ni04Ni0xMSB4ZnNw cm9ncy1kZXZdIyBtb2Rwcm9iZSBzY3NpX2RlYnVnIGRldl9zaXplX21iPTEyOCBwaHlzYmxrX2V4 cD0zCj4gPj4gW3Jvb3RAZGhjcC02Ni04Ni0xMSB4ZnNwcm9ncy1kZXZdIyBibG9ja2RldiAtLWdl dGJzeiAtLWdldHBic3ogLS1nZXRzcyAvZGV2L3NkYwo+ID4+IDQwOTYKPiA+PiA0MDk2Cj4gPj4g NTEyCj4gPj4gW3Jvb3RAZGhjcC02Ni04Ni0xMSB4ZnNwcm9ncy1kZXZdIyBta2ZzIC10IHhmcyAt bCB2ZXJzaW9uPTEgLW0gY3JjPTEgL2Rldi9zZGMKClNvIHRoaXMgaXMgYW4gaW52YWxpZCBmaWxl c3lzdGVtIGNvbmZpZ3VyYXRpb24uIEl0IHNob3VsZCBiZQpkZXRlY3RlZCBhcyBzdWNoIGR1cmlu ZyBjb21tYW5kIGxpbmUgcGFyc2luZyBhbmQgcmVqZWN0ZWQgYmVmb3JlIHdlCmdldCBhbnl3aGVy ZSBuZWFyIGNoZWNraW5nIHRvcG9sb2d5IGNvbnN0cmFpbnRzLiBJbiBta2ZzCnRlcm1zLCBpdCdz IGEgY29uZmxpY3Rpbmcgb3B0aW9uIHNldC4KCj4gQW5kIHRoZSBjdWxwcml0IGlzIGluIG1rZnMs IHNvbWUgZm9ydHkgbGluZXMgYmVmb3JlIHRoZSBjcmMgJiBsb2cgdmVyc2lvbiBjaGVjazoKPiAK PiAyMDI2IOKHpSAgICAgICB9IGVsc2UgaWYgKGxzZWN0b3JzaXplID4gWEZTX01JTl9TRUNUT1JT SVpFICYmICFsc3UgJiYgIWxzdW5pdCkgewo+IDIwMjcg4oelICAgICAgIOKHpSAgICAgICBsc3Ug PSBibG9ja3NpemU7Cj4gMjAyOCDih6UgICAgICAg4oelICAgICAgIHNiX2ZlYXQubG9nX3ZlcnNp b24gPSAyOwo+IDIwMjkg4oelICAgICAgIH0KPiAKPiBUaGUgcG9zc2libGUgc29sdXRpb25zIEkg Y2FuIHRoaW5rIG9mIGFyZToKCk5vbmUgb2Ygd2hpY2ggcmVhbGx5IGFwcGVhbCBiZWNhdXNlLCBJ TU8sIHRoZXkgYXJlIHRyeWluZyB0byBzb2x2ZQp0aGUgd3JvbmcgcHJvYmxlbS4KClRoZSB3aG9s ZSBwb2ludCBvZiBtb3ZpbmcgdG8gdGFibGUgYmFzZWQgY29tbWFuZCBsaW5lIG9wdGlvbiBwYXJz aW5nCmlzIHRoYXQgd2UgY2FuIGVuY29kZSB0aGVzZSBzb3J0cyBvZiBjb25mbGljdHMgaW50byB0 aGUgb3B0aW9uCnRhYmxlLiBUaGUgY29uZmxpY3QgcmVzb2x1dGlvbiBpbiB0aGUgb3B0aW9uIHRh YmxlIGlzIGN1cnJlbnRseSBub3QKY29tcGxldGUgLSBpdCBjYW4gb25seSBlbmNvZGUgYW5kIGRl dGVjdCBjb25mbGljdHMgd2l0aGluIGEKc3Vib3B0aW9uIHR5cGUsIGJ1dCBub3QgYWNyb3NzIHN1 Ym9wdGlvbiB0eXBlcyAoZS5nLiB3aXRoaW4gLWQKc3Vib3B0aW9ucywgYnV0IG5vdCBiZXR3ZWVu IC1kIGFuZCAtbCBzdWJvcHRpb25zKS4KClRoaXMgaXMgc2ltcGx5IGJlY2F1c2UgSSBuZXZlciBn b3QgYXMgZmFyIGFzIGltcGxlbWVudGluZyB0aGlzIGxldmVsCm9mIGNvbmZsaWN0IGVuY29kaW5n L3Jlc29sdXRpb24uIEluIGVzc2VuY2UsIHRoZSBjb25mbGljdCBhcnJheQpuZWVkcyB0byBkZWZp bmUgdGhlIHN1YiBvcHRpb24gdHlwZSwgdGhlIHN1Ym9wdGlvbiB0aGF0IGlzCmluIGNvbmZsaWN0 IGFuZCB0aGUgdmFsdWUgdGhhdCBpdCBjb25mbGljdHMgYWdhaW5zdC4gSGVuY2UgdGhlCmNvbmZs aWN0cyB0YWJsZSBjYW4gdGhlbiBlbmNvZGUgc3VjaCB0aGluZ3MgYXMgInZlcnNpb24gMSBsb2dz IGFyZQppbnZhbGlkIGZvciBDUkMgZW5hYmxlZCBmaWxlc3lzdGVtcyIgYW5kIHZpY2UgdmVyc2Eu CgpDaGVlcnMsCgpEYXZlLgotLSAKRGF2ZSBDaGlubmVyCmRhdmlkQGZyb21vcmJpdC5jb20KCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCnhmcyBtYWlsaW5n IGxpc3QKeGZzQG9zcy5zZ2kuY29tCmh0dHA6Ly9vc3Muc2dpLmNvbS9tYWlsbWFuL2xpc3RpbmZv L3hmcwo=