[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 10/16] conf: Add separate defaults addition and validation for XML parsing



On 03/01/13 20:10, Laine Stump wrote:
On 02/20/2013 12:06 PM, Peter Krempa wrote:
This patch adds instrumentation that will ultimately allow to split out
filling of defaults and input validation from the XML parser to separate
functions.

With this patch, after the XML is parsed, a callback to the driver is
issued requesing to fill and validate driver specific details of the
configuration. This allows to use sensible defaults and checks on a per
driver basis at the time the XML is parsed.

Two callback pointers are exposed in the virCaps object:
* virDriverDeviceDefCallback - called for a single device parsed and for
                                every single device in a domain config.
                                A virDomainDeviceDefPtr is passed.
* virDriverDomainDefCallback - called if a domain definition is parsed
                                device specific callbacks were called.
                                A virDomainDefPtr is passed.

Errors may be reported in those callbacks resulting in a XML parsing
failure.

Additionally two internal filler functions are added:
virDomainDeviceDefUpdateDefaultsInternal and
virDomainDefUpdateDefaultsInternal that are meant to be used for
separating the validation and defaults assignment code from the parser
function.
---
  src/conf/capabilities.h |  6 +++++
  src/conf/domain_conf.c  | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 78 insertions(+)

diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index cc01765..56cd09f 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -171,6 +171,12 @@ struct _virCaps {
      bool hasWideScsiBus;
      const char *defaultInitPath;

+
+    /* these callbacks are called after a XML definition of a device or domain
+     * is parsed. They are meant to validate and fill driver-specific values. */
+    int (*virDriverDeviceDefCallback)(void *); /* virDomainDeviceDefPtr is passed */
+    int (*virDriverDomainDefCallback)(void *); /* virDomainDefPtr is passed */

If you know that  virDriverDeviceDefCallback is always given a
virDomainDeviceDefPtr, why prototype it as void*? Same question for
virDriverDomainDefCallback.

Those two types are defined in conf/domain_conf.h. conf/domain_conf.h in return needs conf/capabilities.h. This creates a circular dependency chain if I try to include domain_conf.h in capabilities.h. I tried to to come up with a different solution than void * but none of the others were nicer than that. The options were:

1) use a separate header file for one of the types
2) use a extern declaration
3) include domain_conf.h into capabilities.h after all needed types were declared.

I'll welcome any tips how to solve this problem. (cc-d Dan and Eric).


Additionally, in the callback for a device, we will need to have the
virDomainDefPtr (e.g. so that we can see what machinetype was selected
for the domain). And in both of these callbacks we will need to get the
virCapsPtr so that (for example), we can have access to information
about which devices are available for which machine types, the default
pci addresses of integrated devices, etc.

So, I think the callbacks should be like this:

   int (*virDriverDeviceDefCallback) (virCapsPtr caps, virDomainDefPtr
*domdef, virDomainDeviceDefPtr *devdef);
   int (*virDriverDomainDefCallback) (virCapsPtr caps, virDomainDefPtr
*domdef);


Yep, seems reasonable to me.

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]