Re: [Qemu-devel] [PATCH 4/4] blocksize: add blkconf_blocksize call to all block devices

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 03/09/14 17:46, Stefan Hajnoczi wrote:
> On Tue, Jul 29, 2014 at 02:27:19PM +0200, Ekaterina Tumanova wrote:
>> This patch add the blkconf_blocksize call to all
>> devices, which use DEFINE_BLOCK_PROPERTIES.
>> If the underlying driver function fails, blkconf_blocksizes
>> will set blocksizes to default (512) value.
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> ---
>>  hw/block/nvme.c          | 1 +
>>  hw/block/virtio-blk.c    | 1 +
>>  hw/ide/qdev.c            | 1 +
>>  hw/scsi/scsi-disk.c      | 1 +
>>  hw/usb/dev-storage.c     | 1 +
>>  include/hw/block/block.h | 4 ++--
>>  6 files changed, 7 insertions(+), 2 deletions(-)
> 
> Wasn't this NACKed before on the grounds that it is likely to upset the

Not yet ;-) (just other other comments)

> guest after live migration?  QEMU doesn't automatically query the
> storage because these parameters must be preserved across migration.

Would it be more acceptable if this auto detection is only running on init,
but on migration the target will use the block size of the source?

> 
> The knowledge of these fields belongs in the management tool that
> orchestrates migration, not QEMU.

I think the main problem that this patch tries to address is, is not migration. 
It tries to make the whole guest work an pass-through dasd. It guess that this problem
did not happen on x86 yet as most disks are 512 and most file system will cope with
sector size changes.
Maybe this will change as soon as you run a guest on a real disk (no image file) and
the disk happens to be a 4Kn disk.

Question is: Do we want all users to specify the block size of the underlying disk,
just because its a 4Kn-type disk?

Or in other words:
shall we force everybody to change
qemu-system-x86_64 -hda /dev/sdc

into 
qemu-system-x86_64 -drive file=/dev/sdc,if=none,id=mydisk -device ide-drive,bus=ide.0,drive=mydisk,physical_block_size=4096
for a 4Kn device?

Or for the libvirt case, we need
   <blockio logical_block_size='4096' physical_block_size='4096'/>

Or is your suggestion to let libvirt detect the block size for host devices?


> If you want specific parameters, please put them in your guest
> configuration.  QEMU and libvirt support that.
> 
> I'm concerned that this patch serious is likely to break things and
> autodetection doesn't add much value since the management tool needs to
> be aware of this information anyway.

I am totally with you, that we should make this patch in a way to not break anything on other platforms.

Christian