[Qemu-devel] drive_add vs. pci_add

[Qemu-devel] drive_add vs. pci_add

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

Hi,

sorry, it's still early, but isn't the monitor command 'drive_add'
completely redundant to 'pci_add ... storage'? If yes, and drive_add is
only there for legacy users, I would mask its help from the monitor
interface to avoid confusion.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: drive_add vs. pci_add

[Jan Kiszka]

Jan Kiszka wrote:
> Hi,
> 
> sorry, it's still early, but isn't the monitor command 'drive_add'
> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
> only there for legacy users, I would mask its help from the monitor
> interface to avoid confusion.

One coffee later, the picture is clear: drive_add adds a drive to an
existing controller (one should read the doc completely). 'pci_add ...
storage' adds a storage controller, but can also add a first drive to
that controller. The latter feature definitely requires a better
documentation than "Hot-add PCI device".

Jan

Re: [Qemu-devel] drive_add vs. pci_add

[Avi Kivity]

On 06/25/2009 09:46 AM, Jan Kiszka wrote:
> Hi,
>
> sorry, it's still early, but isn't the monitor command 'drive_add'
> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
> only there for legacy users, I would mask its help from the monitor
> interface to avoid confusion.
>    

What we really want is pci_add storage to add a storage controller, and 
drive_add to attach a drive to that controller.  I don't think that's 
what happens now.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] drive_add vs. pci_add

[Paul Brook]

On Thursday 25 June 2009, Avi Kivity wrote:
> On 06/25/2009 09:46 AM, Jan Kiszka wrote:
> > Hi,
> >
> > sorry, it's still early, but isn't the monitor command 'drive_add'
> > completely redundant to 'pci_add ... storage'? If yes, and drive_add is
> > only there for legacy users, I would mask its help from the monitor
> > interface to avoid confusion.
>
> What we really want is pci_add storage to add a storage controller, and
> drive_add to attach a drive to that controller.  I don't think that's
> what happens now.

Part of the problem is that we don't currently isolate configs for different 
pats of the device stack. There are several different layers at which hotplug 
can occur:

- Device - e.g. PCI hotplug. If done properly this shouldn't care whether 
you're adding a NIC, VGA, SCSI HBA, or whatever.
- Drive - Adding/removing drives to an existing HBA.
- Media - e.g. changing the contents of a CDROM drive.

Depending on the subsystem, hotplug may only be supported at some levels. For 
example harddrives don't support media hotplug, IDE+virtio do not support 
drive hotplug, and the sun4m ESP SCSI adapter does not support device 
hotplug[1].

Paul

[1] I've no idea if actual sun4m SBUS hardware supports hotplug, but It 
suspect it does not.

Re: [Qemu-devel] drive_add vs. pci_add

[Anthony Liguori]

Paul Brook wrote:
> On Thursday 25 June 2009, Avi Kivity wrote:
>   
>> On 06/25/2009 09:46 AM, Jan Kiszka wrote:
>>     
>>> Hi,
>>>
>>> sorry, it's still early, but isn't the monitor command 'drive_add'
>>> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
>>> only there for legacy users, I would mask its help from the monitor
>>> interface to avoid confusion.
>>>       
>> What we really want is pci_add storage to add a storage controller, and
>> drive_add to attach a drive to that controller.  I don't think that's
>> what happens now.
>>     
>
> Part of the problem is that we don't currently isolate configs for different 
> pats of the device stack. There are several different layers at which hotplug 
> can occur:
>
> - Device - e.g. PCI hotplug. If done properly this shouldn't care whether 
> you're adding a NIC, VGA, SCSI HBA, or whatever.
> - Drive - Adding/removing drives to an existing HBA.
> - Media - e.g. changing the contents of a CDROM drive.
>   

Also, we probably want to separate the definition of storage from the 
act of connecting the storage via hotplug.  If I were going to add new 
monitor commands today, I may do something like:

# Create a new BlockDriverState with a symbolic name of 'Foo'
(qemu) storage_add Foo file=/path/to/image,snapshot=on

# Create a new pci device with a symbolic name of 'Bar'
(qemu) pci_create dev=virtio-blk,name=Bar

# Connect the storage backend Foo to the storage frontend Bar
(qemu) storage_connect Foo Bar

# Attach PCI device via hotplug
(qemu) pci_hotplug Bar addr=0.01.0

Beyond pci_hotplug, you could also introduce pci_insert/remove 
commands.  This could be used if starting a guest in the stopped state 
to insert a PCI card or in combination with -no-shutdown.

For attaching SCSI disks, you would probably introduce something like:

# Create a new scsi-disk named Bar
(qemu) scsi_create_lun target=0,lun=1,name=Bar

# Connect Foo storage backend to Bar storage frontend
(qemu) storage_connect Foo Bar

