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