Received-SPF: pass (google.com: domain of linux-ext4+bounces-2305-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <a3bd9271-c010-4eb5-8814-0f9247ff4117@oracle.com>
Date: Mon, 6 May 2024 17:56:42 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 4/4] btrfs-progs: convert: support ext2 unwritten file
 data extents
To: Qu Wenruo <wqu@suse.com>, Qu Wenruo <quwenruo.btrfs@gmx.com>,
        linux-btrfs@vger.kernel.org
Cc: dsterba@suse.com, y16267966@gmail.com, linux-ext4@vger.kernel.org
References: <cover.1714963428.git.anand.jain@oracle.com>
 <91f25251b1d57ee972179d707d13b453f43b5614.1714963428.git.anand.jain@oracle.com>
 <4c6ce351-e1fe-483a-8a9b-a1abb2324ea1@gmx.com>
 <fa9aa138-476d-413f-ac02-35156baacd66@suse.com>
Content-Language: en-GB
From: Anand Jain <anand.jain@oracle.com>
Autocrypt: addr=anand.jain@oracle.com; keydata=
 xsFNBGQG2+MBEAC42714sRj0ptcjHWMJgkltgglCKCpcjdLTyoFY9dljqJdvrOeojl4N1Ztb
 qMwsnsoFkPiVMUnnU/FgypRlPOzaB4w0R9MTzfvpHKAUNMbaYLquldGJhfuYpTgikr5GztZU
 VGKGsKc4NJzWh6Mfqit2jwurS18RmjxR2dBDKKb5+M5xQ66M5Of2SuuzaM6UnT1vctDN/hWr
 MDqx3CNeQ8Va0i1iCStsdS3ExG6nBVZkL9ZCHHZHi/oqe4bG4vvevRlx57s+uS4WKpAsjlKD
 Z/WHxer9bffB9GuOCngrOTWiXtf1qmgXNs5kXlfb6O3uRv1xnfqTAHdxJ8/pwSShl2aDScdW
 7S265QZ92+ygEJeoviTc8FyrhKkV5c4hAMa9QeiuP6Sk7Mk1G0D/d/DlHQCncQZ/St6q5ESX
 M1LbFLp4amx2yELX0/2lLBXj5s0vQd4mbyz29K5TfiYB/BsEWzSA0gTM9MPdJL3FhIei5VsD
 SQ197dkp3pzqII7/rw77sQs6TFin555Q4DSMsKvKvm/vULpknXMe0DdrHw8ybrY2AjWiTs2W
 1Re7VPORkKxEK7prZ62hghiEvGyZHh0RpnI0aD57R8P3RLJ5P7mCMKimK46SC9fw+zWfWZJA
 EIDccuxTfaLdGPMO8GJ2HnKbvAFbI+nMoSYRvJ6ULvcsH9bPPwARAQABzTJBbmFuZCBTdXZl
 ZXIgSmFpbiAoT3JhY2xlKSA8YW5hbmQuamFpbkBvcmFjbGUuY29tPsLBmAQTAQgAQhYhBKPX
 ZMgfwKRZ10YTjD2+pVga3ljYBQJkBtvjAhsDBQkSzAMABQsJCAcCAyICAQYVCgkICwIEFgID
 AQIeBwIXgAAKCRA9vqVYGt5Y2EAVD/98XUcG+lHTLFvrBn/l+egW5BiJUiUuLIti9wMmj3lg
 Ndv6myanBwjK+v0+RZJ6Vr+oazwTiki6RgnxT3LN9u79T4C7vGuVjqZ205a1vGVN309oMPDm
 +rF4qstsNBMTyE6FfLD1n4ONqgMLATRuk5rPAyfIXQyKy5UomLZo+ISHjpDUt4sXnrsYMz/N
 Ht5w7LRQMmKva92T5cReAvyU8guCHTiG6oN3RCQKlyRmZnFCXa2ov+hYhBrpNikFtPOojGnQ
 JZ/i7RHIU7/ku0/NCGLe+3osdjxaItjkcLP6U7R+WrViweSKocwrtqVIlizSvaDv4MxYM2oM
 aHoAcolFcrpUaqgnUAjhwYRc6CNdB5MroTzrzGnacJ4y7xBlql0+HlrlNho2AVLqvXmar5fp
 uwUHYTeUwsixVHfJL+1sow3Ky7Q5SknDQKd7V7X9X1qs862fuuBD3iPLR4YY5SstF1P0lFrr
 QjNS85TaHFkFhKrXGvhe1WGhum5Fc0hVx88gQBZ2gdw8z4GWKC5esxbvv0lI2UhP89q2ClsY
 ZFS0/Odo0eGgfyxqUGtrouK4cMVXVP+LJb168xt6yOuPMTOLJH/CT9/b3LygcWxn4m/2+XbY
 w1aLKoaO1cKAMSObubp1nQIy+idTnQeY69oKQpxYp97EH7bhYBWfLp/kKJEB98hJeM7BTQRk
 BtvjARAA6w/uFi14uDJ1jAlGWYUpBELdj1NgSAWw6CR6GiS9XPlvtn1uApa80cy/Hm1mqYQJ
 FtC+H3Q0uJRZYol2dvDRJYfDmC4bwoO/mru8ZpHVF2c2rVehDvgzxYJeqH9fJi6fymr9rOa6
 tjX0v8FGKD2pnU8yPXsMNeADdl2lL+XPwVoVhAxx8bpotl8nG14TXQcBNuKxbU4oWRjUZif2
 32CAXkngOnE/dwo68L6tfwBaKNCtXXjv7BMXylXjByMciW1hsR+wwOObWioW8R9uQEDWSNv1
 EwXre7VnuIksrt53Ohfuz458eF5Lg6qKGMYYuLmNwRbFPBeZvx6989P2zKuQn3I6YxzA2sdo
 YIhwJHbJNsf971H3CMFORqiLZY9OQ3Ef6FaLW+KM0p9ezuT9bAomQm6xGJDWC93hM/xLXAd7
 LJxhhxj9rQTwSwxm5eQg0ODntYXeEVfJw/gW0eMf5ivTjzKEF22oTswsEKjnsaZ2UZNPi9Pj
 WbPTEWCzGe4jHLqgY70F7f+OgCoI6Qyvb4+UfXyKez+zuo05Q8TxSFa3diFP5/mRokFMzrmF
 lgnUIyPYrHJWAhizZQveSNQ/5M0C9cVykxhaGaF6r0z8JRxhXi0hAlFIDaGye1k+UB8ZoENq
 JVOcjH5uVcXjdqzEXCa9OCDCJrHYCTu+dxyvR6iFXZUAEQEAAcLBfAQYAQgAJhYhBKPXZMgf
 wKRZ10YTjD2+pVga3ljYBQJkBtvjAhsMBQkSzAMAAAoJED2+pVga3ljYwXIP/2B74x/gNE4c
 5/TGzX3oKEdflBGadVjkirOGM1yjIEqstnCF1UIABhyLJYv9IRaNzhx+ieBDD8knEVAAXvp+
 3b0cnmct+kyvOnXwYpCDJSZcJRE25f8fyTyvo17rUCdP8CennzfB0CFMeis7JhyC3b4ZRaLm
 M87gx9ZJA6z5SLarw5zeI5rHmpQ8FK4hGH82AJeedHKcE+RR8rNOyHpdKDHIEtTxWXTZAC+q
 TxCzgLS6y0OOXDGPifcHjSkW7mSrnVXb/FTIqxVC8ClHwSomp2IQLwqPaew+QNFT3RII7QbK
 vQyq+V0TMXGo7zQQ23SN1N08Nj7E6m/hHffFZvRJ1ibZdHaDDCeDXEZoklttj78325i8yV/C
 XDL6MeirxiJyB8P+Y9eSrIDTQP0jKBPQa6N66QeBSJnMFuDBbP82lovdszeCJq5XhwjDgQ3b
 zAKqel0LTK4JTAlKeYjX678eVUcDAkdfurkLDbYcPd6sOveHr1Wuz3aFgtPVVnVzg3rwi5oH
 rffHVDSAu23bB/YgL+OHJ+EzKIqR+qLaYt0Y+e12zhFBSazVC6NFFQY0A+BV7PPnOLdKF1IE
 kbRwSOU3mzvks433LMKj9vmt5TyRU99OsAIn/BY2nCP3FURwQ1NKQ2vpJ8KnkLCGePkjefcQ
 y4F0QrzFk5Hg4pvnpDur6Cbp
