On Fri, Aug 18, 2017 at 09:44:25AM -0700, Omar Sandoval wrote:
> 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.
Yes, I think your idea is correct. All unknown flags should be ignored
by userspace. This is not the first time the API (lo_flags) is extended
by a new flag.
Karel
--
Karel Zak <[email protected]>
http://karelzak.blogspot.com