Skip to content
Snippets Groups Projects
Select Git revision
  • feature/libtcg-rebase default protected
1 result
Search by author
  • Any Author
  • Anton Anton anjo
1 author
  1. Nov 08, 2023
  3. Nov 07, 2023
  5. Nov 03, 2023
  7. Nov 01, 2023
    • Steve Sistare's avatar
      cpr: relax blockdev migration blockers · e0ee3a8f
      Steve Sistare authored 
      
Some blockdevs block migration because they do not support sharing across
hosts and/or do not support dirty bitmaps.  These prohibitions do not apply
if the old and new qemu processes do not run concurrently, and if new qemu
starts on the same host as old, which is the case for cpr.  Narrow the scope
of these blockers so they only apply to normal mode.  They will not block
cpr modes when they are added in subsequent patches.

No functional change until a new mode is added.

Signed-off-by: default avatarSteve Sistare <steven.sistare@oracle.com>
Reviewed-by: default avatarJuan Quintela <quintela@redhat.com>
Signed-off-by: default avatarJuan Quintela <quintela@redhat.com>
Message-ID: <1698263069-406971-4-git-send-email-steven.sistare@oracle.com>
      e0ee3a8f
  9. Oct 20, 2023
  11. Oct 12, 2023
  13. Sep 29, 2023
  15. May 19, 2023
    • Kevin Wolf's avatar
      block: Call .bdrv_co_create(_opts) unlocked · 4db7ba3b
      Kevin Wolf authored 
      
These are functions that modify the graph, so they must be able to take
a writer lock. This is impossible if they already hold the reader lock.
If they need a reader lock for some of their operations, they should
take it internally.

Many of them go through blk_*(), which will always take the lock itself.
Direct calls of bdrv_*() need to take the reader lock. Note that while
locking for bdrv_co_*() calls is checked by TSA, this is not the case
for the mixed_coroutine_fns bdrv_*(). Holding the lock is still required
when they are called from coroutine context like here!

This effectively reverts 4ec8df01, but adds some internal locking
instead.

Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Message-Id: <20230510203601.418015-2-kwolf@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      4db7ba3b
  17. May 10, 2023
  19. Feb 23, 2023
  21. Feb 17, 2023
  23. Feb 01, 2023
  25. Dec 15, 2022
  27. Oct 27, 2022
  29. Jul 12, 2022
  31. Mar 07, 2022
  33. 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
  35. Mar 31, 2021
  37. 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
  39. Jul 06, 2020
    • Vladimir Sementsov-Ogievskiy's avatar
      block/vdi: return ZERO block-status when appropriate · 2ea0332f
      Vladimir Sementsov-Ogievskiy authored 
      
In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.

Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.

There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).

BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".

We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi at this patch and
vpc with the following) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.

After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.

Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <20200528094405.145708-4-vsementsov@virtuozzo.com>
Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      2ea0332f
  41. May 18, 2020
  43. May 05, 2020
  45. Apr 30, 2020
  47. Mar 26, 2020
  49. 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
  51. Aug 19, 2019
  53. 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