Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-25 Thread Dr. David Alan Gilbert
* Don Slutz (dsl...@verizon.com) wrote:
 On 11/21/14 05:49, Dr. David Alan Gilbert wrote:
 * Markus Armbruster (arm...@redhat.com) wrote:
 Don Slutzdsl...@verizon.com  writes:
 
 On 11/19/14 07:29, Markus Armbruster wrote:
 Don Slutzdsl...@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 Slutzdsl...@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):
 [...]
 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.
 So you have a CD-ROM on the source, but not on the destination?
 
 Yes, QEMU in the source has an empty CD-ROM, but not on the destination.
 xen
 does not know that QEMU added a CD-ROM drive in hdc by default.
 
 That can't work.  I guess it broke for you in an unusual way (target
 crashes) rather than the usual way (target rejects migration data for a
 device it doesn't have) due to our convoluted IDE data structures.  With
 your patch applied it should break the usual way.  Does it?
 
 Nope. It does not break at all.  The migration works.  The target accepts
 the hdc data.
 It looks like to me that all 4 IDE state data is sent.
 
 
 Management tools should use -nodefaults.  But if it mixes default and
 -nodefaults in migration, recreating the stuff it got by default but
 doesn't get with -nodefaults is its own responsibility.
 
 Yes. Since xen did not ask for a CD-ROM, it did not expect to need to create
 it.   xen was fixed to use -nodefaults.
 
 Well, mostly - we wouldn't expect a migration to work if the source/dest
 didn't match exactly; but QEMU shouldn't seg.
 
 Well, this change prevents a seg.  So you are in favor of having this
 backported?

Yes.

Dave

 
 
-Don Slutz
 
 Dave
 
 --
 Dr. David Alan Gilbert /dgilb...@redhat.com  / Manchester, UK
 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-25 Thread Markus Armbruster
Dr. David Alan Gilbert dgilb...@redhat.com writes:

 * Don Slutz (dsl...@verizon.com) wrote:
 On 11/21/14 05:49, Dr. David Alan Gilbert wrote:
 * Markus Armbruster (arm...@redhat.com) wrote:
[...]
 Management tools should use -nodefaults.  But if it mixes default and
 -nodefaults in migration, recreating the stuff it got by default but
 doesn't get with -nodefaults is its own responsibility.
 
 Yes. Since xen did not ask for a CD-ROM, it did not expect to need to create
 it.   xen was fixed to use -nodefaults.
 
 Well, mostly - we wouldn't expect a migration to work if the source/dest
 didn't match exactly; but QEMU shouldn't seg.
 
 Well, this change prevents a seg.  So you are in favor of having this
 backported?

 Yes.

I gladly defer to Dave's judgement here.



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-24 Thread Don Slutz

On 11/21/14 05:49, Dr. David Alan Gilbert wrote:

* Markus Armbruster (arm...@redhat.com) wrote:

Don Slutzdsl...@verizon.com  writes:


On 11/19/14 07:29, Markus Armbruster wrote:

Don Slutzdsl...@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 Slutzdsl...@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):

[...]

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.

So you have a CD-ROM on the source, but not on the destination?


Yes, QEMU in the source has an empty CD-ROM, but not on the 
destination.  xen

does not know that QEMU added a CD-ROM drive in hdc by default.


That can't work.  I guess it broke for you in an unusual way (target
crashes) rather than the usual way (target rejects migration data for a
device it doesn't have) due to our convoluted IDE data structures.  With
your patch applied it should break the usual way.  Does it?


Nope. It does not break at all.  The migration works.  The target 
accepts the hdc data.

It looks like to me that all 4 IDE state data is sent.



Management tools should use -nodefaults.  But if it mixes default and
-nodefaults in migration, recreating the stuff it got by default but
doesn't get with -nodefaults is its own responsibility.


Yes. Since xen did not ask for a CD-ROM, it did not expect to need to create
it.   xen was fixed to use -nodefaults.


Well, mostly - we wouldn't expect a migration to work if the source/dest
didn't match exactly; but QEMU shouldn't seg.


Well, this change prevents a seg.  So you are in favor of having this
backported?


   -Don Slutz


Dave

--
Dr. David Alan Gilbert /dgilb...@redhat.com  / Manchester, UK





Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-21 Thread Markus Armbruster
Don Slutz dsl...@verizon.com writes:

 On 11/19/14 07:29, Markus Armbruster wrote:
 Don Slutz dsl...@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 dsl...@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):
