Skip to content
Snippets Groups Projects
  1. Sep 29, 2021
    • Vladimir Sementsov-Ogievskiy's avatar
      block: use int64_t instead of uint64_t in driver write handlers · e75abeda
      Vladimir Sementsov-Ogievskiy authored
      
      We are generally moving to int64_t for both offset and bytes parameters
      on all io paths.
      
      Main motivation is realization of 64-bit write_zeroes operation for
      fast zeroing large disk chunks, up to the whole disk.
      
      We chose signed type, to be consistent with off_t (which is signed) and
      with possibility for signed return type (where negative value means
      error).
      
      So, convert driver write handlers parameters which are already 64bit to
      signed type.
      
      While being here, convert also flags parameter to be BdrvRequestFlags.
      
      Now let's consider all callers. Simple
      
        git grep '\->bdrv_\(aio\|co\)_pwritev\(_part\)\?'
      
      shows that's there three callers of driver function:
      
       bdrv_driver_pwritev() and bdrv_driver_pwritev_compressed() in
       block/io.c, both pass int64_t, checked by bdrv_check_qiov_request() to
       be non-negative.
      
       qcow2_save_vmstate() does bdrv_check_qiov_request().
      
      Still, the functions may be called directly, not only by drv->...
      Let's check:
      
      git grep '\.bdrv_\(aio\|co\)_pwritev\(_part\)\?\s*=' | \
      awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
      while read func; do git grep "$func(" | \
      grep -v "$func(BlockDriverState"; done
      
      shows several callers:
      
      qcow2:
        qcow2_co_truncate() write at most up to @offset, which is checked in
          generic qcow2_co_truncate() by bdrv_check_request().
        qcow2_co_pwritev_compressed_task() pass the request (or part of the
          request) that already went through normal write path, so it should
          be OK
      
      qcow:
        qcow_co_pwritev_compressed() pass int64_t, it's updated by this patch
      
      quorum:
        quorum_co_pwrite_zeroes() pass int64_t and int - OK
      
      throttle:
        throttle_co_pwritev_compressed() pass int64_t, it's updated by this
        patch
      
      vmdk:
        vmdk_co_pwritev_compressed() pass int64_t, it's updated by this
        patch
      
      Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Message-Id: <20210903102807.27127-5-vsementsov@virtuozzo.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      e75abeda
    • Vladimir Sementsov-Ogievskiy's avatar
      block: use int64_t instead of uint64_t in driver read handlers · f7ef38dd
      Vladimir Sementsov-Ogievskiy authored
      
      We are generally moving to int64_t for both offset and bytes parameters
      on all io paths.
      
      Main motivation is realization of 64-bit write_zeroes operation for
      fast zeroing large disk chunks, up to the whole disk.
      
      We chose signed type, to be consistent with off_t (which is signed) and
      with possibility for signed return type (where negative value means
      error).
      
      So, convert driver read handlers parameters which are already 64bit to
      signed type.
      
      While being here, convert also flags parameter to be BdrvRequestFlags.
      
      Now let's consider all callers. Simple
      
        git grep '\->bdrv_\(aio\|co\)_preadv\(_part\)\?'
      
      shows that's there three callers of driver function:
      
       bdrv_driver_preadv() in block/io.c, passes int64_t, checked by
         bdrv_check_qiov_request() to be non-negative.
      
       qcow2_load_vmstate() does bdrv_check_qiov_request().
      
       do_perform_cow_read() has uint64_t argument. And a lot of things in
       qcow2 driver are uint64_t, so converting it is big job. But we must
       not work with requests that don't satisfy bdrv_check_qiov_request(),
       so let's just assert it here.
      
      Still, the functions may be called directly, not only by drv->...
      Let's check:
      
      git grep '\.bdrv_\(aio\|co\)_preadv\(_part\)\?\s*=' | \
      awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
      while read func; do git grep "$func(" | \
      grep -v "$func(BlockDriverState"; done
      
      The only one such caller:
      
          QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, &data, 1);
          ...
          ret = bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0);
      
      in tests/unit/test-bdrv-drain.c, and it's OK obviously.
      
      Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Message-Id: <20210903102807.27127-4-vsementsov@virtuozzo.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      [eblake: fix typos]
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      f7ef38dd
  2. Sep 15, 2020
  3. Jul 14, 2020
    • Eric Blake's avatar
      qcow: Tolerate backing_fmt= · 344acbd6
      Eric Blake authored
      
      qcow has no space in the metadata to store a backing format, and there
      are existing qcow images backed both by raw or by other formats
      (usually qcow) images, reliant on probing to tell the difference.  On
      the bright side, because we probe every time, raw files are marked as
      probed and we thus forbid a commit action into the backing file where
      guest-controlled contents could change the result of the probe next
      time around (the iotest added here proves that).
      
      Still, allowing the user to specify the backing format during
      creation, even if we can't record it, is a good thing.  This patch
      blindly allows any value that resolves to a known driver, even if the
      user's request is a mismatch from what probing finds; then the next
      patch will further enhance things to verify that the user's request
      matches what we actually probe.  With this and the next patch in
      place, we will finally be ready to deprecate the creation of images
      where a backing format was not explicitly specified by the user.
      
      Note that this is only for QemuOpts usage; there is no change to the
      QAPI to allow a format through -blockdev.
      
      Add a new iotest 301 just for qcow, to demonstrate the latest
      behavior, and to make it easier to show the improvements made in the
      next patch.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200706203954.341758-6-eblake@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      344acbd6
  4. Jul 10, 2020
    • Markus Armbruster's avatar
      error: Avoid error_propagate() after migrate_add_blocker() · 386f6c07
      Markus Armbruster authored
      
      When migrate_add_blocker(blocker, &errp) is followed by
      error_propagate(errp, err), we can often just as well do
      migrate_add_blocker(..., errp).
      
      Do that with this Coccinelle script:
      
          @@
          expression blocker, err, errp;
          expression ret;
          @@
          -    ret = migrate_add_blocker(blocker, &err);
          -    if (err) {
          +    ret = migrate_add_blocker(blocker, errp);
          +    if (ret < 0) {
                   ... when != err;
          -        error_propagate(errp, err);
                   ...
               }
      
          @@
          expression blocker, err, errp;
          @@
          -    migrate_add_blocker(blocker, &err);
          -    if (err) {
          +    if (migrate_add_blocker(blocker, errp) < 0) {
                   ... when != err;
          -        error_propagate(errp, err);
                   ...
               }
      
      Double-check @err is not used afterwards.  Dereferencing it would be
      use after free, but checking whether it's null would be legitimate.
      
      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-43-armbru@redhat.com>
      386f6c07
    • Markus Armbruster's avatar
      qapi: Smooth another visitor error checking pattern · b11a093c
      Markus Armbruster authored
      
      Convert
      
          visit_type_FOO(v, ..., &ptr, &err);
          ...
          if (err) {
              ...
          }
      
      to
      
          visit_type_FOO(v, ..., &ptr, errp);
          ...
          if (!ptr) {
              ...
          }
      
      for functions that set @ptr to non-null / null on success / error.
      
      Eliminate error_propagate() that are now unnecessary.  Delete @err
      that are now unused.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200707160613.848843-40-armbru@redhat.com>
      b11a093c
    • Markus Armbruster's avatar
      error: Eliminate error_propagate() with Coccinelle, part 1 · 668f62ec
      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.  Convert
      
          if (!foo(..., &err)) {
              ...
              error_propagate(errp, err);
              ...
              return ...
          }
      
      to
      
          if (!foo(..., errp)) {
              ...
              ...
              return ...
          }
      
      where nothing else needs @err.  Coccinelle script:
      
          @rule1 forall@
          identifier fun, err, errp, lbl;
          expression list args, args2;
          binary operator op;
          constant c1, c2;
          symbol false;
          @@
               if (
          (
          -        fun(args, &err, args2)
          +        fun(args, errp, args2)
          |
          -        !fun(args, &err, args2)
          +        !fun(args, errp, args2)
          |
          -        fun(args, &err, args2) op c1
          +        fun(args, errp, args2) op c1
          )
                  )
               {
                   ... when != err
                       when != lbl:
                       when strict
          -        error_propagate(errp, err);
                   ... when != err
          (
                   return;
          |
                   return c2;
          |
                   return false;
          )
               }
      
          @rule2 forall@
          identifier fun, err, errp, lbl;
          expression list args, args2;
          expression var;
          binary operator op;
          constant c1, c2;
          symbol false;
          @@
          -    var = fun(args, &err, args2);
          +    var = fun(args, errp, args2);
               ... when != err
               if (
          (
                   var
          |
                   !var
          |
                   var op c1
          )
                  )
               {
                   ... when != err
                       when != lbl:
                       when strict
          -        error_propagate(errp, err);
                   ... when != err
          (
                   return;
          |
                   return c2;
          |
                   return false;
          |
                   return var;
          )
               }
      
          @depends on rule1 || rule2@
          identifier err;
          @@
          -    Error *err = NULL;
               ... when != err
      
      Not exactly elegant, I'm afraid.
      
      The "when != lbl:" is necessary to avoid transforming
      
               if (fun(args, &err)) {
                   goto out
               }
               ...
           out:
               error_propagate(errp, err);
      
      even though other paths to label out still need the error_propagate().
      For an actual example, see sclp_realize().
      
      Without the "when strict", Coccinelle transforms vfio_msix_setup(),
      incorrectly.  I don't know what exactly "when strict" does, only that
      it helps here.
      
      The match of return is narrower than what I want, but I can't figure
      out how to express "return where the operand doesn't use @err".  For
      an example where it's too narrow, see vfio_intx_enable().
      
      Silently fails to convert hw/arm/armsse.c, because Coccinelle gets
      confused by ARMSSE being used both as typedef and function-like macro
      there.  Converted manually.
      
      Line breaks tidied up manually.  One nested declaration of @local_err
      deleted manually.  Preexisting unwanted blank line dropped in
      hw/riscv/sifive_e.c.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20200707160613.848843-35-armbru@redhat.com>
      668f62ec
  5. May 18, 2020
  6. May 05, 2020
  7. Apr 30, 2020
  8. Mar 26, 2020
  9. Oct 28, 2019
    • 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
    • Hanna Reitz's avatar
      block: Do not truncate file node when formatting · 26536c7f
      Hanna Reitz authored
      
      There is no reason why the format drivers need to truncate the protocol
      node when formatting it.  When using the old .bdrv_co_create_ops()
      interface, the file will be created with no size option anyway, which
      generally gives it a size of 0.  (Exceptions are block devices, which
      cannot be truncated anyway.)
      
      When using blockdev-create, the user must have given the file node some
      size anyway, so there is no reason why we should override that.
      
      qed is an exception, it needs the file to start completely empty (as
      explained by c743849b).
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Message-id: 20190918095144.955-4-mreitz@redhat.com
      Reviewed-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      26536c7f
  10. Jul 08, 2019
  11. Jun 04, 2019
    • Kevin Wolf's avatar
      block: Add BlockBackend.ctx · d861ab3a
      Kevin Wolf authored
      
      This adds a new parameter to blk_new() which requires its callers to
      declare from which AioContext this BlockBackend is going to be used (or
      the locks of which AioContext need to be taken anyway).
      
      The given context is only stored and kept up to date when changing
      AioContexts. Actually applying the stored AioContext to the root node
      is saved for another commit.
      
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      d861ab3a
  12. Apr 30, 2019
  13. 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: Add BDS.auto_backing_file · 998c2019
      Hanna Reitz authored
      
      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: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-5-mreitz@redhat.com
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      998c2019
  14. Feb 22, 2019
  15. Dec 12, 2018
  16. Nov 05, 2018
    • Peter Maydell's avatar
      block/qcow: Don't take address of fields in packed structs · a5fdff18
      Peter Maydell authored
      
      Taking the address of a field in a packed struct is a bad idea, because
      it might not be actually aligned enough for that pointer type (and
      thus cause a crash on dereference on some host architectures). Newer
      versions of clang warn about this. Avoid the bug by not using the
      "modify in place" byte swapping functions.
      
      There are a few places where the in-place swap function is
      used on something other than a packed struct field; we convert
      those anyway, for consistency.
      
      This patch was produced with the following spatch script:
      
      @@
      expression E;
      @@
      -be16_to_cpus(&E);
      +E = be16_to_cpu(E);
      @@
      expression E;
      @@
      -be32_to_cpus(&E);
      +E = be32_to_cpu(E);
      @@
      expression E;
      @@
      -be64_to_cpus(&E);
      +E = be64_to_cpu(E);
      @@
      expression E;
      @@
      -cpu_to_be16s(&E);
      +E = cpu_to_be16(E);
      @@
      expression E;
      @@
      -cpu_to_be32s(&E);
      +E = cpu_to_be32(E);
      @@
      expression E;
      @@
      -cpu_to_be64s(&E);
      +E = cpu_to_be64(E);
      
      Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
      Reviewed-by: default avatarRichard Henderson <richard.henderson@linaro.org>
      Tested-by: default avatarJohn Snow <jsnow@redhat.com>
      Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      a5fdff18
  17. Jul 30, 2018
    • KONRAD Frederic's avatar
      qcow: fix a reference leak · 41b65134
      KONRAD Frederic authored
      Since 42a3e1ab qemu asserts when using the
      vvfat driver:
      
      git clone git://qemu.org/qemu.git
      
      
      cd qemu
      ./configure --target-list=ppc-softmmu --enable-debug
      make -j8
      mkdir foo
      touch foo/hello
      ./ppc-softmmu/qemu-system-ppc -M prep --nographic --monitor null             \
                                    -hda fat:rw:./foo
      
      "Ctrl-C"
      
      qemu-system-ppc: block.c:3368: bdrv_close_all: Assertion                     \
         `((&all_bdrv_states)->tqh_first == ((void *)0))' failed.
      
      This is because we reference bs twice in qcow_co_create(..) one time in
      bdrv_open_blockdev_ref(..) and in blk_insert_bs(..) but we unref it only once
      in blk_unref which leads to the reference leak.
      
      Note that I didn't tested much QCOW after this change as I don't use it much.
      
      Signed-off-by: default avatarKONRAD Frederic <frederic.konrad@adacore.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      41b65134
  18. Jun 29, 2018
  19. Jun 15, 2018
    • Markus Armbruster's avatar
    • Markus Armbruster's avatar
      block: Clean up a misuse of qobject_to() in .bdrv_co_create_opts() · 92adf9db
      Markus Armbruster authored
      
      The following pattern occurs in the .bdrv_co_create_opts() methods of
      parallels, qcow, qcow2, qed, vhdx and vpc:
      
          qobj = qdict_crumple_for_keyval_qiv(qdict, errp);
          qobject_unref(qdict);
          qdict = qobject_to(QDict, qobj);
          if (qdict == NULL) {
               ret = -EINVAL;
               goto done;
          }
      
          v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
          [...]
          ret = 0;
      done:
          qobject_unref(qdict);
          [...]
          return ret;
      
      If qobject_to() fails, we return failure without setting errp.  That's
      wrong.  As far as I can tell, it cannot fail here.  Clean it up
      anyway, by removing the useless conversion.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      92adf9db
    • Markus Armbruster's avatar
      block: Fix -blockdev for certain non-string scalars · e5af0da1
      Markus Armbruster authored
      
      Configuration flows through the block subsystem in a rather peculiar
      way.  Configuration made with -drive enters it as QemuOpts.
      Configuration made with -blockdev / blockdev-add enters it as QAPI
      type BlockdevOptions.  The block subsystem uses QDict, QemuOpts and
      QAPI types internally.  The precise flow is next to impossible to
      explain (I tried for this commit message, but gave up after wasting
      several hours).  What I can explain is a flaw in the BlockDriver
      interface that leads to this bug:
      
          $ qemu-system-x86_64 -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234
          qemu-system-x86_64: -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234: Internal error: parameter user invalid
      
      QMP blockdev-add is broken the same way.
      
      Here's what happens.  The block layer passes configuration represented
      as flat QDict (with dotted keys) to BlockDriver methods
      .bdrv_file_open().  The QDict's members are typed according to the
      QAPI schema.
      
      nfs_file_open() converts it to QAPI type BlockdevOptionsNfs, with
      qdict_crumple() and a qobject input visitor.
      
      This visitor comes in two flavors.  The plain flavor requires scalars
      to be typed according to the QAPI schema.  That's the case here.  The
      keyval flavor requires string scalars.  That's not the case here.
      nfs_file_open() uses the latter, and promptly falls apart for members
      @user, @group, @tcp-syn-count, @readahead-size, @page-cache-size,
      @debug.
      
      Switching to the plain flavor would fix -blockdev, but break -drive,
      because there the scalars arrive in nfs_file_open() as strings.
      
      The proper fix would be to replace the QDict by QAPI type
      BlockdevOptions in the BlockDriver interface.  Sadly, that's beyond my
      reach right now.
      
      Next best would be to fix the block layer to always pass correctly
      typed QDicts to the BlockDriver methods.  Also beyond my reach.
      
      What I can do is throw another hack onto the pile: have
      nfs_file_open() convert all members to string, so use of the keyval
      flavor actually works, by replacing qdict_crumple() by new function
      qdict_crumple_for_keyval_qiv().
      
      The pattern "pass result of qdict_crumple() to
      qobject_input_visitor_new_keyval()" occurs several times more:
      
      * qemu_rbd_open()
      
        Same issue as nfs_file_open(), but since BlockdevOptionsRbd has only
        string members, its only a latent bug.  Fix it anyway.
      
      * parallels_co_create_opts(), qcow_co_create_opts(),
        qcow2_co_create_opts(), bdrv_qed_co_create_opts(),
        sd_co_create_opts(), vhdx_co_create_opts(), vpc_co_create_opts()
      
        These work, because they create the QDict with
        qemu_opts_to_qdict_filtered(), which creates only string scalars.
        The function sports a TODO comment asking for better typing; that's
        going to be fun.  Use qdict_crumple_for_keyval_qiv() to be safe.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      e5af0da1
    • Hanna Reitz's avatar
      block: Add block-specific QDict header · 609f45ea
      Hanna Reitz authored
      
      There are numerous QDict functions that have been introduced for and are
      used only by the block layer.  Move their declarations into an own
      header file to reflect that.
      
      While qdict_extract_subqdict() is in fact used outside of the block
      layer (in util/qemu-config.c), it is still a function related very
      closely to how the block layer works with nested QDicts, namely by
      sometimes flattening them.  Therefore, its declaration is put into this
      header as well and util/qemu-config.c includes it with a comment stating
      exactly which function it needs.
      
      Suggested-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Message-Id: <20180509165530.29561-7-mreitz@redhat.com>
      [Copyright note tweaked, superfluous includes dropped]
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      609f45ea
  20. May 31, 2018
  21. May 15, 2018
    • Eric Blake's avatar
      block: Merge .bdrv_co_writev{,_flags} in drivers · e18a58b4
      Eric Blake authored
      
      We have too many driver callback interfaces; simplify the mess
      somewhat by merging the flags parameter of .bdrv_co_writev_flags()
      into .bdrv_co_writev().  Note that as long as a driver doesn't set
      .supported_write_flags, the flags argument will be 0 and behavior is
      identical.  Also note that the public function bdrv_co_writev() still
      lacks a flags argument; so the driver signature is thus intentionally
      slightly different.  But that's not the end of the world, nor the first
      time that the driver interface differs slightly from the public
      interface.
      
      Ideally, we should be rewriting all of these drivers to use modern
      byte-based interfaces.  But that's a more invasive patch to write
      and audit, compared to the simplification done here.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarDaniel P. Berrangé <berrange@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      e18a58b4
  22. May 04, 2018
  23. Mar 19, 2018
  24. Mar 02, 2018
Loading