Bug 1161563

Summary: invalid QEMU NOTEs in vmcore that is dumped for multi-VCPU guests
Product: Red Hat Enterprise Linux 7 Reporter: Laszlo Ersek <lersek>
Component: qemu-kvmAssignee: Laszlo Ersek <lersek>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.1CC: hhuang, huding, juzhang, lersek, lmiksik, michen, rbalakri, virt-maint, xfu, xwei
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: qemu-kvm-1.5.3-80.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-03-05 08:13:20 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1157798    

Description Laszlo Ersek 2014-11-07 10:57:43 UTC
*** Description of problem:

In qemu-kvm-1.5.3-77.el7, the write_elf64_notes() and write_elf32_notes()
functions are broken due to upstream commit c72bf4682.

    commit c72bf468259935a80ea185f2cbe807c3da9c1bbd
    Author: Jens Freimann <jfrei.ibm.com>
    Date:   Fri Apr 19 16:45:06 2013 +0200

        cpu: Move cpu_write_elfXX_note() functions to CPUState

        Convert cpu_write_elfXX_note() functions to CPUClass methods and
        pass CPUState as argument. Update target-i386 accordingly.

        Signed-off-by: Jens Freimann <jfrei.ibm.com>
        [AF: Retain stubs as CPUClass' default method implementation;
         style changes]
        Signed-off-by: Andreas Färber <afaerber>

This commit changed the signature of the following functions so that they
take CPUState rather than CPUArchState:
- cpu_write_elf64_note()
- cpu_write_elf64_qemunote()
- cpu_write_elf32_note()
- cpu_write_elf32_qemunote()

The callers of these functions, write_elf64_notes() and
write_elf32_notes(), each iterate over CPUArchState objects (starting from
"first_cpu") *twice*, the first loop calling cpu_write_elfXX_note(), the
second loop calling cpu_write_elfXX_qemunote(). The loop variable is
called "env". When calling the above functions after c72bf468, "env" (of
type CPUArchState) needs to be converted to CPUState, with the
ENV_GET_CPU() macro.

Now, even before c72bf468, the *first* loop in each of both callers used
to do the conversion already, because cpu_write_elfXX_note() needs a CPU
index, and that's only reachable via cpu_index(ENV_GET_CPU(env)).
Therefore the first loop in each caller already set the "cpu" local
variable correctly, for each "env" in question.

However, the *second* loop in each caller had never done that, because
cpu_write_elfXX_qemunote() had never needed a CPUState for anything.

Upstream commit c72bf4682 simply replaced "env" with "cpu" in both loop
bodies (in both callers). This was correct for the first loops (because
they already had set "cpu" correctly), but the commit missed to add

    cpu = ENV_GET_CPU(env);

to the second loops. Hence cpu_write_elfXX_qemunote() is always called
with the last "cpu" value inherited from the first loop! (Which is why the
bug is invisible for single-VCPU guests.)

Add the missing assignments.

For upstream, this was silently fixed in

    commit 182735efaf956ccab50b6d74a4fed163e0f35660
    Author: Andreas Färber <afaerber>
    Date:   Wed May 29 22:29:20 2013 +0200

        cpu: Make first_cpu and next_cpu CPUState

        Move next_cpu from CPU_COMMON to CPUState.
        Move first_cpu variable to qom/cpu.h.

        gdbstub needs to use CPUState::env_ptr for now.
        cpu_copy() no longer needs to save and restore cpu_next.

        Acked-by: Paolo Bonzini <pbonzini>
        [AF: Rebased, simplified cpu_copy()]
        Signed-off-by: Andreas Färber <afaerber>

because it obviated and eliminated the

    cpu = ENV_GET_CPU(env);

conversions completely. The bug-introducing commit c72bf4682 had been
released in v1.5.0 (and it persisted even into 1.5.3, which is why we have
it in RHEL). The silent / unintended fix (182735ef) is part of v1.6.0 (and
we never backported that commit to RHEL-7.0).

*** Version-Release number of selected component (if applicable):
qemu-kvm-1.5.3-77.el7

*** How reproducible:
Not very easily reproducible externally. Bug found by eyeballing the code.
Sanity testing by QE with dump-guest-memory will be enough for verifying the
fix.

Comment 6 Miroslav Rezanina 2014-11-21 07:34:49 UTC
Fix included in qemu-kvm-1.5.3-80.el7

Comment 8 Xiaoqing Wei 2014-12-02 02:37:01 UTC
Hello Laszlo,

what kind of guest cfg is sufficient to verify this bz ?

does guest cfg and test steps in below fine ?
https://bugzilla.redhat.com/show_bug.cgi?id=1157798#c12

it's a RHEL 7.1 VM with 2vcpu 4G RAM
both full memory dump(ELF) and partialily compressed dump(snappy, lzo, zlib) tests performed

two methods of elf dump covered.
virsh dump --memory-only rhel7.1-bios /root/dump

virsh qemu-monitor-command rhel7.1-bios --pretty  '
{
  "execute": "dump-guest-memory",
  "arguments": {
    "paging": false,
    "protocol": "file:/tmp/vmcore.elf",
    "format": "elf"
  }
}'

OVMF guest covered too :-)

if above test steps or cfg insufficient, pls tell me.

Thank you,
Xiaoqing Wei.

Comment 9 Laszlo Ersek 2014-12-02 07:55:19 UTC
(In reply to Xiaoqing Wei from comment #8)

> does guest cfg and test steps in below fine ?
> https://bugzilla.redhat.com/show_bug.cgi?id=1157798#c12

Yes, those config & steps are sufficient.

Thanks!

Comment 10 Xiaoqing Wei 2014-12-02 08:36:30 UTC
Thanks(In reply to Laszlo Ersek from comment #9)
> (In reply to Xiaoqing Wei from comment #8)
> 
> > does guest cfg and test steps in below fine ?
> > https://bugzilla.redhat.com/show_bug.cgi?id=1157798#c12
> 
> Yes, those config & steps are sufficient.
> 

Thanks !

now moving to VERIFIED.

Comment 12 errata-xmlrpc 2015-03-05 08:13:20 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0349.html