[libvirt] [PATCH 07/11] conf: Extend virDomainDefValidate(Callback) for parseOpaque

Marc Hartmayer posted 11 patches 7 years, 4 months ago
[libvirt] [PATCH 07/11] conf: Extend virDomainDefValidate(Callback) for parseOpaque
Posted by Marc Hartmayer 7 years, 4 months ago
Add @parseOpaque argument to virDomainDefValidate and
virDomainDefValidateCallback, but don't use it for now since it's not
ensured that it's always a non-NULL value.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
 src/conf/domain_conf.c  | 11 +++++++----
 src/conf/domain_conf.h  |  6 ++++--
 src/qemu/qemu_domain.c  |  3 ++-
 src/qemu/qemu_process.c |  2 +-
 src/vz/vz_driver.c      |  3 ++-
 5 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e61f04ea2271..ae7f3ed95faf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6271,6 +6271,7 @@ virDomainDefValidateInternal(const virDomainDef *def)
  * @caps: driver capabilities object
  * @parseFlags: virDomainDefParseFlags
  * @xmlopt: XML parser option object
+ * @parseOpaque: opaque data and it might be NULL (for QEMU driver it's qemuCaps)
  *
  * This validation function is designed to take checks of globally invalid
  * configurations that the parser needs to accept so that VMs don't vanish upon
@@ -6284,12 +6285,14 @@ int
 virDomainDefValidate(virDomainDefPtr def,
                      virCapsPtr caps,
                      unsigned int parseFlags,
-                     virDomainXMLOptionPtr xmlopt)
+                     virDomainXMLOptionPtr xmlopt,
+                     void *parseOpaque)
 {
     struct virDomainDefPostParseDeviceIteratorData data = {
         .caps = caps,
         .xmlopt = xmlopt,
         .parseFlags = parseFlags,
+        .parseOpaque = parseOpaque,
     };
 
     /* validate configuration only in certain places */
@@ -6298,7 +6301,7 @@ virDomainDefValidate(virDomainDefPtr def,
 
     /* call the domain config callback */
     if (xmlopt->config.domainValidateCallback &&
-        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0)
+        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv, data.parseOpaque) < 0)
         return -1;
 
     /* iterate the devices */
@@ -21063,7 +21066,7 @@ virDomainObjParseXML(xmlDocPtr xml,
         goto error;
 
     /* valdiate configuration */
-    if (virDomainDefValidate(obj->def, caps, flags, xmlopt) < 0)
+    if (virDomainDefValidate(obj->def, caps, flags, xmlopt, parseOpaque) < 0)
         goto error;
 
     return obj;
@@ -21154,7 +21157,7 @@ virDomainDefParseNode(xmlDocPtr xml,
         goto cleanup;
 
     /* valdiate configuration */
-    if (virDomainDefValidate(def, caps, flags, xmlopt) < 0)
+    if (virDomainDefValidate(def, caps, flags, xmlopt, parseOpaque) < 0)
         goto cleanup;
 
     VIR_STEAL_PTR(ret, def);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e30a4b2fe7b9..3642d5eb6cba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2696,7 +2696,8 @@ typedef void (*virDomainDefPostParseDataFree)(void *parseOpaque);
  * config. */
 typedef int (*virDomainDefValidateCallback)(const virDomainDef *def,
                                             virCapsPtr caps,
-                                            void *opaque);
+                                            void *opaque,
+                                            void *domainOpaque);
 
 /* Called once per device, for adjusting per-device settings while
  * leaving the overall domain otherwise unchanged.  */
@@ -2812,7 +2813,8 @@ bool virDomainDeviceAliasIsUserAlias(const char *aliasStr);
 int virDomainDefValidate(virDomainDefPtr def,
                          virCapsPtr caps,
                          unsigned int parseFlags,
-                         virDomainXMLOptionPtr xmlopt);
+                         virDomainXMLOptionPtr xmlopt,
+                         void *parseOpaque);
 
 static inline bool
 virDomainObjIsActive(virDomainObjPtr dom)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index eb80711597cb..52d2aa435a36 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3990,7 +3990,8 @@ qemuDomainDefValidateMemory(const virDomainDef *def)
 static int
 qemuDomainDefValidate(const virDomainDef *def,
                       virCapsPtr caps ATTRIBUTE_UNUSED,
-                      void *opaque)
+                      void *opaque,
+                      void *parseOpaque ATTRIBUTE_UNUSED)
 {
     virQEMUDriverPtr driver = opaque;
     virQEMUCapsPtr qemuCaps = NULL;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6c5a6472d8cd..fed2c0f545e3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5161,7 +5161,7 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver,
      * VM that was running before (migration, snapshots, save). It's more
      * important to start such VM than keep the configuration clean */
     if ((flags & VIR_QEMU_PROCESS_START_NEW) &&
-        virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0)
+        virDomainDefValidate(vm->def, caps, 0, driver->xmlopt, qemuCaps) < 0)
         return -1;
 
     return 0;
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index bedc6316db8d..16c44c2f2215 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -250,7 +250,8 @@ vzDomainDefPostParse(virDomainDefPtr def,
 static int
 vzDomainDefValidate(const virDomainDef *def,
                     virCapsPtr caps ATTRIBUTE_UNUSED,
-                    void *opaque)
+                    void *opaque,
+                    void *parserOpaque ATTRIBUTE_UNUSED)
 {
     if (vzCheckUnsupportedControllers(def, opaque) < 0)
         return -1;
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/11] conf: Extend virDomainDefValidate(Callback) for parseOpaque
Posted by John Ferlan 7 years, 4 months ago

