diff mbox

btrfs-progs: let mkfs return nozero value on thin provision device

Message ID 20180403083945.13485-1-suy.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Su Yue April 3, 2018, 8:39 a.m. UTC
when mkfs.btrfs on a thin provision device which has very small
backing size and big virtual size, all code works well in
mkfs.btrfs until close_ctree() is called.
close_ctree() fails to sync device due to small backing size
while closing devices.
However, mkfs returns 0 in such situation which causes failure of
xfstests generic/405.

So, let mkfs returns nonzero value if previous steps succeeded but
close_ctree() failed.
Then xfstests generic/405 passes now.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 mkfs/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Qu Wenruo April 3, 2018, 8:58 a.m. UTC | #1
On 2018年04月03日 16:39, Su Yue wrote:
> when mkfs.btrfs on a thin provision device which has very small
> backing size and big virtual size, all code works well in
> mkfs.btrfs until close_ctree() is called.
> close_ctree() fails to sync device due to small backing size
> while closing devices.
> However, mkfs returns 0 in such situation which causes failure of
> xfstests generic/405.
> 
> So, let mkfs returns nonzero value if previous steps succeeded but
> close_ctree() failed.
> Then xfstests generic/405 passes now.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  mkfs/main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 5a717f701cd5..9f7f2396df8f 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -1285,6 +1285,12 @@ out:
>  		}
>  	}
>  
> +	if (!ret && close_ret) {
> +		ret = close_ret;
> +		error("failed to close ctree, the filesystem may be inconsistent: %d",
> +		      ret);
> +	}
> +
>  	btrfs_close_all_devices();
>  	free(label);
>  
>
David Sterba April 9, 2018, 1:25 p.m. UTC | #2
On Tue, Apr 03, 2018 at 04:39:45PM +0800, Su Yue wrote:
> when mkfs.btrfs on a thin provision device which has very small
> backing size and big virtual size, all code works well in
> mkfs.btrfs until close_ctree() is called.
> close_ctree() fails to sync device due to small backing size
> while closing devices.
> However, mkfs returns 0 in such situation which causes failure of
> xfstests generic/405.
> 
> So, let mkfs returns nonzero value if previous steps succeeded but
> close_ctree() failed.
> Then xfstests generic/405 passes now.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Applied, thanks. Can you please write a test for that?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Su Yue April 9, 2018, 3:46 p.m. UTC | #3
Sorry for the previous reply in HTML format which is not delivered to
the mail list.

I mean there is already xfstests generic/405 for the situation.
Thin provision devices need to call dmsetup to setup.
I wonder If calling dmsetup in progs tests is acceptable or not.
If it is, I will do it.

Thanks,
Su

On Mon, Apr 9, 2018 at 9:25 PM, David Sterba <dsterba@suse.cz> wrote:
> On Tue, Apr 03, 2018 at 04:39:45PM +0800, Su Yue wrote:
>> when mkfs.btrfs on a thin provision device which has very small
>> backing size and big virtual size, all code works well in
>> mkfs.btrfs until close_ctree() is called.
>> close_ctree() fails to sync device due to small backing size
>> while closing devices.
>> However, mkfs returns 0 in such situation which causes failure of
>> xfstests generic/405.
>>
>> So, let mkfs returns nonzero value if previous steps succeeded but
>> close_ctree() failed.
>> Then xfstests generic/405 passes now.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>
> Applied, thanks. Can you please write a test for that?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba April 9, 2018, 4:31 p.m. UTC | #4
On Mon, Apr 09, 2018 at 11:46:32PM +0800, Damenly Su wrote:
> Sorry for the previous reply in HTML format which is not delivered to
> the mail list.
> 
> I mean there is already xfstests generic/405 for the situation.
> Thin provision devices need to call dmsetup to setup.
> I wonder If calling dmsetup in progs tests is acceptable or not.
> If it is, I will do it.

Yes dmsetup is already used eg in test
mkfs-tests/005-long-device-name-for-ssd .
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/mkfs/main.c b/mkfs/main.c
index 5a717f701cd5..9f7f2396df8f 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1285,6 +1285,12 @@  out:
 		}
 	}
 
+	if (!ret && close_ret) {
+		ret = close_ret;
+		error("failed to close ctree, the filesystem may be inconsistent: %d",
+		      ret);
+	}
+
 	btrfs_close_all_devices();
 	free(label);