Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ostree admin deploy: Add --no-prune option #1418

Closed
wants to merge 2 commits into from

Conversation

wmanley
Copy link
Member

@wmanley wmanley commented Jan 15, 2018

If you want cleanup, but don't want to prune the repo. Pruning can be quite expensive so ostree admin deploy can be much faster without pruning.

On the NVidia Tegra TK1 pruning can take 20s, even when there is nothing to delete.

TODO:

  • test it

@cgwalters
Copy link
Member

I only quickly glanced at this so far, but I'm a bit uncertain as to the need for the flag and the option? Can't we just port the cli to use ostree_sysroot_simple_write_deployment_with_options() from a5d5333 ?

In the next commit I will add --no-prune which will affect cleaning.  By
doing this refactor we avoid having to add a NO_PRUNE flag.
If you want cleanup, but don't want to prune the repo.  Pruning can
be quite expensive so ostree admin deploy can be much faster without
pruning.
@wmanley
Copy link
Member Author

wmanley commented Jan 15, 2018

Can't we just port the cli to use ostree_sysroot_simple_write_deployment_with_options() from a5d5333 ?

I had a go at that and we end up duplicating the logic from ostree_sysroot_simple_write_deployment that comes up with the list of new deployments. Instead I have just passed the NO_CLEAN flag and done the cleaning ourselves in ot_admin_builtin_deploy. The only theoretical difference is that now the cleaning will happen with a read-write /boot rather than a read-only one.

I haven't tested it yet though...

@cgwalters
Copy link
Member

Going back over this I'm uncertain why we don't just forcibly do prepare_cleanup() before starting to write deployments or so. But that'd require some more tricky investigation.

I'm not worried about the read/write vs ro /boot yet, that logic is going to need a lot more work that should end up going along with #1265

So this LGTM.

@rh-atomic-bot r+ 1731f42

@rh-atomic-bot
Copy link

⌛ Testing commit 1731f42 with merge 2098f75...

rh-atomic-bot pushed a commit that referenced this pull request Jan 16, 2018
If you want cleanup, but don't want to prune the repo.  Pruning can
be quite expensive so ostree admin deploy can be much faster without
pruning.

Closes: #1418
Approved by: cgwalters
@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing 2098f75 to master...

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.

3 participants