QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
534 stars 46 forks source link

Run thin_trim when activating thin pools #7514

Open DemiMarie opened 2 years ago

DemiMarie commented 2 years ago

How to file a helpful issue

The problem you're addressing (if any)

Running blkdiscard on thin volumes is slow.

The solution you'd like

LVM should run thin_trim during volume activation. This requires an (easy) LVM patch.

The value to a user, and who that user might be

Users will benefit from improved VM shutdown time.

brendanhoar commented 2 years ago

Would this be during "volume activation" or during "thin pool activation"?

B

DemiMarie commented 2 years ago

Pool activation. thin_trim cannot be run on metadata that the kernel is currently using, as doing so would likely cause data loss. The only safe time to run it is when the thin pool has not been activated.

DemiMarie commented 2 years ago

The reason this is easy is because LVM already calls thin_check to check that the metadata is consistent before activating a pool. It turns out that LVM already knows what the path to the data volume is, so it can just call thin_trim at that point.

brendanhoar commented 2 years ago

Pool activation.

Right. I was being a bit pedantic on the wording of the ticket/issue. :)

B

brendanhoar commented 2 years ago

The reason this is easy is because LVM already calls thin_check to check that the metadata is consistent before activating a pool. It turns out that LVM already knows what the path to the data volume is, so it can just call thin_trim at that point.

Two things:

  1. Obviously thin_trim (and activation) should not be called if thin_check reports errors or if attempts to low-risk resolve those errors fail.

  2. May I advocate for thin_trim invocation both during pre-pool activation during normal startup as well as post-pool deactivation during normal shutdown.

B

DemiMarie commented 2 years ago

The reason this is easy is because LVM already calls thin_check to check that the metadata is consistent before activating a pool. It turns out that LVM already knows what the path to the data volume is, so it can just call thin_trim at that point.

Two things:

  1. Obviously thin_trim (and activation) should not be called if thin_check reports errors or if attempts to low-risk resolve those errors fail.

Indeed so; this would make no sense and be dangerous.

  1. May I advocate for thin_trim invocation both during pre-pool activation during normal startup as well as post-pool deactivation during normal shutdown.

This might be a more difficult patch. My current plan is to call thin_trim after any successful call to thin_check, which is easy due to how the LVM codebase is structured.

brendanhoar commented 2 years ago

May I advocate for thin_trim invocation both during pre-pool activation during normal startup as well as post-pool deactivation during normal shutdown. This might be a more difficult patch. My current plan is to call thin_trim after any successful call to thin_check, which is easy due to how the LVM codebase is structured.

Hmm, it'd be nice to get the "opportunistic anti-forensics" benefit during a normal shutdown and not require a restart to obtain it.

B