[RFCv2 00/46] RFC: Generate parsexml/formatbuf functions based on directives

Shi Lei shi_lei at massclouds.com
Mon Dec 7 03:58:38 UTC 2020


On 2020-12-05 at 01:41, DanielP. Berrangé wrote:
>On Fri, Sep 04, 2020 at 11:34:52AM +0800, Shi Lei wrote:
>> V1 here: [https://www.redhat.com/archives/libvir-list/2020-June/msg00357.html]
>
>
>> For those new and changed directives, illustrate them by an example:
>>
>> struct _virDomainGraphicsAuthDef {  /* genparse, genformat:separate */
>>     char *passwd;                   /* xmlattr, formatflag:VIR_DOMAIN_DEF_FORMAT_SECURE */
>>     bool expires;
>>     time_t validTo;                 /* xmlattr:passwdValidTo, specified:expires */
>>     virDomainGraphicsAuthConnectedType connected;   /* xmlattr */
>> };
>>
>> struct _virDomainGraphicsListenDef {    /* genparse:withhook, genformat */
>>     virDomainGraphicsListenType type;   /* xmlattr */
>>     char *address;                      /* xmlattr, formathook */
>>     char *network;                      /* xmlattr, formatflag:VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK */
>>     char *socket;                       /* xmlattr, formathook */
>>     int fromConfig;                     /* xmlattr, formatflag:%VIR_DOMAIN_DEF_FORMAT_STATUS */
>>     bool autoGenerated;                 /* xmlattr, formatflag:%VIR_DOMAIN_DEF_FORMAT_STATUS */
>> };
>>
>> struct _virDomainGraphicsSDLDef {   /* genparse, genformat:separate */
>>     char *display;                  /* xmlattr */
>>     char *xauth;                    /* xmlattr */
>>     bool fullscreen;                /* xmlattr */
>>     virTristateBool gl;             /* xmlattr:gl/enable */
>> };
>>
>> struct _virDomainGraphicsDef {  /* genparse:concisehook, genformat */
>>     virObjectPtr privateData;
>>     virDomainGraphicsType type;                         /* xmlattr */
>>
>>     size_t nListens;
>>     virDomainGraphicsListenDefPtr listens;  /* xmlelem, array:nListens */
>>
>>     union {
>>         virDomainGraphicsSDLDef sdl;                    /* xmlgroup */
>>         virDomainGraphicsVNCDef vnc;                    /* xmlgroup */
>>         virDomainGraphicsRDPDef rdp;                    /* xmlgroup */
>>         virDomainGraphicsDesktopDef desktop;            /* xmlgroup */
>>         virDomainGraphicsSpiceDef spice;                /* xmlgroup */
>>         virDomainGraphicsEGLHeadlessDef egl_headless;   /* xmlgroup */
>>     } data;                                             /* xmlswitch:type */
>> };
>>
>>
>> Explanation for these directives:
>>
>>     - genformat[:separate|onlyattrs|onlyelems]
>>
>>       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'.
>>
>>       When 'genformat:separate' is specified, generate two formatbuf functions
>>       rather than a single full-mode formatbuf function.
>>       One for formatting attributes and another for formatting elements.
>>       These function names are based on struct-name and suffixed with 'FormatAttr'
>>       and 'FormatElem' respectively.
>>
>>       The 'onlyattrs' and 'onlyelems' are just like 'separate', but only
>>       generate one of those two functions according to its denotation.
>>
>>     - xmlattr[:[parentname/]thename]
>>
>>       Parse/Format the field as an XML attribute or
>>       attribute wrapped by an XML element.
>>       If only 'thename' is specified, use it as the XML attribute name;
>>       or use the filed name.
>>       The 'parentname' is the name of the attribute's parent element.
>>       If 'parentname/thename' is specified, the corresponding form is
>>       <parentname thename='..' />.
>>
>>     - xmlgroup
>>
>>       The field is a struct, but its corresponding form in XML is a group
>>       rather than an element.
>>
>>     - xmlswitch:thename
>>
>>       Only for discriminated union. 'thename' is the name of its relative enum.
>>       The name of each union member should match a shortname of the enum.
>>
>>     - array[:countername]
>>
>>       Parse/Format the field as an array.
>>       Each array field must have an related counter field, which name is
>>       specified by 'countername'.
>>       If 'countername' is omitted, follow the pattern:
>>       n + 'field_name'.
>>
>>     - specified[:thename]
>>
>>       This field has an related field to indicate its existence, and
>>       'thename' specifies the name of this related field.
>>       When 'thename' is omitted, follow the pattern:
>>       'field_name' + '_specified'.
>>
>>     - formatflag:[!|%]flag
>>
>>       This field will be formatted and written out to XML only if the 'flag'
>>       hits a target flagset.
>>       The target flagset is passed into the formatbuf function through the
>>       argument 'opaque'.
>>
>>       Adding a '!' before 'flag' means NOT hitting.
>>
>>       Adding a '%' before 'flag' means that flag hitting-check is the unique
>>       condition for formatting this field. For example,
>>       for 'passwd' in 'virDomainGraphicsAuthDef', the directive is:
>>
>>         formatflag:VIR_DOMAIN_DEF_FORMAT_SECURE
>>
>>       then the generated code:
>>
>>         if (def->passwd && (virXMLFlag(opaque) & VIR_DOMAIN_DEF_FORMAT_SECURE))
>>             virBufferEscapeString(buf, " passwd='%s'", def->passwd);
>>
>>       If '%' is inserted like this:
>>
>>         formatflag:%VIR_DOMAIN_DEF_FORMAT_SECURE
>>
>>       then the generated code:
>>
>>         if ((virXMLFlag(opaque) & VIR_DOMAIN_DEF_FORMAT_SECURE))
>>             virBufferEscapeString(buf, " passwd='%s'", def->passwd);
>>
>>     - formathook
>>       Introduce hooks to handle the field if xmlgen can't deal with it now.
>>
>>       E.g., virDomainGraphicsListenDef have two fields with 'formathook',
>>       which are 'address' and 'socket'.
>>       The xmlgen will generate the declaration of some hooks for formatting
>>       these fields and developers should implement them.
>>
>>       1) Check the declaration of hook by a commandline.
>>
>>         # ./scripts/xmlgen/go show virDomainGraphicsListenDef -kf
>>
>>         int
>>         virDomainGraphicsListenDefFormatHook(const virDomainGraphicsListenDef *def,
>>                                              const void *parent,
>>                                              const void *opaque,
>>                                              virBufferPtr addressBuf,
>>                                              virBufferPtr socketBuf);
>>
>>         bool
>>         virDomainGraphicsListenDefCheckHook(const virDomainGraphicsListenDef *def,
>>                                             const void *parent,
>>                                             void *opaque,
>>                                             bool result);
>>
>>       2) Implement these two hooks in src/conf/domain_conf.c.
>>
>>       2.1) virXXXFormatHook
>>         It is the hook for formatting field 'address' and 'socket'.
>>         The 'addressBuf' and 'socketBuf' are used for output destinations respectively.
>>
>>       2.2) virXXXCheckHook
>>         For structs, the xmlgen generates virXXXCheck function to come with
>>         the virXXXFormatBuf. The virXXXCheck reports whether the corresponding
>>         XML element is null.
>>
>>         The virXXXCheckHook intercepts the 'result' of virXXXCheck. It changes 'result'
>>         or just forwards it according to those fields with 'formathook'.
>
>It is probably a good idea to put all this doucmentation from
>the cover letter  into a docs/xmlgenerator.rst file as it'll be
>useful reference for developers in future. 

