1. Patchew
  2. Libvirt
  3. [PATCH v4 0/5] lxc: Add VCPU features for LXC
  4. View patch

[PATCH v4 2/5] lxc: Add HPET device into allowed devices

Julio Faracco posted 5 patches 5 years, 11 months ago
[PATCH v4 2/5] lxc: Add HPET device into allowed devices
Posted by Julio Faracco 5 years, 11 months ago 
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@gmail.com>
---
 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;
-
                 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");
+                }
+                break;
             }
         }
     } else {
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index eba6bfe0bf..518967ee83 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1566,13 +1566,15 @@ virLXCControllerSetupTimers(virLXCControllerPtr ctrl)
         dev_t dev;
         virDomainTimerDefPtr timer = def->clock.timers[i];
 
+        if (!timer->present)
+            continue;
+
         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,
@@ -1580,9 +1582,6 @@ virLXCControllerSetupTimers(virLXCControllerPtr ctrl)
                            virDomainTimerNameTypeToString(timer->name));
             return -1;
         case VIR_DOMAIN_TIMER_NAME_RTC:
-            if (!timer->present)
-                break;
-
             if (stat("/dev/rtc", &sb) < 0) {
                 if (errno == EACCES)
                     return -1;
@@ -1605,6 +1604,32 @@ virLXCControllerSetupTimers(virLXCControllerPtr ctrl)
                 return -1;
             }
 
+            if (lxcContainerChown(ctrl->def, path) < 0)
+                return -1;
+            break;
+        case VIR_DOMAIN_TIMER_NAME_HPET:
+            if (stat("/dev/hpet", &sb) < 0) {
+                if (errno == EACCES)
+                    return -1;
+
+                virReportSystemError(errno,
+                                     _("Path '%s' is not accessible"),
+                                     path);
+                return -1;
+            }
+
+            path = g_strdup_printf("/%s/%s.dev/%s", LXC_STATE_DIR,
+                                   ctrl->def->name, "/hpet");
+
+            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;
-- 
2.20.1
Re: [PATCH v4 2/5] lxc: Add HPET device into allowed devices
Posted by Daniel P. Berrangé 5 years, 11 months ago 
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@gmail.com>
> ---
>  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 :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.