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