Okay. I'll do it.

>
>
>
>>  meson.build                      |    5 +
>>  po/POTFILES.in                   |    3 +
>>  scripts/meson.build              |    8 +
>>  scripts/xmlgen/directive.py      | 1115 ++++++++++++++++++++
>>  scripts/xmlgen/go                |    7 +
>>  scripts/xmlgen/main.py           |  439 ++++++++
>>  scripts/xmlgen/utils.py          |  121 +++
>>  src/conf/domain_conf.c           | 1650 +++++++++---------------------
>>  src/conf/domain_conf.h           |  179 ++--
>>  src/conf/meson.build             |   41 +
>>  src/conf/network_conf.c          |  467 ++-------
>>  src/conf/network_conf.h          |   54 +-
>>  src/conf/virconftypes.h          |   18 +
>>  src/libvirt_private.syms         |    9 +
>>  src/meson.build                  |    6 +
>>  src/qemu/qemu_command.c          |    4 +
>>  src/qemu/qemu_driver.c           |    2 +
>>  src/qemu/qemu_hotplug.c          |    2 +
>>  src/qemu/qemu_migration_cookie.c |    1 +
>>  src/qemu/qemu_process.c          |    5 +
>>  src/qemu/qemu_validate.c         |    2 +
>>  src/util/virsocketaddr.c         |   42 +
>>  src/util/virsocketaddr.h         |   26 +-
>>  src/util/virstring.c             |   57 ++
>>  src/util/virstring.h             |    9 +
>>  src/util/virxml.c                |  105 ++
>>  src/util/virxml.h                |    6 +
>>  src/vmx/vmx.c                    |    1 +
>>  tests/meson.build                |    1 +
>>  tools/meson.build                |    2 +
>
>I think it'd be a good idea to have a test case to validate the
>XML generator. For example create a simpe tests/xmlgen/demo.h that
>illustrates all the different features we can use.
>
>Then have tests/xmlgen/demo.generated.{c,h}, and the write a
>test case that generates a new copy of demo.generated.{c,h}
>and compares to what we have in git.
>
>This will help us validate that changes to the xmlgenerator in
>future don't result in unexpected changes to the generated
>code. 

Yes. I agree.

Regards,
Shi Lei

>
>
>Regards,
>Daniel
>--
>|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o- https://fstop138.berrange.com :|
>|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>




More information about the libvir-list mailing list