1. Patchew
  2. Libvirt
  3. [PATCH 0/7] Various watchdog fixes
  4. View patch

[PATCH 7/7] qemu: Forbid most duplicated watchdogs

Martin Kletzander posted 7 patches 1 year, 7 months ago
[PATCH 7/7] qemu: Forbid most duplicated watchdogs
Posted by Martin Kletzander 1 year, 7 months ago 
Most of them are platform devices and only i6300esb can be plugged
multiple times into different PCI slots.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_validate.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b2c3fd1785bc..2b205348f597 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1110,19 +1110,21 @@ qemuValidateDomainDefTPMs(const virDomainDef *def)
 static int
 qemuValidateDomainDefWatchdogs(const virDomainDef *def)
 {
-    bool found_itco = false;
+    bool watchdogs[VIR_DOMAIN_WATCHDOG_MODEL_LAST] = {0};
     size_t i = 0;
 
     for (i = 0; i < def->nwatchdogs; i++) {
+        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB)
+            continue;
 
-        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_ITCO) {
-            if (found_itco) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                               _("Multiple iTCO watchdogs are not supported"));
-                return -1;
-            }
-            found_itco = true;
+        if (watchdogs[def->watchdogs[i]->model]) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("domain can only have one watchdog with model '%1$s'"),
+                           virDomainWatchdogModelTypeToString(def->watchdogs[i]->model));
+            return -1;
         }
+
+        watchdogs[def->watchdogs[i]->model] = true;
     }
 
     return 0;
-- 
2.40.0
Re: [PATCH 7/7] qemu: Forbid most duplicated watchdogs
Posted by Michal Prívozník 1 year, 7 months ago 
On 4/19/23 16:07, Martin Kletzander wrote:
> Most of them are platform devices and only i6300esb can be plugged
> multiple times into different PCI slots.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/qemu/qemu_validate.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index b2c3fd1785bc..2b205348f597 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1110,19 +1110,21 @@ qemuValidateDomainDefTPMs(const virDomainDef *def)
>  static int
>  qemuValidateDomainDefWatchdogs(const virDomainDef *def)
>  {
> -    bool found_itco = false;
> +    bool watchdogs[VIR_DOMAIN_WATCHDOG_MODEL_LAST] = {0};


We usually us virBitmap for this...

>      size_t i = 0;
>  
>      for (i = 0; i < def->nwatchdogs; i++) {
> +        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB)
> +            continue;
>  
> -        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_ITCO) {
> -            if (found_itco) {
> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                               _("Multiple iTCO watchdogs are not supported"));
> -                return -1;
> -            }
> -            found_itco = true;
> +        if (watchdogs[def->watchdogs[i]->model]) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("domain can only have one watchdog with model '%1$s'"),
> +                           virDomainWatchdogModelTypeToString(def->watchdogs[i]->model));
> +            return -1;
>          }
> +
> +        watchdogs[def->watchdogs[i]->model] = true;
>      }
>  
>      return 0;

It should be fairly trivial to rewrite.

Michal
Re: [PATCH 7/7] qemu: Forbid most duplicated watchdogs
Posted by Martin Kletzander 1 year, 7 months ago 
On Wed, Apr 19, 2023 at 04:53:36PM +0200, Michal Prívozník wrote:
>On 4/19/23 16:07, Martin Kletzander wrote:
>> Most of them are platform devices and only i6300esb can be plugged
>> multiple times into different PCI slots.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/qemu/qemu_validate.c | 18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index b2c3fd1785bc..2b205348f597 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -1110,19 +1110,21 @@ qemuValidateDomainDefTPMs(const virDomainDef *def)
>>  static int
>>  qemuValidateDomainDefWatchdogs(const virDomainDef *def)
>>  {
>> -    bool found_itco = false;
>> +    bool watchdogs[VIR_DOMAIN_WATCHDOG_MODEL_LAST] = {0};
>
>
>We usually us virBitmap for this...
>
>>      size_t i = 0;
>>
>>      for (i = 0; i < def->nwatchdogs; i++) {
>> +        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB)
>> +            continue;
>>
>> -        if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_ITCO) {
>> -            if (found_itco) {
>> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -                               _("Multiple iTCO watchdogs are not supported"));
>> -                return -1;
>> -            }
>> -            found_itco = true;
>> +        if (watchdogs[def->watchdogs[i]->model]) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                           _("domain can only have one watchdog with model '%1$s'"),
>> +                           virDomainWatchdogModelTypeToString(def->watchdogs[i]->model));
>> +            return -1;
>>          }
>> +
>> +        watchdogs[def->watchdogs[i]->model] = true;
>>      }
>>
>>      return 0;
>
>It should be fairly trivial to rewrite.
>

