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

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.

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