[libvirt] [PATCH 0/3] fix some issue around host-passthough cpu model

We support add feature in host-passthough cpu model, but there are some place need fix. Luyao Huang (3): cpu:x86: fix cannot keep cpu feature after migrate/restore qemu: add a check for non-migratable cpu flags docs: fix a small xml error in docs docs/formatdomain.html.in | 2 +- src/cpu/cpu_x86.c | 9 ++++----- src/qemu/qemu_migration.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) -- 1.8.3.1

https://bugzilla.redhat.com/show_bug.cgi?id=1207095 When we set feature when cpu model is host-passthrough and start the vm, we can see these settings in the XML, however after migrate/restore these flags will be covered by host cpu models. As cpu model host-passthrough can set feature by users now, so we need check if there is a feature when do updatecpu(). Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/cpu/cpu_x86.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index bf1867b..81ab2ca 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2087,8 +2087,7 @@ x86UpdateCustom(virCPUDefPtr guest, static int x86UpdateHostModel(virCPUDefPtr guest, - const virCPUDef *host, - bool passthrough) + const virCPUDef *host) { virCPUDefPtr oldguest = NULL; const struct x86_map *map; @@ -2118,7 +2117,7 @@ x86UpdateHostModel(virCPUDefPtr guest, } } } - for (i = 0; !passthrough && i < oldguest->nfeatures; i++) { + for (i = 0; i < oldguest->nfeatures; i++) { if (virCPUDefUpdateFeature(guest, oldguest->features[i].name, oldguest->features[i].policy) < 0) @@ -2143,11 +2142,11 @@ x86Update(virCPUDefPtr guest, case VIR_CPU_MODE_HOST_MODEL: guest->match = VIR_CPU_MATCH_EXACT; - return x86UpdateHostModel(guest, host, false); + return x86UpdateHostModel(guest, host); case VIR_CPU_MODE_HOST_PASSTHROUGH: guest->match = VIR_CPU_MATCH_MINIMUM; - return x86UpdateHostModel(guest, host, true); + return x86UpdateHostModel(guest, host); case VIR_CPU_MODE_LAST: break; -- 1.8.3.1

If user set this feature in vm xml: <cpu mode='host-passthrough'> <feature policy='require' name='invtsc'/> </cpu> we won't report error when do migrate. So remove def->cpu->mode check, because we can set feature for host-passthrough model now. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d34bb02..9c76e79 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2004,7 +2004,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, return false; } - if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { + if (def->cpu) { for (i = 0; i < def->cpu->nfeatures; i++) { virCPUFeatureDefPtr feature = &def->cpu->features[i]; -- 1.8.3.1

Signed-off-by: Luyao Huang <lhuang@redhat.com> --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..577d647 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1019,7 +1019,7 @@ ...</pre> <pre> - <cpu mode='host-passthrough'/> + <cpu mode='host-passthrough'> <feature policy='disable' name='lahf_lm'/> ...</pre> -- 1.8.3.1

On 03/30/2015 04:49 AM, Luyao Huang wrote:
We support add feature in host-passthough cpu model, but there are some place need fix.
Luyao Huang (3): cpu:x86: fix cannot keep cpu feature after migrate/restore qemu: add a check for non-migratable cpu flags docs: fix a small xml error in docs
docs/formatdomain.html.in | 2 +- src/cpu/cpu_x86.c | 9 ++++----- src/qemu/qemu_migration.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-)
Looks like these were orphaned ;-)... Looking through history of the changes, it seems you're essentially undoing what Jan changed for https://bugzilla.redhat.com/show_bug.cgi?id=1147584 in commit id 'f53bb1a'. I'm assuming these were missed on the initial pass, but bumping them should grab his attention. I think patch 3/3 is ACK-able and I'll push that separately since it's unrelated. Your patch 4/3 undoes commit id 'ec5f817f'. John

On 05/05/2015 08:30 PM, John Ferlan wrote:
On 03/30/2015 04:49 AM, Luyao Huang wrote:
We support add feature in host-passthough cpu model, but there are some place need fix.
Luyao Huang (3): cpu:x86: fix cannot keep cpu feature after migrate/restore qemu: add a check for non-migratable cpu flags docs: fix a small xml error in docs
docs/formatdomain.html.in | 2 +- src/cpu/cpu_x86.c | 9 ++++----- src/qemu/qemu_migration.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-)
Looks like these were orphaned ;-)...
Thanks a lot for 'saving' my patches again and again ;-)
Looking through history of the changes, it seems you're essentially undoing what Jan changed for https://bugzilla.redhat.com/show_bug.cgi?id=1147584 in commit id 'f53bb1a'. I'm assuming these were missed on the initial pass, but bumping them should grab his attention.
Yes, as your said i try to undoing the Jan's changes, but we support add features line with host-passthough cpu model, some code for host-passthrough model work not well host-passthough with features, and there is a new problem around the vm save/restore with host-passthough with features (you can see this bug https://bugzilla.redhat.com/show_bug.cgi?id=1207095). Also as host-passthough can have the features now, I think we shouldn't skip the cpu features check for host-passthough model. But seems my patches will cause some regression issue, i will recheck and retest them.
I think patch 3/3 is ACK-able and I'll push that separately since it's unrelated.
Yes, it is a small issue.
Your patch 4/3 undoes commit id 'ec5f817f'.
Thanks for pointing out that, i will recheck these patches and write a new version these days. Thanks a lot for your review.
John
Luyao
participants (2)
-
John Ferlan
-
Luyao Huang