Revision tags: v9.1.0 |
|
#
9da6bd39 |
| 31-Jul-2024 |
Kevin Wolf <kwolf@redhat.com> |
scsi-disk: Always report RESERVATION_CONFLICT to guest
In the case of scsi-block, RESERVATION_CONFLICT is not a backend error, but indicates that the guest tried to make a request that it isn't allo
scsi-disk: Always report RESERVATION_CONFLICT to guest
In the case of scsi-block, RESERVATION_CONFLICT is not a backend error, but indicates that the guest tried to make a request that it isn't allowed to execute. Pass the error to the guest so that it can decide what to do with it.
Without this, if we stop the VM in response to a RESERVATION_CONFLICT (as is the default policy in management software such as oVirt or KubeVirt), it can happen that the VM cannot be resumed any more because every attempt to resume it immediately runs into the same error and stops the VM again.
One case that expects RESERVATION_CONFLICT errors to be visible in the guest is running the validation tests in Windows 2019's Failover Cluster Manager, which intentionally tries to execute invalid requests to see if they are properly rejected.
Buglink: https://issues.redhat.com/browse/RHEL-50000 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240731123207.27636-5-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
8a049562 |
| 31-Jul-2024 |
Kevin Wolf <kwolf@redhat.com> |
scsi-disk: Add warning comments that host_status errors take a shortcut
scsi_block_sgio_complete() has surprising behaviour in that there are error cases in which it directly completes the request a
scsi-disk: Add warning comments that host_status errors take a shortcut
scsi_block_sgio_complete() has surprising behaviour in that there are error cases in which it directly completes the request and never calls the passed callback. In the current state of the code, this doesn't seem to result in bugs, but with future code changes, we must be careful to never rely on the callback doing some cleanup until this code smell is fixed. For now, just add warnings to make people aware of the trap.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240731123207.27636-4-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
622a7016 |
| 31-Jul-2024 |
Kevin Wolf <kwolf@redhat.com> |
scsi-block: Don't skip callback for sgio error status/driver_status
Instead of calling into scsi_handle_rw_error() directly from scsi_block_sgio_complete() and skipping the normal callback, go throu
scsi-block: Don't skip callback for sgio error status/driver_status
Instead of calling into scsi_handle_rw_error() directly from scsi_block_sgio_complete() and skipping the normal callback, go through the normal cleanup path by calling the callback with a positive error value.
The important difference here is not only that the code path is cleaner, but that the callbacks set r->req.aiocb = NULL. If we skip setting this and the error action is BLOCK_ERROR_ACTION_STOP, resuming the VM runs into an assertion failure in scsi_read_data() or scsi_write_data() because the dangling aiocb pointer is unexpected.
Fixes: a108557bbf ("scsi: inline sg_io_sense_from_errno() into the callers.") Buglink: https://issues.redhat.com/browse/RHEL-50000 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240731123207.27636-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
cfe08808 |
| 31-Jul-2024 |
Kevin Wolf <kwolf@redhat.com> |
scsi-disk: Use positive return value for status in dma_readv/writev
In some error cases, scsi_block_sgio_complete() never calls the passed callback, but directly completes the request. This leads to
scsi-disk: Use positive return value for status in dma_readv/writev
In some error cases, scsi_block_sgio_complete() never calls the passed callback, but directly completes the request. This leads to bugs because its error paths are not exact copies of what the callback would normally do.
In preparation to fix this, allow passing positive return values to the callbacks that represent the status code that should be used to complete the request.
scsi_handle_rw_error() already handles positive values for its ret parameter because scsi_block_sgio_complete() calls directly into it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240731123207.27636-2-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
b4912afa |
| 24-May-2024 |
Hyman Huang <yong.huang@smartx.com> |
scsi-disk: Fix crash for VM configured with USB CDROM after live migration
For VMs configured with the USB CDROM device:
-drive file=/path/to/local/file,id=drive-usb-disk0,media=cdrom,readonly=on..
scsi-disk: Fix crash for VM configured with USB CDROM after live migration
For VMs configured with the USB CDROM device:
-drive file=/path/to/local/file,id=drive-usb-disk0,media=cdrom,readonly=on... -device usb-storage,drive=drive-usb-disk0,id=usb-disk0...
QEMU process may crash after live migration, to reproduce the issue, configure VM (Guest OS ubuntu 20.04 or 21.10) with the following XML:
<disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/path/to/share_fs/cdrom.iso'/> <target dev='sda' bus='usb'/> <readonly/> <address type='usb' bus='0' port='2'/> </disk> <controller type='usb' index='0' model='piix3-uhci'/>
Do the live migration repeatedly, crash may happen after live migratoin, trace log at the source before live migration is as follows:
324808@1711972823.521945:usb_uhci_frame_start nr 319 324808@1711972823.521978:usb_uhci_qh_load qh 0x35cb5400 324808@1711972823.521989:usb_uhci_qh_load qh 0x35cb5480 324808@1711972823.521997:usb_uhci_td_load qh 0x35cb5480, td 0x35cbe000, ctrl 0x0, token 0xffe07f69 324808@1711972823.522010:usb_uhci_td_nextqh qh 0x35cb5480, td 0x35cbe000 324808@1711972823.522022:usb_uhci_qh_load qh 0x35cb5680 324808@1711972823.522030:usb_uhci_td_load qh 0x35cb5680, td 0x75ac5180, ctrl 0x19800000, token 0x3c903e1 324808@1711972823.522045:usb_uhci_packet_add token 0x103e1, td 0x75ac5180 324808@1711972823.522056:usb_packet_state_change bus 0, port 2, ep 2, packet 0x559f9ba14b00, state undef -> setup 324808@1711972823.522079:usb_msd_cmd_submit lun 0, tag 0x472, flags 0x00000080, len 10, data-len 8 324808@1711972823.522107:scsi_req_parsed target 0 lun 0 tag 1138 command 74 dir 1 length 8 324808@1711972823.522124:scsi_req_parsed_lba target 0 lun 0 tag 1138 command 74 lba 4096 324808@1711972823.522139:scsi_req_alloc target 0 lun 0 tag 1138 324808@1711972823.522169:scsi_req_continue target 0 lun 0 tag 1138 324808@1711972823.522181:scsi_req_data target 0 lun 0 tag 1138 len 8 324808@1711972823.522194:usb_packet_state_change bus 0, port 2, ep 2, packet 0x559f9ba14b00, state setup -> complete 324808@1711972823.522209:usb_uhci_packet_complete_success token 0x103e1, td 0x75ac5180 324808@1711972823.522219:usb_uhci_packet_del token 0x103e1, td 0x75ac5180 324808@1711972823.522232:usb_uhci_td_complete qh 0x35cb5680, td 0x75ac5180
trace log at the destination after live migration is as follows:
3286206@1711972823.951646:usb_uhci_frame_start nr 320 3286206@1711972823.951663:usb_uhci_qh_load qh 0x35cb5100 3286206@1711972823.951671:usb_uhci_qh_load qh 0x35cb5480 3286206@1711972823.951680:usb_uhci_td_load qh 0x35cb5480, td 0x35cbe000, ctrl 0x1000000, token 0xffe07f69 3286206@1711972823.951693:usb_uhci_td_nextqh qh 0x35cb5480, td 0x35cbe000 3286206@1711972823.951702:usb_uhci_qh_load qh 0x35cb5700 3286206@1711972823.951709:usb_uhci_td_load qh 0x35cb5700, td 0x75ac5240, ctrl 0x39800000, token 0xe08369 3286206@1711972823.951727:usb_uhci_queue_add token 0x8369 3286206@1711972823.951735:usb_uhci_packet_add token 0x8369, td 0x75ac5240 3286206@1711972823.951746:usb_packet_state_change bus 0, port 2, ep 1, packet 0x56066b2fb5a0, state undef -> setup 3286206@1711972823.951766:usb_msd_data_in 8/8 (scsi 8) 2024-04-01 12:00:24.665+0000: shutting down, reason=crashed
The backtrace reveals the following:
Program terminated with signal SIGSEGV, Segmentation fault. 0 __memmove_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:312 312 movq -8(%rsi,%rdx), %rcx [Current thread is 1 (Thread 0x7f0a9025fc00 (LWP 3286206))] (gdb) bt 0 __memmove_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:312 1 memcpy (__len=8, __src=<optimized out>, __dest=<optimized out>) at /usr/include/bits/string_fortified.h:34 2 iov_from_buf_full (iov=<optimized out>, iov_cnt=<optimized out>, offset=<optimized out>, buf=0x0, bytes=bytes@entry=8) at ../util/iov.c:33 3 iov_from_buf (bytes=8, buf=<optimized out>, offset=<optimized out>, iov_cnt=<optimized out>, iov=<optimized out>) at /usr/src/debug/qemu-6-6.2.0-75.7.oe1.smartx.git.40.x86_64/include/qemu/iov.h:49 4 usb_packet_copy (p=p@entry=0x56066b2fb5a0, ptr=<optimized out>, bytes=bytes@entry=8) at ../hw/usb/core.c:636 5 usb_msd_copy_data (s=s@entry=0x56066c62c770, p=p@entry=0x56066b2fb5a0) at ../hw/usb/dev-storage.c:186 6 usb_msd_handle_data (dev=0x56066c62c770, p=0x56066b2fb5a0) at ../hw/usb/dev-storage.c:496 7 usb_handle_packet (dev=0x56066c62c770, p=p@entry=0x56066b2fb5a0) at ../hw/usb/core.c:455 8 uhci_handle_td (s=s@entry=0x56066bd5f210, q=0x56066bb7fbd0, q@entry=0x0, qh_addr=qh_addr@entry=902518530, td=td@entry=0x7fffe6e788f0, td_addr=<optimized out>, int_mask=int_mask@entry=0x7fffe6e788e4) at ../hw/usb/hcd-uhci.c:885 9 uhci_process_frame (s=s@entry=0x56066bd5f210) at ../hw/usb/hcd-uhci.c:1061 10 uhci_frame_timer (opaque=opaque@entry=0x56066bd5f210) at ../hw/usb/hcd-uhci.c:1159 11 timerlist_run_timers (timer_list=0x56066af26bd0) at ../util/qemu-timer.c:642 12 qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:656 13 qemu_clock_run_all_timers () at ../util/qemu-timer.c:738 14 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:542 15 qemu_main_loop () at ../softmmu/runstate.c:739 16 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at ../softmmu/main.c:52 (gdb) frame 5 (gdb) p ((SCSIDiskReq *)s->req)->iov $1 = {iov_base = 0x0, iov_len = 0} (gdb) p/x s->req->tag $2 = 0x472
When designing the USB mass storage device model, QEMU places SCSI disk device as the backend of USB mass storage device. In addition, USB mass device driver in Guest OS conforms to the "Universal Serial Bus Mass Storage Class Bulk-Only Transport" specification in order to simulate the transform behavior between a USB controller and a USB mass device. The following shows the protocol hierarchy:
+----------------+ CDROM driver | scsi command | CDROM +----------------+
+-----------------------+ USB mass | USB Mass Storage Class| USB mass storage driver | Bulk-Only Transport | storage device +-----------------------+
+----------------+ USB Controller | USB Protocol | USB device +----------------+
In the USB protocol layer, between the USB controller and USB device, at least two USB packets will be transformed when guest OS send a read operation to USB mass storage device:
1. The CBW packet, which will be delivered to the USB device's Bulk-Out endpoint. In order to simulate a read operation, the USB mass storage device parses the CBW and converts it to a SCSI command, which would be executed by CDROM(represented as SCSI disk in QEMU internally), and store the result data of the SCSI command in a buffer.
2. The DATA-IN packet, which will be delivered from the USB device's Bulk-In endpoint(fetched directly from the preceding buffer) to the USB controller.
We consider UHCI to be the controller. The two packets mentioned above may have been processed by UHCI in two separate frame entries of the Frame List , and also described by two different TDs. Unlike the physical environment, a virtualized environment requires the QEMU to make sure that the result data of CBW is not lost and is delivered to the UHCI controller.
Currently, these types of SCSI requests are not migrated, so QEMU cannot ensure the result data of the IO operation is not lost if there are inflight emulated SCSI requests during the live migration.
Assume for the moment that the USB mass storage device is processing the CBW and storing the result data of the read operation to a buffre, live migration happens and moves the VM to the destination while not migrating the result data of the read operation.
After migration, when UHCI at the destination issues a DATA-IN request to the USB mass storage device, a crash happens because USB mass storage device fetches the result data and get nothing.
The scenario this patch addresses is this one.
Theoretically, any device that uses the SCSI disk as a back-end would be affected by this issue. In this case, it is the USB CDROM.
To fix it, inflight emulated SCSI request be migrated during live migration, similar to the DMA SCSI request.
Signed-off-by: Hyman Huang <yong.huang@smartx.com> Message-ID: <878c8f093f3fc2f584b5c31cb2490d9f6a12131a.1716531409.git.yong.huang@smartx.com> [Do not bump migration version, introduce compat property instead. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
75997e18 |
| 04-Jun-2024 |
Kevin Wolf <kwolf@redhat.com> |
scsi-disk: Don't silently truncate serial number
Before this commit, scsi-disk accepts a string of arbitrary length for its "serial" property. However, the value visible on the guest is actually tru
scsi-disk: Don't silently truncate serial number
Before this commit, scsi-disk accepts a string of arbitrary length for its "serial" property. However, the value visible on the guest is actually truncated to 36 characters. This limitation doesn't come from the SCSI specification, it is an arbitrary limit that was initially picked as 20 and later bumped to 36 by commit 48b62063.
Similarly, device_id was introduced as a copy of the serial number, limited to 20 characters, but commit 48b62063 forgot to actually bump it.
As long as we silently truncate the given string, extending the limit is actually not a harmless change, but break the guest ABI. This is the most important reason why commit 48b62063 was really wrong (and it's also why we can't change device_id to be in sync with the serial number again and use 36 characters now, it would be another guest ABI breakage).
In order to avoid future breakage, don't silently truncate the serial number string any more, but just error out if it would be truncated.
Buglink: https://issues.redhat.com/browse/RHEL-3542 Suggested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20240604161755.63448-1-kwolf@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
00a17d80 |
| 12-Apr-2024 |
Philippe Mathieu-Daudé <philmd@linaro.org> |
hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1. Using qemu_hexdump_line both fixes the deprecation warning and simplifies t
hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1. Using qemu_hexdump_line both fixes the deprecation warning and simplifies the code base.
Note that this drops the "0x" prefix to every byte, which should be of no consequence to tracing.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20240412073346.458116-9-richard.henderson@linaro.org>
show more ...
|
#
2d7b39a6 |
| 20-Dec-2023 |
Richard Henderson <richard.henderson@linaro.org> |
hw/scsi: Constify VMState
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20231221031652.119827-52-richard.henderson@linaro.org>
|
#
e7fc3c4a |
| 05-Dec-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
scsi: remove outdated AioContext lock comment
The SCSI subsystem no longer uses the AioContext lock. Request processing runs exclusively in the BlockBackend's AioContext since "scsi: only access SCS
scsi: remove outdated AioContext lock comment
The SCSI subsystem no longer uses the AioContext lock. Request processing runs exclusively in the BlockBackend's AioContext since "scsi: only access SCSIDevice->requests from one thread" and hence the lock is unnecessary.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID: <20231205182011.1976568-13-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
4f36b138 |
| 05-Dec-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
scsi: remove AioContext locking
The AioContext lock no longer has any effect. Remove it.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID:
scsi: remove AioContext locking
The AioContext lock no longer has any effect. Remove it.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID: <20231205182011.1976568-9-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
10bcb0d9 |
| 05-Dec-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
scsi: assert that callbacks run in the correct AioContext
Since the removal of AioContext locking, the correctness of the code relies on running requests from a single AioContext at any given time.
scsi: assert that callbacks run in the correct AioContext
Since the removal of AioContext locking, the correctness of the code relies on running requests from a single AioContext at any given time.
Add assertions that verify that callbacks are invoked in the correct AioContext.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20231205182011.1976568-3-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
14042268 |
| 04-Dec-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
scsi: don't lock AioContext in I/O code path
blk_aio_*() doesn't require the AioContext lock and the SCSI subsystem's internal state also does not anymore.
Signed-off-by: Stefan Hajnoczi <stefanha@
scsi: don't lock AioContext in I/O code path
blk_aio_*() doesn't require the AioContext lock and the SCSI subsystem's internal state also does not anymore.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20231204164259.1515217-4-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
be2b619a |
| 13-Sep-2023 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: ensure that FORMAT UNIT commands are terminated
Otherwise when a FORMAT UNIT command is issued, the SCSI layer can become confused because it can find itself in the situation where it thi
scsi-disk: ensure that FORMAT UNIT commands are terminated
Otherwise when a FORMAT UNIT command is issued, the SCSI layer can become confused because it can find itself in the situation where it thinks there is still data to be transferred which can cause the next emulated SCSI command to fail.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Fixes: 6ab71761 ("scsi-disk: add FORMAT UNIT command") Tested-by: Thomas Huth <thuth@redhat.com> Message-ID: <20230913204410.65650-4-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
7cfcc79b |
| 25-Sep-2023 |
Thomas Huth <thuth@redhat.com> |
hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]
We are doing things like
nb_sectors /= (s->qdev.blocksize / BDRV_SECTOR_SIZE);
in the code here (e.g. in scsi_disk_emu
hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]
We are doing things like
nb_sectors /= (s->qdev.blocksize / BDRV_SECTOR_SIZE);
in the code here (e.g. in scsi_disk_emulate_mode_sense()), so if the blocksize is smaller than BDRV_SECTOR_SIZE (=512), this crashes with a division by 0 exception. Thus disallow block sizes of 256 bytes to avoid this situation.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1813 CVE: 2023-42467 Signed-off-by: Thomas Huth <thuth@redhat.com> Message-ID: <20230925091854.49198-1-thuth@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
766aa2de |
| 16-May-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
virtio-scsi: implement BlockDevOps->drained_begin()
The virtio-scsi Host Bus Adapter provides access to devices on a SCSI bus. Those SCSI devices typically have a BlockBackend. When the BlockBackend
virtio-scsi: implement BlockDevOps->drained_begin()
The virtio-scsi Host Bus Adapter provides access to devices on a SCSI bus. Those SCSI devices typically have a BlockBackend. When the BlockBackend enters a drained section, the SCSI device must temporarily stop submitting new I/O requests.
Implement this behavior by temporarily stopping virtio-scsi virtqueue processing when one of the SCSI devices enters a drained section. The new scsi_device_drained_begin() API allows scsi-disk to message the virtio-scsi HBA.
scsi_device_drained_begin() uses a drain counter so that multiple SCSI devices can have overlapping drained sections. The HBA only sees one pair of .drained_begin/end() calls.
After this commit, virtio-scsi no longer depends on hw/virtio's ioeventfd aio_set_event_notifier(is_external=true). This commit is a step towards removing the aio_disable_external() API.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230516190238.8401-19-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v8.0.0 |
|
#
abfcd276 |
| 21-Feb-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race
dma_blk_cb() only takes the AioContext lock around ->io_func(). That means the rest of dma_blk_cb() is not protected. In particular, the DM
dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race
dma_blk_cb() only takes the AioContext lock around ->io_func(). That means the rest of dma_blk_cb() is not protected. In particular, the DMAAIOCB field accesses happen outside the lock.
There is a race when the main loop thread holds the AioContext lock and invokes scsi_device_purge_requests() -> bdrv_aio_cancel() -> dma_aio_cancel() while an IOThread executes dma_blk_cb(). The dbs->acb field determines how cancellation proceeds. If dma_aio_cancel() sees dbs->acb == NULL while dma_blk_cb() is still running, the request can be completed twice (-ECANCELED and the actual return value).
The following assertion can occur with virtio-scsi when an IOThread is used:
../hw/scsi/scsi-disk.c:368: scsi_dma_complete: Assertion `r->req.aiocb != NULL' failed.
Fix the race by holding the AioContext across dma_blk_cb(). Now dma_aio_cancel() under the AioContext lock will not see inconsistent/intermediate states.
Cc: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230221212218.1378734-3-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
7b7fc3d0 |
| 21-Feb-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
scsi: protect req->aiocb with AioContext lock
If requests are being processed in the IOThread when a SCSIDevice is unplugged, scsi_device_purge_requests() -> scsi_req_cancel_async() races with I/O c
scsi: protect req->aiocb with AioContext lock
If requests are being processed in the IOThread when a SCSIDevice is unplugged, scsi_device_purge_requests() -> scsi_req_cancel_async() races with I/O completion callbacks. Both threads load and store req->aiocb. This can lead to assert(r->req.aiocb == NULL) failures and undefined behavior.
Protect r->req.aiocb with the AioContext lock to prevent the race.
Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230221212218.1378734-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c86422c5 |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since the callback traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Because now this function creates a new coroutine and polls, we need to take the AioContext lock where it is missing, for the only reason that internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls in bdrv_open_driver, because this function has so many callers in so many context that it can easily lead to deadlocks. Therefore the new rule for bdrv_open_driver is that the caller must always hold the AioContext lock of the given bs (except if it is a coroutine), because the function calls bdrv_refresh_total_sectors() which is now a co_wrapper.
Once the rwlock is ultimated and placed in every place it needs to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext lock.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-7-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.2.0 |
|
#
298c31de |
| 04-Aug-2022 |
John Millikin <john@john-millikin.com> |
scsi-disk: support setting CD-ROM block size via device options
SunOS expects CD-ROM devices to have a block size of 512, and will fail to mount or install using QEMU's default block size of 2048.
scsi-disk: support setting CD-ROM block size via device options
SunOS expects CD-ROM devices to have a block size of 512, and will fail to mount or install using QEMU's default block size of 2048.
When initializing the SCSI device, allow the `physical_block_size' block device option to override the default block size.
Signed-off-by: John Millikin <john@john-millikin.com> Message-Id: <20220804122950.1577012-1-john@john-millikin.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
fe9d8927 |
| 17-Aug-2022 |
John Millikin <john@john-millikin.com> |
scsi: Add buf_len parameter to scsi_req_new()
When a SCSI command is received from the guest, the CDB length implied by the first byte might exceed the number of bytes the guest sent. In this case s
scsi: Add buf_len parameter to scsi_req_new()
When a SCSI command is received from the guest, the CDB length implied by the first byte might exceed the number of bytes the guest sent. In this case scsi_req_new() will read uninitialized data, causing unpredictable behavior.
Adds the buf_len parameter to scsi_req_new() and plumbs it through the call stack.
Signed-off-by: John Millikin <john@john-millikin.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127 Message-Id: <20220817053458.698416-1-john@john-millikin.com> [Fill in correct length for adapters other than ESP. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
55794c90 |
| 30-Jul-2022 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: ensure block size is non-zero and changes limited to bits 8-15
The existing code assumes that the block size can be generated from p[1] << 8 in multiple places which ignores the top and b
scsi-disk: ensure block size is non-zero and changes limited to bits 8-15
The existing code assumes that the block size can be generated from p[1] << 8 in multiple places which ignores the top and bottom 8 bits. If the block size is allowed to be set to an arbitrary value then this causes a mismatch between the value written by the guest in the block descriptor and the value subsequently read back using READ CAPACITY causing the guest to generate requests that can crash QEMU.
For now restrict block size changes to bits 8-15 and also ignore requests to set the block size to 0 which causes the SCSI emulation to crash in at least one place with a divide by zero error.
Fixes: 356c4c441e ("scsi-disk: allow MODE SELECT block descriptor to set the block size") Closes: https://gitlab.com/qemu-project/qemu/-/issues/1112 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220730122656.253448-3-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
54a53a00 |
| 30-Jul-2022 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: fix overflow when block size is not a multiple of BDRV_SECTOR_SIZE
In scsi_disk_emulate_write_same() the number of host sectors to transfer is calculated as (s->qdev.blocksize / BDRV_SECT
scsi-disk: fix overflow when block size is not a multiple of BDRV_SECTOR_SIZE
In scsi_disk_emulate_write_same() the number of host sectors to transfer is calculated as (s->qdev.blocksize / BDRV_SECTOR_SIZE) which is then used to copy data in block size chunks to the iov buffer.
Since the loop copying the data to the iov buffer uses a fixed increment of s->qdev.blocksize then using a block size that isn't a multiple of BDRV_SECTOR_SIZE introduces a rounding error in the iov buffer size calculation such that the iov buffer copy overflows the space allocated.
Update the iov buffer copy for() loop so that it will use the smallest of either the current block size or the remaining transfer count to prevent the overflow.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220730122656.253448-2-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
356c4c44 |
| 22-Jun-2022 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: allow MODE SELECT block descriptor to set the block size
The MODE SELECT command can contain an optional block descriptor that can be used to set the device block size. If the block descr
scsi-disk: allow MODE SELECT block descriptor to set the block size
The MODE SELECT command can contain an optional block descriptor that can be used to set the device block size. If the block descriptor is present then update the block size on the SCSI device accordingly.
This allows CDROMs to be used with A/UX which requires a CDROM drive which is capable of switching from a 2048 byte sector size to a 512 byte sector size.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-13-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
4536fba0 |
| 22-Jun-2022 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: allow the MODE_PAGE_R_W_ERROR AWRE bit to be changeable for CDROM drives
A/UX sends a MODE_PAGE_R_W_ERROR command with the AWRE bit set to 0 when enumerating CDROM drives. Since the bit i
scsi-disk: allow the MODE_PAGE_R_W_ERROR AWRE bit to be changeable for CDROM drives
A/UX sends a MODE_PAGE_R_W_ERROR command with the AWRE bit set to 0 when enumerating CDROM drives. Since the bit is currently hardcoded to 1 then indicate that the AWRE bit can be changed (even though we don't care about the value) so that the MODE_PAGE_R_W_ERROR page can be set successfully.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-12-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
#
389e18eb |
| 22-Jun-2022 |
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> |
scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh
When A/UX configures the CDROM device it sends a truncated MODE SELECT request for page 1 (MODE_PAGE_R_W_ERROR) which is only 6
scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh
When A/UX configures the CDROM device it sends a truncated MODE SELECT request for page 1 (MODE_PAGE_R_W_ERROR) which is only 6 bytes in length rather than 10. This seems to be due to bug in Apple's code which calculates the CDB message length incorrectly.
The work at [1] suggests that this truncated request is accepted on real hardware whereas in QEMU it generates an INVALID_PARAM_LEN sense code which causes A/UX to get stuck in a loop retrying the command in an attempt to succeed.
Alter the mode page request length check so that truncated requests are allowed if the SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk is enabled, whilst also adding a trace event to enable the condition to be detected.
[1] https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-10-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|