[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 1/4] block: Pass bdrv_* methods to bs->file by default

[adding libvirt for block size discussion]

On 07/04/2017 04:44 AM, Kevin Wolf wrote:
> Am 04.07.2017 um 10:40 hat Manos Pitsidianakis geschrieben:
>> The following functions fail if bs->drv does not implement them:
>> bdrv_probe_blocksizes
>> bdrv_probe_geometry
>> bdrv_truncate
>> bdrv_has_zero_init
>> bdrv_get_info
>> bdrv_media_changed
>> bdrv_eject
>> bdrv_lock_medium
>> bdrv_co_ioctl
>> Instead, the call should be passed to bs->file if it exists, to allow
>> filter drivers to support those methods without implementing them.

>> +        return bdrv_probe_blocksizes(bs->file->bs, bsz);
>>      }
>>      return -ENOTSUP;
> This change actually changes existing behaviour. Basically, all of your
> additions also apply for qcow2 images or other image formats, and we
> need to check whether they make sense there.
> bdrv_probe_blocksizes() is used for the default values for the physical
> and logical block sizes for block devices if no value is explicitly
> specified in -device. This makes sense as it will allow the guest to
> optimise its requests so that they align with the block size that is
> optimal for the storage that contains the image. (I also think that we
> probably should implement .bdrv_probe_blocksizes() not only for
> host_device, but also for regular files to get optimal performance by
> default.)
> Changing the defaults can be a problem for cross-version live migration
> if the default values are used. Management tools that want to do live
> migration are supposed to explicitly provide all options they can, so
> this should be okay.
> Eric, does libvirt set the physical/logical block size explicitly? If
> not, it would be good if it started to do so.

Libvirt supports user control of block size via this XML in <disk>:
    <blockio logical_block_size='512' physical_block_size='4096'/>
which it translates to command-line logical_block_size= and
physical_block_size=.  But I didn't see anywhere that libvirt tries to
do anything with block size in QMP, which makes me wonder if that is a
libvirt bug for not allowing a block size when hot-plugging a drive, or
a weakness in QMP.

Your point that if libvirt isn't always specifying block size, then we
risk the block size changing on migration (that is, libvirt should be
patched to specify block size always, and not just when the XML
specifies it).

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]