From: Amir Goldstein <amir73il@gmail.com>
To: Chengguang Xu <cgxu519@icloud.com>
Cc: Eryu Guan <eguan@redhat.com>, fstests <fstests@vger.kernel.org>,
overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [PATCH v3 1/3] common/rc: add scratch shutdown support for overlayfs
Date: Wed, 3 Jan 2018 16:54:09 +0200 [thread overview]
Message-ID: <CAOQ4uxhPzjEpxz1sroca+QLhd9us8orkGetzo9Zz8wC4B8RuZA@mail.gmail.com> (raw)
In-Reply-To: <698CC47A-DC4A-4705-B112-6AAC36A088A4@icloud.com>
On Wed, Jan 3, 2018 at 4:25 PM, Chengguang Xu <cgxu519@icloud.com> wrote:
>>
>> 在 2018年1月3日,下午8:58,Amir Goldstein <amir73il@gmail.com> 写道:
>>
>> On Wed, Jan 3, 2018 at 2:44 PM, Chengguang Xu <cgxu519@icloud.com> wrote:
>>>
>> [...]
>>> In order to add overlay support in some requirement checks like _require_metadata_journaling,
>>> I think it’s better save underlying filesystem type to $OVL_BASE_FSTYP and doing proper check based
>>> on it.
>>>
>>> Currently $OVL_BASE_FSTYP and $FSTYP all set to “overlay”, Is there any specific reason for it?
>>>
>>
>> In _overlay_config_override():
>> # Config file may specify base fs type, but we obay -overlay flag
>> export OVL_BASE_FSTYP="$FSTYP"
>> export FSTYP=overlay
>>
>> So either your setup is wrong or there is a bug.
>> $OVL_BASE_FSTYP *should* contain the base fs type,
>> but only if you defined FSTYPE in your config file when running ./check -overlay
>
> Yeah, I didn’t specify by hand in config file and just supposed to detect automatically.
> I didn’t check very carefully but seems just slight modification like below could let it
> support auto detection.
>
Sorry, I do not wish invest time to review this change because:
1. I don't see the need to support auto detect of base fs type (feel
free to explain)
2. I ran a lot of test to sanitize the new overlay config option with many
configurations and this adds a lot more variants. I you wish to push this
forward and have a good claim for the need, please specify which tests
you ran to sanitize your change before requesting review.
If you wish to contribute to the xfstests overlay infrastructure there
are other features
that would bring more gain IMO, for example:
- Support mkfs and MKFS_OPTIONS for base fs
- Try to increase the number of generic tests that can be run with
basefs+overlay
similar to your effort with the shutdown group tests
Thanks!
Amir.
next prev parent reply other threads:[~2018-01-03 14:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-12 3:09 [PATCH v3 1/3] common/rc: add scratch shutdown support for overlayfs Chengguang Xu
2017-12-12 3:09 ` [PATCH v3 2/3] common/rc: add a check case in _require_xfs_io_command() to support syncfs Chengguang Xu
2017-12-12 3:09 ` [PATCH v3 3/3] generic/470: add syncfs test Chengguang Xu
2017-12-12 6:00 ` Amir Goldstein
2017-12-12 6:07 ` Chengguang Xu
2017-12-12 9:24 ` [PATCH v3 3/3] generic/471: " Eryu Guan
2017-12-12 9:56 ` Amir Goldstein
2017-12-12 10:05 ` Chengguang Xu
2017-12-12 11:00 ` Amir Goldstein
2017-12-12 13:30 ` Eryu Guan
2017-12-12 5:58 ` [PATCH v3 1/3] common/rc: add scratch shutdown support for overlayfs Amir Goldstein
2017-12-12 9:16 ` Eryu Guan
2017-12-12 10:20 ` Chengguang Xu
2017-12-12 13:39 ` Eryu Guan
2018-01-03 12:44 ` Chengguang Xu
2018-01-03 12:58 ` Amir Goldstein
2018-01-03 14:25 ` Chengguang Xu
2018-01-03 14:54 ` Amir Goldstein [this message]
2018-01-10 2:35 ` Chengguang Xu
2017-12-14 3:35 ` Chengguang Xu
2017-12-14 4:32 ` Eryu Guan
2017-12-14 5:31 ` Chengguang Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAOQ4uxhPzjEpxz1sroca+QLhd9us8orkGetzo9Zz8wC4B8RuZA@mail.gmail.com \
--to=amir73il@gmail.com \
--cc=cgxu519@icloud.com \
--cc=eguan@redhat.com \
--cc=fstests@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.