[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/31] include/sysemu/block-backend: split header into I/O
From: |
Hanna Reitz |
Subject: |
Re: [PATCH v5 04/31] include/sysemu/block-backend: split header into I/O and global state (GS) API |
Date: |
Fri, 10 Dec 2021 15:21:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 |
On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote:
Similarly to the previous patches, split block-backend.h
in block-backend-io.h and block-backend-global-state.h
In addition, remove "block/block.h" include as it seems
it is not necessary anymore, together with "qemu/iov.h"
block-backend-common.h contains the structures shared between
the two headers, and the functions that can't be categorized as
I/O or global state.
Assertions are added in the next patch.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
include/sysemu/block-backend-common.h | 84 ++++++
include/sysemu/block-backend-global-state.h | 121 +++++++++
include/sysemu/block-backend-io.h | 139 ++++++++++
include/sysemu/block-backend.h | 269 +-------------------
block/block-backend.c | 9 +-
5 files changed, 353 insertions(+), 269 deletions(-)
create mode 100644 include/sysemu/block-backend-common.h
create mode 100644 include/sysemu/block-backend-global-state.h
create mode 100644 include/sysemu/block-backend-io.h
[...]
diff --git a/include/sysemu/block-backend-global-state.h
b/include/sysemu/block-backend-global-state.h
new file mode 100644
index 0000000000..77009bf7a2
--- /dev/null
+++ b/include/sysemu/block-backend-global-state.h
[...]
+int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
I don’t quite follow what makes blk_ioctl() a GS function. (It’s
possible that I forgot something about this from v4, though, and can’t
find it anymore...)
I suppose it can be said that in the context of the block layer, ioctl
functions are generally used outside of I/O threads as something that
kind of affects the global data state, but... bdrv_co_ioctl() is in
block-io.h, and internally we definitely do use ioctl in I/O threads
(various uses in file-posix.c, e.g. to zero out areas). I also don’t
understand why blk_ioctl() is GS, and blk_aio_ioctl() is I/O.
And if you have a virtio-scsi device in an iothread, and then create a
scsi-generic device on it, will bdrv_co_ioctl() not run in the I/O
thread still...? Hm, testing (on patch 6), it appears that not – I
think that’s because it mostly uses blk_aio_ioctl(), apart from
*_realize() and scsi_SG_IO_FROM_DEV() (whatever that is; and this makes
me a bit cautious, too). Still, I’m not quite sure how it’s more global
state than e.g. writing zeroes or, well, blk_aio_ioctl().
However, testing brought some other problems to light:
$ ls /dev/sg?
/dev/sg0
$ sudo modprobe scsi_debug max_luns=1 num_tgts=1 add_host=1
$ ls /dev/sg?
/dev/sg0 /dev/sg1
$ ./qemu-system-x86_64 \
-object iothread,id=foo \
-device virtio-scsi,iothread=foo \
-blockdev host_device,filename=/dev/sg1,node-name=bar \
-device scsi-generic,drive=bar
qemu-system-x86_64: ../block/block-backend.c:2038:
blk_set_guest_block_size: Assertion `qemu_in_main_thread()' failed.
[1] 121353 IOT instruction (core dumped) ./qemu-system-x86_64
-object iothread,id=foo -device virtio-scsi,iothread=foo
And:
$ ./qemu-system-x86_64 \
-object iothread,id=foo \
-device virtio-scsi,iothread=foo \
-blockdev null-co,read-zeroes=true,node-name=bar \
-device scsi-hd,drive=bar \
-cdrom ~/tmp/arch.iso \
-enable-kvm \
-m 1g
qemu-system-x86_64: ../block/block-backend.c:1918:
blk_enable_write_cache: Assertion `qemu_in_main_thread()' failed.
[1] 127203 IOT instruction (core dumped) ./qemu-system-x86_64
-object iothread,id=foo -device virtio-scsi,iothread=foo
If I boot from a CD under virtio-scsi (in an iothread), I also get an
error for blk_lock_medium(), and if I eject such a CD, for blk_eject()
and blk_get_attached_dev_id() (called from blk_eject()).
It appears like scsi_disk_emulate_command() is always run in the BB’s
AioContext, and so everything it (indirectly) calls cannot assume to be
in the main thread. As for blk_set_guest_block_size(), that one’s
called from scsi-generic’s scsi_read_complete().
Hanna
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v5 04/31] include/sysemu/block-backend: split header into I/O and global state (GS) API,
Hanna Reitz <=