2014-03-31 15:46 GMT+02:00 Michal Privoznik <mprivozn@xxxxxxxxxx>:
>
> 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
>>
>>
>
>> +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.
>
No, because there is no return statement in the if block, so the
unlock call after the if block is sufficient.
>>
>> + 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@xxxxxxxxxxxxxx>
>> + * Copyright (C) 2009-2012, 2014 Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
>> *
>> * 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."
I added a comment about this now.
>> + }
>> +
>> + 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
Thanks, pushed... finally :)
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list