On Fri, Aug 18, 2017 at 11:22:39AM +0200, Karel Zak wrote: > On Fri, Aug 18, 2017 at 01:22:26AM -0700, Omar Sandoval wrote: > > On Fri, Aug 18, 2017 at 10:12:51AM +0200, Hannes Reinecke wrote: > > > On 08/18/2017 10:05 AM, Omar Sandoval wrote: > > > > On Fri, Aug 18, 2017 at 09:56:26AM +0200, Hannes Reinecke wrote: > > > >> On 08/18/2017 09:47 AM, Omar Sandoval wrote: > > > [ .. ] > > > >>> > > > >>> I actually checked losetup, it works just fine with LO_FLAGS_BLOCKSIZE > > > >>> always set and lo_init[0] always filled in. > > > >>> > > > >> The original argument I had with the util-linux maintainer did not > > > >> revolve so much around technical details :-) > > > > > > > > Karel, what were your concerns here? > > > > > > > It wasn't Karel, it was our guy. > > > Doesn't make it any better, though... > > > > I just went through the code and util-linux doesn't mention lo_init at > > all except for the definition, and everywhere it's using lo_flags would > > work fine with the behavior I implemented here. Unless there's an actual > > issue someone can point out, I see no reason to not do it this way. > > Hmm.. we have loopdev API enhancement in Linus' tree and losetup > maintainer have no clue about it ;-) Ha, glad you're hearing about it now before it's too late :) > Anyway, I like the idea with LO_FLAGS_BLOCKSIZE and lo_init[]. It > seems like a backwardly compatible way how to make loopdevs usable > for dd(1) images from non-512 devices, etc. > > For now nothing uses lo_init[], so it seems no problem to use it for > LOOP_GET_STATUS64. Do you see any issues with the behavior I added in this patch? Namely, LOOP_GET_STATUS{,64} will always return lo_flags with LO_FLAGS_BLOCKSIZE set and lo_init[0] will always contain the blocksize. Older userspace which doesn't know about LO_FLAGS_BLOCKSIZE will just ignore it and the LOOP_GET_STATUS64; modify stuff; LOOP_SET_STATUS64 pattern will still work. > Note that we use LOOP_GET_STATUS64 and struct loop_info64 only, old API > around "struct loop_info" is no more used by util-linux. > > The important detail is that for losetup (to show mapped devices) is > more important /sys than LOOP_GET_STATUS64. The /sys is better > because it does not require root permissions and it makes losetup > (and lsblk) usable for regular users. Ah, I'll spin a followup patch to add it to sysfs. > I guess /sys/block/loopN/queue/minimum_io_size is affected by Hannes' > LO_FLAGS_BLOCKSIZE patches, so non-512 I/O sizes for loopdevs are > visible in "lsblk -t /dev/loopN" output, right? Yup: # lsblk -t /dev/loop0 NAME ALIGNMENT MIN-IO OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE RA WSAME loop0 0 4096 0 4096 4096 1 mq-deadline 256 128 0B > I'll add --blocksize <size> to losetup, and BLKSZ column to output to > make the new feature usable also for end-users. Thank you!