-
Notifications
You must be signed in to change notification settings - Fork 908
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
distro: add FreeBSD code path for eject #4838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the Dragonflybsd class inherits from the Freebsd class this actually overrides dragonfly's eject_media() function as well, which I don't think we want.
I think we also need the following:
diff --git a/cloudinit/distros/dragonflybsd.py b/cloudinit/distros/dragonflybsd.py
index 5032ff5a7..9a7a53eee 100644
--- a/cloudinit/distros/dragonflybsd.py
+++ b/cloudinit/distros/dragonflybsd.py
@@ -2,8 +2,13 @@
#
# This file is part of cloud-init. See LICENSE file for license information.
-import cloudinit.distros.freebsd
+from cloudinit.distros import Distro as base
+from cloudinit.distros import freebsd
-class Distro(cloudinit.distros.freebsd.Distro):
+class Distro(freebsd.Distro):
home_dir = "/home"
+
+ @staticmethod
+ def eject_media(device: str) -> None:
+ base.eject_media(device)
yeah, we could do that, but dragonfly has camcontrol(8). cam is very old, as you can see from the history section. more importantly: i have misread https://man.dragonflybsd.org/?command=eject§ion=ANY this is just the same (unmaintained) eject port: https://man.freebsd.org/cgi/man.cgi?eject so, that commit message needs adapting. |
That sounds good, thanks @igalic! |
n.b.: Tomorrow has turned into much later, as I'm at a conference right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting to state "Request changes" state for now until commit message is updated - then I'll merge.
53d82a4
to
37f11d5
Compare
OpenBSD, and NetBSD both have an eject(1), so they should be covered in the default code path. FreeBSD and Dragonfly however, do not have eject in base. Here, eject is an (unmaintained) port. In base, we do however, have camcontrol(8) and cdcontrol(1), both of which have an eject subcommand. Let's use camcontrol(8) here. Sponsored by: The FreeBSD Foundation
37f11d5
to
a344fc8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
OpenBSD, and NetBSD both have an eject(1), so they should be covered in the default code path. FreeBSD and Dragonfly however, do not have eject in base. Here, eject is an (unmaintained) port. In base, we do however, have camcontrol(8) and cdcontrol(1), both of which have an eject subcommand. Let's use camcontrol(8) here. Sponsored by: The FreeBSD Foundation
OpenBSD, and NetBSD both have an eject(1), so they should be covered in the default code path. FreeBSD and Dragonfly however, do not have eject in base. Here, eject is an (unmaintained) port. In base, we do however, have camcontrol(8) and cdcontrol(1), both of which have an eject subcommand. Let's use camcontrol(8) here. Sponsored by: The FreeBSD Foundation
OpenBSD, and NetBSD both have an eject(1), so they should be covered in the default code path. FreeBSD and Dragonfly however, do not have eject in base. Here, eject is an (unmaintained) port. In base, we do however, have camcontrol(8) and cdcontrol(1), both of which have an eject subcommand. Let's use camcontrol(8) here. Sponsored by: The FreeBSD Foundation
OpenBSD, and NetBSD both have an eject(1), so they should be covered in the default code path. FreeBSD and Dragonfly however, do not have eject in base. Here, eject is an (unmaintained) port. In base, we do however, have camcontrol(8) and cdcontrol(1), both of which have an eject subcommand. Let's use camcontrol(8) here. Sponsored by: The FreeBSD Foundation
Proposed Commit Message
Additional Context
Test Steps
Checklist
Merge type