[...]


 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.

So you have a CD-ROM on the source, but not on the destination?

That can't work.  I guess it broke for you in an unusual way (target
crashes) rather than the usual way (target rejects migration data for a
device it doesn't have) due to our convoluted IDE data structures.  With
your patch applied it should break the usual way.  Does it?

Management tools should use -nodefaults.  But if it mixes default and
-nodefaults in migration, recreating the stuff it got by default but
doesn't get with -nodefaults is its own responsibility.



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-21 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote:
 Don Slutz dsl...@verizon.com writes:
 
  On 11/19/14 07:29, Markus Armbruster wrote:
  Don Slutz dsl...@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 dsl...@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):
 [...]
 
 
  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.
 
 So you have a CD-ROM on the source, but not on the destination?
 
 That can't work.  I guess it broke for you in an unusual way (target
 crashes) rather than the usual way (target rejects migration data for a
 device it doesn't have) due to our convoluted IDE data structures.  With
 your patch applied it should break the usual way.  Does it?
 
 Management tools should use -nodefaults.  But if it mixes default and
 -nodefaults in migration, recreating the stuff it got by default but
 doesn't get with -nodefaults is its own responsibility.

Well, mostly - we wouldn't expect a migration to work if the source/dest
didn't match exactly; but QEMU shouldn't seg.

Dave

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-20 Thread Don Slutz

On 11/19/14 07:29, Markus Armbruster wrote:

Don Slutz dsl...@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 dsl...@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 

Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-19 Thread Markus Armbruster
Don Slutz dsl...@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 dsl...@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?

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;
}

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.


  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;



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-18 Thread Paolo Bonzini


On 17/11/2014 22:20, Don Slutz wrote:
 The other callers to blk_set_enable_write_cache() in this file
 already check for s-blk == NULL.
 
 Signed-off-by: Don Slutz dsl...@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.
 
  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;
 

Reviewed-by: Paolo Bonzini pbonz...@redhat.com




Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-18 Thread Stefan Hajnoczi
On Mon, Nov 17, 2014 at 04:20:39PM -0500, Don Slutz wrote:
 The other callers to blk_set_enable_write_cache() in this file
 already check for s-blk == NULL.
 
 Signed-off-by: Don Slutz dsl...@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.
 
  hw/ide/core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com


pgp82QmAe9sMB.pgp
Description: PGP signature


Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-18 Thread Kevin Wolf
Am 17.11.2014 um 22:20 hat Don Slutz geschrieben:
 The other callers to blk_set_enable_write_cache() in this file
 already check for s-blk == NULL.
 
 Signed-off-by: Don Slutz dsl...@verizon.com
 ---
 
 I think this is a bugfix that should be back ported to stable
 releases.

Please remember to include a 'Cc: qemu-sta...@nongnu.org' line in your
commit messages for such patches. I've added it and applied the patch to
my block branch, thanks. (Saw it too late for my -rc2 pull request,
though, so it'll have to wait for -rc3.)

Kevin



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-18 Thread Stefano Stabellini
Konrad,
I think we should have this fix in Xen 4.5. Should I go ahead and
backport it?

On Mon, 17 Nov 2014, Don Slutz wrote:
 The other callers to blk_set_enable_write_cache() in this file
 already check for s-blk == NULL.
 
 Signed-off-by: Don Slutz dsl...@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.
 
  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;
 -- 
 1.8.4
 



Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-18 Thread Peter Maydell
On 18 November 2014 11:41, Kevin Wolf kw...@redhat.com wrote:
 Am 17.11.2014 um 22:20 hat Don Slutz geschrieben:
 The other callers to blk_set_enable_write_cache() in this file
 already check for s-blk == NULL.

 Signed-off-by: Don Slutz dsl...@verizon.com
 ---

 I think this is a bugfix that should be back ported to stable
 releases.

 Please remember to include a 'Cc: qemu-sta...@nongnu.org' line in your
 commit messages for such patches. I've added it and applied the patch to
 my block branch, thanks. (Saw it too late for my -rc2 pull request,
 though, so it'll have to wait for -rc3.)

Now applied directly to master so it will make -rc2.

thanks
-- PMM



[Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/ide/core.c: Prevent SIGSEGV during migration

2014-11-17 Thread Don Slutz
The other callers to blk_set_enable_write_cache() in this file
already check for s-blk == NULL.

Signed-off-by: Don Slutz dsl...@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.

 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;
-- 
1.8.4