Re: [libvirt] [PATCH 1/2] esx: Add libcurl based stream driver
2014-03-31 15:46 GMT+02:00 Michal Privoznik mpriv...@redhat.com:
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.bo...@googlemail.com
+ * Copyright (C) 2009-2012, 2014 Matthias Bolte
matthias.bo...@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.
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] esx: Add libcurl based stream driver
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 000..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.bo...@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
Re: [libvirt] [PATCH 1/2] esx: Add libcurl based stream driver
On Mon, Mar 31, 2014 at 03:46:43PM +0200, Michal Privoznik wrote: 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 000..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.bo...@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); Only if there are actually any VIR_DEBUG statements in the file. You'd see a compile failure if you had debug statements, but no log init. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