In-Reply-To: <fa9aa138-476d-413f-ac02-35156baacd66@suse.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?dERyRkRpWm5pL2NrbnZUbE1DRFlHYXFzcXh4REJteXZ1ekFlVkZua013cWxo?=
 =?utf-8?B?Q3RKdm9OdVNxTEcxL0hYdHgycmdodUgySlZjWitsLyt4VEw5YVRwb2w5dlNH?=
 =?utf-8?B?K29WZklhRi95dDcrSm5pa21qY3UzRlIvN093bkFhWkFnTGh5K21zWC9GQjVz?=
 =?utf-8?B?L01UWXhYMFZvc1QrRWpUZ1pBcUVQSjV5amxyUGh1QjllTGtteHE4NlI2RFl1?=
 =?utf-8?B?bmJMbXJsUUNORHZuWTdzL1YydXJiVXVlWnJhR2plQUtteWd4dXp6T0haRnJw?=
 =?utf-8?B?cXZYSkNXL0h1RnFuZmU1VFNGL0oyZE85RGloMVBIN1ZaZE8rZVhwVmJhWElG?=
 =?utf-8?B?eXhxaDNTLy9SUU9OSE5qSjA2MTE4dzE5cXZJdmRGcDBqazFkbWsvWUZBdlNa?=
 =?utf-8?B?eU5aREpCYngxUnAzdCtNeE12eTd5MnRvZDltRjQ3MXRqVWwrNkI5QUdKblZB?=
 =?utf-8?B?NnZnWFIwUjYrTWNZbTlUbHJEMGsza2hhelNtbWJrMkNqMHVoU1Z6UnJWcUtC?=
 =?utf-8?B?emhXbGE1VWNaK0YyUWhGdzg4dHd1SUtLMXVINWpMTitsaXVWYWhXRkJoWFV5?=
 =?utf-8?B?eWZFMFR1VUlXN2NBTkp6RXZWWlk5OGhCa2w2NG8rbXdtTUpxUk9FSTZiVHpn?=
 =?utf-8?B?d1c4SHg2VU9tNXZ4Z2ZMZ0VmUjVLM2VkdEdNeTFVTjFRV01RWWlLejg4ODJj?=
 =?utf-8?B?Q3J3ZndqVWFFUDU1VWd3ZXg1aHpLQlN5STFmNmRwaExjdjA5c0xMOXZMQVJY?=
 =?utf-8?B?c2p6RGVBeE1Bc2ZDNm5CbDJFc1VIQjFSVDRQL3V5YWI0WTNraG1mRDRjb0Ro?=
 =?utf-8?B?V2NjeWhUQ1VQbDNGMlg4OHRnR2tZNzBBaVdSUnkxMzZIdTM5WkxhbDI4akQr?=
 =?utf-8?B?RmZQeTB6S2NKNC9nclBXbHpJN3FSY2dxV3pIemVaUDZjNmt2WlFFQzhUaEh6?=
 =?utf-8?B?K2NPWHMydXd1RFdjd2RUbVlRRnhYZUZ5c29iZGc3ckxOUUhTU1czMzUrUU56?=
 =?utf-8?B?azkyckpuNVNTbUtBRERjQTZaR1dHUHhxbnYrK0lhS2h1NjhGYXFwVlpmV2dn?=
 =?utf-8?B?NFFBV241YVhxNkROM0t6VzhCUTVoaXkzdFpvOW5RN1JBTjZObUFKS0VYaGE5?=
 =?utf-8?B?Y2hwWTcrU2VwMWhLR2Q1Tm9aK0JSZ1hFKzF6NVgwL3EyaXYzdVFEQUl4RmZG?=
 =?utf-8?B?MGlLZHVlOHhOSmpJeS9KcC8wN1BnN0dmMkQ5Z2QxR1BYZGJnNk9ia0luZ1Ji?=
 =?utf-8?B?d3ZVSnJoZnNBbkgrdG8wdzAwZVVUbFFod25oWEc0YXRXWnZ5ODhYa2N6aTZ2?=
 =?utf-8?B?OGNNZTRacHVCVVpmRnBDZGJFNjNLZUpDbjZNRVhXR0tjYXlJbFViZlI0V3VV?=
 =?utf-8?B?YzdXMDZXZXFYREZmNXp5bEE3Nkl0Yko5bk9RUnhiZVRVSWQ3ZXd1UGdkeFp0?=
 =?utf-8?B?ZEFTZ0x5VmV6a1dVVFhjczE3VlhQTFJlQXcySGFWaVIxVzdocUVwWEljTE1r?=
 =?utf-8?B?bHJHNEN0Snp0NnBKQnMyNEs1bjgvSjFmYWIvQW45R2l5VUhId2JhOTN5SjF4?=
 =?utf-8?B?WFRwa1hJVEhiN1FBOHFYNk1YSytNVFJPV2RiTXpkdFZvN2JvdGRQT3VTcERB?=
 =?utf-8?B?YWltRHdkY0NlZWJZUloxc1NNWnF0SXhwanM2c3lHY1lzUFhNWjZpZ1lsZksw?=
 =?utf-8?B?VFhmc0F5d3NybzViNDVTaEtNVUJ4TWhhN0dtMVBmRnZmTjN3RUhVS3Rza25P?=
 =?utf-8?B?NDBpZG5xVTU1UTFMOWlucCtZZW5NT3J0Nnk0OWVhQ0NUZkhNQWlZQnBhM0w4?=
 =?utf-8?B?OWVWN3ZKa1JSNDA3bEQxelZZeHhiNXJzeVc5ZE5YbDk2cXFDU2t4bHJZZ2pi?=
 =?utf-8?B?Z2NEZ2xxTitZeS9MM2MyZ3Bob0NYbWxQTXF3SEcvb3Fha3NEd0JVTUUzWHVS?=
 =?utf-8?B?OWtJL2dhbkVGNW5nWisxcUVDQ2lSNVlCejN0RGEwU1ExWVlxOHNweTUzcUZR?=
 =?utf-8?B?K2laQzhoLzVNbGZhV2FLMG5GQndjSVd1Vm96Sk5XdEV3RVZVeHB1d216bDNV?=
 =?utf-8?B?QjFuTTg0ZHBDTTBwOUQ1OGNJTytuYmJCWWFUb1BKVjFjYkxZSmZDaVJkNFBn?=
 =?utf-8?B?Rnk0N0ExNWNHYWNvdzRTRVpESENNR1hWZ2dmNFlOQjVBSjZnVGRwbnV4Rk5h?=
 =?utf-8?B?VHc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	F4FTuUr27BSV17zw3wn7ojpJBBtdN9Gcr39lRN25Rf8mRy69C/Mk5AMc8yhRGqfZ84Upll5vk9sQXKwSdmghTOsz5/4ej03OBFEXWSLAG/V2OlJ3BpZeQtBkvp80Ai5RwO2ZJFCzMGf+kI4NqWdHGs8lH+GavADvmJ5pDxZLoH6/EewRJNu/AnWjy+1/GAv9GCIOUClBYXvPVnP2orDzamN3nJx30sGca+EIka49IDjo31Bnh0yzjyTJcGXZM28q6qS1G+yRfNOvRsvTEDtHTSmMukZXtiSR+qMFHpnIf/+SNM+0xkRJT5o8qnxoTrWTomPNaQb2gpXi/OonNwZRiOtdXIcmfuNhcDk6CirRKf73GdFh3YKFZupd58RFHMvKlDguE22MENxqLd68zO9k8tWh6o8iQxoLj3zgs8j7e0t2b62Qs7HBL+UTUxDtTiZn93wsU5qMjvrXS07uzTAdLo/t0aA0+kLLqit0RmV3lS4+Wpa4fnVqIO3zLR9yt7ra5dcVbE8+DFoq0SP7c4BBbWJgXN1rm28IF6U31gEEWkQXCMwMe9MxxugVTNKA4HDDdrNSDMOI5zz9bd3XKCrpFzWHCzublOh/Bg01j4k0dRI=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 98acf4ec-55db-43d1-272f-08dc6db2d92c
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5706.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 09:56:49.6327
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XWH83P9EsHZWbrDiLQrutl2Gscal7flb/R+2dhxFBKO8+D+pScGtx7pk8duj2vtKaqt7Gq2NQXPd4nnbIBiUAQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5043
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-06_06,2024-05-03_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 spamscore=0
 suspectscore=0 adultscore=0 phishscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2405060067
