1:40 p.m.
Patch #1 changes the vmcoreinfo XML schema slightly to be more future
proof, this is technically an API break but I'm not sure it matters,
see the patch for more details.
Patch #3 adds vmcoreinfo reporting to domain capabilities. The
schema doesn't follow the typical pattern so if anyone has other ideas
please comment
Cole Robinson (3):
conf: format/parse <vmcoreinfo> as tristate
conf: Add a comment warning about boolean feature XML
domain_capabilities: Report <vmcoreinfo> support
docs/formatdomaincaps.html.in | 5 +++++
docs/schemas/domaincaps.rng | 7 +++++++
docs/schemas/domaincommon.rng | 6 +++++-
src/conf/domain_capabilities.c | 2 ++
src/conf/domain_capabilities.h | 1 +
src/conf/domain_conf.c | 6 ++++--
src/qemu/qemu_capabilities.c | 3 +++
tests/domaincapsschemadata/basic.xml | 1 +
tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 1 +
tests/domaincapsschemadata/full.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv.xml | 1 +
tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml | 1 +
tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +-
33 files changed, 53 insertions(+), 4 deletions(-)
--
2.17.0
1:40 p.m.
New subject: [libvirt] [PATCH 1/3] conf: format/parse <vmcoreinfo> as tristate
<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(a)redhat.com>
---
src/conf/domain_conf.c | 4 ++--
tests/qemuxml2xmloutdata/vmcoreinfo.xml | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
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>
--
2.17.0
10:09 a.m.
New subject: [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
1:40 p.m.
New subject: [libvirt] [PATCH 2/3] conf: Add a comment warning about boolean feature XML
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(a)redhat.com>
---
docs/schemas/domaincommon.rng | 6 +++++-
src/conf/domain_conf.c | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
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;
--
2.17.0
10:09 a.m.
New subject: [libvirt] [PATCH 2/3] conf: Add a comment warning about boolean feature XML
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(a)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(a)redhat.com>
John
1:40 p.m.
New subject: [libvirt] [PATCH 3/3] domain_capabilities: Report <vmcoreinfo> support
Report domaincaps <features><vmcoreinfo supported='yes'/> if the
guest
config accepts <features><vmcoreinfo state='on'/>
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
This bucks the domaincapabilities trend of always having a child
enum if supported='yes'. Following that trend we would give us
this XML when vmcoreinfo is supported:
<vmcoreinfo supported='yes'>
<enum name='state'>
<value>on</value>
<value>off</value>
</enum>
</vmcoreinfo>
Which is verbose but fine. But it's unclear what we do in the
case when vmcoreinfo isn't supported... do we do:
<vmcoreinfo supported='no'/>
which may not be entirely accurate because the code will still accept
<vmcoreinfo enabled='off'/>. Or do we do:
<vmcoreinfo supported='yes'>
<enum name='state'>
<value>off</value>
</enum>
</vmcoreinfo>
Which is weird IMO. I'm not certain what the semantics of
'supported' are meant to be so I went with this minimal option
but I'm not tied to it
docs/formatdomaincaps.html.in | 5 +++++
docs/schemas/domaincaps.rng | 7 +++++++
src/conf/domain_capabilities.c | 2 ++
src/conf/domain_capabilities.h | 1 +
src/qemu/qemu_capabilities.c | 3 +++
tests/domaincapsschemadata/basic.xml | 1 +
tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 1 +
tests/domaincapsschemadata/full.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv.xml | 1 +
tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml | 1 +
30 files changed, 43 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 6bfcaf61c..acdc1cf8a 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -417,6 +417,7 @@
<value>3</value>
</enum>
</gic>
+ <vmcoreinfo supported='yes'/>
</features>
</domainCapabilities>
</pre>
@@ -441,5 +442,9 @@
<code>gic</code> element.</dd>
</dl>
+ <h4><a id="elementsvmcoreinfo">vmcoreinfo</a></h4>
+
+ <p>Reports whether the vmcoreinfo feature can be enabled</p>
+
</body>
</html>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 39053181e..bace0e44a 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -173,6 +173,7 @@
<element name='features'>
<interleave>
<ref name='gic'/>
+ <ref name='vmcoreinfo'/>
</interleave>
</element>
</define>
@@ -184,6 +185,12 @@
</element>
</define>
+ <define name='vmcoreinfo'>
+ <element name='vmcoreinfo'>
+ <ref name='supported'/>
+ </element>
+ </define>
+
<define name='value'>
<zeroOrMore>
<element name='value'>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f7d9be50f..ccdccd695 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -587,6 +587,8 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
virBufferAdjustIndent(&buf, 2);
virDomainCapsFeatureGICFormat(&buf, &caps->gic);
+ virBufferAsprintf(&buf, "<vmcoreinfo
supported='%s'/>\n",
+ caps->vmcoreinfo ? "yes" : "no");
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</features>\n");
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index fa4c1e442..5bb028f63 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -157,6 +157,7 @@ struct _virDomainCaps {
/* add new domain devices here */
virDomainCapsFeatureGIC gic;
+ bool vmcoreinfo;
/* add new domain features here */
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f379fc6d2..0543c0194 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4830,6 +4830,9 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
}
+ domCaps->vmcoreinfo = virQEMUCapsGet(qemuCaps,
+ QEMU_CAPS_DEVICE_VMCOREINFO);
+
if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 ||
virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps) < 0 ||
virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps,
diff --git a/tests/domaincapsschemadata/basic.xml b/tests/domaincapsschemadata/basic.xml
index 6b788d914..e937b3607 100644
--- a/tests/domaincapsschemadata/basic.xml
+++ b/tests/domaincapsschemadata/basic.xml
@@ -17,5 +17,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/bhyve_basic.x86_64.xml
b/tests/domaincapsschemadata/bhyve_basic.x86_64.xml
index 95a8038c2..70e36e97e 100644
--- a/tests/domaincapsschemadata/bhyve_basic.x86_64.xml
+++ b/tests/domaincapsschemadata/bhyve_basic.x86_64.xml
@@ -27,5 +27,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml
b/tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml
index 662d0db9a..19b76da45 100644
--- a/tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml
+++ b/tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml
@@ -44,5 +44,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/bhyve_uefi.x86_64.xml
b/tests/domaincapsschemadata/bhyve_uefi.x86_64.xml
index f8c79b99b..c278df038 100644
--- a/tests/domaincapsschemadata/bhyve_uefi.x86_64.xml
+++ b/tests/domaincapsschemadata/bhyve_uefi.x86_64.xml
@@ -36,5 +36,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
index ab6ef9f2e..93bdad2eb 100644
--- a/tests/domaincapsschemadata/full.xml
+++ b/tests/domaincapsschemadata/full.xml
@@ -107,5 +107,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/libxl-xenfv-usb.xml
b/tests/domaincapsschemadata/libxl-xenfv-usb.xml
index 6a9e3d96f..dfa992cb7 100644
--- a/tests/domaincapsschemadata/libxl-xenfv-usb.xml
+++ b/tests/domaincapsschemadata/libxl-xenfv-usb.xml
@@ -70,5 +70,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/libxl-xenfv.xml
b/tests/domaincapsschemadata/libxl-xenfv.xml
index d48e69948..c74632a13 100644
--- a/tests/domaincapsschemadata/libxl-xenfv.xml
+++ b/tests/domaincapsschemadata/libxl-xenfv.xml
@@ -69,5 +69,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/libxl-xenpv-usb.xml
b/tests/domaincapsschemadata/libxl-xenpv-usb.xml
index d1a3918ef..8277a9efd 100644
--- a/tests/domaincapsschemadata/libxl-xenpv-usb.xml
+++ b/tests/domaincapsschemadata/libxl-xenpv-usb.xml
@@ -60,5 +60,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/libxl-xenpv.xml
b/tests/domaincapsschemadata/libxl-xenpv.xml
index c0e3193fe..c816b2053 100644
--- a/tests/domaincapsschemadata/libxl-xenpv.xml
+++ b/tests/domaincapsschemadata/libxl-xenpv.xml
@@ -59,5 +59,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
index 8d1ad8657..d79e8015f 100644
--- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml
@@ -108,5 +108,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml
b/tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml
index 9cba942fb..982f31a9a 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml
@@ -115,5 +115,6 @@
<value>3</value>
</enum>
</gic>
+ <vmcoreinfo supported='yes'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
index 4029e9e38..0aa14b127 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
@@ -77,5 +77,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.12.0.s390x.xml
b/tests/domaincapsschemadata/qemu_2.12.0.s390x.xml
index 50680c1ef..693b6dc7a 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0.s390x.xml
@@ -74,5 +74,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
index 851d914bc..797ecfae7 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
@@ -139,5 +139,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='yes'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml
b/tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml
index 7c019b230..5e4ac26f7 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml
@@ -112,5 +112,6 @@
<value>2</value>
</enum>
</gic>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
index 3b1428062..5bc69e717 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
@@ -108,5 +108,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml
b/tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml
index 98c72d36d..0543e3f56 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml
@@ -81,5 +81,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
index de8188623..21198b500 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml
@@ -113,5 +113,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
index b93d00ece..1e6140091 100644
--- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
@@ -74,5 +74,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
index d860cd833..8ae794f69 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml
@@ -114,5 +114,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
index ee40d1e84..ac6317755 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
@@ -155,5 +155,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
index 33161f7b8..d227926c6 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml
@@ -114,5 +114,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
index 07e1db641..513b71c1b 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
@@ -122,5 +122,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
index 6c2bbac19..38fddf586 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml
@@ -146,5 +146,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
diff --git a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
index 2fc3b7213..501fcc682 100644
--- a/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml
@@ -123,5 +123,6 @@
</devices>
<features>
<gic supported='no'/>
+ <vmcoreinfo supported='no'/>
</features>
</domainCapabilities>
--
2.17.0
10:09 a.m.
New subject: [libvirt] [PATCH 3/3] domain_capabilities: Report <vmcoreinfo> support
On 04/17/2018 02:40 PM, Cole Robinson wrote:
Report domaincaps <features><vmcoreinfo
supported='yes'/> if the guest
config accepts <features><vmcoreinfo state='on'/>
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
This bucks the domaincapabilities trend of always having a child
enum if supported='yes'. Following that trend we would give us
this XML when vmcoreinfo is supported:
<vmcoreinfo supported='yes'>
<enum name='state'>
<value>on</value>
<value>off</value>
</enum>
</vmcoreinfo>
Which is verbose but fine. But it's unclear what we do in the
case when vmcoreinfo isn't supported... do we do:
<vmcoreinfo supported='no'/>
which may not be entirely accurate because the code will still accept
<vmcoreinfo enabled='off'/>. Or do we do:
<vmcoreinfo supported='yes'>
<enum name='state'>
<value>off</value>
</enum>
</vmcoreinfo>
Which is weird IMO. I'm not certain what the semantics of
'supported' are meant to be so I went with this minimal option
but I'm not tied to it
I don't mind this explanation - I think the reason GIC has the enum is
because the domain xml <gic... > will allow an optional version
attribute that has possible values of 2 or 3. Since the vmcoreinfo only
cares about supported or not and there's no other attribute, then I
think we're good as you've written this (my opinion may not be held by
all others in the group and I reserve the right to have it changed ;-)).
My question is should the <features> be used for other things and how
many features have been added that we missed also adding a domcap for
that an up the stack consumer could use?
One example that comes to mind because I'm working on it now, I'd
'reuse' this code to do something similar for <genid/> - which is
actually a device in qemu terms, but it's either supported or not. It's
not a domain <features...> element, but rather a more general metadata
element.
Likewise, if we consider IOThreads a domain capability - should we
report it too as supported or not based on capability? I'm sure there's
others, but those come to mind as two that I
docs/formatdomaincaps.html.in | 5 +++++
docs/schemas/domaincaps.rng | 7 +++++++
src/conf/domain_capabilities.c | 2 ++
src/conf/domain_capabilities.h | 1 +
src/qemu/qemu_capabilities.c | 3 +++
tests/domaincapsschemadata/basic.xml | 1 +
tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 1 +
tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 1 +
tests/domaincapsschemadata/full.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenfv.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv-usb.xml | 1 +
tests/domaincapsschemadata/libxl-xenpv.xml | 1 +
tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml | 1 +
tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml | 1 +
30 files changed, 43 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 6bfcaf61c..acdc1cf8a 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -417,6 +417,7 @@
<value>3</value>
</enum>
</gic>
+ <vmcoreinfo supported='yes'/>
</features>
</domainCapabilities>
</pre>
@@ -441,5 +442,9 @@
<code>gic</code> element.</dd>
</dl>
+ <h4><a
id="elementsvmcoreinfo">vmcoreinfo</a></h4>
+
+ <p>Reports whether the vmcoreinfo feature can be enabled</p>
s/enabled/endabled./
+
</body>
</html>
With that small adjustment, consider this:
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Again, please wait for 4.4.0 before pushing...
Also a question, did you hand generate the bhyve and libxl changes
(other than the libxl*-usb.xml)? When reusing this for genid, I used the
VIR_TEST_REGENERATE_OUTPUT=1 which generated most of the .xml output,
but a few didn't show so I'm curious mostly.
4:34 p.m.
New subject: [libvirt] [PATCH 3/3] domain_capabilities: Report <vmcoreinfo> support
On 04/27/2018 11:09 AM, John Ferlan wrote:
On 04/17/2018 02:40 PM, Cole Robinson wrote:
> Report domaincaps <features><vmcoreinfo supported='yes'/> if the
guest
> config accepts <features><vmcoreinfo state='on'/>
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> This bucks the domaincapabilities trend of always having a child
> enum if supported='yes'. Following that trend we would give us
> this XML when vmcoreinfo is supported:
>
> <vmcoreinfo supported='yes'>
> <enum name='state'>
> <value>on</value>
> <value>off</value>
> </enum>
> </vmcoreinfo>
>
> Which is verbose but fine. But it's unclear what we do in the
> case when vmcoreinfo isn't supported... do we do:
>
> <vmcoreinfo supported='no'/>
>
> which may not be entirely accurate because the code will still accept
> <vmcoreinfo enabled='off'/>. Or do we do:
>
> <vmcoreinfo supported='yes'>
> <enum name='state'>
> <value>off</value>
> </enum>
> </vmcoreinfo>
>
> Which is weird IMO. I'm not certain what the semantics of
> 'supported' are meant to be so I went with this minimal option
> but I'm not tied to it
>
I don't mind this explanation - I think the reason GIC has the enum is
because the domain xml <gic... > will allow an optional version
attribute that has possible values of 2 or 3. Since the vmcoreinfo only
cares about supported or not and there's no other attribute, then I
think we're good as you've written this (my opinion may not be held by
all others in the group and I reserve the right to have it changed ;-)).
My question is should the <features> be used for other things and how
many features have been added that we missed also adding a domcap for
that an up the stack consumer could use?
One example that comes to mind because I'm working on it now, I'd
'reuse' this code to do something similar for <genid/> - which is
actually a device in qemu terms, but it's either supported or not. It's
not a domain <features...> element, but rather a more general metadata
element.
Likewise, if we consider IOThreads a domain capability - should we
report it too as supported or not based on capability? I'm sure there's
others, but those come to mind as two that I
> docs/formatdomaincaps.html.in | 5 +++++
> docs/schemas/domaincaps.rng | 7 +++++++
> src/conf/domain_capabilities.c | 2 ++
> src/conf/domain_capabilities.h | 1 +
> src/qemu/qemu_capabilities.c | 3 +++
> tests/domaincapsschemadata/basic.xml | 1 +
> tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 1 +
> tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 1 +
> tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 1 +
> tests/domaincapsschemadata/full.xml | 1 +
> tests/domaincapsschemadata/libxl-xenfv-usb.xml | 1 +
> tests/domaincapsschemadata/libxl-xenfv.xml | 1 +
> tests/domaincapsschemadata/libxl-xenpv-usb.xml | 1 +
> tests/domaincapsschemadata/libxl-xenpv.xml | 1 +
> tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.12.0-virt.aarch64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.12.0.s390x.xml | 1 +
> tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 +
> tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 +
> tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml | 1 +
> tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml | 1 +
> 30 files changed, 43 insertions(+)
>
> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index 6bfcaf61c..acdc1cf8a 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -417,6 +417,7 @@
> <value>3</value>
> </enum>
> </gic>
> + <vmcoreinfo supported='yes'/>
> </features>
> </domainCapabilities>
> </pre>
> @@ -441,5 +442,9 @@
> <code>gic</code> element.</dd>
> </dl>
>
> + <h4><a
id="elementsvmcoreinfo">vmcoreinfo</a></h4>
> +
> + <p>Reports whether the vmcoreinfo feature can be enabled</p>
s/enabled/endabled./
> +
> </body>
> </html>
With that small adjustment, consider this:
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Again, please wait for 4.4.0 before pushing...
Also a question, did you hand generate the bhyve and libxl changes
(other than the libxl*-usb.xml)? When reusing this for genid, I used the
VIR_TEST_REGENERATE_OUTPUT=1 which generated most of the .xml output,
but a few didn't show so I'm curious mostly.
Thanks, I made your suggested changes and pushed. And yeah I had to hand
edit the those files, hopefully it's correct. Looks like Martin's latest
patch missed doing the same... would be nice to figure out a way around
it. Perhaps adjusting the test cases to report 'skip' if the HV isn't
compiled it, that should help it stick out in the test output a bit more
- Cole
2395
days inactive
2411
days old
7 comments
2 participants
participants (2)
-
Cole Robinson -
John Ferlan