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

pkg/servicereaper: build this for FreeBSD as well as Linux #1531

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

dfr
Copy link
Contributor

@dfr dfr commented Jun 28, 2023

This is needed to support 'podman system service'.

This is needed to support 'podman system service'.

Signed-off-by: Doug Rabson <[email protected]>
@dfr dfr force-pushed the freebsd-servicereaper branch from 3727e65 to c118afa Compare June 28, 2023 15:19
@Luap99
Copy link
Member

Luap99 commented Jun 28, 2023

So far we only use that for rootlessport/slirp4netns and AFAIK that is not used on freebsd? Maybe it is better to nop the call out in podman on bsd?

@dfr
Copy link
Contributor Author

dfr commented Jun 28, 2023

So far we only use that for rootlessport/slirp4netns and AFAIK that is not used on freebsd? Maybe it is better to nop the call out in podman on bsd?

I'll try that - thanks for the extra context

@dfr
Copy link
Contributor Author

dfr commented Jun 28, 2023

There is a call site in podman/cmd/podman/system/service/service_abi.go which doesn't seem to be related to slirp4netns (unless I'm misunderstanding something).

@Luap99
Copy link
Member

Luap99 commented Jun 28, 2023

yeah it calls Start() but that just spawns a goroutine and doesn't do anything useful unless there are other callers the call AddPID()

so unless you need a process reaper and plan on using AddPID() there is no reason to call Start().

@dfr
Copy link
Contributor Author

dfr commented Jun 28, 2023

Ok, it looks like I don't need this in podman on FreeBSD for now. I'll do some more testing over there and close this PR.

@Luap99
Copy link
Member

Luap99 commented Jun 28, 2023

I mean we can merge this here, this package seems to compile fine on freebsd. But it still makes sense to NOP it out for bsd in the podman service as you safe on goroutine (so bit of cpu and memory).

@dfr
Copy link
Contributor Author

dfr commented Jun 28, 2023

Makes sense - I'll decouple things on the podman side before I make a PR to enable 'podman system service' on FreeBSD

@rhatdan
Copy link
Member

rhatdan commented Jun 28, 2023

/approve
/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 28, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dfr, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 65a52f4 into containers:main Jun 28, 2023
@dfr dfr deleted the freebsd-servicereaper branch June 29, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants