[libvirt] [PATCH 1/2] esx: Add libcurl based stream driver

Michal Privoznik mprivozn at redhat.com
Mon Mar 31 13:46:43 UTC 2014


On 30.03.2014 21:03, Matthias Bolte wrote:
> This allows to implement libvirt functions that use streams, such as
> virDoaminScreenshot, without the need to store the downloaded data in
> a temporary file first. The stream driver directly interacts with
> libcurl to send and receive data.
>
> The driver uses the libcurl multi interface that allows to do a transfer
> in multiple curl_multi_perform() calls. The easy interface would do the
> whole transfer in a single curl_easy_perform() call. This doesn't work
> with the libvirt stream API that is driven by multiple calls to the
> virStreamSend() and virStreamRecv() functions.
>
> The curl_multi_wait() function is used to do blocking operations. But it
> was added in libcurl 7.28.0. For older versions it is emulated using the
> socket callback of the multi interface.
>
> The current driver only supports blocking operations. There is already
> some code in place for non-blocking mode but it's incomplete. As you can
> tell from the copyright date I implemeted this in 2012, but never came
> around to publish it then. I did some work in 2013 and now it's 2014 and
> I don't want to hold it back any longer.
> ---
>   po/POTFILES.in       |   1 +
>   src/Makefile.am      |   1 +
>   src/esx/esx_stream.c | 478 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/esx/esx_stream.h |  32 ++++
>   src/esx/esx_vi.c     | 222 +++++++++++++++++++++++-
>   src/esx/esx_vi.h     |  19 +-
>   6 files changed, 749 insertions(+), 4 deletions(-)
>   create mode 100644 src/esx/esx_stream.c
>   create mode 100644 src/esx/esx_stream.h
>

> diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
> new file mode 100644
> index 0000000..fb9abbc
> --- /dev/null
> +++ b/src/esx/esx_stream.c
> @@ -0,0 +1,478 @@
> +/*
> + * esx_stream.c: libcurl based stream driver
> + *
> + * Copyright (C) 2012-2014 Matthias Bolte <matthias.bolte at googlemail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +
> +#include "internal.h"
> +#include "datatypes.h"
> +#include "viralloc.h"
> +#include "virstring.h"
> +#include "esx_stream.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_ESX

I believe we want something like this too:

VIR_LOG_INIT("esx.esx_stream");

> +
> +/*
> + * This libcurl based stream driver cannot use a libcurl easy handle alone
> + * because curl_easy_perform would do the whole transfer before it returns.
> + * But there is no place in the stream handling concept that would allow for
> + * such a call to be made. The stream is driven by esxStream(Send|Recv) which
> + * is probably called multiple times to send/receive the stream in chunks.
> + * Therefore, a libcurl multi handle is used that allows to perform the data
> + * transfer in chunks and also allows to support non-blocking operations.
> + *
> + * In the upload direction esxStreamSend is called to push data into the
> + * stream and libcurl will call esxVI_CURL_ReadStream to pull data out of
> + * the stream to upload it via HTTP(S). To realize this esxStreamSend calls
> + * esxStreamTransfer that uses esxVI_MultiCURL_(Wait|Perform) to drive the
> + * transfer and makes libcurl read up the data passed to esxStreamSend.
> + *
> + * In the download direction esxStreamRecv is called to pull data out of the
> + * stream and libcurl will call esxVI_CURL_WriteStream to push data into the
> + * stream that it has downloaded via HTTP(S). To realize this esxStreamRecv
> + * calls esxStreamTransfer that uses esxVI_MultiCURL_(Wait|Perform) to drive
> + * the transfer and makes libcurl write to the buffer passed to esxStreamRecv.
> + *
> + * The download direction requires some extra logic because libcurl might
> + * call esxVI_CURL_WriteStream with more data than there is space left in the
> + * buffer passed to esxStreamRecv. But esxVI_CURL_WriteStream is not allowed
> + * to handle only a part of the incoming data, it needs to handle it all at
> + * once. Therefore the stream driver manages a backlog buffer that holds the
> + * extra data that didn't fit into the esxStreamRecv buffer anymore. The next
> + * time esxStreamRecv is called it'll read the data from the backlog buffer
> + * first before asking libcurl for more data.
> + *
> + * Typically libcurl will call esxVI_CURL_WriteStream with up to 16kb data
> + * this means that the typically maximum backlog size should be 16kb as well.
> + */