On 9/20/18 1:44 PM, Marc Hartmayer wrote:
> Add @parseOpaque argument to virDomainDefValidate and
> virDomainDefValidateCallback, but don't use it for now since it's not
> ensured that it's always a non-NULL value.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>  src/conf/domain_conf.c  | 11 +++++++----
>  src/conf/domain_conf.h  |  6 ++++--
>  src/qemu/qemu_domain.c  |  3 ++-
>  src/qemu/qemu_process.c |  2 +-
>  src/vz/vz_driver.c      |  3 ++-
>  5 files changed, 16 insertions(+), 9 deletions(-)
> 

I like this idea especially since the Validate paths are the ones where
qemuCaps seem to be most useful.

Couple of nits below

John

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e61f04ea2271..ae7f3ed95faf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6271,6 +6271,7 @@ virDomainDefValidateInternal(const virDomainDef *def)
>   * @caps: driver capabilities object
>   * @parseFlags: virDomainDefParseFlags
>   * @xmlopt: XML parser option object
> + * @parseOpaque: opaque data and it might be NULL (for QEMU driver it's qemuCaps)
>   *
>   * This validation function is designed to take checks of globally invalid
>   * configurations that the parser needs to accept so that VMs don't vanish upon
> @@ -6284,12 +6285,14 @@ int
>  virDomainDefValidate(virDomainDefPtr def,
>                       virCapsPtr caps,
>                       unsigned int parseFlags,
> -                     virDomainXMLOptionPtr xmlopt)
> +                     virDomainXMLOptionPtr xmlopt,
> +                     void *parseOpaque)
>  {
>      struct virDomainDefPostParseDeviceIteratorData data = {
>          .caps = caps,
>          .xmlopt = xmlopt,
>          .parseFlags = parseFlags,
> +        .parseOpaque = parseOpaque,
>      };
>  
>      /* validate configuration only in certain places */
> @@ -6298,7 +6301,7 @@ virDomainDefValidate(virDomainDefPtr def,
>  
>      /* call the domain config callback */
>      if (xmlopt->config.domainValidateCallback &&
> -        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0)
> +        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv, data.parseOpaque) < 0)
>          return -1;
>  
>      /* iterate the devices */
> @@ -21063,7 +21066,7 @@ virDomainObjParseXML(xmlDocPtr xml,
>          goto error;
>  
>      /* valdiate configuration */

May as well fix the typo above *validate

> -    if (virDomainDefValidate(obj->def, caps, flags, xmlopt) < 0)
> +    if (virDomainDefValidate(obj->def, caps, flags, xmlopt, parseOpaque) < 0)
>          goto error;
>  
>      return obj;
> @@ -21154,7 +21157,7 @@ virDomainDefParseNode(xmlDocPtr xml,
>          goto cleanup;
>  
>      /* valdiate configuration */

Consistency is the key ;-)

> -    if (virDomainDefValidate(def, caps, flags, xmlopt) < 0)
> +    if (virDomainDefValidate(def, caps, flags, xmlopt, parseOpaque) < 0)
>          goto cleanup;
>  
>      VIR_STEAL_PTR(ret, def);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index e30a4b2fe7b9..3642d5eb6cba 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2696,7 +2696,8 @@ typedef void (*virDomainDefPostParseDataFree)(void *parseOpaque);
>   * config. */
>  typedef int (*virDomainDefValidateCallback)(const virDomainDef *def,
>                                              virCapsPtr caps,
> -                                            void *opaque);
> +                                            void *opaque,
> +                                            void *domainOpaque);
>  
>  /* Called once per device, for adjusting per-device settings while
>   * leaving the overall domain otherwise unchanged.  */
> @@ -2812,7 +2813,8 @@ bool virDomainDeviceAliasIsUserAlias(const char *aliasStr);
>  int virDomainDefValidate(virDomainDefPtr def,
>                           virCapsPtr caps,
>                           unsigned int parseFlags,
> -                         virDomainXMLOptionPtr xmlopt);
> +                         virDomainXMLOptionPtr xmlopt,
> +                         void *parseOpaque);
>  
>  static inline bool
>  virDomainObjIsActive(virDomainObjPtr dom)
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index eb80711597cb..52d2aa435a36 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3990,7 +3990,8 @@ qemuDomainDefValidateMemory(const virDomainDef *def)
>  static int
>  qemuDomainDefValidate(const virDomainDef *def,
>                        virCapsPtr caps ATTRIBUTE_UNUSED,
> -                      void *opaque)
> +                      void *opaque,
> +                      void *parseOpaque ATTRIBUTE_UNUSED)
>  {
>      virQEMUDriverPtr driver = opaque;
>      virQEMUCapsPtr qemuCaps = NULL;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 6c5a6472d8cd..fed2c0f545e3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5161,7 +5161,7 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver,
>       * VM that was running before (migration, snapshots, save). It's more
>       * important to start such VM than keep the configuration clean */
>      if ((flags & VIR_QEMU_PROCESS_START_NEW) &&
> -        virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0)
> +        virDomainDefValidate(vm->def, caps, 0, driver->xmlopt, qemuCaps) < 0)
>          return -1;
>  
>      return 0;
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index bedc6316db8d..16c44c2f2215 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -250,7 +250,8 @@ vzDomainDefPostParse(virDomainDefPtr def,
>  static int
>  vzDomainDefValidate(const virDomainDef *def,
>                      virCapsPtr caps ATTRIBUTE_UNUSED,
> -                    void *opaque)
> +                    void *opaque,
> +                    void *parserOpaque ATTRIBUTE_UNUSED)