# Attach lun 'Bar' to existing SCSI controller named 'Controller'
(qemu) scsi_attach_lun Bar Controller

Regards,

Anthony Liguori

Re: [Qemu-devel] drive_add vs. pci_add

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]

Anthony Liguori wrote:
> Paul Brook wrote:
>> On Thursday 25 June 2009, Avi Kivity wrote:
>>  
>>> On 06/25/2009 09:46 AM, Jan Kiszka wrote:
>>>    
>>>> Hi,
>>>>
>>>> sorry, it's still early, but isn't the monitor command 'drive_add'
>>>> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
>>>> only there for legacy users, I would mask its help from the monitor
>>>> interface to avoid confusion.
>>>>       
>>> What we really want is pci_add storage to add a storage controller, and
>>> drive_add to attach a drive to that controller.  I don't think that's
>>> what happens now.
>>>     
>>
>> Part of the problem is that we don't currently isolate configs for
>> different pats of the device stack. There are several different layers
>> at which hotplug can occur:
>>
>> - Device - e.g. PCI hotplug. If done properly this shouldn't care
>> whether you're adding a NIC, VGA, SCSI HBA, or whatever.
>> - Drive - Adding/removing drives to an existing HBA.
>> - Media - e.g. changing the contents of a CDROM drive.
>>   
> 
> Also, we probably want to separate the definition of storage from the
> act of connecting the storage via hotplug.

What would be the benefit of be able to define detached storages?

>  If I were going to add new
> monitor commands today, I may do something like:
> 
> # Create a new BlockDriverState with a symbolic name of 'Foo'
> (qemu) storage_add Foo file=/path/to/image,snapshot=on
> 
> # Create a new pci device with a symbolic name of 'Bar'
> (qemu) pci_create dev=virtio-blk,name=Bar

Support for creating pci devices and attaching storages offline would
surely be useful.

> 
> # Connect the storage backend Foo to the storage frontend Bar
> (qemu) storage_connect Foo Bar
> 
> # Attach PCI device via hotplug
> (qemu) pci_hotplug Bar addr=0.01.0
> 
> Beyond pci_hotplug, you could also introduce pci_insert/remove
> commands.  This could be used if starting a guest in the stopped state
> to insert a PCI card or in combination with -no-shutdown.

Shouldn't this be handled more generically one day by (re-)reading a
machine configuration (from a file or from a QMP session)?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] drive_add vs. pci_add

[Paul Brook]

On Thursday 25 June 2009, Anthony Liguori wrote:
> Paul Brook wrote:
> > On Thursday 25 June 2009, Avi Kivity wrote:
> >> On 06/25/2009 09:46 AM, Jan Kiszka wrote:
> >>> Hi,
> >>>
> >>> sorry, it's still early, but isn't the monitor command 'drive_add'
> >>> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
> >>> only there for legacy users, I would mask its help from the monitor
> >>> interface to avoid confusion.
> >>
> >> What we really want is pci_add storage to add a storage controller, and
> >> drive_add to attach a drive to that controller.  I don't think that's
> >> what happens now.
> >
> > Part of the problem is that we don't currently isolate configs for
> > different pats of the device stack. There are several different layers at
> > which hotplug can occur:
> >
> > - Device - e.g. PCI hotplug. If done properly this shouldn't care whether
> > you're adding a NIC, VGA, SCSI HBA, or whatever.
> > - Drive - Adding/removing drives to an existing HBA.
> > - Media - e.g. changing the contents of a CDROM drive.
>
> Also, we probably want to separate the definition of storage from the
> act of connecting the storage via hotplug.  If I were going to add new
> monitor commands today, I may do something like:

I'm not sure about the exact sequences you describe, but yes, this is the sort 
of thing I'm getting at.  The exact details need a fair amount of 
consideration, we want to make sure we're not working around creating things 
at the wrong time. For example, you should not be connection SCSI drives to an 
LSI HBA, you should be connecting them to the scsi bus provided by that 
device.

Paul

Re: [Qemu-devel] drive_add vs. pci_add

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

Avi Kivity wrote:
> On 06/25/2009 09:46 AM, Jan Kiszka wrote:
>> Hi,
>>
>> sorry, it's still early, but isn't the monitor command 'drive_add'
>> completely redundant to 'pci_add ... storage'? If yes, and drive_add is
>> only there for legacy users, I would mask its help from the monitor
>> interface to avoid confusion.
>>    
> 
> What we really want is pci_add storage to add a storage controller, and
> drive_add to attach a drive to that controller.  I don't think that's
> what happens now.
> 

The latter should already happen, but we probably need to drop that
"first drive included" from the former. Also, there is no drive_remove.

I'm going to did into the whole hotplug user interface later, it has
some room for improvements.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]