Skip to content

Conversation

@tomponline
Copy link
Member

@tomponline tomponline commented Jan 14, 2025

This reverts commit ab910ac from #14761.

It is causing failures with our LVM CI tests due to the additional time it takes to do a slow blkdiscard --zeroout run when resetting the volume after an initial snapshot is transferred.

As the other commit in this PR deals with resetting the newly created volume, no old data from previously deleted volumes will be present, and this commit is just designed to ensure that "holes" of zeroes in the incoming data stream don't corrupt the volume when using the sparse writer.

As we've not landed the sparse writer yet, all of the bytes from the incoming data stream are written to the volume and thus there is no issue.

So we can hold off from landing this commit until we land the sparse writer.

…arse write"

This reverts commit ab910ac.

It is causing failures with our LVM CI tests due to the additional time it takes to
do a slow `blkdiscard --zeroout` run when resetting the volume after an initial
snapshot is transferred.

As the other commit in this PR deals with resetting the newly created volume,
no old data from previously deleted volumes will be present, and this commit
is just designed to ensure that "holes" of zeroes in the incoming data stream
don't corrupt the volume when using the sparse writer.

As we've not landed the sparse writer yet, all of the bytes from the incoming
data stream are written to the volume and thus there is no issue.

So we can hold off from landing this commit until we land the sparse writer.

Signed-off-by: Thomas Parrott <[email protected]>
@tomponline tomponline self-assigned this Jan 14, 2025
@tomponline tomponline requested a review from hamistao January 14, 2025 08:17
@tomponline tomponline changed the title Revert "lxd/storage/drivers/generic/vfs: Truncate/Discard ahead of sparse write" Storage: Revert "lxd/storage/drivers/generic/vfs: Truncate/Discard ahead of sparse write" Jan 14, 2025
Copy link

@hamistao hamistao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clear commit message :))
I was able to understand a process outside my comfort zone in a single read.

@tomponline tomponline merged commit 0872972 into canonical:main Jan 14, 2025
26 checks passed
@tomponline tomponline deleted the tp-reset branch January 14, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants