Block layer patches

- scsi-disk: Apply error policy for host_status errors again
 - qcow2: Fix qemu-img info crash with missing crypto header
 - qemu-img bench: Fix division by zero for zero-sized images
 - test-bdrv-drain: Fix data races
 -----BEGIN PGP SIGNATURE-----
 
 iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmf1HdQRHGt3b2xmQHJl
 ZGhhdC5jb20ACgkQfwmycsiPL9Z9QxAAlKjkXt5mshcMPPNAIFkBarvF318T8azh
 5A4soABMpgZBceXaadWMEkBiYGW7jvoBwRVivVNB7jLfar3jchfW8xEAerLXMpAE
 O6n6vwXQz5fy1w5VqJuA/lA/5ZGdt8P7NvvOGcd00GySo6df2lOBtCbDjtwT5t6a
 0w6b5d/qSIsfm7wEIh7Vh8HjQ88WoOXSti9xQppyd48onNRT+6p2XtyXD75EeZi+
 uYS/NNwViNVRD2df3q4Thi3Q9AMhlDn8yZUqgMpwupbZcXNgjdfMNMPUUmRTNDrO
 33byZu+nrrq+Qz5xTSekD9anV4M1yJ+aWYxL7BI2RP87u4OgcZuCgNcFHzZ2j9BJ
 xrV0wPdh1xdY8kn/5+X27/gC5cjb5AYoiA4SGZJsZpcvYnBz/jRIMoUY9HVc1Y+N
 hW/endbNTpQYlEzmTb6RRccV7gTsD8V+Dc5TOg/RLgpdxahiZg0JAxT4sUkb52Ij
 CH5kPRkEsluSXf86qFyDitMlE/SCl4bL9xoHnydgeaMJovMRAT6I/UpUdLkgsacL
 ul6snvKPRXXP6PnM8hKHJmZwzKyzJVaVnQSG4TefNQTLIro3ZgVKzUek4dmpIHmg
 hn9GOqENeS3soKg1vyniWEsNdg/t6YvEfFutJk5LJVRb5F18sht9IIYWNJKdWxuV
 S7S3kAlMXow=
 =Dv5w
 -----END PGP SIGNATURE-----

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- scsi-disk: Apply error policy for host_status errors again
- qcow2: Fix qemu-img info crash with missing crypto header
- qemu-img bench: Fix division by zero for zero-sized images
- test-bdrv-drain: Fix data races

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmf1HdQRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9Z9QxAAlKjkXt5mshcMPPNAIFkBarvF318T8azh
# 5A4soABMpgZBceXaadWMEkBiYGW7jvoBwRVivVNB7jLfar3jchfW8xEAerLXMpAE
# O6n6vwXQz5fy1w5VqJuA/lA/5ZGdt8P7NvvOGcd00GySo6df2lOBtCbDjtwT5t6a
# 0w6b5d/qSIsfm7wEIh7Vh8HjQ88WoOXSti9xQppyd48onNRT+6p2XtyXD75EeZi+
# uYS/NNwViNVRD2df3q4Thi3Q9AMhlDn8yZUqgMpwupbZcXNgjdfMNMPUUmRTNDrO
# 33byZu+nrrq+Qz5xTSekD9anV4M1yJ+aWYxL7BI2RP87u4OgcZuCgNcFHzZ2j9BJ
# xrV0wPdh1xdY8kn/5+X27/gC5cjb5AYoiA4SGZJsZpcvYnBz/jRIMoUY9HVc1Y+N
# hW/endbNTpQYlEzmTb6RRccV7gTsD8V+Dc5TOg/RLgpdxahiZg0JAxT4sUkb52Ij
# CH5kPRkEsluSXf86qFyDitMlE/SCl4bL9xoHnydgeaMJovMRAT6I/UpUdLkgsacL
# ul6snvKPRXXP6PnM8hKHJmZwzKyzJVaVnQSG4TefNQTLIro3ZgVKzUek4dmpIHmg
# hn9GOqENeS3soKg1vyniWEsNdg/t6YvEfFutJk5LJVRb5F18sht9IIYWNJKdWxuV
# S7S3kAlMXow=
# =Dv5w
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 08 Apr 2025 09:00:04 EDT
# gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg:                issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin:
  test-bdrv-drain: Fix data races
  scsi-disk: Apply error policy for host_status errors again
  qcow2: Don't crash qemu-img info with missing crypto header
  qemu-img: fix division by zero in bench_cb() for zero-sized images

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
Stefan Hajnoczi 2025-04-08 09:59:33 -04:00
commit c302660920
8 changed files with 167 additions and 30 deletions

