Lock from the time system repository is loaded until exit

[evan-goode]

Related: #2129

This is another step towards proper locking so that multiple DNF5 processes don't interfere with each other.

Adds Base::lock_system_repo and Base::unlock_system_repo:

/// Acquire an advisory lock on the installroot's system repository.
/// The lock will be automatically released when Base goes out of scope, or manually when unlock_system_repo is called.
/// Can be called multiple times to upgrade or downgrade a READ lock to a WRITE lock or vice versa.
/// Should be called before the system repo is loaded, and the lock should be held until all transactions are
/// complete and other processes can safely re-read the RPMDB and resolve transactions.
/// @throw libdnf5::SystemError if an unexpected error occurs when locking
/// @return true if acquiring the lock succeeded, false otherwise
bool lock_system_repo(
    libdnf5::utils::LockAccessType access = libdnf5::utils::LockAccessType::WRITE,
    libdnf5::utils::LockBlockingType blocking = libdnf5::utils::LockBlockingType::NON_BLOCKING);

/// Release the lock obtained by lock_system_repo.
/// Idempotent. No-op if there is currently no lock.
/// @throw libdnf5::SystemError if an unexpected error occurs when unlocking
void unlock_system_repo();

And calls Base::lock_system_repo from Context::load_repos before the system repo is loaded.

The idea here is that we need to hold a lock the entire time from before the transaction is resolved until after the transaction is complete and the RPMDB is written. Otherwise, another DNF process could resolve a transaction using a soon-to-be-invalid version of the system repo and get an invalid transaction.

This approach is an improvement over DNF4 which blocks while another transaction is running but does not block before loading the system repo and resolving a new transaction.

A helpful message is printed listing the other processes DNF5 is waiting for:

$ sudo dnf5 install rsms-inter-fonts
Waiting for other processes to finish:
62409	dnf5 install blender

[evan-goode]

Now the non-privileged commands acquire a READ lock, not a WRITE lock.

I'm still trying to figure out how this should work for unprivileged users. If a root DNF process has not yet created /run/dnf, then an unprivileged user doesn't have sufficient permissions to create /run/dnf and they can't obtain any lock, either read or write. So I'm thinking the lockfile needs to be persistent somewhere, always owned by root with mode 0664 or 0644. That way unprivileged users will always be able to open a O_RDONLY FD, but they won't be able to delete the lockfile or write garbage to it.

Maybe we could put it in /usr/lib/sysimage/libdnf5? That should work with installroots too.

We'd also need to extend Locker to not delete the lockfiles, that will likely be an ABI-breaking change.

[evan-goode]

I've moved the lockfile to /usr/lib/sysimage/libdnf5/system-repo.lock and refactored Locker with a p_impl.

Now I think we just need some flag to ignore the locks in the case where e.g. a privileged user doesn't care about an unprivileged user's read lock on the system repo.

[evan-goode]

I think this is in a good place for initial review. I ended up making an API-breaking change to add a load_system flag to Context::load_repos.

[kontura]

While I don't think these breaking changes will actually break anyone's code I am not thrilled about bumping the major version so soon again.

I think we could get around it by rather introducing new API but I am not sure if there is any real technical reason for this.

[kontura]

If I understand correctly this value is derived from (it duplicates) information already present in the Context. Is there a plan to use it somehow differently later on?

Currently I would rather make cmd_requires_privileges(dnf5::Context context) a private method of Context and use it directly. It would require introducing a private header file for Context, where we would likely move Context::Impl, but I think that would be a good move regardless.
What do you think?

[evan-goode]

It may be useful in the future for commands (especially those provided by plugins) to declare for themselves whether they will modify the system rather than doing the heuristics in cmd_requires_privileges. Also if we ever do something about https://issues.redhat.com/browse/RHEL-67915, there would be a distinction between "will modify the system repository" (inside the installroot) and "needs root privileges".

But for now your suggestion is maybe better, like you said, a context_private.hpp would be a good idea.

[kontura]

While we are doing changes here, I am wondering whether the Context.load_repos(...) API should be public at all. If I am not mistaken this API is for dnf5 plugins and those will already be using it through the dnf5 main function.
I see it is now used in makecache command but that feels more like an oversight to me because it results in calling it twice. I believe it could easily be removed.

This would need a private header for Context as welll though.

[evan-goode]

I agree, it could be made private.

[evan-goode]

While I don't think these breaking changes will actually break anyone's code I am not thrilled about bumping the major version so soon again.

I think we could get around it by rather introducing new API but I am not sure if there is any real technical reason for this.

Most of the ABI/API changes are in Locker. So we could create a LockerV2 or something, yes. IMO it's better to just do the ABI/API break. In my view, there is not much harm in a major version bump other than the ABI/API break that it signifies. Stable F43 is still on DNF 5.2 so we would not need to do backporting to 5.3.