nit: @parseOpaque

>  {
>      if (vzCheckUnsupportedControllers(def, opaque) < 0)
>          return -1;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/11] conf: Extend virDomainDefValidate(Callback) for parseOpaque
Posted by Marc Hartmayer 7 years, 4 months ago
On Sat, Sep 29, 2018 at 05:34 PM +0200, John Ferlan <jferlan@redhat.com> wrote:
> On 9/20/18 1:44 PM, Marc Hartmayer wrote:
>> Add @parseOpaque argument to virDomainDefValidate and
>> virDomainDefValidateCallback, but don't use it for now since it's not
>> ensured that it's always a non-NULL value.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> ---
>>  src/conf/domain_conf.c  | 11 +++++++----
>>  src/conf/domain_conf.h  |  6 ++++--
>>  src/qemu/qemu_domain.c  |  3 ++-
>>  src/qemu/qemu_process.c |  2 +-
>>  src/vz/vz_driver.c      |  3 ++-
>>  5 files changed, 16 insertions(+), 9 deletions(-)
>>
>
> I like this idea especially since the Validate paths are the ones where
> qemuCaps seem to be most useful.
>
> Couple of nits below
>
> John
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index e61f04ea2271..ae7f3ed95faf 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -6271,6 +6271,7 @@ virDomainDefValidateInternal(const virDomainDef *def)
>>   * @caps: driver capabilities object
>>   * @parseFlags: virDomainDefParseFlags
>>   * @xmlopt: XML parser option object
>> + * @parseOpaque: opaque data and it might be NULL (for QEMU driver it's qemuCaps)
>>   *
>>   * This validation function is designed to take checks of globally invalid
>>   * configurations that the parser needs to accept so that VMs don't vanish upon
>> @@ -6284,12 +6285,14 @@ int
>>  virDomainDefValidate(virDomainDefPtr def,
>>                       virCapsPtr caps,
>>                       unsigned int parseFlags,
>> -                     virDomainXMLOptionPtr xmlopt)
>> +                     virDomainXMLOptionPtr xmlopt,
>> +                     void *parseOpaque)
>>  {
>>      struct virDomainDefPostParseDeviceIteratorData data = {
>>          .caps = caps,
>>          .xmlopt = xmlopt,
>>          .parseFlags = parseFlags,
>> +        .parseOpaque = parseOpaque,
>>      };
>>
>>      /* validate configuration only in certain places */
>> @@ -6298,7 +6301,7 @@ virDomainDefValidate(virDomainDefPtr def,
>>
>>      /* call the domain config callback */
>>      if (xmlopt->config.domainValidateCallback &&
>> -        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0)
>> +        xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv, data.parseOpaque) < 0)
>>          return -1;
>>
>>      /* iterate the devices */
>> @@ -21063,7 +21066,7 @@ virDomainObjParseXML(xmlDocPtr xml,
>>          goto error;
>>
>>      /* valdiate configuration */
>
> May as well fix the typo above *validate

Will change.

>
>> -    if (virDomainDefValidate(obj->def, caps, flags, xmlopt) < 0)
>> +    if (virDomainDefValidate(obj->def, caps, flags, xmlopt, parseOpaque) < 0)
>>          goto error;
>>
>>      return obj;
>> @@ -21154,7 +21157,7 @@ virDomainDefParseNode(xmlDocPtr xml,
>>          goto cleanup;
>>
>>      /* valdiate configuration */
>
> Consistency is the key ;-)

Yep :D

>
>> -    if (virDomainDefValidate(def, caps, flags, xmlopt) < 0)
>> +    if (virDomainDefValidate(def, caps, flags, xmlopt, parseOpaque) < 0)
>>          goto cleanup;
>>

[…snip…]

>>  static int
>>  vzDomainDefValidate(const virDomainDef *def,
>>                      virCapsPtr caps ATTRIBUTE_UNUSED,
>> -                    void *opaque)
>> +                    void *opaque,
>> +                    void *parserOpaque ATTRIBUTE_UNUSED)
>
> nit: @parseOpaque

Okay.

>
>>  {
>>      if (vzCheckUnsupportedControllers(def, opaque) < 0)
>>          return -1;
>>
>

Thanks for the review!

--
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list