| Message ID | 97094e3df1ef092f1b1aa2c19a118f3fd91837e2.1523989580.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | conf: <vmcoreinfo> tweaks | expand |
On 04/17/2018 02:40 PM, Cole Robinson wrote: > <features><vmcoreinfo/> is a bare boolean XML property. We don't really > use this format anymore and instead prefer tristate <X state=on|off/> > since it's required for modeling on/off/default. If for example future > qemu started enabling vmcoreinfo by default we wouldn't have any way > for the user to turn this off. > > Convert it to tristate. For writing XML this is semanticly the same, > <vmcoreinfo/> is processed as <vmcoreinfo state='on'/>. > > For apps reading guest XML this is technically an API change, > as they might misinterpret <vmcoreinfo state='off'/>, however this > has only been present in libvirt since 3.10.0 and I don't think any > apps are dependent on this yet ^^ perhaps partially a --- type comment - how much you leave in the commit is your call > > Signed-off-by: Cole Robinson <crobinso@redhat.com> > --- > src/conf/domain_conf.c | 4 ++-- > tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > FWIW: Looks like HAP changed too (commit id '9d243e08') - so from the aspect of precedence, this would seem to be OK then. Two things missing from this commit then would be the change to formatdomain.html.in to describe the XML format and domaincommon.rng (e.g. from patch 2). I'd be fine if patch 1 and 2 were merged since all that would be left in 2 would be the comment addition which would seem to be a good idea for this patch. Please wait for 4.4.0 before pushing and with the formatdomain change consider this, Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4dad8e3b2..648057ad4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19281,7 +19281,6 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_HYPERV: - case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_KVM: def->features[val] = VIR_TRISTATE_SWITCH_ON; break; @@ -19300,6 +19299,7 @@ virDomainDefParseXML(xmlDocPtr xml, } break; + case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_HAP: case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: @@ -26870,7 +26870,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_VIRIDIAN: - case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_PRIVNET: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_ABSENT: @@ -26891,6 +26890,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; + case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_HAP: case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: diff --git a/tests/qemuxml2xmloutdata/vmcoreinfo.xml b/tests/qemuxml2xmloutdata/vmcoreinfo.xml index d0cd2f2ce..48b75d7d4 100644 --- a/tests/qemuxml2xmloutdata/vmcoreinfo.xml +++ b/tests/qemuxml2xmloutdata/vmcoreinfo.xml @@ -9,7 +9,7 @@ <boot dev='hd'/> </os> <features> - <vmcoreinfo/> + <vmcoreinfo state='on'/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff>
<features><vmcoreinfo/> is a bare boolean XML property. We don't really use this format anymore and instead prefer tristate <X state=on|off/> since it's required for modeling on/off/default. If for example future qemu started enabling vmcoreinfo by default we wouldn't have any way for the user to turn this off. Convert it to tristate. For writing XML this is semanticly the same, <vmcoreinfo/> is processed as <vmcoreinfo state='on'/>. For apps reading guest XML this is technically an API change, as they might misinterpret <vmcoreinfo state='off'/>, however this has only been present in libvirt since 3.10.0 and I don't think any apps are dependent on this yet Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 4 ++-- tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list