1. Patchew
  2. Libvirt
  3. [libvirt PATCH v2 0/8] qemu: Add support for the armvtimer timer
  4. View patch

[libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer

Andrea Bolognani posted 8 patches 6 years ago
[libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer
Posted by Andrea Bolognani 6 years ago 
Its use is limited to certain guest types, and it only supports
a subset of all possible tick policies.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 68348464a8..8036886508 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
             break;
 
         case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
+            if (def->virtType != VIR_DOMAIN_VIRT_KVM ||
+                !qemuDomainIsARMVirt(def)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Configuring the '%s' timer is not supported "
+                                 "for virtType=%s arch=%s machine=%s guests"),
+                               virDomainTimerNameTypeToString(timer->name),
+                               virDomainVirtTypeToString(def->virtType),
+                               virArchToString(def->os.arch),
+                               def->os.machine);
+                return -1;
+            }
+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Configuring the '%s' timer is not supported "
+                                 "with this QEMU binary"),
+                               virDomainTimerNameTypeToString(timer->name));
+                return -1;
+            }
+
+            switch (timer->tickpolicy) {
+            case -1:
+            case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
+            case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
+                break;
+            case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
+            case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("The '%s' timer does not support "
+                                 "tickpolicy '%s'"),
+                               virDomainTimerNameTypeToString(timer->name),
+                               virDomainTimerTickpolicyTypeToString(timer->tickpolicy));
+                return -1;
+            }
             break;
         }
     }
-- 
2.24.1
Re: [libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer
Posted by Ján Tomko 5 years, 12 months ago 
On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
>Its use is limited to certain guest types, and it only supports
>a subset of all possible tick policies.
>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
> src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 68348464a8..8036886508 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
>             break;
>
>         case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:

Missing check for present == 0.

>+            if (def->virtType != VIR_DOMAIN_VIRT_KVM ||
>+                !qemuDomainIsARMVirt(def)) {
>+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                               _("Configuring the '%s' timer is not supported "
>+                                 "for virtType=%s arch=%s machine=%s guests"),
>+                               virDomainTimerNameTypeToString(timer->name),
>+                               virDomainVirtTypeToString(def->virtType),
>+                               virArchToString(def->os.arch),
>+                               def->os.machine);
>+                return -1;
>+            }
>+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) {
>+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                               _("Configuring the '%s' timer is not supported "
>+                                 "with this QEMU binary"),
>+                               virDomainTimerNameTypeToString(timer->name));
>+                return -1;
>+            }
>+
>+            switch (timer->tickpolicy) {
>+            case -1:
>+            case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
>+            case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
>+                break;
>+            case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
>+            case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
>+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                               _("The '%s' timer does not support "
>+                                 "tickpolicy '%s'"),

Please join the last two lines.

>+                               virDomainTimerNameTypeToString(timer->name),
>+                               virDomainTimerTickpolicyTypeToString(timer->tickpolicy));
>+                return -1;

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer
Posted by Andrea Bolognani 5 years, 12 months ago 
On Thu, 2020-02-13 at 14:28 +0100, Ján Tomko wrote:
> On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
> > +++ b/src/qemu/qemu_domain.c
> > @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
> >             break;
> > 
> >         case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
> 
> Missing check for present == 0.

Good catch! Assuming Drew confirms the timer can't be disabled, then
we should definitely error out for present='no'.

Do you want me to respin, or can I just squash in the diff below?


diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 29ec203413..143ddc508e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5441,6 +5441,12 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
                                def->os.machine);
                 return -1;
             }
+            if (timer->present == 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("The '%s' timer can't be disabled"),
+                               virDomainTimerNameTypeToString(timer->name));
+                return -1;
+            }
             if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("Configuring the '%s' timer is not supported "
-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer
Posted by Ján Tomko 5 years, 12 months ago 
On Thu, Feb 13, 2020 at 05:45:14PM +0100, Andrea Bolognani wrote:
>On Thu, 2020-02-13 at 14:28 +0100, Ján Tomko wrote:
>> On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
>> > +++ b/src/qemu/qemu_domain.c
>> > @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
>> >             break;
>> >
>> >         case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
>>
>> Missing check for present == 0.
>
>Good catch! Assuming Drew confirms the timer can't be disabled, then
>we should definitely error out for present='no'.
>
>Do you want me to respin, or can I just squash in the diff below?
>
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 29ec203413..143ddc508e 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -5441,6 +5441,12 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
>                                def->os.machine);
>                 return -1;
>             }
>+            if (timer->present == 0) {
>+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                               _("The '%s' timer can't be disabled"),
>+                               virDomainTimerNameTypeToString(timer->name));
>+                return -1;
>+            }
>             if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) {
>                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                _("Configuring the '%s' timer is not supported "

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH v2 4/8] qemu: Validate configuration for the armvtimer timer
Posted by Masayoshi Mizuma 5 years, 12 months ago 
On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
> Its use is limited to certain guest types, and it only supports
> a subset of all possible tick policies.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 68348464a8..8036886508 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def,
>              break;
>  
>          case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
> +            if (def->virtType != VIR_DOMAIN_VIRT_KVM ||
> +                !qemuDomainIsARMVirt(def)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("Configuring the '%s' timer is not supported "
> +                                 "for virtType=%s arch=%s machine=%s guests"),
> +                               virDomainTimerNameTypeToString(timer->name),
> +                               virDomainVirtTypeToString(def->virtType),
> +                               virArchToString(def->os.arch),
> +                               def->os.machine);
> +                return -1;
> +            }
> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("Configuring the '%s' timer is not supported "
> +                                 "with this QEMU binary"),
> +                               virDomainTimerNameTypeToString(timer->name));
> +                return -1;
> +            }
> +
> +            switch (timer->tickpolicy) {
> +            case -1:
> +            case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
> +            case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
> +                break;
> +            case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
> +            case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("The '%s' timer does not support "
> +                                 "tickpolicy '%s'"),
> +                               virDomainTimerNameTypeToString(timer->name),
> +                               virDomainTimerTickpolicyTypeToString(timer->tickpolicy));
> +                return -1;
> +            }
>              break;
>          }
>      }
> -- 
> 2.24.1
> 

Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.