9:58 p.m.
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 :|