On 2020-12-05 at 02:55, 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] >> >> 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 >-- Thanks for reviewing this series. For the first blocker, I think we can wait until April 2021, since we have spent a lot time on it. According to your comment, I think I can make some preparations for the initial merge during these 4~5 months. (1) Implement some testcases for the generator to make sure its proper functions in the future. (2) Add some docs to explain and demonstrate this generator. My English is not well, so I will try my best to do it and I may need a bit long time. (3) For the 'default' item of enums, I think we can add an extra directive to direct the generator how to generate code for enum, as I mentioned in the last email. Or we can continue to talk about other solutions. (4) Next series will only include conversion about network, so that we can focus on the docs, testcases and this generator itself. I plan to finish these jobs and post the next series in the next month, which will be still a RFC. So you and other people can have sufficent time to review and comment it and I will have sufficent time to improve it :-) Regards, Shi Lei
