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

[Ekaterina Tumanova]

On 09/03/2014 07:46 PM, 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 <address@hidden>
> > Reviewed-by: David Hildenbrand <address@hidden>
> > Acked-by: Cornelia Huck <address@hidden>
> > ---
> >   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
> guest after live migration?  QEMU doesn't automatically query the
> storage because these parameters must be preserved across migration.

Sorry, haven't found this discussion in the mail list. Do you have a link?

As far as I understand, the xxxxxx_init functions of the qemu block 
devices, which contain blkconf_blocksize calls, will
be called again on the destination host before the guest is resumed. And 
since migration requests qemu to be brought on the same disk, the 
configuration will receive the same block size from the ioctl, as
before. What do I miss?

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

For case of DASDs we need QEMU to know the underlying blocksize.
And you mentioned in your review comment to the Einar's initial patch
that you request this to be implemented for all architectures:

"Detecting the underlying block size is a generally useful configuration
option.  This should not be s390-specific, so no need to rename
DEFINE_BLOCK_PROPERTIES()."

http://qemu.11.n7.nabble.com/Qemu-devel-PATCH-V3-0-2-hd-geometry-c-Integrate-HDIO-GETGEO-in-guessing-tt185124.html#none

> 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.

Can you please explain what do you mean by "AUTOdetection"?
Do you simply mean "detection by ioctl" or "detection performed without
guest request"?

> Stefan

Thanks!