qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Don Slutz <dslutz@verizon.com>
To: Markus Armbruster <armbru@redhat.com>, Don Slutz <dslutz@verizon.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration
Date: Thu, 20 Nov 2014 13:31:58 -0500	[thread overview]
Message-ID: <546E339E.4030202@terremark.com> (raw)
In-Reply-To: <87k32r8jc8.fsf@blackfin.pond.sub.org>

On 11/19/14 07:29, Markus Armbruster wrote:
> Don Slutz <dslutz@verizon.com> writes:
>
>> The other callers to blk_set_enable_write_cache() in this file
>> already check for s->blk == NULL.
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> ---
>>
>> I think this is a bugfix that should be back ported to stable
>> releases.
>>
>> I also think this should be done in xen's copy of QEMU for 4.5 with
>> back port(s) to active stable releases.
>>
>> Note: In 2.1 and earlier the routine is
>> bdrv_set_enable_write_cache(); variable is s->bs.
> Got a reproducer?

yes.  Migrating a guest from xen 4.2 or 4.3 to xen 4.4 (or 4.5-unstable) on
CentOS 6.3 with xen_emul_unplug=unnecessary and no cdrom defined.


>
> I'm asking because I believe s->identify_set implies s->blk.
> s->identify_set is initialized to zero, and gets set to non-zero exactly
> on the first successful IDENTIFY DEVICE or IDENTIFY PACKET DEVICE, in
> ide_identify(), ide_atapi_identify() or ide_cfata_identify(),
> respectively.  Only called via cmd_identify() / cmd_identify_packet()
> via ide_exec_cmd().  The latter immediately fails when !s->blk:
>
>      s = idebus_active_if(bus);
>      /* ignore commands to non existent slave */
>      if (s != bus->ifs && !s->blk) {
>          return;
>      }

I do think that you are right.  I have now spent more time on why I am
seeing this.


> Even if I'm right, your patch is fine, because it makes this spot more
> obviously correct, and consistent with the other uses of
> blk_set_enable_write_cache().  The case for stable is weak, though.
>

I had not fully tracked down what is happening before sending the bugfix.
I have now done more debugging, and have tracked it down to xen 4.4
now using "-nodefaults" with QEMU.

I needed to add output to QEMU to track this down because I have long
command lines...

