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

Device Unix #6113

Merged
merged 26 commits into from Sep 3, 2019

Conversation

@tomponline
Copy link
Member

commented Aug 22, 2019

Migration of unix-char and unix-block devices into the new device interface.

This PR at a top level does the following:

  • Moves the inotify event handling code into the device package under the file device_utils_unix_events and changes it to support the concept of handler subscriptions rather than iterating container config directly.
  • Adds a unixCommon device implementation that is used for unix-char and unix-block type devices.
  • Modifies the device event registration process so that it is done via post start hooks to guarantee it happens when the container is running and after it has started.
  • Fixes the bug that prevents dynamic monitoring of new unix devices from working unless LXD is restarted.
  • Cleans up inotify watchers that are not needed when an instance is stopped.

Part of #5819

  • Statically adding devices whilst running and at boot time
  • Add validation of major and minor unix device numbers
  • Dynamic adding of devices using inotify
  • Tests
  • Fix bug where adding new missing device was not detected until restart of LXD

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch 3 times, most recently from 0feddee to 4ae5a4a Aug 22, 2019

@lxc-jenkins

This comment has been minimized.

Copy link

commented Aug 27, 2019

Testsuite passed

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch 7 times, most recently from 48cdaaf to 77d8f33 Aug 27, 2019

@tomponline tomponline marked this pull request as ready for review Aug 27, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2019

jenkins: test this please

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch 3 times, most recently from d2160be to 43a819f Aug 28, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2019

@stgraber ready for review.

@lxc-jenkins

This comment has been minimized.

Copy link

commented Aug 28, 2019

Testsuite passed

@tomponline tomponline referenced this pull request Aug 29, 2019
5 of 5 tasks complete

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch from 43a819f to 2f72862 Aug 29, 2019

tomponline added 10 commits Aug 22, 2019
test: Removes tmpfs references from gpu tests
Tests may break if not run on tmpfs otherwise.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/device/utils/unix: Various small improvements
- Improves comments
- Removes duplicated path and source logic
- Removes duplicated default mode

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device: Links up unix-char and unix-block devices
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device: Moves empty device type validation into device package
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container: Removes unused unix-char and unix-block validation
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container: Moves missing device type validation into device package
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Adds safety net for deviceStop() in case no device ret…
…urned

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Removes unused unix-char and unix-block code
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
devices: Removes inotify code
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
devices: Renames USBDevice to USBEvent
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/usb: Removes unused function
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch from 2f72862 to 4506ba1 Aug 29, 2019

@lxc-jenkins

This comment has been minimized.

Copy link

commented Aug 29, 2019

Testsuite passed

1 similar comment
@lxc-jenkins

This comment has been minimized.

Copy link

commented Aug 29, 2019

Testsuite passed

lxd/device/unixCommon.go Outdated Show resolved Hide resolved
@stgraber

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

Sorry for some of the confusion :)

Overall, this looks good and I think the only remaining actions are:

  • Rename unixCommon.go to unix_common.go (we never use camel case in the file names)
  • Split out the inotify* functions into their own file
tomponline added 12 commits Aug 22, 2019
test: Adds tests for unix-char and unix-block devices
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/unix/common: Adds implementation for unix-char and unix-block …
…devices

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/device/utils/unix/events: Adds unix event handling functions
Moves inotify related functions into this file too as they were tightly coupled to the unix event handlers.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Removes device Register() after device start
Moves to using post start hooks so that Register is optional, controlled by the device, and always occurs after the instance has started or the device has started (when hot plugging).

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
daemon: Reorganised the uevent and inotify event handler startup to o…
…ccur before device registration.

This is to allow devices to trigger new inotify watches as devices are registered, and to ensure that instances are started before executing their event handlers.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/device/utils/unix: Moves some config validation functions into…
… device package

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/gpu: Used device package validation functions
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/proxy: Used device package validation functions
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/usb: Updates Register() to be called by post start hook
Also updates util function names.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
shared/container: Removes device related validation functions
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
container/lxc: Removes unused setupUnixDevice()
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
device/device/utils/inotify: Moves inotify functions to device package
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch from 61eff65 to c612d35 Sep 3, 2019

@tomponline

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2019

thanks @stgraber those changes are now made.

@lxc-jenkins

This comment has been minimized.

Copy link

commented Sep 3, 2019

Testsuite passed

@tomponline tomponline requested a review from stgraber Sep 3, 2019

@tomponline tomponline force-pushed the tomponline:tp-device-unix branch from 45ad387 to c612d35 Sep 3, 2019

@stgraber stgraber merged commit 99d2735 into lxc:master Sep 3, 2019

4 of 5 checks passed

Testsuite Build finished.
Details
Branch target Branch target is correct
Details
DCO All commits signed-off
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tomponline tomponline deleted the tomponline:tp-device-unix branch Sep 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.