[2/3] conf: Add a comment warning about boolean feature XML

Message ID 18601ec4d9e0e022f5d411eb60051114b7b462ab.1523989580.git.crobinso@redhat.com
State New
Headers show
Series
  • conf: <vmcoreinfo> tweaks
Related show

Commit Message

Cole Robinson April 17, 2018, 6:40 p.m.
This is the old style and we really shouldn't be adding any more
examples like this. Add a comment to warn devs away

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 docs/schemas/domaincommon.rng | 6 +++++-
 src/conf/domain_conf.c        | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

John Ferlan April 27, 2018, 3:09 p.m. | #1
On 04/17/2018 02:40 PM, Cole Robinson wrote:
> This is the old style and we really shouldn't be adding any more

> examples like this. Add a comment to warn devs away

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>  docs/schemas/domaincommon.rng | 6 +++++-

>  src/conf/domain_conf.c        | 2 ++

>  2 files changed, 7 insertions(+), 1 deletion(-)

> 


The domaincommon.rng change should go in patch 1.

I'll leave it up to you whether to combine the domain_conf.c change as well.

Reviewed-by: John Ferlan <jferlan@redhat.com>


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05..015c5b737 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5036,7 +5036,11 @@ 
 
   <define name="vmcoreinfo">
     <element name="vmcoreinfo">
-      <empty/>
+      <optional>
+        <attribute name="state">
+          <ref name="virOnOff"/>
+        </attribute>
+      </optional>
     </element>
   </define>
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 648057ad4..e303c503e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26871,6 +26871,8 @@  virDomainDefFormatInternal(virDomainDefPtr def,
             case VIR_DOMAIN_FEATURE_PAE:
             case VIR_DOMAIN_FEATURE_VIRIDIAN:
             case VIR_DOMAIN_FEATURE_PRIVNET:
+                /* NOTE: This is for old style <opt/> booleans. New XML
+                 * should use the explicit state=on|off output below */
                 switch ((virTristateSwitch) def->features[i]) {
                 case VIR_TRISTATE_SWITCH_ABSENT:
                     break;