1. 21 11月, 2017 1 次提交
    • K
      block: Don't use BLK_PERM_CONSISTENT_READ for format probing · dacaa162
      Kevin Wolf 提交于
      For format probing, we don't really care whether all of the image
      content is consistent. The only thing we're looking at is the image
      header, and specifically the magic numbers that are expected to never
      change, no matter how inconsistent the guest visible disk content is.
      
      Therefore, don't request BLK_PERM_CONSISTENT_READ. This allows to use
      format probing, e.g. in the context of 'qemu-img info', even while the
      guest visible data in the image is inconsistent during a running block
      job.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NFam Zheng <famz@redhat.com>
      dacaa162
  2. 18 11月, 2017 4 次提交
    • 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
    • M
      block: Guard against NULL bs->drv · d470ad42
      Max Reitz 提交于
      We currently do not guard everywhere against a NULL bs->drv where we
      should be doing so.  Most of the places fixed here just do not care
      about that case at all.
      
      Some care implicitly, e.g. through a prior function call to
      bdrv_getlength() which would always fail for an ejected BDS.  Add an
      assert there to make it more obvious.
      
      Other places seem to care, but do so insufficiently: Freeing clusters in
      a qcow2 image is an error-free operation, but it may leave the image in
      an unusable state anyway.  Giving qcow2_free_clusters() an error code is
      not really viable, it is much easier to note that bs->drv may be NULL
      even after a successful driver call.  This concerns bdrv_co_flush(), and
      the way the check is added to bdrv_co_pdiscard() (in every iteration
      instead of only once).
      
      Finally, some places employ at least an assert(bs->drv); somewhere, that
      may be reasonable (such as in the reopen code), but in
      bdrv_has_zero_init(), it is definitely not.  Returning 0 there in case
      of an ejected BDS saves us much headache instead.
      Reported-by: NR. Nageswara Sastry <nasastry@in.ibm.com>
      Buglink: https://bugs.launchpad.net/qemu/+bug/1728660Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20171110203111.7666-4-mreitz@redhat.com
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      d470ad42
    • M
      block: qobject_is_equal() in bdrv_reopen_prepare() · 54fd1b0d
      Max Reitz 提交于
      Currently, bdrv_reopen_prepare() assumes that all BDS options are
      strings. However, this is not the case if the BDS has been created
      through the json: pseudo-protocol or blockdev-add.
      
      Note that the user-invokable reopen command is an HMP command, so you
      can only specify strings there. Therefore, specifying a non-string
      option with the "same" value as it was when originally created will now
      return an error because the values are supposedly similar (and there is
      no way for the user to circumvent this but to just not specify the
      option again -- however, this is still strictly better than just
      crashing).
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Message-id: 20171114180128.17076-5-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      54fd1b0d
    • K
      block: Fix permissions in image activation · dafe0960
      Kevin Wolf 提交于
      Inactive images generally request less permissions for their image files
      than they would if they were active (in particular, write permissions).
      Activating the image involves extending the permissions, therefore.
      
      drv->bdrv_invalidate_cache() can already require write access to the
      image file, so we have to update the permissions earlier than that.
      The current code does it only later, so we have to move up this part.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      dafe0960
  3. 17 11月, 2017 2 次提交
  4. 26 10月, 2017 1 次提交
    • P
      block: don't add 'driver' to options when referring to backing via node name · 6bff597b
      Peter Krempa 提交于
      When referring to a backing file of an image via node name
      bdrv_open_backing_file would add the 'driver' option to the option list
      filling it with the backing format driver. This breaks construction of
      the backing chain via -blockdev, as bdrv_open_inherit reports an error
      if both 'reference' and 'options' are provided.
      
      $ qemu-img create -f raw /tmp/backing.raw 64M
      $ qemu-img create -f qcow2 -F raw -b /tmp/backing.raw /tmp/test.qcow2
      $ qemu-system-x86_64 \
        -blockdev driver=file,filename=/tmp/backing.raw,node-name=backing \
        -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing
      qemu-system-x86_64: -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing: Could not open backing file: Cannot reference an existing block device with additional options or a new filename
      Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6bff597b
  5. 06 10月, 2017 5 次提交
    • K
      commit: Remove overlay_bs · bde70715
      Kevin Wolf 提交于
      We don't need to make any assumptions about the graph layout above the
      top node of the commit operation any more. Remove the use of
      bdrv_find_overlay() and related variables from the commit job code.
      
      bdrv_drop_intermediate() doesn't use the 'active' parameter any more, so
      we can just drop it.
      
      The overlay node was previously added to the block job to get a
      BLK_PERM_GRAPH_MOD. We really need to respect those permissions in
      bdrv_drop_intermediate() now, but as long as we haven't figured out yet
      how BLK_PERM_GRAPH_MOD is actually supposed to work, just leave a TODO
      comment there.
      
      With this change, it is now possible to perform another block job on an
      overlay node without conflicts. qemu-iotests 030 is changed accordingly.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      bde70715
    • K
      commit: Support multiple roots above top node · 61f09cea
      Kevin Wolf 提交于
      This changes the commit block job to support operation in a graph where
      there is more than a single active layer that references the top node.
      
      This involves inserting the commit filter node not only on the path
      between the given active node and the top node, but between the top node
      and all of its parents.
      
      On completion, bdrv_drop_intermediate() must consider all parents for
      updating the backing file link. These parents may be backing files
      themselves and as such read-only; reopen them temporarily if necessary.
      Previously this was achieved by the bdrv_reopen() calls in the commit
      block job that made overlay_bs read-write for the whole duration of the
      block job, even though write access is only needed on completion.
      
      Now that we consider all parents, overlay_bs is meaningless. It is left
      in place in this commit, but we'll remove it soon.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      61f09cea
    • K
      block: Introduce BdrvChildRole.update_filename · 6858eba0
      Kevin Wolf 提交于
      There is no good reason for bdrv_drop_intermediate() to know the active
      layer above the subchain it is operating on - even more so, because
      the assumption that there is a single active layer above it is not
      generally true.
      
      In order to prepare removal of the active parameter, use a BdrvChildRole
      callback to update the backing file string in the overlay image instead
      of directly calling bdrv_change_backing_file().
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      6858eba0
    • E
      dirty-bitmap: Avoid size query failure during truncate · 1b6cc579
      Eric Blake 提交于
      We've previously fixed several places where we failed to account
      for possible errors from bdrv_nb_sectors().  Fix another one by
      making bdrv_dirty_bitmap_truncate() take the new size from the
      caller instead of querying itself; then adjust the sole caller
      bdrv_truncate() to pass the size just determined by a successful
      resize, or to reuse the size given to the original truncate
      operation when refresh_total_sectors() was not able to confirm the
      actual size (the two sizes can potentially differ according to
      rounding constraints), thus avoiding sizing the bitmaps to -1.
      This also fixes a bug where not all failure paths in
      bdrv_truncate() would set errp.
      
      Note that bdrv_truncate() is still a bit awkward.  We may want
      to revisit it later and clean up things to better guarantee that
      a resize attempt either fails cleanly up front, or cannot fail
      after guest-visible changes have been made (if temporary changes
      are made, then they need to be cleanly rolled back).  But that
      is a task for another day; for now, the goal is the bare minimum
      fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Reviewed-by: NJohn Snow <jsnow@redhat.com>
      Reviewed-by: NFam Zheng <famz@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      1b6cc579
    • E
      block: Make bdrv_img_create() size selection easier to read · a8b42a1c
      Eric Blake 提交于
      All callers of bdrv_img_create() pass in a size, or -1 to read the
      size from the backing file.  We then set that size as the QemuOpt
      default, which means we will reuse that default rather than the
      final parameter to qemu_opt_get_size() several lines later.  But
      it is rather confusing to read subsequent checks of 'size == -1'
      when it looks (without seeing the full context) like size defaults
      to 0; it also doesn't help that a size of 0 is valid (for some
      formats).
      
      Rework the logic to make things more legible.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Snow <jsnow@redhat.com>
      Reviewed-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NFam Zheng <famz@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      a8b42a1c
  6. 26 9月, 2017 5 次提交
  7. 05 9月, 2017 2 次提交
  8. 04 9月, 2017 3 次提交
  9. 23 8月, 2017 1 次提交
  10. 08 8月, 2017 3 次提交
  11. 02 8月, 2017 2 次提交
    • M
      block: fix leaks in bdrv_open_driver() · 180ca19a
      Manos Pitsidianakis 提交于
      bdrv_open_driver() is called in two places, bdrv_new_open_driver() and
      bdrv_open_common(). In the latter, failure cleanup in is in its caller,
      bdrv_open_inherit(), which unrefs the bs->file of the failed driver open
      if it exists.
      
      Let's move the bs->file cleanup to bdrv_open_driver() to take care of
      all callers and do not set bs->drv to NULL unless the driver's open
      function failed. When bs is destroyed by removing its last reference, it
      calls bdrv_close() which checks bs->drv to perform the needed cleanups
      and also call the driver's close function. Since it cleans up options
      and opaque we must take care not leave dangling pointers.
      
      The error paths in bdrv_open_driver() are now two:
      If open fails, drv->bdrv_close() should not be called. Unref the child
      if it exists, free what we allocated and set bs->drv to NULL. Return the
      error and let callers free their stuff.
      
      If open succeeds but we fail after, return the error and let callers
      unref and delete their bs, while cleaning up their allocations.
      Signed-off-by: NManos Pitsidianakis <el13635@mail.ntua.gr>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      180ca19a
    • M
      block: fix dangling bs->explicit_options in block.c · 998cbd6a
      Manos Pitsidianakis 提交于
      In some error paths it is possible to QDECREF a freed dangling
      explicit_options, resulting in a heap overflow crash.  For example
      bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls
      bdrv_close which also unrefs it.
      Signed-off-by: NManos Pitsidianakis <el13635@mail.ntua.gr>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      998cbd6a
  12. 24 7月, 2017 1 次提交
    • K
      block: Skip implicit nodes in query-block/blockstats · d3c8c674
      Kevin Wolf 提交于
      Commits 0db832f4 and 6cdbceb1 introduced the automatic insertion of filter
      nodes above the top layer of mirror and commit block jobs. The
      assumption made there was that since libvirt doesn't do node-level
      management of the block layer yet, it shouldn't be affected by added
      nodes.
      
      This is true as far as commands issued by libvirt are concerned. It only
      uses BlockBackend names to address nodes, so any operations it performs
      still operate on the root of the tree as intended.
      
      However, the assumption breaks down when you consider query commands,
      which return data for the wrong node now. These commands also return
      information on some child nodes (bs->file and/or bs->backing), which
      libvirt does make use of, and which refer to the wrong nodes, too.
      
      One of the consequences is that oVirt gets wrong information about the
      image size and stops the VM in response as long as a mirror or commit
      job is running:
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1470634
      
      This patch fixes the problem by hiding the implicit nodes created
      automatically by the mirror and commit block jobs in the output of
      query-block and BlockBackend-based query-blockstats as long as the user
      doesn't indicate that they are aware of those nodes by providing a node
      name for them in the QMP command to start the block job.
      
      The node-based commands query-named-block-nodes and query-blockstats
      with query-nodes=true still show all nodes, including implicit ones.
      This ensures that users that are capable of node-level management can
      still access the full information; users that only know BlockBackends
      won't use these commands.
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NPeter Krempa <pkrempa@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Tested-by: NEric Blake <eblake@redhat.com>
      d3c8c674
  13. 18 7月, 2017 1 次提交
    • J
      qemu-img: Check for backing image if specified during create · 6e6e55f5
      John Snow 提交于
      Or, rather, force the open of a backing image if one was specified
      for creation. Using a similar -unsafe option as rebase, allow qemu-img
      to ignore the backing file validation if possible.
      
      It may not always be possible, as in the existing case when a filesize
      for the new image was not specified.
      
      This is accomplished by shifting around the conditionals in
      bdrv_img_create, such that a backing file is always opened unless we
      provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag
      when -u is provided to create.
      
      Sorry for the heinous looking diffstat, but it's mostly whitespace.
      
      Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786Signed-off-by: NJohn Snow <jsnow@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6e6e55f5
  14. 13 7月, 2017 1 次提交
  15. 11 7月, 2017 8 次提交