[3/4] SCSI-Hotdel: Implement drive_hot_del

This commit implements the method drive_hot_del() that allows
for hot-removing drives. It also introduces a new
monitor command drive_del, which acts as counterpart
to drive_add.

Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pci-hotplug.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-monitor.hx  |   14 +++++++-
 sysemu.h         |    1 +
 3 files changed, 106 insertions(+), 1 deletions(-)

Hi,

> +     dinfo = drive_get(type, bus, unit);
> +     if (!dinfo) {
> +         monitor_printf(mon, "Trying to remove non-existent device\n");
> +	 return;
> +     }

No.  Just don't do this silly if/bus/unit parsing.  At very minimum use 
drive_get_by_id() here, then have something like 'drive_del $id'.

IMHO much better would be to go qdev instead though.  We should have 
generic device_add + device_del monitor commands which work for any 
device, pretty much like the -device command line switch.

cheers,
   Gerd

Gerd Hoffmann wrote:
>    Hi,
> 
>> +     dinfo = drive_get(type, bus, unit);
>> +     if (!dinfo) {
>> +         monitor_printf(mon, "Trying to remove non-existent device\n");
>> +	 return;
>> +     }
> 
> No.  Just don't do this silly if/bus/unit parsing.  At very minimum use 
> drive_get_by_id() here, then have something like 'drive_del $id'.
> 
> IMHO much better would be to go qdev instead though.  We should have 
> generic device_add + device_del monitor commands which work for any 
> device, pretty much like the -device command line switch.

That makes sense, but I'd nevertheless prefer to stick with the
more traditional approach right now, replacing the
bus/unit parsing with an ID-based variant. However, is there any
standard way to get from an instance of DriveInfo to
the corresponding instance of SCSIDevice respectively
SCSIDeviceInfo? It's a bit unclear to me if there is
no such connection, of if I'm just overlooking something.

Thanks, Wolfgang

On 09/30/09 17:45, Wolfgang Mauerer wrote:
> Gerd Hoffmann wrote:
>>     Hi,
>>
>>> +     dinfo = drive_get(type, bus, unit);
>>> +     if (!dinfo) {
>>> +         monitor_printf(mon, "Trying to remove non-existent device\n");
>>> +	 return;
>>> +     }
>>
>> No.  Just don't do this silly if/bus/unit parsing.  At very minimum use
>> drive_get_by_id() here, then have something like 'drive_del $id'.
>>
>> IMHO much better would be to go qdev instead though.  We should have
>> generic device_add + device_del monitor commands which work for any
>> device, pretty much like the -device command line switch.
>
> That makes sense, but I'd nevertheless prefer to stick with the
> more traditional approach right now, replacing the
> bus/unit parsing with an ID-based variant.

Have a look at the "qdev: bus management updates." patch series posted a 
few days ago.  It adds device_add+device_del.

> However, is there any
> standard way to get from an instance of DriveInfo to
> the corresponding instance of SCSIDevice respectively
> SCSIDeviceInfo?

No.  It is linked the other way around: The device has a reference to 
the DriveInfo.  Which is one of the reasons why I think drive_del isn't 
that a great idea.

cheers,
   Gerd

Gerd Hoffmann wrote:
> On 09/30/09 17:45, Wolfgang Mauerer wrote:
>> Gerd Hoffmann wrote:
>>>     Hi,
>>>
>>>> +     dinfo = drive_get(type, bus, unit);
>>>> +     if (!dinfo) {
>>>> +         monitor_printf(mon, "Trying to remove non-existent device\n");
>>>> +	 return;
>>>> +     }
>>> No.  Just don't do this silly if/bus/unit parsing.  At very minimum use
>>> drive_get_by_id() here, then have something like 'drive_del $id'.
>>>
>>> IMHO much better would be to go qdev instead though.  We should have
>>> generic device_add + device_del monitor commands which work for any
>>> device, pretty much like the -device command line switch.
>> That makes sense, but I'd nevertheless prefer to stick with the
>> more traditional approach right now, replacing the
>> bus/unit parsing with an ID-based variant.
> 
> Have a look at the "qdev: bus management updates." patch series posted a 
> few days ago.  It adds device_add+device_del.

Thanks for the pointer. The reason why I'd (still :-)) like
to stick to the non-qdev variant is because I would want
the patch to be applicable to older qemu releases for various
reasons, and backporting all the required qdev  changes does
not seem to be a reasonable option (but please tell me if you
think this would make sense)

>> However, is there any
>> standard way to get from an instance of DriveInfo to
>> the corresponding instance of SCSIDevice respectively
>> SCSIDeviceInfo?
> 
> No.  It is linked the other way around: The device has a reference to 
> the DriveInfo.  Which is one of the reasons why I think drive_del isn't 
> that a great idea.

Cheers, Wolfgang

On 10/01/09 09:54, Wolfgang Mauerer wrote:
> Thanks for the pointer. The reason why I'd (still :-)) like
> to stick to the non-qdev variant is because I would want
> the patch to be applicable to older qemu releases for various
> reasons, and backporting all the required qdev  changes does
> not seem to be a reasonable option (but please tell me if you
> think this would make sense)

Yes, backporting all qdev isn't going to fly.

Note that support for looking up drives by name is available in 0.11+ 
only too, so you might have to stick with the lookup by pci address, 
depending on which versions you plan to backport it ...

cheers,
   Gerd