On 11/28/2017 07:56 PM, Kevin Wolf wrote:
> Am 28.11.2017 um 13:10 hat Denis V. Lunev geschrieben:
>> There is the following crash reported from the field in QEMU 2.9:
>> bdrv_inc_in_flight (bs=bs@entry=0x0)
>> blk_aio_prwv
>> blk_aio_preadv
>> ide_buffered_readv
>> cd_read_sector
>> ide_data_readw
>> portio_read
>> memory_region_read_accessor
>> access_with_adjusted_size
>> memory_region_dispatch_read1
>> memory_region_dispatch_read
>> address_space_read_continue
>> address_space_read_full
>> address_space_read
>> address_space_rw
>> kvm_handle_io
>> kvm_cpu_exec
>> qemu_kvm_cpu_thread_fn
>> start_thread
>> clone
>> Indeed, the CDROM device without media has blk->bs == NULL. We should
>> check that the media is really available for the device like has been done
>> in SCSI code.
>>
>> May be the patch adds a bit more check than necessary, but this is not be
>> the problem. We should always stay on the safe side.
>>
>> Signed-off-by: Denis V. Lunev <d...@openvz.org>
>> CC: John Snow <js...@redhat.com>
>> CC: Kevin Wolf <kw...@redhat.com>
>> CC: Stefan Hajnoczi <stefa...@redhat.com>
>> ---
>> hw/ide/atapi.c | 32 ++++++++++++++++++++++++++++----
>> hw/ide/core.c | 4 ++--
>> 2 files changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index c0509c8bf5..fa50c0ccf6 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -119,6 +119,11 @@ cd_read_sector_sync(IDEState *s)
>>
>> trace_cd_read_sector_sync(s->lba);
>>
>> + if (!blk_is_available(s->blk)) {
>> + ret = -ENOMEDIUM;
>> + goto fail;
>> + }
>> +
>> switch (s->cd_sector_size) {
>> case 2048:
>> ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
>> @@ -132,8 +137,8 @@ cd_read_sector_sync(IDEState *s)
>> }
>> break;
>> default:
>> - block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
>> - return -EIO;
>> + ret = -EIO;
>> + goto fail;
>> }
>>
>> if (ret < 0) {
>> @@ -145,6 +150,10 @@ cd_read_sector_sync(IDEState *s)
>> }
>>
>> return ret;
>> +
>> +fail:
>> + block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
>> + return ret;
>> }
> I'm not sure if we can do anything about blk_aio_* in the short time
> that we have until 2.11, so maybe we need to fix some callers like IDE
> (we won't catch all of them anyway), but at least the synchronous one
> should be easily handled in blk_prwv() by returning -ENOMEDIUM before we
> access blk_bs(blk).
>
> AIO is trickier because we need to schedule a BH, and blk_drain() can't
> execute that BH yet unless we increase bs->in_flight - which we
> obviously can't do for a NULL bs->in_flight. The proper solution
> involves a separate blk->in_flight counter for the BB and a blk_drain()
> implementation that considers it.
>
> Kevin
I have double checked that SCSI code is correct.
AHCI comes through IDE thus with this patch applied
we will be safe at emulation layer.
Den
