Skip to content
Snippets Groups Projects
  1. Jul 10, 2020
    • Markus Armbruster's avatar
      error: Eliminate error_propagate() with Coccinelle, part 2 · af175e85
      Markus Armbruster authored
      
      When all we do with an Error we receive into a local variable is
      propagating to somewhere else, we can just as well receive it there
      right away.  The previous commit did that with a Coccinelle script I
      consider fairly trustworthy.  This commit uses the same script with
      the matching of return taken out, i.e. we convert
      
          if (!foo(..., &err)) {
              ...
              error_propagate(errp, err);
              ...
          }
      
      to
      
          if (!foo(..., errp)) {
              ...
              ...
          }
      
      This is unsound: @err could still be read between afterwards.  I don't
      know how to express "no read of @err without an intervening write" in
      Coccinelle.  Instead, I manually double-checked for uses of @err.
      
      Suboptimal line breaks tweaked manually.  qdev_realize() simplified
      further to placate scripts/checkpatch.pl.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200707160613.848843-36-armbru@redhat.com>
      af175e85
    • Markus Armbruster's avatar
      qemu-option: Use returned bool to check for failure · 235e59cf
      Markus Armbruster authored
      
      The previous commit enables conversion of
      
          foo(..., &err);
          if (err) {
              ...
          }
      
      to
      
          if (!foo(..., &err)) {
              ...
          }
      
      for QemuOpts functions that now return true / false on success /
      error.  Coccinelle script:
      
          @@
          identifier fun = {
              opts_do_parse, parse_option_bool, parse_option_number,
              parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
              qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
              qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
              qemu_opts_validate
          };
          expression list args, args2;
          typedef Error;
          Error *err;
          @@
          -    fun(args, &err, args2);
          -    if (err)
          +    if (!fun(args, &err, args2))
               {
                   ...
               }
      
      A few line breaks tidied up manually.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Message-Id: <20200707160613.848843-15-armbru@redhat.com>
      [Conflict with commit 0b6786a9 "block/amend: refactor qcow2 amend
      options" resolved by rerunning Coccinelle on master's version]
      235e59cf
  2. May 28, 2020
    • Eric Blake's avatar
      qcow2: Expose bitmaps' size during measure · 5d72c68b
      Eric Blake authored
      It's useful to know how much space can be occupied by qcow2 persistent
      bitmaps, even though such metadata is unrelated to the guest-visible
      data.  Report this value as an additional QMP field, present when
      measuring an existing image and output format that both support
      bitmaps.  Update iotest 178 and 190 to updated output, as well as new
      coverage in 190 demonstrating non-zero values made possible with the
      recently-added qemu-img bitmap command (see 3b51ab4b).
      
      The new 'bitmaps size:' field is displayed automatically as part of
      'qemu-img measure' any time it is present in QMP (that is, any time
      both the source image being measured and destination format support
      bitmaps, even if the measurement is 0 because there are no bitmaps
      present).  If the field is absent, it means that no bitmaps can be
      copied (source, destination, or both lack bitmaps, including when
      measuring based on size rather than on a source image).  This behavior
      is compatible with an upcoming patch adding 'qemu-img convert
      --bitmaps': that command will fail in the same situations where this
      patch omits the field.
      
      The addition of a new field demonstrates why we should always
      zero-initialize qapi C structs; while the qcow2 driver still fully
      populates all fields, the raw and crypto drivers had to be tweaked to
      avoid uninitialized data.
      
      Consideration was also given towards having a 'qemu-img measure
      --bitmaps' which errors out when bitmaps are not possible, and
      otherwise sums the bitmaps into the existing allocation totals rather
      than displaying as a separate field, as a potential convenience
      factor.  But this was ultimately decided to be more complexity than
      necessary when the QMP interface was sufficient enough with bitmaps
      remaining a separate field.
      
      See also: https://bugzilla.redhat.com/1779904
      
      
      
      Reported-by: default avatarNir Soffer <nsoffer@redhat.com>
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200521192137.1120211-3-eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      5d72c68b
  3. May 18, 2020
  4. May 08, 2020
    • Eric Blake's avatar
      block: Drop unused .bdrv_has_zero_init_truncate · 47e0b38a
      Eric Blake authored
      
      Now that there are no clients of bdrv_has_zero_init_truncate, none of
      the drivers need to worry about providing it.
      
      What's more, this eliminates a source of some confusion: a literal
      reading of the documentation as written in ceaca56f and implemented in
      commit 1dcaf527 claims that a driver which returns 0 for
      bdrv_has_zero_init_truncate() must not return 1 for
      bdrv_has_zero_init(); this condition was violated for parallels, qcow,
      and sometimes for vdi, although in practice it did not matter since
      those drivers also lacked .bdrv_co_truncate.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200428202905.770727-10-eblake@redhat.com>
      Acked-by: default avatarRichard W.M. Jones <rjones@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      47e0b38a
  5. Apr 30, 2020
  6. Mar 26, 2020
  7. Oct 28, 2019
    • Hanna Reitz's avatar
      block: Let format drivers pass @exact · e61a28a9
      Hanna Reitz authored
      
      When truncating a format node, the @exact parameter is generally handled
      simply by virtue of the format storing the new size in the image
      metadata.  Such formats do not need to pass on the parameter to their
      file nodes.
      
      There are exceptions, though:
      - raw and crypto cannot store the image size, and thus must pass on
        @exact.
      
      - When using qcow2 with an external data file, it just makes sense to
        keep its size in sync with the qcow2 virtual disk (because the
        external data file is the virtual disk).  Therefore, we should pass
        @exact when truncating it.
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Message-id: 20190918095144.955-7-mreitz@redhat.com
      Reviewed-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      e61a28a9
    • Hanna Reitz's avatar
      block: Add @exact parameter to bdrv_co_truncate() · c80d8b06
      Hanna Reitz authored
      
      We have two drivers (iscsi and file-posix) that (in some cases) return
      success from their .bdrv_co_truncate() implementation if the block
      device is larger than the requested offset, but cannot be shrunk.  Some
      callers do not want that behavior, so this patch adds a new parameter
      that they can use to turn off that behavior.
      
      This patch just adds the parameter and lets the block/io.c and
      block/block-backend.c functions pass it around.  All other callers
      always pass false and none of the implementations evaluate it, so that
      this patch does not change existing behavior.  Future patches take care
      of that.
      
      Suggested-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Message-id: 20190918095144.955-5-mreitz@redhat.com
      Reviewed-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      c80d8b06
  8. Aug 19, 2019
  9. Jun 12, 2019
  10. Mar 26, 2019
  11. Mar 12, 2019
    • Alberto Garcia's avatar
      block: Add a 'mutable_opts' field to BlockDriver · 8a2ce0bc
      Alberto Garcia authored
      
      If we reopen a BlockDriverState and there is an option that is present
      in bs->options but missing from the new set of options then we have to
      return an error unless the driver is able to reset it to its default
      value.
      
      This patch adds a new 'mutable_opts' field to BlockDriver. This is
      a list of runtime options that can be modified during reopen. If an
      option in this list is unspecified on reopen then it must be reset (or
      return an error).
      
      Signed-off-by: default avatarAlberto Garcia <berto@igalia.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      8a2ce0bc
  12. Feb 25, 2019
    • Hanna Reitz's avatar
      block: Add strong_runtime_opts to BlockDriver · 2654267c
      Hanna Reitz authored
      
      This new field can be set by block drivers to list the runtime options
      they accept that may influence the contents of the respective BDS. As of
      a follow-up patch, this list will be used by the common
      bdrv_refresh_filename() implementation to decide which options to put
      into BDS.full_open_options (and consequently whether a JSON filename has
      to be created), thus freeing the drivers of having to implement that
      logic themselves.
      
      Additionally, this patch adds the field to all of the block drivers that
      need it and sets it accordingly.
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-22-mreitz@redhat.com
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      2654267c
    • Hanna Reitz's avatar
      block: Use bdrv_refresh_filename() to pull · f30c66ba
      Hanna Reitz authored
      
      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: default avatarMax Reitz <mreitz@redhat.com>
      Message-id: 20190201192935.18394-2-mreitz@redhat.com
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      f30c66ba
  13. Aug 15, 2018
  14. Jul 10, 2018
  15. Jul 09, 2018
  16. Jun 29, 2018
    • Kevin Wolf's avatar
      block: Convert .bdrv_truncate callback to coroutine_fn · 061ca8a3
      Kevin Wolf authored
      
      bdrv_truncate() is an operation that can block (even for a quite long
      time, depending on the PreallocMode) in I/O paths that shouldn't block.
      Convert it to a coroutine_fn so that we have the infrastructure for
      drivers to make their .bdrv_co_truncate implementation asynchronous.
      
      This change could potentially introduce new race conditions because
      bdrv_truncate() isn't necessarily executed atomically any more. Whether
      this is a problem needs to be evaluated for each block driver that
      supports truncate:
      
      * file-posix/win32, gluster, iscsi, nfs, rbd, ssh, sheepdog: The
        protocol drivers are trivially safe because they don't actually yield
        yet, so there is no change in behaviour.
      
      * copy-on-read, crypto, raw-format: Essentially just filter drivers that
        pass the request to a child node, no problem.
      
      * qcow2: The implementation modifies metadata, so it needs to hold
        s->lock to be safe with concurrent I/O requests. In order to avoid
        double locking, this requires pulling the locking out into
        preallocate_co() and using qcow2_write_caches() instead of
        bdrv_flush().
      
      * qed: Does a single header update, this is fine without locking.
      
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      061ca8a3
  17. Jun 01, 2018
  18. May 15, 2018
    • Hanna Reitz's avatar
      block: Support BDRV_REQ_WRITE_UNCHANGED in filters · 228345bf
      Hanna Reitz authored
      
      Update the rest of the filter drivers to support
      BDRV_REQ_WRITE_UNCHANGED.  They already forward write request flags to
      their children, so we just have to announce support for it.
      
      This patch does not cover the replication driver because that currently
      does not support flags at all, and because it just grabs the WRITE
      permission for its children when it can, so we should be fine just
      submitting the incoming WRITE_UNCHANGED requests as normal writes.
      
      It also does not cover format drivers for similar reasons.  They all use
      bdrv_format_default_perms() as their .bdrv_child_perm() implementation
      so they just always grab the WRITE permission for their file children
      whenever possible.  In addition, it often would be difficult to
      ascertain whether incoming unchanging writes end up as unchanging writes
      in their files.  So we just leave them as normal potentially changing
      writes.
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
      Message-id: 20180421132929.21610-7-mreitz@redhat.com
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      228345bf
  19. Mar 02, 2018
  20. Sep 04, 2017
  21. Jul 11, 2017
  22. Jul 10, 2017
  23. Jun 26, 2017
  24. Apr 28, 2017
Loading