On 06/22/2016 04:05 AM, Kevin Wolf wrote: >>>> /* Set request alignment */ >>>> - align = qemu_opt_get_size(opts, "align", bs->request_alignment); >>>> - if (align > 0 && align < INT_MAX && !(align & (align - 1))) { >>>> - bs->request_alignment = align; >>>> + align = qemu_opt_get_size(opts, "align", >>>> + bs->request_alignment ?: BDRV_SECTOR_SIZE); >>>
>>> I think you should either just unconditionally use BDRV_SECTOR_SIZE or >>> even better just store 0 in s->align if the option isn't given. Your >>> implementation of blkdebug_refresh_limits() already leaves the default >>> bs->request_alignment alone if s->align == 0. >> >> I like that idea; I guess that means I instead need to tweak the logic >> to test if "align" is present in opts (in which case it must be >> non-zero), or absent (in which case leaving things as 0 is a nicer >> approach than trying to guess which default to stick things to). > > Except that you don't have to check all of that explicitly, the default > value for absent options is what the third parameter is for: > > align = qemu_opt_get_size(opts, "align", 0); Previously, we explicitly reject the user passing in an explicit 'align':0. If I go with this approach of 0 as the default, then I may need to tweak docs. But that sounds better, so I'll try it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature