Am 24.06.2016 um 16:15 hat Eric Blake geschrieben: > On 06/24/2016 12:43 AM, Fam Zheng wrote: > > On Thu, 06/23 16:37, Eric Blake wrote: > >> Sector-based limits are awkward to think about; in our on-going > >> quest to move to byte-based interfaces, convert max_discard and > >> discard_alignment. Rename them, using 'pdiscard' as an aid to > >> track which remaining discard interfaces need conversion, and so > >> that the compiler will help us catch the change in semantics > >> across any rebased code. The BlockLimits type is now completely > >> byte-based; and in iscsi.c, sector_limits_lun2qemu() is no > >> longer needed. > >> > >> pdiscard_alignment is made unsigned (we use power-of-2 alignments > >> as bitmasks, where unsigned is easier to think about) while > >> leaving max_pdiscard signed (since we still have an 'int' > >> interface); this is comparable to what commit cf081fc did for > >> write zeroes limits. We may later want to make everything an > >> unsigned 64-bit limit - but that requires a bigger code audit. > >> > > >> - /* optimal alignment for discard requests in sectors */ > >> - int64_t discard_alignment; > >> + /* optimal alignment for discard requests in bytes, must be power > >> + * of 2, less than max_discard if that is set, and multiple of > > > > s/max_discard/max_pdiscard/ > > Maintainer could touch it up on pull request.
Okay, no problem. > >> /* align request */ > >> - if (bs->bl.discard_alignment && > >> - num >= bs->bl.discard_alignment && > >> - sector_num % bs->bl.discard_alignment) { > >> - if (num > bs->bl.discard_alignment) { > >> - num = bs->bl.discard_alignment; > >> + if (discard_alignment && > >> + num >= discard_alignment && > >> + sector_num % discard_alignment) { > >> + if (num > discard_alignment) { > >> + num = discard_alignment; > >> } > >> - num -= sector_num % bs->bl.discard_alignment; > >> + num -= sector_num % discard_alignment; > > > > Or just > > > > num = discard_alignment - sector_num % discard_alignment; > > > > without the if. > > > > Sure. It all gets simplified later when I switch to bdrv_co_pdiscard(). > Up to the maintainer. This is an actual code change and not a bug fix, so I'll leave this one alone. We can always have a follow-up patch, but as you say your other work will simplify it anyway, I guess that's not necessary. Kevin
pgpRU5yQ17y4_.pgp
Description: PGP signature