Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Fri, Nov 29, 2019 at 08:59:39AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> > +/**
> > + * popen_write - write data to the child stdin
> > + * @handle:	popen handle
> > + * @buf:	data to write
> > + * @count:	number of bytes to write
> > + * @flags:	a flag representing stdin peer
> > + *
> > + * Returns number of bytes written or -1 on error.
> > + */
> > +ssize_t
> > +popen_write(struct popen_handle *handle, void *buf,
> > +	    size_t count, unsigned int flags)
> > +{
> > +	if (!popen_may_io(handle, STDIN_FILENO, flags))
> > +		return -1;
> > +
> > +	say_debug("popen: %d: write idx %d",
> > +		  handle->pid, STDIN_FILENO);
> > +
> > +	return write(handle->fds[STDIN_FILENO], buf, count);
> 
> I don't understand, is this blocking or not?

It does block "coio" thread, similar to the regular fio.write().

> If this is blocking, where is it supposed to be called from?

It is called from coio, with default priority and wait, 1:1
mapping to fio.write()

> Shouldn't you be using a non-blocking I/O like coio does?

Actually I fear I don't understand this moment -- I wrapped
the popen_write into coio_custom calls, this is what you
mean, or womething else?

> 
> Besides, even though I realize it's a pipe, so real world
> return values may be different, but write() can return a partial
> result even in blocking mode. Why did you choose to design an API
> which will require caller to handle partial writes?

It is first draft to gather comments. I already fixed a similar
issue for regular fio.write() (the patch is flying around) and
will address the partial writes on the next series.

But I think you noticed already that our fio.write() is simply
broken by design -- we do return true/false but instead we should
return bytes written to the caller. It is always up to the caller
what to do with partial writes because only the caller knowns the
context of the call. Can he proceed with partial writes, or he
should abort the whole write and set offset back to the original
position? For this reason exactly the system call return the number
of bytes written not true/false.

Anyway I have to address partial writes.

	Cyrill