On Mon, Feb 24, 2020 at 11:24:24AM -0300, Julio Faracco wrote:
This commit share host Real Time Clock device (rtc) into LXC
containers
to support hardware clock. This should be available setting up a `rtc`
timer under clock section. Since this option is not emulated, it should
be available only for `localtime` clock. This option should be readonly
due to security reasons.
Before:
root# hwclock --verbose
hwclock from util-linux 2.32.1
System Time: 1581877557.598365
Trying to open: /dev/rtc0
Trying to open: /dev/rtc
Trying to open: /dev/misc/rtc
No usable clock interface found.
hwclock: Cannot access the Hardware Clock via any known method.
Now:
root# hwclock
2020-02-16 18:23:55.374134+00:00
root# hwclock -w
hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
Permission denied
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
docs/formatdomain.html.in | 2 +-
src/lxc/lxc_cgroup.c | 36 +++++++++++++++++++++
src/lxc/lxc_controller.c | 68 +++++++++++++++++++++++++++++++++++++++
3 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4fef2a0a97..5598bf41b4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2465,7 +2465,7 @@
being modified, and can be one of
"platform" (currently unsupported),
"hpet" (libxl, xen, qemu), "kvmclock" (qemu),
- "pit" (qemu), "rtc" (qemu), "tsc" (libxl, qemu
-
+ "pit" (qemu), "rtc" (qemu, lxc), "tsc" (libxl,
qemu -
<span class="since">since 3.2.0</span>),
"hypervclock"
(qemu - <span class="since">since 1.2.2</span>) or
"armvtimer" (qemu - <span class="since">since
6.1.0</span>).
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 4ebe5ef467..6a103055a4 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -337,6 +337,42 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
VIR_CGROUP_DEVICE_RWM) < 0)
return -1;
+ VIR_DEBUG("Allowing timers char devices");
+
+ /* Sync'ed with Host clock */
+ if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME) {
+ for (i = 0; i < def->clock.ntimers; i++) {
+ virDomainTimerDefPtr timer = def->clock.timers[i];
+
+ switch ((virDomainTimerNameType)timer->name) {
+ case VIR_DOMAIN_TIMER_NAME_PLATFORM:
+ case VIR_DOMAIN_TIMER_NAME_TSC:
+ case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
+ case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
+ case VIR_DOMAIN_TIMER_NAME_PIT:
+ case VIR_DOMAIN_TIMER_NAME_HPET:
+ case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
+ case VIR_DOMAIN_TIMER_NAME_LAST:
+ break;
+ case VIR_DOMAIN_TIMER_NAME_RTC:
+ if (!timer->present)
+ break;
This field is not a boolean, it is a tri-state.
* -1 == the hypervisor default - historically with LXC this means
no /dev/rtc device
* 0 == disable it (matches the default for LXC historically)
* 1 == enable it
So this check needs to be "if (timer->present != 1) "
+
+ if (virFileExists("/dev/rtc")) {
+ if (virCgroupAllowDevicePath(cgroup, "/dev/rtc",
+ VIR_CGROUP_DEVICE_READ,
+ false) < 0)
+ return -1;
+ } else {
+ VIR_DEBUG("Ignoring non-existent device /dev/rtc");
We should be raising an error if the user requested RTC and it doesn't
exist.
+ }
+ break;
+ }
+ }
+ } else {
+ VIR_DEBUG("Ignoring non-localtime clock");
+ }
The setup for <timer> elements should be independant of whether or
not localtime is set.
Ideally we would raise an unsupported config error for containers
which don't have localtime set, but there's way too large a risk
of causing regressions if we do this.
I'd suggest we just don't check the clock offset at all, or at
least do the check independantly of the timer setup.
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c3dec0859c..eba6bfe0bf 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1550,6 +1550,71 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr
ctrl)
}
+static int
+virLXCControllerSetupTimers(virLXCControllerPtr ctrl)
+{
+ g_autofree char *path = NULL;
+ size_t i;
+ struct stat sb;
+ virDomainDefPtr def = ctrl->def;
+
+ /* Not sync'ed with Host clock */
+ if (def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME)
+ return 0;
+
+ for (i = 0; i < def->clock.ntimers; i++) {
+ dev_t dev;
+ virDomainTimerDefPtr timer = def->clock.timers[i];
+
+ switch ((virDomainTimerNameType)timer->name) {
+ case VIR_DOMAIN_TIMER_NAME_PLATFORM:
+ case VIR_DOMAIN_TIMER_NAME_TSC:
+ case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
+ case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
+ case VIR_DOMAIN_TIMER_NAME_PIT:
+ case VIR_DOMAIN_TIMER_NAME_HPET:
+ case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
+ case VIR_DOMAIN_TIMER_NAME_LAST:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported timer type (name) '%s'"),
+ virDomainTimerNameTypeToString(timer->name));
+ return -1;
+ case VIR_DOMAIN_TIMER_NAME_RTC:
+ if (!timer->present)
+ break;
Same comments as previously.
+
+ if (stat("/dev/rtc", &sb) < 0) {
+ if (errno == EACCES)
+ return -1;
Why this special case ? You're returning an error here without
setting any error message which is certainly not right.
+
+ virReportSystemError(errno,
+ _("Path '%s' is not
accessible"),
+ path);
+ return -1;
+ }
No error reporting if /dev/rtc is missing.
+
+ path = g_strdup_printf("/%s/%s.dev/%s", LXC_STATE_DIR,
+ ctrl->def->name, "/rtc");
+
+ dev = makedev(major(sb.st_rdev), minor(sb.st_rdev));
+ if (mknod(path, S_IFCHR, dev) < 0 ||
+ chmod(path, sb.st_mode)) {
+ virReportSystemError(errno,
+ _("Failed to make device %s"),
+ path);
+ return -1;
+ }
+
+ if (lxcContainerChown(ctrl->def, path) < 0)
+ return -1;
+ break;
+ }
+ }
+
+ return 0;
+}
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|