> +static int
> +esxStreamClose(virStreamPtr stream, bool finish)
> +{
> +    int result = 0;
> +    esxStreamPrivate *priv = stream->privateData;
> +
> +    if (!priv)
> +        return 0;
> +
> +    virMutexLock(&priv->curl->lock);
> +
> +    if (finish && priv->backlog_used > 0) {

I think you want to unlock the curl lock here.

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Stream has untransferred data left"));
> +        result = -1;
> +    }
> +
> +    stream->privateData = NULL;
> +
> +    virMutexUnlock(&priv->curl->lock);
> +
> +    esxFreeStreamPrivate(&priv);
> +
> +    return result;
> +}
> +

> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index 6188139..ba34bfd 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -2,7 +2,7 @@
>    * esx_vi.c: client for the VMware VI API 2.5 to manage ESX hosts
>    *
>    * Copyright (C) 2010-2012 Red Hat, Inc.
> - * Copyright (C) 2009-2012 Matthias Bolte <matthias.bolte at googlemail.com>
> + * Copyright (C) 2009-2012, 2014 Matthias Bolte <matthias.bolte at googlemail.com>
>    *
>    * This library is free software; you can redistribute it and/or
>    * modify it under the terms of the GNU Lesser General Public
> @@ -22,6 +22,7 @@
>
>   #include <config.h>
>
> +#include <poll.h>
>   #include <libxml/parser.h>
>   #include <libxml/xpathInternals.h>
>
> @@ -662,6 +663,68 @@ esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl)
>    * MultiCURL
>    */
>
> +#if ESX_EMULATE_CURL_MULTI_WAIT
> +
> +static int
> +esxVI_MultiCURL_SocketCallback(CURL *handle ATTRIBUTE_UNUSED,
> +                               curl_socket_t fd, int action,
> +                               void *callback_opaque,
> +                               void *socket_opaque ATTRIBUTE_UNUSED)
> +{
> +    esxVI_MultiCURL *multi = callback_opaque;
> +    size_t i;
> +    struct pollfd *pollfd = NULL;
> +    struct pollfd dummy;
> +
> +    if (action & CURL_POLL_REMOVE) {
> +        for (i = 0; i < multi->npollfds; ++i) {
> +            if (multi->pollfds[i].fd == fd) {
> +                VIR_DELETE_ELEMENT(multi->pollfds, i, multi->npollfds);
> +                break;
> +            }
> +        }
> +    } else {
> +        for (i = 0; i < multi->npollfds; ++i) {
> +            if (multi->pollfds[i].fd == fd) {
> +                pollfd = &multi->pollfds[i];
> +                break;
> +            }
> +        }
> +
> +        if (pollfd == NULL) {
> +            if (VIR_APPEND_ELEMENT(multi->pollfds, multi->npollfds, dummy) < 0) {
> +                return 0;

Okay, this is strange. But I see why you can't return -1. From the 
curl_multi_socket() documentation:

"The callback MUST return 0."

> +            }
> +
> +            pollfd = &multi->pollfds[multi->npollfds - 1];
> +        }
> +
> +        pollfd->fd = fd;
> +        pollfd->events = 0;
> +
> +        if (action & CURL_POLL_IN)
> +            pollfd->events |= POLLIN;
> +
> +        if (action & CURL_POLL_OUT)
> +            pollfd->events |= POLLOUT;
> +    }
> +
> +    return 0;
> +}
> +

ACK with the nits fixed.

Michal




More information about the libvir-list mailing list