Re: [libvirt] [PATCH v2 1/5] domain: Fix unknown flags diagnosis in virDomainGetXMLDesc

> Since this is limited to the GetXMLDesc processing wouldn't this limit > the exposure in such a way that some new flag expecting some default > action would not return expected or "new" results on the newer libvirt > vs. some older one? That is limited to VIR_DOMAIN_XML_COMMON_FLAGS > (SECURE, INACTIVE, MIGRATABLE). If say, CHECKPOINTABLE was added to that > list, the get wouldn't fail because it doesn't know the flag, but it > also wouldn't return any XML related to the new flag, right? Indeed - and that's how I found it: I'm trying to add VIR_DOMAIN_XML_SNAPSHOTS and VIR_DOMAIN_XML_CHECKPOINTS flags, and was confused when I patched virsh to support the new flag but the old libvirt didn't react to it in any way (I then confirmed that libvirt running on RHEL 6 _does_ react to the unknown flag). Missing output is not a security breach, but is annoying enough to be worth comments somewhere as to the fact that the problem exists (worse would be if we wanted to add a flag comparable in nature to VIR_DOMAIN_XML_SECURE, where failure to obey the flag results in leaking XML information that should have been suppressed). > > Secondary to that knowing this would require someone to read this > specific commit message to garnish at least a small understanding of the > issue. Perhaps some comments in domain_conf.h near the new > VIR_DOMAIN_XML_COMMON_FLAGS would help ensure someone doesn't expand > those without understanding the impact. Similarly, near the flags > definition either a similar noteworthy comment or a comment to go read > the domain_conf.h comment. There's a quick comment added before > virDomainDefFormatConvertXMLFlags in this patch that could be expandable > instead. Doesn't mean someone will read and/or understand it, but at > least leaves a trail. Sure, I can add some strategic comments.