On Mon, Feb 24, 2020 at 11:24:25AM -0300, Julio Faracco wrote:
> This commit is related to RTC timer device too. HPET is being shared
> from host device through `localtime` clock. This timer is available
> creating a new timer using `hpet` name.
>
> Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx>
> ---
> docs/formatdomain.html.in | 2 +-
> src/lxc/lxc_cgroup.c | 17 +++++++++++++----
> src/lxc/lxc_controller.c | 33 +++++++++++++++++++++++++++++----
> 3 files changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 5598bf41b4..8571db89dc 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2464,7 +2464,7 @@
> The <code>name</code> attribute selects which timer is
> being modified, and can be one of
> "platform" (currently unsupported),
> - "hpet" (libxl, xen, qemu), "kvmclock" (qemu),
> + "hpet" (libxl, xen, qemu, lxc), "kvmclock" (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
> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index 6a103055a4..997a5c3dfa 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -344,20 +344,19 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
> for (i = 0; i < def->clock.ntimers; i++) {
> virDomainTimerDefPtr timer = def->clock.timers[i];
>
> + if (!timer->present)
> + break;
> +
> 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;
Instead of moving the check here, just put it in the right place
immediately in the previous patch. Note the comment about this not
being a boolean, but rather a tri-state.
> -
> if (virFileExists("/dev/rtc")) {
> if (virCgroupAllowDevicePath(cgroup, "/dev/rtc",
> VIR_CGROUP_DEVICE_READ,
> @@ -367,6 +366,16 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
> VIR_DEBUG("Ignoring non-existent device /dev/rtc");
> }
> break;
> + case VIR_DOMAIN_TIMER_NAME_HPET:
> + if (virFileExists("/dev/hpet")) {
> + if (virCgroupAllowDevicePath(cgroup, "/dev/hpet",
> + VIR_CGROUP_DEVICE_READ,
> + false) < 0)
> + return -1;
> + } else {
> + VIR_DEBUG("Ignoring non-existent device /dev/hpet");
> + }
Same comment about needing to report an error here.
> + case VIR_DOMAIN_TIMER_NAME_HPET:
> + if (stat("/dev/hpet", &sb) < 0) {
> + if (errno == EACCES)
> + return -1;
Same strange special case missing error message reporting.
> +
> + virReportSystemError(errno,
> + _("Path '%s' is not accessible"),
> + path);
> + return -1;
> + }
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 :|
