[Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Dr. David Alan Gilbert]

In the continuing journeys of trying to migrate a q35 guest with ovmf,
I've just hit this assert:

qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.

This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.

It looks like that's the efi var store stuff:

#4  0x00007f8073ec3947 in bdrv_co_do_pwritev (bs=0x7f8074faf580, offset=<optimized out>, bytes=131072, 
    qiov=0x7f805d5feb50, flags=(unknown: 0)) at /root/git/qemu/block/io.c:1297
#5  0x00007f8073eb6846 in blk_co_pwritev (blk=<optimized out>, offset=<optimized out>, bytes=<optimized out>, 
    qiov=<optimized out>, flags=<optimized out>) at /root/git/qemu/block/block-backend.c:713
#6  0x00007f8073eb688c in blk_write_entry (opaque=0x7f805d5feb70) at /root/git/qemu/block/block-backend.c:736
#7  0x00007f8073f2782a in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>)
    at /root/git/qemu/util/coroutine-ucontext.c:78
#8  0x00007f806aff9110 in ?? () from /lib64/libc.so.6
#9  0x00007f805d5fe3c0 in ?? ()
#10 0x0000000000000000 in ?? ()
(gdb) up
#1  0x00007f806afe8ce8 in abort () from /lib64/libc.so.6
(gdb) 
#2  0x00007f806afe0566 in __assert_fail_base () from /lib64/libc.so.6
(gdb) 
#3  0x00007f806afe0612 in __assert_fail () from /lib64/libc.so.6
(gdb) 
#4  0x00007f8073ec3947 in bdrv_co_do_pwritev (bs=0x7f8074faf580, offset=<optimized out>, bytes=131072, 
    qiov=0x7f805d5feb50, flags=(unknown: 0)) at /root/git/qemu/block/io.c:1297
1297	    bool use_local_qiov = false;
(gdb) p *bs
$1 = {total_sectors = 256, read_only = 0, open_flags = 10242, encrypted = 0, valid_key = 0, sg = 0, 
  copy_on_read = 0, probed = false, drv = 0x7f8074474840 <bdrv_raw>, opaque = 0x0, blk = 0x7f8074fa5920, 
  aio_context = 0x7f8074f61bd0, aio_notifiers = {lh_first = 0x0}, 
  filename = "/var/lib/libvirt/qemu/nvram/f23q35efi_VARS.fd", '\000' <repeats 4040 times>, 
  backing_file = '\000' <repeats 4095 times>, backing_format = '\000' <repeats 15 times>, 
  full_open_options = 0x7f8074fb9b60, 
  exact_filename = "/var/lib/libvirt/qemu/nvram/f23q35efi_VARS.fd", '\000' <repeats 4040 times>, 
  backing = 0x0, file = 0x7f8074fa6020, before_write_notifiers = {notifiers = {lh_first = 0x0}}, 
  serialising_in_flight = 0, throttled_reqs = {{entries = {tqh_first = 0x0, tqh_last = 0x7f8074fb2608}}, {
      entries = {tqh_first = 0x0, tqh_last = 0x7f8074fb2618}}}, io_limits_enabled = false, throttle_state = 0x0, 
  throttle_timers = {timers = {0x0, 0x0}, clock_type = QEMU_CLOCK_REALTIME, read_timer_cb = 0x0, 
    write_timer_cb = 0x0, timer_opaque = 0x0}, pending_reqs = {0, 0}, round_robin = {le_next = 0x0, 
    le_prev = 0x0}, wr_highest_offset = 0, bl = {max_discard = 0, discard_alignment = 0, max_write_zeroes = 0, 
    write_zeroes_alignment = 0, opt_transfer_length = 0, max_transfer_length = 0, min_mem_alignment = 1, 
    opt_mem_alignment = 4096, max_iov = 1024}, zero_beyond_eof = true, request_alignment = 512, 
  node_name = "#block307", '\000' <repeats 22 times>, node_list = {tqe_next = 0x7f8074fc1eb0, 
    tqe_prev = 0x7f8074fb8a00}, bs_list = {tqe_next = 0x7f8074fb5890, tqe_prev = 0x7f8074faa9a0}, monitor_list = {
    tqe_next = 0x0, tqe_prev = 0x0}, dirty_bitmaps = {lh_first = 0x0}, refcnt = 1, tracked_requests = {
    lh_first = 0x0}, op_blockers = {{lh_first = 0x0} <repeats 16 times>}, job = 0x0, inherits_from = 0x0, 
  children = {lh_first = 0x7f8074fa6020}, parents = {lh_first = 0x7f8074fa5a80}, options = 0x7f8074fa04f0, 
  explicit_options = 0x7f8074fb2830, detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, backing_blocker = 0x0, 
  write_threshold_offset = 0, write_threshold_notifier = {notify = 0x0, node = {le_next = 0x0, le_prev = 0x0}}, 
  quiesce_counter = 0}

Full command line:
/opt/qemu-head/bin/qemu-system-x86_64 -name f23q35efib -S -machine pc-q35-2.6,accel=kvm,usb=off,vmport=off -cpu SandyBridge -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on -drive file=/var/lib/libvirt/qemu/nvram/f23q35efi.fd,if=pflash,format=raw,unit=1 -m 4096 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid e938e303-0cff-4863-9a0b-cf7bcca2f532 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-2-f23q35efib/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard -no-hpet -no-shutdown -global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on -device pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x1e -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 -device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,addr=0x1d -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2 -device lsi,id=scsi0,bus=pci.2,addr=0x6 -device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x3 -drive file=/home/vms/f23q35efi.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,cache=none -device virtio-blk-pci,scsi=off,bus=pci.2,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive if=none,id=drive-scsi0-0-0,readonly=on -device scsi-cd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0,bootindex=2 -netdev tap,fd=25,id=hostnet0 -device e1000,netdev=hostnet0,id=net0,mac=52:54:00:1f:62:58,bus=pci.2,addr=0x1 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-2-f23q35efib/org.qemu.guest_agent.0,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -chardev spicevmc,id=charchannel1,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0 -device usb-tablet,id=input0 -spice port=5900,addr=127.0.0.1,disable-ticketing,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,bus=pcie.0,addr=0x1 -device intel-hda,id=sound0,bus=pci.2,addr=0x2 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1 -device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x5 -msg timestamp=on

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

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Paolo Bonzini]

On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
> In the continuing journeys of trying to migrate a q35 guest with ovmf,
> I've just hit this assert:
> 
> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> 
> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.

Try this...

Paolo

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index c475c2a..e96a7b0 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -46,6 +46,7 @@
 #include "exec/address-spaces.h"
 #include "qemu/host-utils.h"
 #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
 
 #define PFLASH_BUG(fmt, ...) \
 do { \
@@ -97,6 +98,7 @@ struct pflash_t {
     MemoryRegion mem;
     char *name;
     void *storage;
+    VMChangeStateEntry *vmstate;
 };
 
 static int pflash_post_load(void *opaque, int version_id);
@@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
     return &fl->mem;
 }
 
+static void postload_update_cb(void *opaque, int running, RunState state)
+{
+    pflash_t *pfl = opaque;
+
+    /* This is called after bdrv_invalidate_cache_all.  */
+    qemu_del_vm_change_state_handler(pfl->vmstate);
+    pfl->vmstate = NULL;
+
+    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
+    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
+}
+
 static int pflash_post_load(void *opaque, int version_id)
 {
     pflash_t *pfl = opaque;
 
     if (!pfl->ro) {
-        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
-        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
+        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
     }
     return 0;
 }

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
> > In the continuing journeys of trying to migrate a q35 guest with ovmf,
> > I've just hit this assert:
> > 
> > qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> > 
> > This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
> 
> Try this...

Well, migration survives; how do I test if pflash is sane after migration?

Dave

> Paolo
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index c475c2a..e96a7b0 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -46,6 +46,7 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/host-utils.h"
>  #include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
>  
>  #define PFLASH_BUG(fmt, ...) \
>  do { \
> @@ -97,6 +98,7 @@ struct pflash_t {
>      MemoryRegion mem;
>      char *name;
>      void *storage;
> +    VMChangeStateEntry *vmstate;
>  };
>  
>  static int pflash_post_load(void *opaque, int version_id);
> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
>      return &fl->mem;
>  }
>  
> +static void postload_update_cb(void *opaque, int running, RunState state)
> +{
> +    pflash_t *pfl = opaque;
> +
> +    /* This is called after bdrv_invalidate_cache_all.  */
> +    qemu_del_vm_change_state_handler(pfl->vmstate);
> +    pfl->vmstate = NULL;
> +
> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> +}
> +
>  static int pflash_post_load(void *opaque, int version_id)
>  {
>      pflash_t *pfl = opaque;
>  
>      if (!pfl->ro) {
> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
>      }
>      return 0;
>  }
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Laszlo Ersek]

On 04/05/16 12:48, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>
>>
>> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
>>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
>>> I've just hit this assert:
>>>
>>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
>>>
>>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
>>
>> Try this...
> 
> Well, migration survives; how do I test if pflash is sane after migration?

You can run sha1sum before / after. The varstore is expected to change
only when the UEFI variable servies are exercised. So, if you boot e.g.
a Linux guest to a login prompt on the source host, checksum the
varstore, then migrate the guest, then verify the checksum on the target
host (or, well, shared storage, if you have set it up), it should match.

Thanks
Laszlo

> 
> Dave
> 
>> Paolo
>>
>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>> index c475c2a..e96a7b0 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -46,6 +46,7 @@
>>  #include "exec/address-spaces.h"
>>  #include "qemu/host-utils.h"
>>  #include "hw/sysbus.h"
>> +#include "sysemu/sysemu.h"
>>  
>>  #define PFLASH_BUG(fmt, ...) \
>>  do { \
>> @@ -97,6 +98,7 @@ struct pflash_t {
>>      MemoryRegion mem;
>>      char *name;
>>      void *storage;
>> +    VMChangeStateEntry *vmstate;
>>  };
>>  
>>  static int pflash_post_load(void *opaque, int version_id);
>> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
>>      return &fl->mem;
>>  }
>>  
>> +static void postload_update_cb(void *opaque, int running, RunState state)
>> +{
>> +    pflash_t *pfl = opaque;
>> +
>> +    /* This is called after bdrv_invalidate_cache_all.  */
>> +    qemu_del_vm_change_state_handler(pfl->vmstate);
>> +    pfl->vmstate = NULL;
>> +
>> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +}
>> +
>>  static int pflash_post_load(void *opaque, int version_id)
>>  {
>>      pflash_t *pfl = opaque;
>>  
>>      if (!pfl->ro) {
>> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
>>      }
>>      return 0;
>>  }
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Dr. David Alan Gilbert]

* Laszlo Ersek (lersek@redhat.com) wrote:
> On 04/05/16 12:48, Dr. David Alan Gilbert wrote:
> > * Paolo Bonzini (pbonzini@redhat.com) wrote:
> >>
> >>
> >> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
> >>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
> >>> I've just hit this assert:
> >>>
> >>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> >>>
> >>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
> >>
> >> Try this...
> > 
> > Well, migration survives; how do I test if pflash is sane after migration?
> 
> You can run sha1sum before / after. The varstore is expected to change
> only when the UEFI variable servies are exercised. So, if you boot e.g.
> a Linux guest to a login prompt on the source host, checksum the
> varstore, then migrate the guest, then verify the checksum on the target
> host (or, well, shared storage, if you have set it up), it should match.

OK, yes that works; and I also tried using efibootmgr to tweak the timeout,
seeing that the sha changed and then check the sha was correct again after
migration.

Dave

> 
> Thanks
> Laszlo
> 
> > 
> > Dave
> > 
> >> Paolo
> >>
> >> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> >> index c475c2a..e96a7b0 100644
> >> --- a/hw/block/pflash_cfi01.c
> >> +++ b/hw/block/pflash_cfi01.c
> >> @@ -46,6 +46,7 @@
> >>  #include "exec/address-spaces.h"
> >>  #include "qemu/host-utils.h"
> >>  #include "hw/sysbus.h"
> >> +#include "sysemu/sysemu.h"
> >>  
> >>  #define PFLASH_BUG(fmt, ...) \
> >>  do { \
> >> @@ -97,6 +98,7 @@ struct pflash_t {
> >>      MemoryRegion mem;
> >>      char *name;
> >>      void *storage;
> >> +    VMChangeStateEntry *vmstate;
> >>  };
> >>  
> >>  static int pflash_post_load(void *opaque, int version_id);
> >> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
> >>      return &fl->mem;
> >>  }
> >>  
> >> +static void postload_update_cb(void *opaque, int running, RunState state)
> >> +{
> >> +    pflash_t *pfl = opaque;
> >> +
> >> +    /* This is called after bdrv_invalidate_cache_all.  */
> >> +    qemu_del_vm_change_state_handler(pfl->vmstate);
> >> +    pfl->vmstate = NULL;
> >> +
> >> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> >> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> >> +}
> >> +
> >>  static int pflash_post_load(void *opaque, int version_id)
> >>  {
> >>      pflash_t *pfl = opaque;
> >>  
> >>      if (!pfl->ro) {
> >> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> >> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> >> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
> >>      }
> >>      return 0;
> >>  }
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Laszlo Ersek]

On 04/05/16 13:25, Dr. David Alan Gilbert wrote:
> * Laszlo Ersek (lersek@redhat.com) wrote:
>> On 04/05/16 12:48, Dr. David Alan Gilbert wrote:
>>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>>>
>>>>
>>>> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
>>>>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
>>>>> I've just hit this assert:
>>>>>
>>>>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
>>>>>
>>>>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
>>>>
>>>> Try this...
>>>
>>> Well, migration survives; how do I test if pflash is sane after migration?
>>
>> You can run sha1sum before / after. The varstore is expected to change
>> only when the UEFI variable servies are exercised. So, if you boot e.g.
>> a Linux guest to a login prompt on the source host, checksum the
>> varstore, then migrate the guest, then verify the checksum on the target
>> host (or, well, shared storage, if you have set it up), it should match.
> 
> OK, yes that works; and I also tried using efibootmgr to tweak the timeout,
> seeing that the sha changed and then check the sha was correct again after
> migration.

Thank you both for fixing up the buggy-on-arrival code I had added.
Laszlo

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Paolo Bonzini]

On 05/04/2016 12:48, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>
>>
>> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
>>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
>>> I've just hit this assert:
>>>
>>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
>>>
>>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
>>
>> Try this...
> 
> Well, migration survives; how do I test if pflash is sane after migration?

Use "-S" on the destination and check if the file has the same contents.

Paolo

> Dave
> 
>> Paolo
>>
>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>> index c475c2a..e96a7b0 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -46,6 +46,7 @@
>>  #include "exec/address-spaces.h"
>>  #include "qemu/host-utils.h"
>>  #include "hw/sysbus.h"
>> +#include "sysemu/sysemu.h"
>>  
>>  #define PFLASH_BUG(fmt, ...) \
>>  do { \
>> @@ -97,6 +98,7 @@ struct pflash_t {
>>      MemoryRegion mem;
>>      char *name;
>>      void *storage;
>> +    VMChangeStateEntry *vmstate;
>>  };
>>  
>>  static int pflash_post_load(void *opaque, int version_id);
>> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
>>      return &fl->mem;
>>  }
>>  
>> +static void postload_update_cb(void *opaque, int running, RunState state)
>> +{
>> +    pflash_t *pfl = opaque;
>> +
>> +    /* This is called after bdrv_invalidate_cache_all.  */
>> +    qemu_del_vm_change_state_handler(pfl->vmstate);
>> +    pfl->vmstate = NULL;
>> +
>> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +}
>> +
>>  static int pflash_post_load(void *opaque, int version_id)
>>  {
>>      pflash_t *pfl = opaque;
>>  
>>      if (!pfl->ro) {
>> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
>>      }
>>      return 0;
>>  }
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Peter Maydell]

On 4 April 2016 at 11:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
>> I've just hit this assert:
>>
>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
>>
>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
>
> Try this...

Hi Paolo -- this bug is on the list of "things we should probably
fix for 2.6", but your fix below doesn't have your Signed-off-by
line. Were you planning to send this as a proper patch, or could
you provide an s-o-b line so somebody else (David?) can take the
code and clean it up into an actual patch?

thanks
-- PMM

>
> Paolo
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index c475c2a..e96a7b0 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -46,6 +46,7 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/host-utils.h"
>  #include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
>
>  #define PFLASH_BUG(fmt, ...) \
>  do { \
> @@ -97,6 +98,7 @@ struct pflash_t {
>      MemoryRegion mem;
>      char *name;
>      void *storage;
> +    VMChangeStateEntry *vmstate;
>  };
>
>  static int pflash_post_load(void *opaque, int version_id);
> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
>      return &fl->mem;
>  }
>
> +static void postload_update_cb(void *opaque, int running, RunState state)
> +{
> +    pflash_t *pfl = opaque;
> +
> +    /* This is called after bdrv_invalidate_cache_all.  */
> +    qemu_del_vm_change_state_handler(pfl->vmstate);
> +    pfl->vmstate = NULL;
> +
> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> +}
> +
>  static int pflash_post_load(void *opaque, int version_id)
>  {
>      pflash_t *pfl = opaque;
>
>      if (!pfl->ro) {
> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
>      }
>      return 0;
>  }

Re: [Qemu-devel] efi var store migration assert (bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.)

[Paolo Bonzini]

On 14/04/2016 17:30, Peter Maydell wrote:
> On 4 April 2016 at 11:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 01/04/2016 19:58, Dr. David Alan Gilbert wrote:
>>> In the continuing journeys of trying to migrate a q35 guest with ovmf,
>>> I've just hit this assert:
>>>
>>> qemu-system-x86_64: /root/git/qemu/block/io.c:1297: bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
>>>
>>> This is just ahead of rc0 - 1458317c8ada834cf39287f6d11a8cb8a37360d6 from yesterday.
>>
>> Try this...
> 
> Hi Paolo -- this bug is on the list of "things we should probably
> fix for 2.6", but your fix below doesn't have your Signed-off-by
> line. Were you planning to send this as a proper patch, or could
> you provide an s-o-b line so somebody else (David?) can take the
> code and clean it up into an actual patch?

Indeed. I had sent the S-o-b to David, but that was a private email so

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,

Paolo

> thanks
> -- PMM
> 
>>
>> Paolo
>>
>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>> index c475c2a..e96a7b0 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -46,6 +46,7 @@
>>  #include "exec/address-spaces.h"
>>  #include "qemu/host-utils.h"
>>  #include "hw/sysbus.h"
>> +#include "sysemu/sysemu.h"
>>
>>  #define PFLASH_BUG(fmt, ...) \
>>  do { \
>> @@ -97,6 +98,7 @@ struct pflash_t {
>>      MemoryRegion mem;
>>      char *name;
>>      void *storage;
>> +    VMChangeStateEntry *vmstate;
>>  };
>>
>>  static int pflash_post_load(void *opaque, int version_id);
>> @@ -944,13 +946,24 @@ MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
>>      return &fl->mem;
>>  }
>>
>> +static void postload_update_cb(void *opaque, int running, RunState state)
>> +{
>> +    pflash_t *pfl = opaque;
>> +
>> +    /* This is called after bdrv_invalidate_cache_all.  */
>> +    qemu_del_vm_change_state_handler(pfl->vmstate);
>> +    pfl->vmstate = NULL;
>> +
>> +    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> +    pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +}
>> +
>>  static int pflash_post_load(void *opaque, int version_id)
>>  {
>>      pflash_t *pfl = opaque;
>>
>>      if (!pfl->ro) {
>> -        DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
>> -        pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
>> +        pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb, pfl);
>>      }
>>      return 0;
>>  }