[Ocfs2-tools-devel] [PATCH 4/6] O2info: Add running codes for '--fs-features'.

[Joel Becker]

On Fri, Apr 23, 2010 at 03:34:53PM -0700, Sunil Mushran wrote:
> > +	rc = ioctl(fd, OCFS2_IOC_INFO, &info);
> > +	if (rc) {
> > +		rc = errno;
> > +		o2i_error(op, "ioctl failed: %s\n", strerror(rc));
> > +		goto out;
> > +	}
> > +
> > +	if (oif.if_req.ir_flags & OCFS2_INFO_FL_FILLED) {
> > +		ofs->compat = oif.if_compat_features;
> > +		ofs->incompat = oif.if_incompat_features;
> > +		ofs->rocompat = oif.if_ro_compat_features;
> > +	}
> > +
> > +out:
> > +	return rc;
> > +}
> >   
> 
> What happens if rc == 0 and flags is not FILLED. Best if
> we ensure that cannot happen. As in, kernel should return
> an error if it is not filled.

	He's correct here.  Remember, we allow !FILLED for forwards
compatibility.  The kernel skips requests it doesn't understand rather
than returning an error.  So if you pass N requests, you have to check
each one to see if FILLED was set to determine whether the kernel
understood it.  The 0 just means there was no actual error.
	But this brings up a good point.  As currently defined, we
return our first error.  Imagine that we fill the first six requests out
of an info_count of eight.  Then the seventh request has a bad pointer.
Currently we return -EFAULT, and the caller misses the six finished
requests.
	I can think of three solutions.  First, we can just leave it
as-is - if there is any error, you get an error and assume nothing is
filled in.
	Next, we could go with read(2) semantics.  Rather than returning
0 on success, we return the number of filled requests.  We only return
an error if we had the error before we filled any request.  Say you have
two requests.  The first request is successfully filled.  The second has
a bad pointer.  You return 1.  The caller tries again passing only the
second request.  Now they get -EFAULT.  Unknown requests are skipped.
So if you pass in eight requests, and the call returns 4, you only
need to walk the requests until you've found four that were FILLED.
	The biggest downside with this method is the same as for
read(2): if you get a short count, you have to call again to find out
why.

--8<-------------------------------------------------------------------
retry:
	rc = ioctl(fd, INFO, &info);
	if (rc < 0)
		goto error;
	assert(rc <= info.info_coutn);
	for (i = 0, seen = 0; i < rc; i++) {
		if (reqs[i].ir_flags & FILLED) {
			/* do something */
			seen++;
		}
	}
	/* Did we get to the end */
	if ((rc < info.info_count) && (i < info.info_count)) {
		info.info_requests = info.info_requests + i;
		info.info_count -= info.info_count;
		goto retry;
	}
-->8-------------------------------------------------------------------

	That's pretty clunky for every user to do.  Of course, we can
wrap in the library somewhat, so that the retry is handled by the
library car.
	Finally, we could do it ASMLib style.  Add req.ir_error and
OCFS2_INFO_FL_ERROR.  Here, we only return an error from the ioctl(2)
when the failure was in the ioctl(2) call itself.  So the ioctl(2)
returns 0 if it exists and the info structure is valid.  Any error on a
particular request is set in ir_error; this causes FL_ERROR to be set.
	The caller now walks the requests and can determined that
FL_ERROR is an error, FL_FILLED is completed, and neither means the
request wasn't supported.

Joel

-- 

 The herd instinct among economists makes sheep look like
 independent thinkers.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127