View File

@ -1721,7 +1721,7 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EINVAL;
goto fail;
}
} else if (!(flags & BDRV_O_NO_IO)) {
} else {
error_setg(errp, "Missing CRYPTO header for crypt method %d",
s->crypt_method_header);
ret = -EINVAL;
@ -1976,7 +1976,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
if (bs->encrypted) {
if (s->crypto) {
/* Encryption works on a sector granularity */
bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);
}

View File

@ -68,10 +68,9 @@ struct SCSIDiskClass {
SCSIDeviceClass parent_class;
/*
* Callbacks receive ret == 0 for success. Errors are represented either as
* negative errno values, or as positive SAM status codes.
*
* Beware: For errors returned in host_status, the function may directly
* complete the request and never call the callback.
* negative errno values, or as positive SAM status codes. For host_status
* errors, the function passes ret == -ENODEV and sets the host_status field
* of the SCSIRequest.
*/
DMAIOFunc *dma_readv;
DMAIOFunc *dma_writev;
@ -225,11 +224,26 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
SCSISense sense = SENSE_CODE(NO_SENSE);
int16_t host_status;
int error;
bool req_has_sense = false;
BlockErrorAction action;
int status;
/*
* host_status should only be set for SG_IO requests that came back with a
* host_status error in scsi_block_sgio_complete(). This error path passes
* -ENODEV as the return value.
*
* Reset host_status in the request because we may still want to complete
* the request successfully with the 'stop' or 'ignore' error policy.
*/
host_status = r->req.host_status;
if (host_status != -1) {
assert(ret == -ENODEV);
r->req.host_status = -1;
}
if (ret < 0) {
status = scsi_sense_from_errno(-ret, &sense);
error = -ret;
@ -289,6 +303,10 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed)
if (acct_failed) {
block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct);
}
if (host_status != -1) {
scsi_req_complete_failed(&r->req, host_status);
return true;
}
if (req_has_sense) {
sdc->update_sense(&r->req);
} else if (status == CHECK_CONDITION) {
@ -409,7 +427,6 @@ done:
scsi_req_unref(&r->req);
}
/* May not be called in all error cases, don't rely on cleanup here */
static void scsi_dma_complete(void *opaque, int ret)
{
SCSIDiskReq *r = (SCSIDiskReq *)opaque;
@ -448,7 +465,6 @@ done:
scsi_req_unref(&r->req);
}
/* May not be called in all error cases, don't rely on cleanup here */
static void scsi_read_complete(void *opaque, int ret)
{
SCSIDiskReq *r = (SCSIDiskReq *)opaque;
@ -585,7 +601,6 @@ done:
scsi_req_unref(&r->req);
}
/* May not be called in all error cases, don't rely on cleanup here */
static void scsi_write_complete(void * opaque, int ret)
{
SCSIDiskReq *r = (SCSIDiskReq *)opaque;
@ -2846,14 +2861,10 @@ static void scsi_block_sgio_complete(void *opaque, int ret)
sg_io_hdr_t *io_hdr = &req->io_header;
if (ret == 0) {
/* FIXME This skips calling req->cb() and any cleanup in it */
if (io_hdr->host_status != SCSI_HOST_OK) {
scsi_req_complete_failed(&r->req, io_hdr->host_status);
scsi_req_unref(&r->req);
return;
}
if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
r->req.host_status = io_hdr->host_status;
ret = -ENODEV;
} else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
ret = BUSY;
} else {
ret = io_hdr->status;

View File

@ -545,6 +545,9 @@ bool job_is_ready(Job *job);
/* Same as job_is_ready(), but called with job lock held. */
bool job_is_ready_locked(Job *job);
/** Returns whether the job is paused. Called with job_mutex *not* held. */
bool job_is_paused(Job *job);
/**
* Request @job to pause at the next pause point. Must be paired with
* job_resume(). If the job is supposed to be resumed by user action, call

6
job.c
View File

@ -251,6 +251,12 @@ bool job_is_cancelled_locked(Job *job)
return job->force_cancel;
}
bool job_is_paused(Job *job)
{
JOB_LOCK_GUARD();
return job->paused;
}
bool job_is_cancelled(Job *job)
{
JOB_LOCK_GUARD();

View File

@ -4488,7 +4488,11 @@ static void bench_cb(void *opaque, int ret)
*/
b->in_flight++;
b->offset += b->step;
if (b->image_size == 0) {
b->offset = 0;
} else {
b->offset %= b->image_size;
}
if (b->write) {
acb = blk_aio_pwritev(b->blk, offset, b->qiov, 0, bench_cb, b);
} else {

View File

@ -0,0 +1,75 @@
#!/usr/bin/env bash
# group: rw quick
#
# Test case for encryption support in qcow2
#
# Copyright (C) 2025 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# creator
owner=kwolf@redhat.com
seq="$(basename $0)"
echo "QA output created by $seq"
status=1 # failure is the default!
_cleanup()
{
_cleanup_test_img
}
trap "_cleanup; exit \$status" 0 1 2 3 15
# get standard environment, filters and checks
. ../common.rc
. ../common.filter
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
_supported_proto file
_require_working_luks
IMG_SIZE=64M
echo
echo "=== Create an encrypted image ==="
echo
_make_test_img --object secret,id=sec0,data=123456 -o encrypt.format=luks,encrypt.key-secret=sec0 $IMG_SIZE
$PYTHON ../qcow2.py "$TEST_IMG" dump-header-exts
_img_info
$QEMU_IMG check \
--object secret,id=sec0,data=123456 \
--image-opts file.filename="$TEST_IMG",encrypt.key-secret=sec0 \
| _filter_qemu_img_check
echo
echo "=== Remove the header extension ==="
echo
$PYTHON ../qcow2.py "$TEST_IMG" del-header-ext 0x0537be77
$PYTHON ../qcow2.py "$TEST_IMG" dump-header-exts
_img_info
$QEMU_IMG check \
--object secret,id=sec0,data=123456 \
--image-opts file.filename="$TEST_IMG",encrypt.key-secret=sec0 2>&1 \
| _filter_qemu_img_check \
| _filter_testdir
# success, all done
echo "*** done"
rm -f $seq.full
status=0

View File

@ -0,0 +1,32 @@
QA output created by qcow2-encryption
=== Create an encrypted image ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Header extension:
magic 0x537be77 (Crypto header)
length 16
data <binary>
Header extension:
magic 0x6803f857 (Feature table)
length 384
data <binary>
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
virtual size: 64 MiB (67108864 bytes)
encrypted: yes
cluster_size: 65536
No errors were found on the image.
=== Remove the header extension ===
Header extension:
magic 0x6803f857 (Feature table)
length 384
data <binary>
qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Missing CRYPTO header for crypt method 2
qemu-img: Could not open 'file.filename=TEST_DIR/t.qcow2,encrypt.key-secret=sec0': Missing CRYPTO header for crypt method 2
*** done

View File

@ -632,6 +632,8 @@ typedef struct TestBlockJob {
BlockDriverState *bs;
int run_ret;
int prepare_ret;
/* Accessed with atomics */
bool running;
bool should_complete;
} TestBlockJob;
@ -667,10 +669,10 @@ static int coroutine_fn test_job_run(Job *job, Error **errp)
/* We are running the actual job code past the pause point in
* job_co_entry(). */
s->running = true;
qatomic_set(&s->running, true);
job_transition_to_ready(&s->common.job);
while (!s->should_complete) {
while (!qatomic_read(&s->should_complete)) {
/* Avoid job_sleep_ns() because it marks the job as !busy. We want to
* emulate some actual activity (probably some I/O) here so that drain
* has to wait for this activity to stop. */
@ -685,7 +687,7 @@ static int coroutine_fn test_job_run(Job *job, Error **errp)
static void test_job_complete(Job *job, Error **errp)
{
TestBlockJob *s = container_of(job, TestBlockJob, common.job);
s->should_complete = true;
qatomic_set(&s->should_complete, true);
}
BlockJobDriver test_job_driver = {
@ -791,7 +793,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
/* job_co_entry() is run in the I/O thread, wait for the actual job
* code to start (we don't want to catch the job in the pause point in
* job_co_entry(). */
while (!tjob->running) {
while (!qatomic_read(&tjob->running)) {
aio_poll(qemu_get_aio_context(), false);
}
}
@ -799,7 +801,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
WITH_JOB_LOCK_GUARD() {
g_assert_cmpint(job->job.pause_count, ==, 0);
g_assert_false(job->job.paused);
g_assert_true(tjob->running);
g_assert_true(qatomic_read(&tjob->running));
g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */
}
@ -825,7 +827,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
*
* paused is reset in the I/O thread, wait for it
*/
while (job->job.paused) {
while (job_is_paused(&job->job)) {
aio_poll(qemu_get_aio_context(), false);
}
}
@ -858,7 +860,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
*
* paused is reset in the I/O thread, wait for it
*/
while (job->job.paused) {
while (job_is_paused(&job->job)) {
aio_poll(qemu_get_aio_context(), false);
}
}
@ -1411,10 +1413,12 @@ static void test_set_aio_context(void)
typedef struct TestDropBackingBlockJob {
BlockJob common;
bool should_complete;
bool *did_complete;
BlockDriverState *detach_also;
BlockDriverState *bs;
/* Accessed with atomics */
bool should_complete;
} TestDropBackingBlockJob;
static int coroutine_fn test_drop_backing_job_run(Job *job, Error **errp)
@ -1422,7 +1426,7 @@ static int coroutine_fn test_drop_backing_job_run(Job *job, Error **errp)
TestDropBackingBlockJob *s =
container_of(job, TestDropBackingBlockJob, common.job);
while (!s->should_complete) {
while (!qatomic_read(&s->should_complete)) {
job_sleep_ns(job, 0);
}
@ -1541,7 +1545,7 @@ static void test_blockjob_commit_by_drained_end(void)
job_start(&job->common.job);
job->should_complete = true;
qatomic_set(&job->should_complete, true);
bdrv_drained_begin(bs_child);
g_assert(!job_has_completed);
bdrv_drained_end(bs_child);
@ -1557,15 +1561,17 @@ static void test_blockjob_commit_by_drained_end(void)
typedef struct TestSimpleBlockJob {
BlockJob common;
bool should_complete;
bool *did_complete;
/* Accessed with atomics */
bool should_complete;
} TestSimpleBlockJob;
static int coroutine_fn test_simple_job_run(Job *job, Error **errp)
{
TestSimpleBlockJob *s = container_of(job, TestSimpleBlockJob, common.job);
while (!s->should_complete) {
while (!qatomic_read(&s->should_complete)) {
job_sleep_ns(job, 0);
}
@ -1700,7 +1706,7 @@ static void test_drop_intermediate_poll(void)
job->did_complete = &job_has_completed;
job_start(&job->common.job);
job->should_complete = true;
qatomic_set(&job->should_complete, true);
g_assert(!job_has_completed);
ret = bdrv_drop_intermediate(chain[1], chain[0], NULL, false);