X-Proofpoint-GUID: xrcWEbPt5F_trgi5GbwJm9aAB6oRkmYa
X-Proofpoint-ORIG-GUID: xrcWEbPt5F_trgi5GbwJm9aAB6oRkmYa


>>> . Remove RFC
>>> . Identify the block with a merged preallocated extent and call 
>>> fail-safe
>>> . Qu has an idea that it could be marked as a hole, which may be 
>>> based on
>>>    top of this patch.
>>
>> Well, my idea of going holes other than preallocated extents is mostly
>> to avoid the extra @prealloc flag parameter.
>>
>> But that's not a big deal for now, as I found the following way to
>> easily crack your v2 patchset:


This patch and the below test case are working as designed it is not
a bug/crack, with the current limitation that it should fail (safer
than silent corruption) (as shown below) when there is a merged 
unwritten data extent.


   ERROR: inode 13 index 0: identified unsupported merged block length 1 
wanted 4


This is an intermediary stage while the full support is being added.


Given this option, the user will have a choice to work on the identified
inode and make it a non-unwritten extent so that btrfs-convert shall be
successful.


>>
>>   # fallocate -l 1G test.img
>>   # mkfs.ext4 -F test.img
>>   # mount test.img $mnt
>>   # xfs_io -f -c "falloc 0 16K" $mnt/file
>>   # sync
>>   # xfs_io -f -c "pwrite 0 4k" -c "pwrite 12k 4k" $mnt/file
>>   # umount $mnt
>>   # ./btrfs-convert test.img
>> btrfs-convert from btrfs-progs v6.8
>>
>> Source filesystem:
>>    Type:           ext2
>>    Label:
>>    Blocksize:      4096
>>    UUID:           0f98aa2a-b1ee-4e91-8815-9b9a7b4af00a
>> Target filesystem:
>>    Label:
>>    Blocksize:      4096
>>    Nodesize:       16384
>>    UUID:           3b8db399-8e25-495b-a41c-47afcb672020
>>    Checksum:       crc32c
>>    Features:       extref, skinny-metadata, no-holes, free-space-tree
>> (default)
>>      Data csum:    yes
>>      Inline data:  yes
>>      Copy xattr:   yes
>> Reported stats:
>>    Total space:      1073741824
>>    Free space:        872349696 (81.24%)
>>    Inode count:           65536
>>    Free inodes:           65523
>>    Block count:          262144
>> Create initial btrfs filesystem
>> Create ext2 image file
>> Create btrfs metadata
>> ERROR: inode 13 index 0: identified unsupported merged block length 1
>> wanted 4
>> ERROR: failed to copy ext2 inode 13: -22
>> ERROR: error during copy_inodes -22
>> WARNING: error during conversion, the original filesystem is not modified
>>


