On Tue, Jan 19, 2016 at 05:08:12PM +0100, Paolo Bonzini wrote:
>
>
> On 19/01/2016 14:09, Daniel P. Berrange wrote:
> > + * This will update @iov so that its head is advanced
> > + * by @skip bytes. To do this, zero or more complete
> > + * elements of @iov will be skipped over. The new head
> > + * of @iov will then have its base & len updated to
> > + * skip the remaining number of bytes. @oldiov will
> > + * hold the original data from the new head of @iov.
> > + */
> > +static void nbd_skip_iov(size_t skip,
> > + struct iovec **iov,
> > + int *niov,
> > + struct iovec *oldiov)
> > {
> > - size_t offset = 0;
> > - int err;
> > -
> > - if (qemu_in_coroutine()) {
> > - if (do_read) {
> > - return qemu_co_recv(fd, buffer, size);
> > + ssize_t done = 0;
> > + size_t i;
> > + for (i = 0; i < *niov; i++) {
> > + if ((*iov)[i].iov_len <= skip) {
> > + done += (*iov)[i].iov_len;
> > + skip -= (*iov)[i].iov_len;
> > } else {
> > - return qemu_co_send(fd, buffer, size);
> > + done += skip;
> > + oldiov->iov_base = (*iov)[i].iov_base;
> > + oldiov->iov_len = (*iov)[i].iov_len;
> > + (*iov)[i].iov_len -= skip;
> > + (*iov)[i].iov_base += skip;
> > + *iov = *iov + i;
> > + *niov = *niov - i;
> > + break;
> > }
> > }
>
> Can you use iov_discard_front instead?
I didn't know that existed - i'll investigate whether it is
possible to use it.
> > + /* XXX see about using qio_channel_yield() in
> > + * future if we can use normal watches instead
> > + * of the AIO handlers */
> > + qemu_coroutine_yield();
>
> This breaks dataplane. The simplest solution is to change
> qio_channel_yield() to just
>
> qio_channel_aio_yield(ioc, qemu_get_aio_context(), condition);
>
> where qio_channel_aio_yield(QIOChannel*, AioContext *, int) is
> implemented by the backends. The implementation could then use
> aio_set_fd_handler, and it would be almost trivial because it would
> reuse common code just like the one in io/channel-watch.c.
>
> Websock is the only complicated case. I think it's fine if you simply
> must not use qio_channel_{,aio_}yield() for websock channels.
>
> That said, I'm not sure you can even rely on nbd_negotiate_continue to
> run in the main I/O thread. So, another possibility is to add an
> alternative watch API similar to AioContext's:
>
> qio_channel_set_handler(QIOChannel*, AioContext*, void(*in)(void*),
> void(*out)(void*), void *opaque)
>
> whose implementation would again be simple for backends other than
> websock, and probably trivial for backends other than buffer and
> websock. Again, it wouldn't be a problem to skip buffer and websock for
> the time being (read, until someone actually needs it).
I'll have a look at this and see what I can do. It shouldn't block
merging of this patch I assume, since the current call to
qemu_coroutine_yield() should be fine - it just my comment suggestion
that would not work. So I'll investigate what you suggest here wrt
to AioContext & QIOChannel as a later followup patch.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
