>On Wed, Jun 10, 2020 at 09:20:28AM +0800, Shi Lei wrote:
>>
>> Last RFC:
[
https://www.redhat.com/archives/libvir-list/2020-April/msg00970.html]
>> In last RFC, I suggested we can generate object-model code based on relax-ng
>> files and Daniel gave it some comments.
>>
>> Follow the suggestion from Daniel, I make another try to generate
parsexml/formatbuf
>> functions automatically.
>>
>>
>> ============
>> Directives
>> ============
>>
>> Still, we need several directives that can direct a tool to generate functions.
>> These directives work on the declarations of structs. For example:
>>
>> typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
>> typedef virNetworkDNSTxtDef *virNetworkDNSTxtDefPtr;
>> struct _virNetworkDNSTxtDef { /* genparse:concisehook, genformat */
>> char *name; /* xmlattr, required */
>> char *value; /* xmlattr */
>> };
>>
>> typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef;
>> typedef virNetworkDNSSrvDef *virNetworkDNSSrvDefPtr;
>> struct _virNetworkDNSSrvDef { /* genparse:withhook, genformat */
>> char *service; /* xmlattr */
>> char *protocol; /* xmlattr */
>> char *domain; /* xmlattr */
>> char *target; /* xmlattr */
>> unsigned int port; /* xmlattr */
>> unsigned int priority; /* xmlattr */
>> unsigned int weight; /* xmlattr */
>> };
>>
>> typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef;
>> typedef virNetworkDNSHostDef *virNetworkDNSHostDefPtr;
>> struct _virNetworkDNSHostDef { /* genparse:withhook, genformat */
>> virSocketAddr ip; /* xmlattr */
>> size_t nnames;
>> char **names; /* xmlelem:hostname, array */
>> };
>>
>> typedef struct _virNetworkDNSForwarder virNetworkDNSForwarder;
>> typedef virNetworkDNSForwarder *virNetworkDNSForwarderPtr;
>> struct _virNetworkDNSForwarder { /* genparse:withhook, genformat */
>> char *domain; /* xmlattr */
>> virSocketAddr addr; /* xmlattr */
>> };
>>
>> typedef struct _virNetworkDNSDef virNetworkDNSDef;
>> typedef virNetworkDNSDef *virNetworkDNSDefPtr;
>> struct _virNetworkDNSDef { /* genparse:withhook, genformat */
>> virTristateBool enable; /* xmlattr */
>> virTristateBool forwardPlainNames; /* xmlattr */
>> size_t nforwarders;
>> virNetworkDNSForwarderPtr forwarders; /* xmlelem, array */
>> size_t ntxts;
>> virNetworkDNSTxtDefPtr txts; /* xmlelem, array */
>> size_t nsrvs;
>> virNetworkDNSSrvDefPtr srvs; /* xmlelem, array */
>> size_t nhosts;
>> virNetworkDNSHostDefPtr hosts; /* xmlelem, array */
>> };
>>
>>
>> Explanation for these directives:
>>
>>
>> - genparse[:withhook|concisehook]
>>
>> Only work on a struct.
>> Generate parsexml function for this struct only if 'genparse' is
specified.
>> The function name is based on the struct-name and suffixed with
'ParseXML'.
>> E.g., for struct virNetworkDNSDef, its parsexml function is
>> virNetworkDNSDefParseXML.
>>
>> If a parsexml function includes some error-checking code, it needs a
>> post-process hook to hold them. Specify 'withhook' or
'concisehook' to
>> setup a hook point in the parsexml function.
>>
>> Executing the tool manually can show the declaration of hook function.
>> E.g. check declaration of 'virNetworkDNSDefParseXMLHook'.
>>
>> # ./build-aux/generator/go show virNetworkDNSDef -kp
>>
>> int
>> virNetworkDNSDefParseXMLHook(xmlNodePtr node,
>> virNetworkDNSDefPtr def,
>> const char *instname,
>> void *opaque,
>> const char *enableStr,
>> const char *forwardPlainNamesStr,
>> int nForwarderNodes,
>> int nTxtNodes,
>> int nSrvNodes,
>> int nHostNodes);
>>
>> Some helper arguments (such as 'enableStr', 'nTxtNodes',
etc.) are
>> passed in to indicate the existence of fields.
>> If these arguments are useless, specify 'concisehook' to omit
them.
>>
>> When 'genparse' is specified, clear function for this struct is
also
>> generated implicitly, it is because the generated parsexml function needs
>> to call the clear function.
>>
>>
>> - genformat
>>
>> Only work on a struct.
>> Generate formatbuf function for this struct only if 'genformat' is
specified.
>> The function name is based on struct-name and suffixed with
'FormatBuf'.
>>
>>
>> - xmlattr[:thename]
>>
>> Parse/Format the field as an XML attribute.
>> If 'thename' is specified, use it as the XML attribute name;
>> or use the filed name.
>>
>>
>> - xmlelem[:thename]
>>
>> Parse/Format the field as a child struct.
>> If 'thename' is specified, use it as the XML element name;
>> or use the filed name.
>>
>>
>> - array
>>
>> Parse/Format the field as an array.
>> Its related field is a counter, which name should follow the pattern:
>> n + 'field_name'.
>>
>>
>> - required
>>
>> The field must have a corresponding item defined in XML.
>
>This all looks pretty reasonable and simple to understand to me.
>
>I think the complex one is going to arrive when we need to deal with
>discriminated unions. eg the <forward
mode='passthrough|nat|hostdev'>
>where the @mode attribute determines which child struct and elements
>are permitted.
I suggest we can now check this in a hook function.
If we deal with discriminated unions, I think we should introduce some directives
or rules to indicate the relationship between union values and child struct.
I'm afraid that the generator and the new directives will be too complicated.
Do you have any suggestion about this?
The current layout of many of our structs is
unhelpful, because while we
treat the elements contents as a union, the struct is often flat. I think
we'll need to fix this as part of adopting the generator, so that we always
use a union for structs.
Taking one of the more simple examples
struct _virDomainHostdevSubsys {
int type; /* enum virDomainHostdevSubsysType */
union {
virDomainHostdevSubsysUSB usb;
virDomainHostdevSubsysPCI pci;
virDomainHostdevSubsysSCSI scsi;
virDomainHostdevSubsysSCSIVHost scsi_host;
virDomainHostdevSubsysMediatedDev mdev;
} u;
};
The "int type" field is the union discriminator and has a string
conversion via the enum.
So we can annotate the union to say which field to use as the
discriminator to switch parsing based on:
struct _virDomainHostdevSubsys {
virDomainHostdevSubsysType type; /* xmlattr */
union {
virDomainHostdevSubsysUSB usb;
virDomainHostdevSubsysPCI pci;
virDomainHostdevSubsysSCSI scsi;
virDomainHostdevSubsysSCSIVHost scsi_host;
virDomainHostdevSubsysMediatedDev mdev;
} u; /* xmlswitch:type */
};
We can declare that the name of each union member should match the string
form of the "type" attribute. eg with type="usb", we expect the union
to
have a field "usb". So with type="usb", we'll parse the
virDomainHostdevSubsysUSB
struct as the content.
Regards,
Daniel
--
|: