1. 29 8月, 2018 1 次提交
  2. 15 8月, 2018 1 次提交
  3. 10 7月, 2018 3 次提交
  4. 18 6月, 2018 1 次提交
    • G
      block: fix QEMU crash with scsi-hd and drive_del · f45280cb
      Greg Kurz 提交于
      Removing a drive with drive_del while it is being used to run an I/O
      intensive workload can cause QEMU to crash.
      
      An AIO flush can yield at some point:
      
      blk_aio_flush_entry()
       blk_co_flush(blk)
        bdrv_co_flush(blk->root->bs)
         ...
          qemu_coroutine_yield()
      
      and let the HMP command to run, free blk->root and give control
      back to the AIO flush:
      
          hmp_drive_del()
           blk_remove_bs()
            bdrv_root_unref_child(blk->root)
             child_bs = blk->root->bs
             bdrv_detach_child(blk->root)
              bdrv_replace_child(blk->root, NULL)
               blk->root->bs = NULL
              g_free(blk->root) <============== blk->root becomes stale
             bdrv_unref(child_bs)
              bdrv_delete(child_bs)
               bdrv_close()
                bdrv_drained_begin()
                 bdrv_do_drained_begin()
                  bdrv_drain_recurse()
                   aio_poll()
                    ...
                    qemu_coroutine_switch()
      
      and the AIO flush completion ends up dereferencing blk->root:
      
        blk_aio_complete()
         scsi_aio_complete()
          blk_get_aio_context(blk)
           bs = blk_bs(blk)
       ie, bs = blk->root ? blk->root->bs : NULL
                  ^^^^^
                  stale
      
      The problem is that we should avoid making block driver graph
      changes while we have in-flight requests. Let's drain all I/O
      for this BB before calling bdrv_root_unref_child().
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      f45280cb
  5. 15 6月, 2018 1 次提交
  6. 01 6月, 2018 1 次提交
  7. 15 5月, 2018 1 次提交
  8. 14 3月, 2018 1 次提交
  9. 08 3月, 2018 1 次提交
    • D
      block: Fix qemu crash when using scsi-block · c060332c
      Deepa Srinivasan 提交于
      Starting qemu with the following arguments causes qemu to segfault:
      ... -device lsi,id=lsi0 -drive file=iscsi:<...>,format=raw,if=none,node-name=
      iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1
      
      This patch fixes blk_aio_ioctl() so it does not pass stack addresses to
      blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. More
      details about the bug follow.
      
      blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the
      coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().
      
      When blk_aio_ioctl() is executed from within a coroutine context (e.g.
      iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to
      the current coroutine's wakeup queue. blk_aio_ioctl() then returns.
      
      When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:
      ....
          BlkRwCo *rwco = &acb->rwco;
      
          rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
                                   rwco->qiov->iov[0].iov_base);  <--- qiov is
                                                                       invalid here
      ...
      
      In the case when blk_aio_ioctl() is called from a non-coroutine context,
      blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls
      qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine
      execution is complete, control returns to blk_aio_ioctl_entry() after the call
      to blk_co_ioctl(). There is no invalid reference after this point, but the
      function is still holding on to invalid pointers.
      
      The fix is to change blk_aio_prwv() to accept a void pointer for the IO buffer
      rather than a QEMUIOVector. blk_aio_prwv() passes this through in BlkRwCo and the
      coroutine function casts it to QEMUIOVector or uses the void pointer directly.
      Signed-off-by: NDeepa Srinivasan <deepa.srinivasan@oracle.com>
      Signed-off-by: NKonrad Rzeszutek Wilk <konrad.wilk@oracle.com>
      Reviewed-by: NMark Kanda <mark.kanda@oracle.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      c060332c
  10. 06 3月, 2018 1 次提交
  11. 03 3月, 2018 2 次提交
  12. 09 2月, 2018 2 次提交
  13. 08 2月, 2018 1 次提交
  14. 21 11月, 2017 1 次提交
  15. 18 11月, 2017 1 次提交
    • M
      block: Make bdrv_next() keep strong references · 5e003f17
      Max Reitz 提交于
      On one hand, it is a good idea for bdrv_next() to return a strong
      reference because ideally nearly every pointer should be refcounted.
      This fixes intermittent failure of iotest 194.
      
      On the other, it is absolutely necessary for bdrv_next() itself to keep
      a strong reference to both the BB (in its first phase) and the BDS (at
      least in the second phase) because when called the next time, it will
      dereference those objects to get a link to the next one.  Therefore, it
      needs these objects to stay around until then.  Just storing the pointer
      to the next in the iterator is not really viable because that pointer
      might become invalid as well.
      
      Both arguments taken together means we should probably just invoke
      bdrv_ref() and blk_ref() in bdrv_next().  This means we have to assert
      that bdrv_next() is always called from the main loop, but that was
      probably necessary already before this patch and judging from the
      callers, it also looks to actually be the case.
      
      Keeping these strong references means however that callers need to give
      them up if they decide to abort the iteration early.  They can do so
      through the new bdrv_next_cleanup() function.
      Suggested-by: NKevin Wolf <kwolf@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20171110172545.32609-1-mreitz@redhat.com
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      5e003f17
  16. 13 11月, 2017 4 次提交
    • A
      block: Leave valid throttle timers when removing a BDS from a backend · c89bcf3a
      Alberto Garcia 提交于
      If a BlockBackend has I/O limits set then its ThrottleGroupMember
      structure uses the AioContext from its attached BlockDriverState.
      Those two contexts must be kept in sync manually. This is not
      ideal and will be fixed in the future by removing the throttling
      configuration from the BlockBackend and storing it in an implicit
      filter node instead, but for now we have to live with this.
      
      When you remove the BlockDriverState from the backend then the
      throttle timers are destroyed. If a new BlockDriverState is later
      inserted then they are created again using the new AioContext.
      
      There are a couple of problems with this:
      
         a) The code manipulates the timers directly, leaving the
            ThrottleGroupMember.aio_context field in an inconsisent state.
      
         b) If you remove the I/O limits (e.g by destroying the backend)
            when the timers are gone then throttle_group_unregister_tgm()
            will attempt to destroy them again, crashing QEMU.
      
      While b) could be fixed easily by allowing the timers to be freed
      twice, this would result in a situation in which we can no longer
      guarantee that a valid ThrottleState has a valid AioContext and
      timers.
      
      This patch ensures that the timers and AioContext are always valid
      when I/O limits are set, regardless of whether the BlockBackend has a
      BlockDriverState inserted or not.
      
      [Fixed "There'a" typo as suggested by Max Reitz <mreitz@redhat.com>
      --Stefan]
      Reported-by: Nsochin jiang <sochin.jiang@huawei.com>
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Message-id: e089c66e7c20289b046d782cea4373b765c5bc1d.1510339534.git.berto@igalia.com
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      c89bcf3a
    • A
      block: Check for inserted BlockDriverState in blk_io_limits_disable() · 48bf7ea8
      Alberto Garcia 提交于
      When you set I/O limits using block_set_io_throttle or the command
      line throttling.* options they are kept in the BlockBackend regardless
      of whether a BlockDriverState is attached to the backend or not.
      
      Therefore when removing the limits using blk_io_limits_disable() we
      need to check if there's a BDS before attempting to drain it, else it
      will crash QEMU. This can be reproduced very easily using HMP:
      
           (qemu) drive_add 0 if=none,throttling.iops-total=5000
           (qemu) drive_del none0
      Reported-by: Nsochin jiang <sochin.jiang@huawei.com>
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 0d3a67ce8d948bb33e08672564714dcfb76a3d8c.1510339534.git.berto@igalia.com
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      48bf7ea8
    • S
      throttle-groups: drain before detaching ThrottleState · dc868fb0
      Stefan Hajnoczi 提交于
      I/O requests hang after stop/cont commands at least since QEMU 2.10.0
      with -drive iops=100:
      
        (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
        (qemu) stop
        (qemu) cont
        ...I/O is stuck...
      
      This happens because blk_set_aio_context() detaches the ThrottleState
      while requests may still be in flight:
      
        if (tgm->throttle_state) {
            throttle_group_detach_aio_context(tgm);
            throttle_group_attach_aio_context(tgm, new_context);
        }
      
      This patch encloses the detach/attach calls in a drained region so no
      I/O request is left hanging.  Also add assertions so we don't make the
      same mistake again in the future.
      Reported-by: NYongxue Hong <yhong@redhat.com>
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20171110151934.16883-1-stefanha@redhat.com
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      dc868fb0
    • Z
      block: all I/O should be completed before removing throttle timers. · 632a7735
      Zhengui 提交于
      In blk_remove_bs, all I/O should be completed before removing throttle
      timers. If there has inflight I/O, removing throttle timers here will
      cause the inflight I/O never return.
      This patch add bdrv_drained_begin before throttle_timers_detach_aio_context
      to let all I/O completed before removing throttle timers.
      
      [Moved declaration of bs as suggested by Alberto Garcia
      <berto@igalia.com>.
      --Stefan]
      Signed-off-by: NZhengui <lizhengui@huawei.com>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 1508564040-120700-1-git-send-email-lizhengui@huawei.com
      Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
      632a7735
  17. 05 9月, 2017 3 次提交
  18. 23 8月, 2017 2 次提交
  19. 15 8月, 2017 1 次提交
  20. 18 7月, 2017 2 次提交
  21. 11 7月, 2017 2 次提交
  22. 26 6月, 2017 1 次提交
  23. 16 6月, 2017 3 次提交
  24. 09 6月, 2017 1 次提交
  25. 11 5月, 2017 2 次提交