All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joerg Vehlow <lkml@jv-coder.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3] squashfs: Add regression test for sanity check bug
Date: Thu, 15 Jul 2021 12:40:03 +0200	[thread overview]
Message-ID: <de3a63e6-3b4c-20ff-3ee6-00fb4d181e98@jv-coder.de> (raw)
In-Reply-To: <YPAJcuQ7irZAI+1Z@yuki>

Hi C

On 7/15/2021 12:09 PM, Cyril Hrubis wrote:
> Hi!
>>> For instance mount_device flag needs implies format_device which implies
>>> needs_device which implies needs_tmpdir.
>> I agree with that. If needs_tmpdir was only required, because
>> needs_device is required, I wouldn't add it.
>> But if needs_device implementation is changed, the test still needs a
>> tmpdir. That's why I would always vote for adding it here.
> The .needs_device flag implies .needs_tmpdir that's a part of the test
> library API.
>
>>> Also the dev_min_size = 1 does not have any efect here, since it can be
>>> used only to request bigger-than-default size and gets ignored here. I
>>> guess that we can merge this as it is and I will add needs_loopdev to
>>> the tst_test structure later which will just allocate loop device and
>>> pass it down to the test.
>> This is true, but the test should also specify what it needs. If for
>> whatever reason DEV_SIZE_MB is redefined to a smaller value, the test
>> would still work.
> The DEV_SIZE_MB will never go backward, this is actually the minimal
> size that is needed in order to create all supported Linux filesystems
> and we need to bump the size up every few years. I think that it was
> 100MB when I started to work on LTP and it got up to 256MB, which is
> also the reason the tests cannot know and why this is hidden in the
> library.
>
>> To be honest, for "1" it doesn't matter. But it it was bigger, it makes
>> total sense to specify the size if the test knows it...
> Also LTP allows to be passed a real block device to the tests, in a case
> that you want to test a block driver, in which case the test has no
> exact controll on the device size, which is also the reason why we
> allow minimal size to be specified but not exact size.
>
>> I don't understand why a lot of developers like implicit definitions so
>> much more over explicit definitions.
>> I could understand it for language intrinsic stuff, because that is (or
>> could be) known to all developers.
>> But for someone, who rarely works on a project or switches between
>> different projects implicit information is bad!
> I think that we can actually make things better by adding all of the
> text I've written above as a top level description in the tst_device.c.
>

I guess we agree to disagree on this matter. It is the same as the 
discussion mailinglist vs. github for patches.
Change the metadata in whatever way you like while merging.

Joerg

  reply	other threads:[~2021-07-15 10:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15  5:08 [LTP] [PATCH v3] squashfs: Add regression test for sanity check bug Joerg Vehlow
2021-07-15  8:00 ` Richard Palethorpe
2021-07-15  8:21 ` xuyang2018.jy
2021-07-15  8:44   ` Joerg Vehlow
2021-07-15  9:27     ` Cyril Hrubis
2021-07-15 10:12       ` Joerg Vehlow
2021-07-15 10:09         ` Cyril Hrubis
2021-07-15 10:40           ` Joerg Vehlow [this message]
2021-07-21 11:47         ` Cyril Hrubis
2021-08-04 14:05 ` Cyril Hrubis

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=de3a63e6-3b4c-20ff-3ee6-00fb4d181e98@jv-coder.de \
    --to=lkml@jv-coder.de \
    --cc=ltp@lists.linux.it \
    /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.