1. 25 2月, 2019 35 次提交
    • M
      iotests.py: Add node_info() · ef7afd63
      Max Reitz 提交于
      This function queries a node; since we cannot do that right now, it
      executes query-named-block-nodes and returns the matching node's object.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NJohn Snow <jsnow@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Message-id: 20190201192935.18394-8-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      ef7afd63
    • M
      iotests.py: Add filter_imgfmt() · f2ea0b20
      Max Reitz 提交于
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20190201192935.18394-7-mreitz@redhat.com
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      f2ea0b20
    • M
      block: Respect backing bs in bdrv_refresh_filename · 90993623
      Max Reitz 提交于
      Basically, bdrv_refresh_filename() should respect all children of a
      BlockDriverState. However, generally those children are driver-specific,
      so this function cannot handle the general case. On the other hand,
      there are only few drivers which use other children than @file and
      @backing (that being vmdk, quorum, and blkverify).
      
      Most block drivers only use @file and/or @backing (if they use any
      children at all). Both can be implemented directly in
      bdrv_refresh_filename.
      
      The user overriding the file's filename is already handled, however, the
      user overriding the backing file is not. If this is done, opening the
      BDS with the plain filename of its file will not be correct, so we may
      not set bs->exact_filename in that case.
      
      iotest 051 contains test cases for overriding the backing file, and so
      its output changes with this patch applied.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-6-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      90993623
    • M
      block: Add BDS.auto_backing_file · 998c2019
      Max Reitz 提交于
      If the backing file is overridden, this most probably does change the
      guest-visible data of a BDS.  Therefore, we will need to consider this
      in bdrv_refresh_filename().
      
      To see whether it has been overridden, we might want to compare
      bs->backing_file and bs->backing->bs->filename.  However,
      bs->backing_file is changed by bdrv_set_backing_hd() (which is just used
      to change the backing child at runtime, without modifying the image
      header), so bs->backing_file most of the time simply contains a copy of
      bs->backing->bs->filename anyway, so it is useless for such a
      comparison.
      
      This patch adds an auto_backing_file BDS field which contains the
      backing file path as indicated by the image header, which is not changed
      by bdrv_set_backing_hd().
      
      Because of bdrv_refresh_filename() magic, however, a BDS's filename may
      differ from what has been specified during bdrv_open().  Then, the
      comparison between bs->auto_backing_file and bs->backing->bs->filename
      may fail even though bs->backing was opened from bs->auto_backing_file.
      To mitigate this, we can copy the real BDS's filename (after the whole
      bdrv_open() and bdrv_refresh_filename() process) into
      bs->auto_backing_file, if we know the former has been opened based on
      the latter.  This is only possible if no options modifying the backing
      file's behavior have been specified, though.  To simplify things, this
      patch only copies the filename from the backing file if no options have
      been specified for it at all.
      
      Furthermore, there are cases where an overlay is created by qemu which
      already contains a BDS's filename (e.g. in blockdev-snapshot-sync).  We
      do not need to worry about updating the overlay's bs->auto_backing_file
      there, because we actually wrote a post-bdrv_refresh_filename() filename
      into the image header.
      
      So all in all, there will be false negatives where (as of a future
      patch) bdrv_refresh_filename() will assume that the backing file differs
      from what was specified in the image header, even though it really does
      not.  However, these cases should be limited to where (1) the user
      actually did override something in the backing chain (e.g. by specifying
      options for the backing file), or (2) the user executed a QMP command to
      change some node's backing file (e.g. change-backing-file or
      block-commit with @backing-file given) where the given filename does not
      happen to coincide with qemu's idea of the backing BDS's filename.
      
      Then again, (1) really is limited to -drive.  With -blockdev or
      blockdev-add, you have to adhere to the schema, so a user cannot give
      partial "unimportant" options (e.g. by just setting backing.node-name
      and leaving the rest to the image header).  Therefore, trying to fix
      this would mean trying to fix something for -drive only.
      
      To improve on (2), we would need a full infrastructure to "canonicalize"
      an arbitrary filename (+ options), so it can be compared against
      another.  That seems a bit over the top, considering that filenames
      nowadays are there mostly for the user's entertainment.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-5-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      998c2019
    • M
      block: Skip implicit nodes for filename info · bb808d5f
      Max Reitz 提交于
      bdrv_refresh_filename() should simply skip all implicit nodes.  They are
      supposed to be invisible to the user, so they should not appear in
      filename information.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-4-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      bb808d5f
    • M
      block: Use children list in bdrv_refresh_filename · e24518e3
      Max Reitz 提交于
      bdrv_refresh_filename() should invoke itself recursively on all
      children, not just on file.
      
      With that change, we can remove the manual invocations in blkverify,
      quorum, commit, mirror, and blklogwrites.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-3-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      e24518e3
    • M
      block: Use bdrv_refresh_filename() to pull · f30c66ba
      Max Reitz 提交于
      Before this patch, bdrv_refresh_filename() is used in a pushing manner:
      Whenever the BDS graph is modified, the parents of the modified edges
      are supposed to be updated (recursively upwards).  However, that is
      nonviable, considering that we want child changes not to concern
      parents.
      
      Also, in the long run we want a pull model anyway: Here, we would have a
      bdrv_filename() function which returns a BDS's filename, freshly
      constructed.
      
      This patch is an intermediate step.  It adds bdrv_refresh_filename()
      calls before every place a BDS.filename value is used.  The only
      exceptions are protocol drivers that use their own filename, which
      clearly would not profit from refreshing that filename before.
      
      Also, bdrv_get_encrypted_filename() is removed along the way (as a user
      of BDS.filename), since it is completely unused.
      
      In turn, all of the calls to bdrv_refresh_filename() before this patch
      are removed, because we no longer have to call this function on graph
      changes.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20190201192935.18394-2-mreitz@redhat.com
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      f30c66ba
    • T
      block/nvme: Remove QEMU_PACKED from naturally aligned NVMeRegs struct · 83c68e14
      Thomas Huth 提交于
      The QEMU_PACKED is causing a compiler warning/error with GCC 9:
      
        CC      block/nvme.o
      block/nvme.c: In function ‘nvme_create_queue_pair’:
      block/nvme.c:209:22: error: taking address of packed member of
       ‘struct <anonymous>’ may result in an unaligned pointer value
       [-Werror=address-of-packed-member]
        209 |     q->sq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale];
      
      All members of the struct are naturally aligned, so there should
      not be the need for QEMU_PACKED here, and the following QEMU_BUILD_BUG_ON
      also ensures that there is no padding. Thus simply remove the QEMU_PACKED
      here.
      
      Buglink: https://bugs.launchpad.net/qemu/+bug/1817525Reported-by: NSatheesh Rajendran <sathnaga@linux.vnet.ibm.com>
      Signed-off-by: NThomas Huth <thuth@redhat.com>
      Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      83c68e14
    • A
      qcow2: Assert that L2 table offsets fit in the L1 table · c1c43990
      Alberto Garcia 提交于
      L1 table entries have a field to store the offset of an L2 table.
      The rest of the bits of the entry are currently reserved except from
      bit 63, which stores the COPIED flag.
      
      The offset is always taken from the entry using L1E_OFFSET_MASK to
      ensure that we only use the bits that belong to that field.
      
      While that mask is used every time we read from the L1 table, it is
      never used when we write to it. Due to the limits set elsewhere in the
      code QEMU can never produce L2 table offsets that don't fit in that
      field so any such offset when allocating an L2 table would indicate a
      bug in QEMU.
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      c1c43990
    • V
      tests: add test-bdrv-graph-mod · 2dbfadf6
      Vladimir Sementsov-Ogievskiy 提交于
      Add two tests of node graph modification.
      Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2dbfadf6
    • V
      block: fix bdrv_check_perm for non-tree subgraph · f962e961
      Vladimir Sementsov-Ogievskiy 提交于
      bdrv_check_perm in it's recursion checks each node in context of new
      permissions for one parent, because of nature of DFS. It works well,
      while children subgraph of top-most updated node is a tree, i.e. it
      doesn't have any kind of loops. But if we have a loop (not oriented,
      of course), i.e. we have two different ways from top-node to some
      child-node, then bdrv_check_perm will do wrong thing:
      
        top
        | \
        |  |
        v  v
        A  B
        |  |
        v  v
        node
      
      It will once check new permissions of node in context of new A
      permissions and old B permissions and once visa-versa. It's a wrong way
      and may lead to corruption of permission system. We may start with
      no-permissions and all-shared for both A->node and B->node relations
      and finish up with non shared write permission for both ways.
      
      The following commit will add a test, which shows this bug.
      
      To fix this situation, let's really set BdrvChild permissions during
      bdrv_check_perm procedure. And we are happy here, as check-perm is
      already written in transaction manner, so we just need to restore
      backed-up permissions in _abort.
      Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      f962e961
    • V
      block: improve should_update_child · 2f30b7c3
      Vladimir Sementsov-Ogievskiy 提交于
      As it already said in the comment, we don't want to create loops in
      parent->child relations. So, when we try to append @to to @c, we should
      check that @c is not in @to children subtree, and we should check it
      recursively, not only the first level. The patch provides BFS-based
      search, to check the relations.
      
      This is needed for further fleecing-hook filter usage: we need to
      append it to source, when the hook is already a parent of target, and
      source may be in a backing chain of target (fleecing-scheme). So, on
      appending, the hook should not became a child (direct or through
      children subtree) of the target.
      Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2f30b7c3
    • K
      aio-posix: Assert that aio_poll() is always called in home thread · 0dc165c1
      Kevin Wolf 提交于
      aio_poll() has an existing assertion that the function is only called
      from the AioContext's home thread if blocking is allowed.
      
      This is not enough, some handlers make assumptions about the thread they
      run in. Extend the assertion to non-blocking calls, too.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      0dc165c1
    • K
      block: Use normal drain for bdrv_set_aio_context() · d70d5954
      Kevin Wolf 提交于
      Now that bdrv_set_aio_context() works inside drained sections, it can
      also use the real drain function instead of open coding something
      similar.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      d70d5954
    • K
      test-bdrv-drain: AioContext switch in drained section · 247d2737
      Kevin Wolf 提交于
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      247d2737
    • K
      block: Fix AioContext switch for drained node · e64f25f3
      Kevin Wolf 提交于
      When a drained node changes its AioContext, we need to move its
      aio_disable_external() to the new context, too.
      
      Without this fix, drain_end will try to reenable the new context, which
      has never been disabled, so an assertion failure is triggered.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      e64f25f3
    • K
      block: Don't poll in bdrv_set_aio_context() · 6c75d761
      Kevin Wolf 提交于
      The explicit aio_poll() call in bdrv_set_aio_context() was added in
      commit c2b6428d as a workaround for bdrv_drain() failing to achieve
      to actually quiesce everything (specifically the NBD client code to
      switch AioContext).
      
      Now that the NBD client has been fixed to complete this operation during
      bdrv_drain(), we don't need the workaround any more.
      
      It was wrong anyway: aio_poll() must always be run in the home thread of
      the AioContext.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      6c75d761
    • K
      nbd: Increase bs->in_flight during AioContext switch · 28e0b2d2
      Kevin Wolf 提交于
      bdrv_drain() must not leave connection_co scheduled, so bs->in_flight
      needs to be increased while the coroutine is waiting to be scheduled
      in the new AioContext after nbd_client_attach_aio_context().
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      28e0b2d2
    • K
      nbd: Use low-level QIOChannel API in nbd_read_eof() · d3bd5b90
      Kevin Wolf 提交于
      Instead of using the convenience wrapper qio_channel_read_all_eof(), use
      the lower level QIOChannel API. This means duplicating some code, but
      we'll need this because this coroutine yield is special: We want it to
      be interruptible so that nbd_client_attach_aio_context() can correctly
      reenter the coroutine.
      
      This moves the bdrv_dec/inc_in_flight() pair into nbd_read_eof(), so
      that connection_co will always sit in this exact qio_channel_yield()
      call when bdrv_drain() returns.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      d3bd5b90
    • K
      nbd: Move nbd_read_eof() to nbd/client.c · a7b78fc9
      Kevin Wolf 提交于
      The only caller of nbd_read_eof() is nbd_receive_reply(), so it doesn't
      have to live in the header file, but can move next to its caller.
      
      Also add the missing coroutine_fn to the function and its caller.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      a7b78fc9
    • K
      io: Remove redundant read/write_coroutine assignments · 2a239e6e
      Kevin Wolf 提交于
      qio_channel_yield() now updates ioc->read_write/coroutine and calls
      qio_channel_set_aio_fd_handlers(), so the code in the handlers has
      become redundant and can be removed.
      
      This does not make a difference in intermediate states because
      aio_co_wake() really enters the coroutine immediately here: These
      handlers are never run in coroutine context, and we're in the right
      AioContext because qio_channel_attach_aio_context() asserts that the
      handlers are inactive.
      
      To make these conditions more obvious, assert the right AioContext.
      Suggested-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2a239e6e
    • K
      io: Make qio_channel_yield() interruptible · 6886ceaf
      Kevin Wolf 提交于
      Similar to how qemu_co_sleep_ns() allows preemption from an external
      coroutine entry, allow reentering qio_channel_yield() early.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6886ceaf
    • K
      nbd: Restrict connection_co reentrance · 5ad81b49
      Kevin Wolf 提交于
      nbd_client_attach_aio_context() schedules connection_co in the new
      AioContext and this way reenters it in any arbitrary place that has
      yielded. We can restrict this a bit to the function call where the
      coroutine actually sits waiting when it's idle.
      
      This doesn't solve any bug yet, but it shows where in the code we need
      to support this random reentrance and where we don't have to care.
      
      Add FIXME comments for the existing bugs that the rest of this series
      will fix.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      5ad81b49
    • K
      virtio-blk: Increase in_flight for request restart BH · 680f2002
      Kevin Wolf 提交于
      virtio_blk_dma_restart_bh() submits new requests, so in order to make
      sure that these requests are not started inside a drained section of the
      attached BlockBackend, we need to make sure that draining the
      BlockBackend waits for the BH to be executed.
      
      This BH is still questionable because its scheduled in the main thread
      instead of the configured iothread. Leave a FIXME comment for this.
      
      But with this fix, enabling the data plane at least waits for these
      requests (in bdrv_set_aio_context()) instead of changing the AioContext
      under their feet and making them run in the wrong thread, causing
      crashes and failures (e.g. due to missing locking).
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      680f2002
    • K
      block-backend: Make blk_inc/dec_in_flight public · c90e2a9c
      Kevin Wolf 提交于
      For some users of BlockBackends, just increasing the in_flight counter
      is easier than implementing separate handlers in BlockDevOps. Make the
      helper functions for this public.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      c90e2a9c
    • D
      qemu-img: fix error reporting for -object · 334c43e2
      Daniel P. Berrangé 提交于
      Error reporting for user_creatable_add_opts_foreach was changed so that
      it no longer called 'error_report_err' in:
      
        commit 7e1e0c11
        Author: Markus Armbruster <armbru@redhat.com>
        Date:   Wed Oct 17 10:26:43 2018 +0200
      
          qom: Clean up error reporting in user_creatable_add_opts_foreach()
      
      Some callers were updated to pass in "&error_fatal" but all the ones in
      qemu-img were left passing NULL. As a result all errors went to
      /dev/null instead of being reported to the user.
      Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NStefano Garzarella <sgarzare@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      334c43e2
    • A
      commit: Replace commit_top_bs on failure after deleting the block job · 2468eed3
      Alberto Garcia 提交于
      If there's an error in commit_start() then the block job must be
      deleted before replacing commit_top_bs, otherwise it will fail because
      of lack of permissions. This happens since the permission system was
      introduced in 8dfba279.
      
      Fortunately this bug doesn't seem to be possible to reproduce at the
      moment without changing the code.
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2468eed3
    • D
      block: don't set the same context · 57830a49
      Denis Plotnikov 提交于
      Adds a fast path on aio context setting preventing
      unnecessary context setting routine.
      Also, it prevents issues with cyclic walk of child
      bds-es appeared because of registering aio walking
      notifiers:
      
      Call stack:
      
      0  __GI_raise
      1  __GI_abort
      2  __assert_fail_base
      3  __GI___assert_fail
      4  bdrv_detach_aio_context (bs=0x55f54d65c000)      <<<
      5  bdrv_detach_aio_context (bs=0x55f54fc8a800)
      6  bdrv_set_aio_context (bs=0x55f54fc8a800, ...)
      7  block_job_attached_aio_context
      8  bdrv_attach_aio_context (bs=0x55f54d65c000, ...) <<<
      9  bdrv_set_aio_context (bs=0x55f54d65c000)
      10 blk_set_aio_context
      11 virtio_blk_data_plane_stop
      12 virtio_bus_stop_ioeventfd
      13 virtio_vmstate_change
      14 vm_state_notify (running=0, state=RUN_STATE_SHUTDOWN)
      15 do_vm_stop (state=RUN_STATE_SHUTDOWN, send_stop=true)
      16 vm_stop (state=RUN_STATE_SHUTDOWN)
      17 main_loop_should_exit
      18 main_loop
      19 main
      
      This can happen because of "new" context attachment to VM disk bds.
      When attaching a new context the corresponding aio context handler is
      called for each of aio_notifiers registered on the VM disk bds context.
      Among those handlers, there is the block_job_attached_aio_context handler
      which sets a new aio context for the block job bds. When doing so,
      the old context is detached from all the block job bds children and one of
      them is the VM disk bds, serving as backing store for the blockjob bds,
      although the VM disk bds is actually the initializer of that process.
      Since the VM disk bds is protected with walking_aio_notifiers flag
      from double processing in recursive calls, the assert fires.
      Signed-off-by: NDenis Plotnikov <dplotnikov@virtuozzo.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      57830a49
    • D
      qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call · 161e612d
      Daniel Henrique Barboza 提交于
      In qcow2_snapshot_create there is the following code block:
      
          /* Generate an ID */
          find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str));
      
          /* Check that the ID is unique */
          if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
              return -EEXIST;
          }
      
      find_new_snapshot_id cycles through all snapshots, getting the id_str
      as an unsigned long int, calculating the max id_max value of all the
      existing id_strs and writing in the id_str pointer id_max + 1:
      
          for(i = 0; i < s->nb_snapshots; i++) {
              sn = s->snapshots + i;
              id = strtoul(sn->id_str, NULL, 10);
              if (id > id_max)
                  id_max = id;
          }
          snprintf(id_str, id_str_size, "%lu", id_max + 1);
      
      Here, sn_info->id_str will have the unique value id_max + 1. Right
      after that, find_snapshot_by_id_and_name is called with
      id = sn_info->id_str and name = NULL. This will cause the function
      to execute the following:
      
          } else if (id) {
              for (i = 0; i < s->nb_snapshots; i++) {
                  if (!strcmp(s->snapshots[i].id_str, id)) {
                      return i;
                  }
              }
          }
      
      In short, we're searching the existing snapshots to see if sn_info->id_str
      matches any existing id, right after we set in the previous line a
      sn_info->id_str value that is already unique.
      
      The first code block goes way back to commit 585f8587, a 2006 commit from
      Fabrice Bellard that simply says "new qcow2 disk image format". No more
      info is provided about this logic in any subsequent commits that moved
      this code block around.
      
      I can't say about the original design, but the current logic is redundant.
      bdrv_snapshot_create is called in aio_context lock, forbidding any
      concurrent call to accidentally create a new snapshot between
      the find_new_snapshot_id and find_snapshot_by_id_and_name calls. What
      we're ending up doing is to cycle through the snapshots two times
      for no viable reason.
      
      This patch eliminates the redundancy by removing the 'id is unique'
      check that calls find_snapshot_by_id_and_name.
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      161e612d
    • D
      block/snapshot: remove bdrv_snapshot_delete_by_id_or_name · 8c04093c
      Daniel Henrique Barboza 提交于
      After the previous patch, the only instance of this function left
      is inside qemu-img.c.
      
      qemu-img is using it inside the 'img_snapshot' function to delete
      snapshots in the SNAPSHOT_DELETE case, based on a "snapshot_name"
      string that refers to the tag, not ID, of the QEMUSnapshotInfo struct.
      This can be verified by checking the SNAPSHOT_CREATE case that
      comes shortly before SNAPSHOT_DELETE. In that case, the same
      "snapshot_name" variable is being strcpy to the 'name' field
      of the QEMUSnapshotInfo struct sn:
      
      pstrcpy(sn.name, sizeof(sn.name), snapshot_name);
      
      Based on that, it is unlikely that "snapshot_name" might contain
      an "id" in SNAPSHOT_DELETE.
      
      This patch changes SNAPSHOT_DELETE to use snapshot_find() and
      snapshot_delete() instead of bdrv_snapshot_delete_by_id_or_name.
      After that, there is no instances left of bdrv_snapshot_delete_by_id_or_name
      in the code, so it is safe to remove it entirely.
      Suggested-by: NMurilo Opsfelder Araujo <muriloo@linux.ibm.com>
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      8c04093c
    • D
      block/snapshot.c: eliminate use of ID input in snapshot operations · 6ca08045
      Daniel Henrique Barboza 提交于
      At this moment, QEMU attempts to create/load/delete snapshots
      by using either an ID (id_str) or a name. The problem is that the code
      isn't consistent of whether the entered argument is an ID or a name,
      causing unexpected behaviors.
      
      For example, when creating snapshots via savevm <arg>, what happens is that
      "arg" is treated as both name and id_str. In a guest without snapshots, create
      a single snapshot via savevm:
      
      (qemu) savevm 0
      (qemu) info snapshots
      List of snapshots present on all disks:
      ID        TAG                 VM SIZE                DATE       VM CLOCK
      --        0                      741M 2018-07-31 13:39:56   00:41:25.313
      
      A snapshot with name "0" is created. ID is hidden from the user, but the
      ID is a non-zero integer that starts at "1". Thus, this snapshot has
      id_str=1, TAG="0". Creating a second snapshot with arg = 1, the first one
      is deleted:
      
      (qemu) savevm 1
      (qemu) info snapshots
      List of snapshots present on all disks:
      ID        TAG                 VM SIZE                DATE       VM CLOCK
      --        1                      741M 2018-07-31 13:42:14   00:41:55.252
      
      What happened?
      
      - when creating the second snapshot, a verification is done inside
      bdrv_all_delete_snapshot to delete any existing snapshots that matches an
      string argument. Here, the code calls bdrv_all_delete_snapshot("1", ...);
      
      - bdrv_all_delete_snapshot calls bdrv_snapshot_find(..., "1") for each
      BlockDriverState of the guest. And this is where things goes tilting:
      bdrv_snapshot_find does a search by both id_str and name. It finds
      out that there is a snapshot that has id_str = 1, stores a reference
      to the snapshot in the sn_info pointer and then returns match found;
      
      - since a match was found, a call to bdrv_snapshot_delete_by_id_or_name() is
      made. This function ignores the pointer written by bdrv_snapshot_find. Instead,
      it deletes the snapshot using bdrv_snapshot_delete() calling it first with
      id_str = 1. If it fails to delete, then it calls it again with name = 1.
      
      - after all that, QEMU creates the new snapshot, that has id_str = 1 and
      name = 1. The user is left wondering that happened with the first snapshot
      created. Similar bugs can be triggered when using loadvm and delvm.
      
      Before contemplating discarding the use of ID input in these operations,
      I've searched the code of what would be the implications. My findings
      are:
      
      - the RBD and Sheepdog drivers don't care. Both uses the 'name' field as
      key in their logic, making id_str = name when appropriate.
      replay-snapshot.c does not make any special use of id_str;
      
      - qcow2 uses id_str as an unique identifier but it is automatically
      calculated, not being influenced by user input. Other than that, there are
      no distinguish operations made only with id_str;
      
      - in blockdev.c, the delete operation uses a match of both id_str AND
      name. Given that id_str is either a copy of 'name' or auto-generated,
      we're fine here.
      
      This gives motivation to not consider ID as a valid user input in HMP
      commands - sticking with 'name' input only is more consistent. To
      accomplish that, the following changes were made in this patch:
      
      - bdrv_snapshot_find() does not match for id_str anymore, only 'name'. The
      function is called in save_snapshot(), load_snapshot(), bdrv_all_delete_snapshot()
      and bdrv_all_find_snapshot(). This change makes the search function more
      predictable and does not change the behavior of any underlying code that uses
      these affected functions, which are related to HMP (which is fine) and the
      main loop inside vl.c (which doesn't care about it anyways);
      
      - bdrv_all_delete_snapshot() does not call bdrv_snapshot_delete_by_id_or_name
      anymore. Instead, it uses the pointer returned by bdrv_snapshot_find to
      erase the snapshot with the exact match of id_str an name. This function
      is called in save_snapshot and hmp_delvm, thus this change  produces the
      intended effect;
      
      - documentation changes to reflect the new behavior. I consider this to
      be an API fix instead of an API change - the user was already creating
      snapshots using 'name', but now he/she will also enjoy a consistent
      behavior.
      
      Ideally we would get rid of the id_str field entirely, but this would have
      repercussions on existing snapshots. Another day perhaps.
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Acked-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6ca08045
    • J
      MAINTAINERS: Remove myself as block maintainer · 5f5246b6
      Jeff Cody 提交于
      I'll not be involved in day-to-day qemu development.  Remove myself as
      maintainer from the remainder of the network block drivers, and revert
      them to the general block layer maintainership.
      
      Move 'sheepdog' to the 'Odd Fixes' support level.
      
      For VHDX, added my personal email address as a maintainer, as I can
      answer questions or send the occassional bug fix.  Leaving it as
      'Supported', instead of 'Odd Fixes', because I think the rest of the
      block layer maintainers and developers will upkeep it as well, if
      needed.
      Signed-off-by: NJeff Cody <jcody@redhat.com>
      Acked-by: NMax Reitz <mreitz@redhat.com>
      Message-Id: <63e205cb84c8f0a10c1bc6d5d6856d72ceb56e41.1537984851.git.jcody@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      5f5246b6
    • J
      MAINTAINERS: Replace myself with John Snow for block jobs · 03283d64
      Jeff Cody 提交于
      I'll not be involved with day-to-day qemu development, and John
      Snow is a block jobs wizard.  Have him take over block job
      maintainership duties.
      Signed-off-by: NJeff Cody <jcody@redhat.com>
      Acked-by: NJohn Snow <jsnow@redhat.com>
      Message-Id: <d56d7c6592e7d68aa72764e9616878394bffbc14.1537984851.git.jcody@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      03283d64
    • P
      Merge remote-tracking branch 'remotes/kraxel/tags/vga-20190222-pull-request' into staging · 59a568b5
      Peter Maydell 提交于
      vga: bugfixes and edid support for virtio-vga
      
      # gpg: Signature made Fri 22 Feb 2019 08:24:25 GMT
      # gpg:                using RSA key 4CB6D8EED3E87138
      # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
      # gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
      # gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
      # Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138
      
      * remotes/kraxel/tags/vga-20190222-pull-request:
        display/virtio: add edid support.
        virtio-gpu: remove useless 'waiting' field
        virtio-gpu: block both 2d and 3d rendering
        virtio-gpu: remove unused config_size
        virtio-gpu: remove unused qdev
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      59a568b5
    • P
      Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190222-pull-request' into staging · 8a4c08b1
      Peter Maydell 提交于
      ui: add support for -display spice-app
      ui: gtk+sdl bugfixes.
      
      # gpg: Signature made Fri 22 Feb 2019 07:53:13 GMT
      # gpg:                using RSA key 4CB6D8EED3E87138
      # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
      # gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
      # gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
      # Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138
      
      * remotes/kraxel/tags/ui-20190222-pull-request:
        display: add -display spice-app launching a Spice client
        spice: use a default name for the server
        qapi: document DisplayType enum
        build-sys: add gio-2.0 check
        char: register spice ports after spice started
        char: move SpiceChardev and open_spice_port() to spice.h header
        spice: do not stop spice if VM is paused
        spice: merge options lists
        spice: avoid spice runtime assert
        char/spice: discard write() if backend is disconnected
        char/spice: trigger HUP event
        ui/gtk: Fix the license information
        sdl2: drop qemu_input_event_send_key_qcode call
        spice: set device address and device display ID in QXL interface
        kbd-state: don't block auto-repeat events
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      8a4c08b1
  2. 22 2月, 2019 5 次提交
    • P
      Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190221.0' into staging · 8eb29f1b
      Peter Maydell 提交于
      VFIO updates 2019-02-21
      
       - Workaround kernel overflow bug in vfio type1 DMA unmap
         (Alex Williamson)
      
       - Refactor vfio container initialization (Eric Auger)
      
      # gpg: Signature made Fri 22 Feb 2019 05:21:07 GMT
      # gpg:                using RSA key 239B9B6E3BB08B22
      # gpg: Good signature from "Alex Williamson <alex.williamson@redhat.com>" [full]
      # gpg:                 aka "Alex Williamson <alex@shazbot.org>" [full]
      # gpg:                 aka "Alex Williamson <alwillia@redhat.com>" [full]
      # gpg:                 aka "Alex Williamson <alex.l.williamson@gmail.com>" [full]
      # Primary key fingerprint: 42F6 C04E 540B D1A9 9E7B  8A90 239B 9B6E 3BB0 8B22
      
      * remotes/awilliam/tags/vfio-updates-20190221.0:
        hw/vfio/common: Refactor container initialization
        vfio/common: Work around kernel overflow bug in DMA unmap
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      8eb29f1b
    • P
      Merge remote-tracking branch 'remotes/rth/tags/pull-hppa-20190221' into staging · a05838cb
      Peter Maydell 提交于
      Fix dino pci config access.
      
      # gpg: Signature made Thu 21 Feb 2019 19:03:26 GMT
      # gpg:                using RSA key 64DF38E8AF7E215F
      # gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>" [full]
      # Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A  05C0 64DF 38E8 AF7E 215F
      
      * remotes/rth/tags/pull-hppa-20190221:
        hw/hppa/dino: mask out lower 2 bits of PCI config addr
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      a05838cb
    • P
      Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190221' into staging · 7817ea16
      Peter Maydell 提交于
      Allow const void * as argument to helpers.
      Remove obsolete TODO file.
      
      # gpg: Signature made Thu 21 Feb 2019 18:59:11 GMT
      # gpg:                using RSA key 64DF38E8AF7E215F
      # gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>" [full]
      # Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A  05C0 64DF 38E8 AF7E 215F
      
      * remotes/rth/tags/pull-tcg-20190221:
        include/exec/helper-head.h: support "const void *" in helper calls
        tcg: Remove TODO file
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      7817ea16
    • P
      Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-feb-21-2019-v2' into staging · 98e139bc
      Peter Maydell 提交于
      MIPS queue for February 21st, 2019, v2
      
      # gpg: Signature made Thu 21 Feb 2019 18:37:04 GMT
      # gpg:                using RSA key D4972A8967F75A65
      # gpg: Good signature from "Aleksandar Markovic <amarkovic@wavecomp.com>" [unknown]
      # gpg: WARNING: This key is not certified with a trusted signature!
      # gpg:          There is no indication that the signature belongs to the owner.
      # Primary key fingerprint: 8526 FBF1 5DA3 811F 4A01  DD75 D497 2A89 67F7 5A65
      
      * remotes/amarkovic/tags/mips-queue-feb-21-2019-v2:
        target/mips: fulong2e: Dynamically generate SPD EEPROM data
        target/mips: fulong2e: Fix bios flash size
        hw/pci-host/bonito.c: Add PCI mem region mapped at the correct address
        target/mips: implement QMP query-cpu-definitions command
        tests/tcg: target/mips: Add wrappers for MSA integer compare instructions
        tests/tcg: target/mips: Change directory name 'bit-counting' to 'bit-count'
        tests/tcg: target/mips: Correct path to headers in some test source files
        hw/misc: mips_itu: Fix 32/64 bit issue in a line involving shift operator
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      98e139bc
    • M
      display: add -display spice-app launching a Spice client · d8aec9d9
      Marc-André Lureau 提交于
      Add a new display backend that will configure Spice to allow a remote
      client to control QEMU in a similar fashion as other QEMU display
      backend/UI like GTK.
      
      For this to work, it will set up Spice server with a unix socket, and
      register a VC chardev that will be exposed as Spice ports. A QMP
      monitor is also exposed as a Spice port, this allows the remote client
      fuller qemu control and state handling.
      
      - doesn't handle VC set_echo() - this doesn't seem a strong
        requirement, very few front-end use it
      - spice options can be tweaked with other -spice arguments
      - Windows support shouldn't be hard to do, but will probably use a TCP
        port instead
      - we may want to watch the child process to quit automatically if it
        crashed
      Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Tested-by: NVictor Toso <victortoso@redhat.com>
      Message-id: 20190221110703.5775-12-marcandre.lureau@redhat.com
      
      [ kraxel: squash incremental fix ]
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      d8aec9d9