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] > > Differ from V1: > > * Move the generator into scripts/xmlgen and rename it 'xmlgen'. > > * Declare virXMLChildNode and virXMLChildNodeSet in libvirt_private.syms. > > * Replace VIR_FREE with g_free and VIR_ALLOC[_N] with g_new0. > > * Adjust virReportError to avoid unnecessary translation. > > * Remove the macro VIR_USED and use G_GNUC_UNUSED to declare arguments. > > * When parsing string member, assign value to it directly instead of > using middle variable. > > * Don't set libclang_path. Just use python-clang's default setting. > > * Use virEscapeString for escaping xml characters. > > * Enable directive 'genformat' with a parameter to support separation mode. > > * Add directive 'xmlswitch' and 'xmlgroup' to support discriminated unions. > > * Allow directive 'array' and 'specified' to carry with a parameter, > which specifies its counterpart explicitly. > > * Enable directive 'xmlattr' with path. > > * Add directive 'formatflag' and 'formathook'. I'm very sorry for the extremely long delay in reviewing this new series. Overall I like this series and would like to see how we can focus on getting at least part of it merged. It is interesting to see how it is forcing a separation of the parsing and validation of data. I also like that it is making the code more consistent in style. The main blocker I think is that we need to temporarily commit the generated files to git, and not run the generator during normal builds, until we ditch RHEL-7 support in April 2021. This should be quite easy to deal with. The next thing is that wierd need to insert a "_NONE" field in to all the enums. I'm not understanding what reqiures this, but I think we need to come up with a different solution to whatever the problem is. It looks like this only affects the DomainGraphics conversion in your series. So we could start by only merging the Network conversion patches, until we figure out a solution to avoid adding _NONE fields. That's it for the main blockers to merge from my POV. After that I think a priority is to get some test coverage of the generator script, and to add the docs about the how the generator annotations work. This can be done after the initial merge. Then there's a large ongoing work to actually convert everything, but there's no rush to finish that, as you've shown that we can do it incrementally. So if you want to re-post an update of the series with the few blocking items addressed, then personally I'd look at trying to merge that unless other people have objections they want to raise that can't be solved after merging the initial support. 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 :|