>> [...]
>>> +
>>> +    memset(&extent, 0, sizeof(struct ext2fs_extent));
>>> +    if (ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent)) {
>>> +        error("ext2fs_extent_get EXT2_EXTENT_CURRENT failed inode %d",
>>> +               src->ext2_ino);
>>> +        ext2fs_extent_free(handle);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (extent.e_pblk != data->disk_block) {
>>> +    error("inode %d index %d found wrong extent e_pblk %llu wanted 
>>> disk_block %llu",
>>> +               src->ext2_ino, index, extent.e_pblk, data->disk_block);
>>> +        ext2fs_extent_free(handle);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (extent.e_len != data->num_blocks) {
>>> +    error("inode %d index %d: identified unsupported merged block 
>>> length %u wanted %llu",
>>> +            src->ext2_ino, index, extent.e_len, data->num_blocks);
>>> +        ext2fs_extent_free(handle);
>>> +        return -EINVAL;
>>> +    }
>>
>> You have to split the extent in this case. As the example I gave, part
>> of the extent can have been written.
>> (And I'm not sure if the e_pblk check is also correct)
>>
>> I believe the example I gave could be a pretty good test case.
>> (Or you can go one step further to interleave every 4K)
> 
> Furthermore, I have to consider what is the best way to iterate all data 
> extents of an ext2 inode.
> 
> Instead of ext2fs_block_iterate2(), I'm wondering if 
> ext2fs_extent_goto() would be a better solution. (As long as if it can 
> handle holes).
> 
> Another thing is, please Cc this series to ext4 mailing list if possible.
> I hope to get some feedback from the ext4 exports as they may have a 
> much better idea than us.
> 

I've tried fixes without success. Empirically, I found
that the main issue is extent optimization and merging,
which ignores the unwritten flag, idk where is this
happening. I think it is during writing the ext4 image
at the inode BTRFS_FIRST_FREE_OBJECTID + 1.

If avoiding this optimization possible, the extent boundary
will align with ext4 and thus its flags.

Thanks, Anand