[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] Consider discard option when writing zeros
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v2] Consider discard option when writing zeros |
Date: |
Wed, 26 Jun 2024 10:42:25 +0200 |
Am 24.06.2024 um 23:12 hat Nir Soffer geschrieben:
> On Mon, Jun 24, 2024 at 7:08 PM Kevin Wolf <kwolf@redhat.com> wrote:
>
> > Am 24.06.2024 um 17:23 hat Stefan Hajnoczi geschrieben:
> > > On Wed, Jun 19, 2024 at 08:43:25PM +0300, Nir Soffer wrote:
> > > > Tested using:
> > >
> > > Hi Nir,
> > > This looks like a good candidate for the qemu-iotests test suite. Adding
> > > it to the automated tests will protect against future regressions.
> > >
> > > Please add the script and the expected output to
> > > tests/qemu-iotests/test/write-zeroes-unmap and run it using
> > > `(cd build && tests/qemu-iotests/check write-zeroes-unmap)`.
> > >
> > > See the existing test cases in tests/qemu-iotests/ and
> > > tests/qemu-iotests/tests/ for examples. Some are shell scripts and
> > > others are Python. I think shell makes sense for this test case. You
> > > can copy the test framework boilerplate from an existing test case.
> >
> > 'du' can't be used like this in qemu-iotests because it makes
> > assumptions that depend on the filesystem. A test case replicating what
> > Nir did manually would likely fail on XFS with its preallocation.
>
> This is why I did not try to add a new qemu-iotest yet.
>
> > Maybe we could operate on a file exposed by the FUSE export that is
> > backed by qcow2, and then you can use 'qemu-img map' on that qcow2 image
> > to verify the allocation status. Somewhat complicated, but I think it
> > could work.
>
> Do we have examples of using the FUSE export? It sounds complicated but
> being able to test on any file system is awesome. The complexity can be
> hidden behind simple test helpers.
We seem to have a few tests that use it, and then the fuse protocol
implementation, too. 308 and file-io-error look relevant.
> Another option is to use a specific file system created for the tests,
> for example on a loop device. We used userstorage[1] in ovirt to test
> on specific file systems with known sector size.
Creating loop devices requires root privileges. If I understand
correctly, userstorage solved that by having a setup phase as root
before running the tests as a normal user? We don't really have that in
qemu-iotests.
Some tests require passwordless sudo and are skipped otherwise, but this
means that in practice they are almost always skipped.
> But more important, are you ok with the change?
>
> I'm not sure about not creating sparse images by default - this is not
> consistent with qemu-img convert and qemu-nbd, which do sparsify by
> default. The old behavior seems better.
Well, your patches make it do what we always claimed it would do, so
that consistency is certainly a good thing. Unmapping on write_zeroes
and ignoring truncate is a weird combination anyway that doesn't really
make any sense to me, so I don't think it's worth preserving. The other
way around could have been more defensible, but that's not how our bug
works.
Now, if ignoring all discard requests is a good default these days is a
separate question and I'm not sure really. Maybe discard=unmap should
be the default (and apply to both discard are write_zeroes, of course).
Kevin
- [PATCH v2] Consider discard option when writing zeros, Nir Soffer, 2024/06/19
- Re: [PATCH v2] Consider discard option when writing zeros, Nir Soffer, 2024/06/19
- Re: [PATCH v2] Consider discard option when writing zeros, Daniel P . Berrangé, 2024/06/26
- Re: [PATCH v2] Consider discard option when writing zeros, Nir Soffer, 2024/06/26
- Re: [PATCH v2] Consider discard option when writing zeros, Kevin Wolf, 2024/06/27
- Re: [PATCH v2] Consider discard option when writing zeros, Nir Soffer, 2024/06/28
Re: [PATCH v2] Consider discard option when writing zeros, Nir Soffer, 2024/06/19