[libvirt] [PATCH] snapshot: ABI stability must include memory sizing

Commit 973fcd8f introduced the ability for qemu to reject snapshot reversion on an ABI incompatibility; but the very example that was first proposed on-list[1] as a demonstration of an ABI incompatibility, namely that of changing the max memory allocation, was not being checked for, resulting in a cryptic failure when running with larger max mem than what the snapshot was created with: error: operation failed: Error -22 while loading VM state This commit merely protects the three variables within mem that are referenced by qemu_command.c, rather than all 7 (the other 4 variables affect cgroup handling, but have no visible effect to the qemu guest). [1] https://www.redhat.com/archives/libvir-list/2010-December/msg00331.html * src/conf/domain_conf.c (virDomainDefCheckABIStability): Add memory sizing checks. --- See also https://bugzilla.redhat.com/show_bug.cgi?id=735553#c5 src/conf/domain_conf.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 11755fe..996e0d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8229,6 +8229,26 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, goto cleanup; } + if (src->mem.max_balloon != dst->mem.max_balloon) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain max memory %ld does not match source %ld"), + dst->mem.max_balloon, src->mem.max_balloon); + goto cleanup; + } + if (src->mem.cur_balloon != dst->mem.cur_balloon) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain current memory %ld does not match source %ld"), + dst->mem.cur_balloon, src->mem.cur_balloon); + goto cleanup; + } + if (src->mem.hugepage_backed != dst->mem.hugepage_backed) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain huge page backing %ld does not match source %ld"), + dst->mem.hugepage_backed, + src->mem.hugepage_backed); + goto cleanup; + } + if (src->vcpus != dst->vcpus) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vpu count %d does not match source %d"), -- 1.7.4.4

On 13.09.2011 22:27, Eric Blake wrote:
Commit 973fcd8f introduced the ability for qemu to reject snapshot reversion on an ABI incompatibility; but the very example that was first proposed on-list[1] as a demonstration of an ABI incompatibility, namely that of changing the max memory allocation, was not being checked for, resulting in a cryptic failure when running with larger max mem than what the snapshot was created with: error: operation failed: Error -22 while loading VM state
This commit merely protects the three variables within mem that are referenced by qemu_command.c, rather than all 7 (the other 4 variables affect cgroup handling, but have no visible effect to the qemu guest).
[1] https://www.redhat.com/archives/libvir-list/2010-December/msg00331.html
* src/conf/domain_conf.c (virDomainDefCheckABIStability): Add memory sizing checks. ---
See also https://bugzilla.redhat.com/show_bug.cgi?id=735553#c5
src/conf/domain_conf.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 11755fe..996e0d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8229,6 +8229,26 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, goto cleanup; }
+ if (src->mem.max_balloon != dst->mem.max_balloon) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain max memory %ld does not match source %ld"), + dst->mem.max_balloon, src->mem.max_balloon); + goto cleanup; + } + if (src->mem.cur_balloon != dst->mem.cur_balloon) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain current memory %ld does not match source %ld"), + dst->mem.cur_balloon, src->mem.cur_balloon); + goto cleanup; + } + if (src->mem.hugepage_backed != dst->mem.hugepage_backed) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain huge page backing %ld does not match source %ld"), + dst->mem.hugepage_backed, + src->mem.hugepage_backed); + goto cleanup; + } + if (src->vcpus != dst->vcpus) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vpu count %d does not match source %d"),
ACK Michal

On 09/14/2011 09:47 AM, Michal Privoznik wrote:
On 13.09.2011 22:27, Eric Blake wrote:
Commit 973fcd8f introduced the ability for qemu to reject snapshot reversion on an ABI incompatibility; but the very example that was first proposed on-list[1] as a demonstration of an ABI incompatibility, namely that of changing the max memory allocation, was not being checked for, resulting in a cryptic failure when running with larger max mem than what the snapshot was created with: error: operation failed: Error -22 while loading VM state
This commit merely protects the three variables within mem that are referenced by qemu_command.c, rather than all 7 (the other 4 variables affect cgroup handling, but have no visible effect to the qemu guest).
[1] https://www.redhat.com/archives/libvir-list/2010-December/msg00331.html
* src/conf/domain_conf.c (virDomainDefCheckABIStability): Add memory sizing checks. ---
See also https://bugzilla.redhat.com/show_bug.cgi?id=735553#c5
ACK
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Michal Privoznik