Re: [Qemu-devel] [RFC] Fix SCSI hotplug with invalid slot

[Kevin Wolf]

[ Whoops, forgot qemu-devel in my original mail, added now ]

Am 12.07.2012 20:11, schrieb Anthony Liguori:
> Kevin Wolf <address@hidden> writes:
> 
>> While trying to hotplug an if=scsi disk with drive_add, I didn't pay
>> attention to using a valid slot (it doesn't matter with the usual
>> if=none...), and so I got:
>>
>> (qemu) drive_add 0 file=/tmp/test.qcow2,if=scsi
>> Segmentation fault.
>>
>> qemu just takes the PCI device at slot 0 and starts working on its first
>> child bus, no matter what device it is, and whether it even has a bus.
>> This NULL pointer access is easy enough to fix, it's what this patch
>> does.
>>
>> However this leaves a second case where the device in the slot does
>> exist, has child buses and still isn't a SCSI bus. For example (here
>> it's IDE):
>>
>> (qemu) drive_add 1 file=/tmp/test.qcow2,if=scsi
>> Object 0x7fadb204bbf0 is not an instance of type SCSI
>> Aborted.
>>
>> I couldn't find any obvious solution for checking if it has the right
>> type without aborting. I'm sure that this is a pretty standard case, but
>> my QOM knowledge is lacking...
>>
>> Signed-off-by: Kevin Wolf <address@hidden>
>> ---
>>  hw/pci-hotplug.c |    8 +++++++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
>> index e7fb780..87d4721 100644
>> --- a/hw/pci-hotplug.c
>> +++ b/hw/pci-hotplug.c
>> @@ -77,10 +77,16 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
>>  static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
>>                          DriveInfo *dinfo, int printinfo)
>>  {
>> +    BusState *bus;
>>      SCSIBus *scsibus;
>>      SCSIDevice *scsidev;
>>  
>> -    scsibus = SCSI_BUS(QLIST_FIRST(&adapter->child_bus));
>> +    bus = QLIST_FIRST(&adapter->child_bus);
>> +    if (bus == NULL) {
> 
> if (bus == NULL || object_dynamic_cast(bus, TYPE_SCSI_BUS) == NULL) {

Ah well, so this is really the official way... Then I could as well use
the result of that instead of calling it a second time in SCSI_BUS().

Should I send an updated patch for this, or is it a more general problem
of the QOM conversions that checks were lost and you'll post a broader fix?

Kevin