Re: [libvirt] [PATCH 1/3] conf: format/parse <vmcoreinfo> as tristate
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(a)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(a)redhat.com>
John