(all I get for ps -ef):
root     14864     1 82 16:42 ?        00:00:09 
/usr/lib/xen/bin/qemu-system-i386 -xen-domid 20 -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-20,server,nowait -mon 
chardev=libxl-cmd,mode=control -name C63-min-tools -machine 
xenfv,vmware_hw=7,xen_platform_pci=240 -hostbridge 82443 -device 
agp-bridge,id=agp,msi=off,msi-x=off,addr=0x12.0 -monitor pty -monitor vc 
-boot menu=on -device 
vmware-pci-bridge,msi=on,msi-x=on,id=pciBridge1.0,addr=0x11.0 -device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.0,multifunction=on,addr=0x15.0 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.1,multifunction=on,addr=0x15.1 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.2,multifunction=on,addr=0x15.2 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.3,multifunction=on,addr=0x15.3 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.4,multifunction=on,addr=0x15.4 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.5,multifunction=on,addr=0x15.5 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.6,multifunction=on,addr=0x15.6 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge5.7,multifunction=on,addr=0x15.7 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.0,multifunction=on,addr=0x16.0 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.1,multifunction=on,addr=0x16.1 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.2,multifunction=on,addr=0x16.2 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.3,multifunction=on,addr=0x16.3 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.4,multifunction=on,addr=0x16.4 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.5,multifunction=on,addr=0x16.5 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.6,multifunction=on,addr=0x16.6 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge6.7,multifunction=on,addr=0x16.7 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.0,multifunction=on,addr=0x17.0 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.1,multifunction=on,addr=0x17.1 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.2,multifunction=on,addr=0x17.2 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.3,multifunction=on,addr=0x17.3 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.4,multifunction=on,addr=0x17.4 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.5,multifunction=on,addr=0x17.5 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.6,multifunction=on,addr=0x17.6 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge7.7,multifunction=on,addr=0x17.7 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.0,multifunction=on,addr=0x18.0 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.1,multifunction=on,addr=0x18.1 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.2,multifunction=on,addr=0x18.2 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.3,multifunction=on,addr=0x18.3 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.4,multifunction=on,addr=0x18.4 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.5,multifunction=on,addr=0x18.5 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.6,multifunction=on,addr=0x18.6 
-device 
vmware-pcie-bridge,msi=on,msi-x=on,id=pciBridge8.7,multifunction=on,addr=0x18.7 
-vnc 0.0.0.0:7,to=99 -serial pty -vga vmware -global 
vmware-svga.vgamem_mb=32 -boot order=cda -device 
vmxnet3,id=nic0,netdev=net0,mac=00:0c:29:86:44:a0,addr=0x3.0x0 -netdev 
type=tap,id=net0,ifname=vif20.0-emu,script=no,downscript=no -device 
e1000_vmw,id=nic1,netdev=net1,mac=00:0c:29:86:44:aa,addr=0x4.0x0 -netdev 
type=tap,id=net1,ifname=vif20.1-emu,script=no,downscript=no -device 
vmxnet3,id=nic2,netdev=net2,mac=00:0c:29:86:44:b4,addr=0x5.0x0 -netdev 
type=tap,id=net2,ifname=vif20.2-emu,script=no,downscript=no -device 
vmxnet3,id=nic3,netdev=net3,mac=00:0c:29:86:44:be,addr=0x6.0x0 -netdev 
type=tap,id=net3,ifname=vif20.3-emu,script=no,downscript=no -device 
vmxnet3,id=nic4,netdev=net4,mac=00:0c:29:86:44:c8,addr=0x8.0x0 -netdev 
type=tap,id=net4,ifname=vif20.4-emu,script=no,downscript=no -device 
vmxnet3,id=nic5,netdev=net5,mac=00:0c:29:86:44:d2,addr=0x9.0x0 -netdev t


Which is missing that option.

The ide that was aborting in this case is the cdrom at hdc that is added
if you do not specify "-nodefaults".

Since this is a "changed" machine config, I am no longer as sure as what
versions this needs to be in.

If I put my QEMU hat on, it does not look like a back port is needed.  
However
for xen it would be nice.

I do not know how the QEMU community feels about migration from a config
without "-nodefaults" to one with "-nodefaults" as the only difference.

    -Don Slutz

>>   hw/ide/core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 00e21cf..d4af5e2 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2401,7 +2401,7 @@ static int ide_drive_post_load(void *opaque, int version_id)
>>   {
>>       IDEState *s = opaque;
>>   
>> -    if (s->identify_set) {
>> +    if (s->blk && s->identify_set) {
>>           blk_set_enable_write_cache(s->blk, !!(s->identify_data[85] & (1 << 5)));
>>       }
>>       return 0;

  reply	other threads:[~2014-11-20 18:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 21:20 [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration Don Slutz
2014-11-18 10:05 ` Paolo Bonzini
2014-11-18 11:37 ` Stefan Hajnoczi
2014-11-18 11:41 ` Kevin Wolf
2014-11-18 18:00   ` Peter Maydell
2014-11-18 14:12 ` Stefano Stabellini
2014-11-19 12:29 ` Markus Armbruster
2014-11-20 18:31   ` Don Slutz [this message]
2014-11-21  8:42     ` Markus Armbruster
2014-11-21 10:49       ` Dr. David Alan Gilbert
2014-11-25  0:48         ` Don Slutz
2014-11-25  8:59           ` Dr. David Alan Gilbert
2014-11-25 11:11             ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=546E339E.4030202@terremark.com \
    --to=dslutz@verizon.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=stefano.stabellini@eu.citrix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).