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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



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


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 :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux