Re: [libvirt] [PATCH 6/8] virQEMUCapsSetFromNodes: Skip setting deprecated flags
On 2/21/19 4:42 PM, Ján Tomko wrote:
Prepare for removal of deprecated flags from the capability
string array and quietly drop flags present in the deprecated
string array if they were not found in the main array.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b12f5914aa..ce50cfdfa5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3460,9 +3460,15 @@ virQEMUCapsSetFromNodes(virQEMUCapsPtr qemuCaps,
}
flag = virQEMUCapsTypeFromString(str);
if (flag < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unknown qemu capabilities flag %s"), str);
- goto cleanup;
+ flag = virQEMUCapsDeprecatedTypeFromString(str);
As mentioned in 2/8 I think this should be replaced with
virStringListHasString().
+ if (flag < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown qemu capabilities flag %s"), str);
+ goto cleanup;
+ } else {
+ VIR_FREE(str);
+ continue;
+ }
}
VIR_FREE(str);
virQEMUCapsSet(qemuCaps, flag);
However, looking at the bigger picture is this a safe thing to do? I
mean, imagine the following scenario:
1) say there is capability X that affects certain part of cmd line. And
assume that those two possibilities are incompatible. If cmd line is
generated one way then migration to a qemu which has cmd line generated
the other way fails.
2) in release R we deprecate X and thus do not format it in
<capabilities/> in status XML.
3) user starts a domain D.
4) user saves D into a file
5) sysadmin downgrades libvirt to R-1
6) user tries to restore D from the saved file
At this point, since X is not in the saved XML, R - 1 assumes that D is
lacking the capability and generates incompatible cmd line which makes
qemu fail on loading migration stream.
IOW, what one libvirt assumes the other might not.
Michal