[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 13/22] migration: convert RDMA to use QIOChan
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v1 13/22] migration: convert RDMA to use QIOChannel interface |
Date: |
Wed, 3 Feb 2016 13:23:04 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* Daniel P. Berrange (address@hidden) wrote:
> On Tue, Feb 02, 2016 at 08:01:36PM +0000, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (address@hidden) wrote:
> > > This converts the RDMA code to provide a subclass of
> > > QIOChannel that uses RDMA for the data transport.
> > >
> > > The RDMA code would be much better off it it could
> > > be split up in a generic RDMA layer, a QIOChannel
> > > impl based on RMDA, and then the RMDA migration
> > > glue. This is left as a future exercise for the brave.
> > >
> > > Signed-off-by: Daniel P. Berrange <address@hidden>
> > > ---
> > > migration/rdma.c | 260
> > > ++++++++++++++++++++++++++++++++++---------------------
> > > 1 file changed, 161 insertions(+), 99 deletions(-)
> > >
>
> > > +static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
> > > + const struct iovec *iov,
> > > + size_t niov,
> > > + int **fds,
> > > + size_t *nfds,
> > > + Error **errp)
> > > +{
> > > + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
> > > + RDMAContext *rdma = rioc->rdma;
> > > RDMAControlHeader head;
> > > int ret = 0;
> > > + ssize_t i;
> > > + size_t done = 0;
> > >
> > > CHECK_ERROR_STATE();
> > >
> > > - /*
> > > - * First, we hold on to the last SEND message we
> > > - * were given and dish out the bytes until we run
> > > - * out of bytes.
> > > - */
> > > - r->len = qemu_rdma_fill(r->rdma, buf, size, 0);
> > > - if (r->len) {
> > > - return r->len;
> > > - }
> > > + for (i = 0; i < niov; i++) {
> > > + size_t want = iov[i].iov_len;
> > > + uint8_t *data = (void *)iov[i].iov_base;
> > >
> > > - /*
> > > - * Once we run out, we block and wait for another
> > > - * SEND message to arrive.
> > > - */
> > > - ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_QEMU_FILE);
> > > + /*
> > > + * First, we hold on to the last SEND message we
> > > + * were given and dish out the bytes until we run
> > > + * out of bytes.
> > > + */
> > > + ret = qemu_rdma_fill(rioc->rdma, data, want, 0);
> > > + if (ret > 0) {
> > > + done += ret;
> > > + if (ret < want) {
> > > + break;
> > > + } else {
> > > + continue;
> > > + }
> >
> > > + }
> > >
> > > - if (ret < 0) {
> > > - rdma->error_state = ret;
> > > - return ret;
> > > - }
> > > + /*
> > > + * Once we run out, we block and wait for another
> > > + * SEND message to arrive.
> > > + */
> > > + ret = qemu_rdma_exchange_recv(rdma, &head,
> > > RDMA_CONTROL_QEMU_FILE);
> > >
> > > - /*
> > > - * SEND was received with new bytes, now try again.
> > > - */
> > > - return qemu_rdma_fill(r->rdma, buf, size, 0);
> > > + if (ret < 0) {
> > > + rdma->error_state = ret;
> > > + return ret;
> > > + }
> > > +
> > > + /*
> > > + * SEND was received with new bytes, now try again.
> > > + */
> > > + ret = qemu_rdma_fill(rioc->rdma, data, want, 0);
> > > + if (ret > 0) {
> > > + done += ret;
> > > + if (ret < want) {
> > > + break;
> > > + }
> > > + }
> >
> > I don't quite understand the behaviour of this loop.
> > If rdma_fill returns less than you wanted for the first iov we break.
> > If it returns 0 then we try and get some more.
> > The weird thing to me is if we have two iov entries; if the
> > amount returned by the qemu_rdma_fill happens to match the size of
> > the 1st iov then I think we end up doing the exchange_recv and
> > waiting for more. Is that what we want? Why?
>
> No, it isn't quite what we want. If we have successfully received
> some data in a preceeding iov, then we shouldn't wait for more data
> for any following iov. I'll rework this bit of code to work better
>
> In fact technically, we should not block for data at all when the
> channel is in non-blocking mode. Fixing that would require some
> refactoring of qemu_rdma_block_for_wrid() so that we could tell
> it to only look for an already completed work request and not
> block
I wouldn't go changing qemu_rdma_block_for_wrid unless you need
to.
Dave
>
> 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 :|
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK