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

Lock a file around the NVT sync #1002

Merged
merged 4 commits into from
Mar 10, 2020

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented Mar 5, 2020

This is required because it is now possible to start a sync outside the gvmd main process, with --rebuild.

Also, the lock file provides a way to check if the sync is running.

Checklist:

@mattmundell mattmundell marked this pull request as ready for review March 5, 2020 20:10
Copy link
Member

@timopollmeier timopollmeier left a comment

Choose a reason for hiding this comment

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

Shouldn't the lock files be released after the update is done? At least it would be be more consistent
with the other use of lockfile_lock_nb and closing the lock fd at the end of the SCAP and CERT syncs.

Also, the other sync lockfiles use "gvm-sync-..." instead of "gvm-syncing-...", but I'm not sure if those or the new NVT one should be changed...

@mattmundell
Copy link
Contributor Author

mattmundell commented Mar 9, 2020

Shouldn't the lock files be released after the update is done? At least it would be be more consistent
with the other use of lockfile_lock_nb and closing the lock fd at the end of the SCAP and CERT syncs.

Also, the other sync lockfiles use "gvm-sync-..." instead of "gvm-syncing-...", but I'm not sure if those or the new NVT one should be changed...

I've tried to be consistent with the other uses of the lockfile_* functions and not with the SecInfo locking.

The SecInfo locking in gvm_migrate_secinfo should be changed to use lockfile_*. Not sure why it is done by hand. The other lockfile cases are all using "gvm-syncing..." style, like "gvm-checking", "gvm-migrating" (except "gvm-create-functions"). So they could all be changed to the "ing" style, but not serious for me.

And the SecInfo locking in sync_secinfo/scap/cert should also use lockfile_*. I get that unlike the NVT sync this doing an unlock, but some other cases of lockfile_* do not unlock because it happens automatically at the end of the process (eg gvm-migrating). Don't mind adding the unlock if it bothers you though.

@mattmundell mattmundell merged commit 051ed94 into greenbone:master Mar 10, 2020
@mattmundell mattmundell deleted the nvt-sync-lock branch March 10, 2020 11:01
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