Re: [Qemu-devel] [PATCH 2/4] block: add block topology options

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/05/2010 07:09 AM, Christoph Hellwig wrote:
> On Wed, Feb 03, 2010 at 01:00:47PM -0600, Anthony Liguori wrote:
>    
>> But I don't think this is the wrong place to do it.  The
>> BlockDriverState reflects that backing device, not the emulated device
>> itself.  In this case, you're trying to set a property of the emulated
>> device.
>>      
> I think that's very borderline.  While the emulated device exposes these
> properties, they are in fact a property of the backing storage, the
> right sector and min/max I/O sizes are determined by the backing storage
> device.
>    

It's guest visible state that should be configured based on the 
properties of the backing store.

However, as you said, live migration complicates things.

drive is not an optimal interface because it mixes host and guest 
state.  But the subset of -drive that you normally use should be 
independent of the guest.  For instance, with:

-drive file=/home/anthony/foo.img,cache=off,aio=native,id=foo -device 
virtio-blk-pci,drive=foo

file, cache, and aio are all host properties.  They have no visible 
impact to the guest and if I change those properties during live 
migration, they take affect without the guest noticing

These topology options still can be specified via -drive, but the proper 
way of splitting things would have them as part of the -device option.  
During live migration, things on the -device side inherited as part of 
the live migration process.

>> I think these need to be qdev properties of the respective devices.
>>  From a UI perspective, you can still expose -drive options for the end
>> user to consume, but this data should be associated with the devices
>> themselves.
>>      
> In addition to not really beeing more logical this would be a lot more
> effort.  We'd need to add properties to all the device, which means
> including dealing with the n+1 ide variants, the virtio-pci proxy, etc.
>    

The way we deal with this with network devices is we have a common 
DEFINE_NIC_PROPERTIES that lets us consistently add properties for all 
network devices.  We need exactly this sort of thing for disk drives for 
the reason you describe.

> If you believe it really needs to be in the qdev properties I'll
> implement it, but I suspect the current version is a better idea.
>    

It definitely a correctness issue.  Once we can describe the full PC via 
qdev, we want to be able to migrate that description as part of the 
migration traffic.  For that to work, we need to have all of the device 
characteristics expressed as qdev properties.

Regards,

Anthony Liguori