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).