Is the following patch OK to squash in?

diff --git i/src/qemu/qemu_validate.c w/src/qemu/qemu_validate.c
index 2b205348f597..fdfb4c6407e2 100644
--- i/src/qemu/qemu_validate.c
+++ w/src/qemu/qemu_validate.c
@@ -26,6 +26,7 @@
  #include "qemu_domain.h"
  #include "qemu_process.h"
  #include "domain_conf.h"
+#include "virbitmap.h"
  #include "virlog.h"
  #include "virutil.h"

@@ -1110,21 +1111,25 @@ qemuValidateDomainDefTPMs(const virDomainDef *def)
  static int
  qemuValidateDomainDefWatchdogs(const virDomainDef *def)
  {
-    bool watchdogs[VIR_DOMAIN_WATCHDOG_MODEL_LAST] = {0};
+    g_autoptr(virBitmap) watchdogs = virBitmapNew(VIR_DOMAIN_WATCHDOG_MODEL_LAST);
      size_t i = 0;

      for (i = 0; i < def->nwatchdogs; i++) {
          if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB)
              continue;

-        if (watchdogs[def->watchdogs[i]->model]) {
+        if (virBitmapIsBitSet(watchdogs, def->watchdogs[i]->model)) {
              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                             _("domain can only have one watchdog with model '%1$s'"),
                             virDomainWatchdogModelTypeToString(def->watchdogs[i]->model));
              return -1;
          }

-        watchdogs[def->watchdogs[i]->model] = true;
+        if (virBitmapSetBit(watchdogs, def->watchdogs[i]->model) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Integrity error in watchdog models"));
+            return -1;
+        }
      }

      return 0;
--
Re: [PATCH 7/7] qemu: Forbid most duplicated watchdogs
Posted by Michal Prívozník 1 year, 7 months ago 
On 4/20/23 08:42, Martin Kletzander wrote:
> On Wed, Apr 19, 2023 at 04:53:36PM +0200, Michal Prívozník wrote:
>> On 4/19/23 16:07, Martin Kletzander wrote:
>>> Most of them are platform devices and only i6300esb can be plugged
>>> multiple times into different PCI slots.
>>>
>>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>>> ---
>>>  src/qemu/qemu_validate.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>>> index b2c3fd1785bc..2b205348f597 100644
>>> --- a/src/qemu/qemu_validate.c
>>> +++ b/src/qemu/qemu_validate.c
>>> @@ -1110,19 +1110,21 @@ qemuValidateDomainDefTPMs(const virDomainDef
>>> *def)
>>>  static int
>>>  qemuValidateDomainDefWatchdogs(const virDomainDef *def)
>>>  {
>>> -    bool found_itco = false;
>>> +    bool watchdogs[VIR_DOMAIN_WATCHDOG_MODEL_LAST] = {0};
>>
>>
>> We usually us virBitmap for this...
>>
>>>      size_t i = 0;
>>>
>>>      for (i = 0; i < def->nwatchdogs; i++) {
>>> +        if (def->watchdogs[i]->model ==
>>> VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB)
>>> +            continue;
>>>
>>> -        if (def->watchdogs[i]->model ==
>>> VIR_DOMAIN_WATCHDOG_MODEL_ITCO) {
>>> -            if (found_itco) {
>>> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> -                               _("Multiple iTCO watchdogs are not
>>> supported"));
>>> -                return -1;
>>> -            }
>>> -            found_itco = true;
>>> +        if (watchdogs[def->watchdogs[i]->model]) {
>>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> +                           _("domain can only have one watchdog with
>>> model '%1$s'"),
>>> +                          
>>> virDomainWatchdogModelTypeToString(def->watchdogs[i]->model));
>>> +            return -1;
>>>          }
>>> +
>>> +        watchdogs[def->watchdogs[i]->model] = true;
>>>      }
>>>
>>>      return 0;
>>
>> It should be fairly trivial to rewrite.
>>
> 
> Is the following patch OK to squash in?

Perfect.

Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.