Re: [libvirt] [PATCH] add compress stream support
2015-09-25 14:30 GMT+03:00 Vasiliy Tolstov : > Also as i see in case ofr strage pool - dir, and image file layout on > disk, fdstream used only to OpenInternal with libvirt iohelper, after > that no fdstream Read/Write function used =(. My fail =(. When libvirt gets VolDownload fdstream uses Read function to read from fd. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add compress stream support
2015-09-25 12:39 GMT+03:00 Vasiliy Tolstov : > I have a problems with enabling compression in fdstream. In my case > use pass flag to VolDownload, in storage_driver i don't have ability > to access privateData of the stream, so i need to create additional > variable in stream struct ? > Or i miss something? Also does the same stream can be used to read and > write in the same time? (libarchive need to init own structure for > read or for write) Also as i see in case ofr strage pool - dir, and image file layout on disk, fdstream used only to OpenInternal with libvirt iohelper, after that no fdstream Read/Write function used =(. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add compress stream support
2015-09-23 16:18 GMT+03:00 Daniel P. Berrange : > nstead pass flags to the virStorageVolDownload/Upload functions, > eg VIR_STORAGE_VOL_STREAM_COMPRESS_ZLIB. This flag would get passed > across to the storage driver, which can then turn on compression in > the virFDStream impl. This would require > > - Add the enum flags to include/libvirt/libvirt-storage.h > - Add libarchive support to src/fdstream.c > - Handle the new flags in src/storage/storage_driver.c to >turn on the libarchive compression in fdsream.c I have a problems with enabling compression in fdstream. In my case use pass flag to VolDownload, in storage_driver i don't have ability to access privateData of the stream, so i need to create additional variable in stream struct ? Or i miss something? Also does the same stream can be used to read and write in the same time? (libarchive need to init own structure for read or for write) -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add compress stream support
23 сент. 2015 г. 16:18 пользователь "Daniel P. Berrange" < berra...@redhat.com> написал: > > On Wed, Sep 23, 2015 at 03:20:51PM +0300, Vasiliy Tolstov wrote: > > 2015-09-23 14:54 GMT+03:00 Daniel P. Berrange : > > > How have you actually tested this in practice - your patch does > > > not change any code to make use of this new feate. You're changing > > > the public API which suggests you expect the client apps to use > > > this when passing a virStreamPtr to the virStorageVolDownload > > > method, but the stream client apps pass is not backed by a > > > virFDStream object. Only libvirtd uses the virFDSteam objects > > > and you've not changed anything to make use ot that. So this is > > > all rather strange still. Can you more clearly state what you > > > are expecting to do. > > > > > > As i'm understand client create stream via virStreamNew , when user > > invoke virStoreVolDownload this function use created stream. > > If i'm correct, libvirt when read/write in > > virStoreVolDownload/virStoreVolUpload uses fdstream functions > > internally. So data compressed or decompressed. > > Or when client create virStreamNew and pass it libvirt not always use > > fdstream functions ? > > It is not quite that simple because we have a client/server architecture. > > So the client app creates a virStreamPtr. This is used by the remote > driver, via a virNetClientStream object to tunnel data over the > libvirtd connection. > So client can't say what stream libvirtd need to create for storage driver? > The libvirtd daemon then creates another virStreamPtr object. This is > used by the straoge driver, via the virFDStream object to fetch the > data the client is requesting. > > So having the client app request compression on its virStreamPtr > does not work, because that stream object is not the one used > by the virFDStream code. > > An alternative approach would be to not try to change the stream API > at all. > > Instead pass flags to the virStorageVolDownload/Upload functions, > eg VIR_STORAGE_VOL_STREAM_COMPRESS_ZLIB. This flag would get passed > across to the storage driver, which can then turn on compression in > the virFDStream impl. This would require > > - Add the enum flags to include/libvirt/libvirt-storage.h > - Add libarchive support to src/fdstream.c > - Handle the new flags in src/storage/storage_driver.c to >turn on the libarchive compression in fdsream.c > > 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
Re: [libvirt] [PATCH] add compress stream support
On Wed, Sep 23, 2015 at 03:20:51PM +0300, Vasiliy Tolstov wrote: > 2015-09-23 14:54 GMT+03:00 Daniel P. Berrange : > > How have you actually tested this in practice - your patch does > > not change any code to make use of this new feate. You're changing > > the public API which suggests you expect the client apps to use > > this when passing a virStreamPtr to the virStorageVolDownload > > method, but the stream client apps pass is not backed by a > > virFDStream object. Only libvirtd uses the virFDSteam objects > > and you've not changed anything to make use ot that. So this is > > all rather strange still. Can you more clearly state what you > > are expecting to do. > > > As i'm understand client create stream via virStreamNew , when user > invoke virStoreVolDownload this function use created stream. > If i'm correct, libvirt when read/write in > virStoreVolDownload/virStoreVolUpload uses fdstream functions > internally. So data compressed or decompressed. > Or when client create virStreamNew and pass it libvirt not always use > fdstream functions ? It is not quite that simple because we have a client/server architecture. So the client app creates a virStreamPtr. This is used by the remote driver, via a virNetClientStream object to tunnel data over the libvirtd connection. The libvirtd daemon then creates another virStreamPtr object. This is used by the straoge driver, via the virFDStream object to fetch the data the client is requesting. So having the client app request compression on its virStreamPtr does not work, because that stream object is not the one used by the virFDStream code. An alternative approach would be to not try to change the stream API at all. Instead pass flags to the virStorageVolDownload/Upload functions, eg VIR_STORAGE_VOL_STREAM_COMPRESS_ZLIB. This flag would get passed across to the storage driver, which can then turn on compression in the virFDStream impl. This would require - Add the enum flags to include/libvirt/libvirt-storage.h - Add libarchive support to src/fdstream.c - Handle the new flags in src/storage/storage_driver.c to turn on the libarchive compression in fdsream.c 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
Re: [libvirt] [PATCH] add compress stream support
2015-09-23 14:54 GMT+03:00 Daniel P. Berrange : > How have you actually tested this in practice - your patch does > not change any code to make use of this new feate. You're changing > the public API which suggests you expect the client apps to use > this when passing a virStreamPtr to the virStorageVolDownload > method, but the stream client apps pass is not backed by a > virFDStream object. Only libvirtd uses the virFDSteam objects > and you've not changed anything to make use ot that. So this is > all rather strange still. Can you more clearly state what you > are expecting to do. As i'm understand client create stream via virStreamNew , when user invoke virStoreVolDownload this function use created stream. If i'm correct, libvirt when read/write in virStoreVolDownload/virStoreVolUpload uses fdstream functions internally. So data compressed or decompressed. Or when client create virStreamNew and pass it libvirt not always use fdstream functions ? -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add compress stream support
On Tue, Sep 22, 2015 at 10:26:19PM +, Vasiliy Tolstov wrote: > Some libvirt functions use streams, this patch add > compress stream support. > So VolumeDownload/VolumeUpload can greatly speedup by using > compressed streams to save network bandtwidth and don't transfer > zero bytes (in case of raw disk format) How have you actually tested this in practice - your patch does not change any code to make use of this new feate. You're changing the public API which suggests you expect the client apps to use this when passing a virStreamPtr to the virStorageVolDownload method, but the stream client apps pass is not backed by a virFDStream object. Only libvirtd uses the virFDSteam objects and you've not changed anything to make use ot that. So this is all rather strange still. Can you more clearly state what you are expecting to do. 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
Re: [libvirt] [PATCH] add compress stream support
2015-09-23 1:26 GMT+03:00 Vasiliy Tolstov : > Some libvirt functions use streams, this patch add > compress stream support. > So VolumeDownload/VolumeUpload can greatly speedup by using > compressed streams to save network bandtwidth and don't transfer > zero bytes (in case of raw disk format) This is new version. Now i'm try to test it. But firstly - i have many Makefile.am writings, to fix linking errors. Can i add globally libarchive cflags and libs to all libvirt libraries? -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add compress stream support
2015-09-22 18:29 GMT+03:00 Daniel P. Berrange : > On Tue, Sep 22, 2015 at 02:10:41PM +, Vasiliy Tolstov wrote: >> use libarchive for compressed stream support > > Can you explain a bit more about how you expect this to be > used ? Thanks for all suggestions. I'm send new version. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] add compress stream support
Some libvirt functions use streams, this patch add
compress stream support.
So VolumeDownload/VolumeUpload can greatly speedup by using
compressed streams to save network bandtwidth and don't transfer
zero bytes (in case of raw disk format)
Signed-off-by: Vasiliy Tolstov
---
configure.ac | 2 ++
daemon/Makefile.am | 3 +++
include/libvirt/libvirt-stream.h | 3 +++
m4/virt-archive.m4 | 29 ++
src/Makefile.am | 17 +++-
src/datatypes.h | 7 +++
src/fdstream.c | 44
src/libvirt-stream.c | 15 ++
tools/Makefile.am| 17 ++--
9 files changed, 130 insertions(+), 7 deletions(-)
create mode 100644 m4/virt-archive.m4
diff --git a/configure.ac b/configure.ac
index 03463b0..58a15bb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,7 @@ LIBVIRT_CHECK_SANLOCK
LIBVIRT_CHECK_SASL
LIBVIRT_CHECK_SELINUX
LIBVIRT_CHECK_SSH2
+LIBVIRT_CHECK_LIBARCHIVE
LIBVIRT_CHECK_SYSTEMD_DAEMON
LIBVIRT_CHECK_UDEV
LIBVIRT_CHECK_WIRESHARK
@@ -2892,6 +2893,7 @@ LIBVIRT_RESULT_SANLOCK
LIBVIRT_RESULT_SASL
LIBVIRT_RESULT_SELINUX
LIBVIRT_RESULT_SSH2
+LIBVIRT_RESULT_LIBARCHIVE
LIBVIRT_RESULT_SYSTEMD_DAEMON
LIBVIRT_RESULT_UDEV
LIBVIRT_RESULT_WIRESHARK
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index be1b5a9..a4d63eb 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -177,17 +177,20 @@ libvirtd_CFLAGS = \
$(XDR_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \
$(WARN_CFLAGS) $(PIE_CFLAGS) \
$(COVERAGE_CFLAGS) \
+ $(LIBARCHIVE_CFLAGS) \
-DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\""
libvirtd_LDFLAGS = \
$(RELRO_LDFLAGS)\
$(PIE_LDFLAGS) \
$(COVERAGE_LDFLAGS) \
+ $(LIBARCHIVE_LDFLAGS) \
$(NO_INDIRECT_LDFLAGS) \
$(NULL)
libvirtd_LDADD = \
$(LIBXML_LIBS) \
+ $(LIBARCHIVE_LIBS) \
$(GNUTLS_LIBS) \
$(SASL_LIBS)\
$(DBUS_LIBS)\
diff --git a/include/libvirt/libvirt-stream.h b/include/libvirt/libvirt-stream.h
index 831640d..c75f03e 100644
--- a/include/libvirt/libvirt-stream.h
+++ b/include/libvirt/libvirt-stream.h
@@ -31,10 +31,13 @@
typedef enum {
VIR_STREAM_NONBLOCK = (1 << 0),
+VIR_STREAM_COMPRESS_GZIP = (1 << 1),
+VIR_STREAM_COMPRESS_XZ = (1 << 2),
} virStreamFlags;
virStreamPtr virStreamNew(virConnectPtr conn,
unsigned int flags);
+
int virStreamRef(virStreamPtr st);
int virStreamSend(virStreamPtr st,
diff --git a/m4/virt-archive.m4 b/m4/virt-archive.m4
new file mode 100644
index 000..9770732
--- /dev/null
+++ b/m4/virt-archive.m4
@@ -0,0 +1,29 @@
+dnl The libarchive.so library
+dnl
+dnl Copyright (C) 2012-2013 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library. If not, see
+dnl
Re: [libvirt] [PATCH] add compress stream support
On Tue, Sep 22, 2015 at 02:10:41PM +, Vasiliy Tolstov wrote:
> use libarchive for compressed stream support
Can you explain a bit more about how you expect this to be
used ?
> Signed-off-by: Vasiliy Tolstov
> ---
> configure.ac | 11 ++--
> include/libvirt/libvirt-stream.h | 6 +
> m4/virt-archive.m4 | 26 +++
> src/fdstream.c | 47 +
> src/libvirt-stream.c | 56
> +++-
> src/libvirt_public.syms | 7 +
> 6 files changed, 150 insertions(+), 3 deletions(-)
> create mode 100644 m4/virt-archive.m4
>
> diff --git a/configure.ac b/configure.ac
> index 03463b0..4018b49 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -250,6 +250,7 @@ LIBVIRT_CHECK_SANLOCK
> LIBVIRT_CHECK_SASL
> LIBVIRT_CHECK_SELINUX
> LIBVIRT_CHECK_SSH2
> +LIBVIRT_CHECK_LIBARCHIVE
This does all the pkg-config checks for libarchive...
> @@ -1603,8 +1604,6 @@ fi
> AC_SUBST([LIBPCAP_CFLAGS])
> AC_SUBST([LIBPCAP_LIBS])
>
> -
> -
Avoid changing unrelated lines please
> dnl
> dnl Checks for the UML driver
> dnl
> @@ -2097,6 +2096,13 @@ AM_CONDITIONAL([WITH_STORAGE_DISK], [test
> "$with_storage_disk" = "yes"])
> AC_SUBST([LIBPARTED_CFLAGS])
> AC_SUBST([LIBPARTED_LIBS])
>
> +if test "$with_libarchive" = "yes" || test "$with_libarchive" = "check"; then
> + PKG_CHECK_MODULES([LIBARCHIVE], [libarchive >= 3.1.2], [],
> [LIBARCHIVE_FOUND=no])
> +fi
> +AC_SUBST([LIBARCHIVE_CFLAGS])
> +AC_SUBST([LIBARCHIVE_LIBS])
...this is redundant given LIBVIRT_CHECK_LIBARCHIVE earlier.
> if test "$with_storage_mpath" = "yes" ||
> test "$with_storage_disk" = "yes"; then
> DEVMAPPER_CFLAGS=
> diff --git a/include/libvirt/libvirt-stream.h
> b/include/libvirt/libvirt-stream.h
> index 831640d..ac48fba 100644
> --- a/include/libvirt/libvirt-stream.h
> +++ b/include/libvirt/libvirt-stream.h
> @@ -35,6 +35,12 @@ typedef enum {
>
> virStreamPtr virStreamNew(virConnectPtr conn,
>unsigned int flags);
> +virStreamPtr virStreamNewLz4(virConnectPtr conn,
> + unsigned int flags);
> +virStreamPtr virStreamNewGzip(virConnectPtr conn,
> + unsigned int flags);
> +virStreamPtr virStreamNewXz(virConnectPtr conn,
> + unsigned int flags);
If we want to expose this in the public API, then we'd really
want an enum for each archive format, so we don't need to keep
adding new APIs for each format.
> diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
> index c16f586..40702dc 100644
> --- a/src/libvirt-stream.c
> +++ b/src/libvirt-stream.c
> @@ -69,6 +73,56 @@ virStreamNew(virConnectPtr conn,
> return st;
> }
>
> +virStreamPtr
> +virStreamNewLz4(virConnectPtr conn,
> +unsigned int flags)
> +{
> +virStreamPtr st;
> +
> +st = virStreamNew(conn, flags);
> +if (st != NULL) {
> +struct virFDStreamData *fdst = st->privateData;
This is not valid. You can't assume anything about what
st->privateData is pointing to - it can be anything that
a virt driver wants to use. For example with the remote
driver it can point to a virNetClientStreamPtr instead
instead of virFDStreamData.
> +fdst->archive = archive_write_new();
> +archive_write_add_filter(fdst->archive, ARCHIVE_FILTER_LZ4);
> +}
> +
> +return st;
> +}
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
[libvirt] [PATCH] add compress stream support
use libarchive for compressed stream support
Signed-off-by: Vasiliy Tolstov
---
configure.ac | 11 ++--
include/libvirt/libvirt-stream.h | 6 +
m4/virt-archive.m4 | 26 +++
src/fdstream.c | 47 +
src/libvirt-stream.c | 56 +++-
src/libvirt_public.syms | 7 +
6 files changed, 150 insertions(+), 3 deletions(-)
create mode 100644 m4/virt-archive.m4
diff --git a/configure.ac b/configure.ac
index 03463b0..4018b49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,7 @@ LIBVIRT_CHECK_SANLOCK
LIBVIRT_CHECK_SASL
LIBVIRT_CHECK_SELINUX
LIBVIRT_CHECK_SSH2
+LIBVIRT_CHECK_LIBARCHIVE
LIBVIRT_CHECK_SYSTEMD_DAEMON
LIBVIRT_CHECK_UDEV
LIBVIRT_CHECK_WIRESHARK
@@ -1603,8 +1604,6 @@ fi
AC_SUBST([LIBPCAP_CFLAGS])
AC_SUBST([LIBPCAP_LIBS])
-
-
dnl
dnl Checks for the UML driver
dnl
@@ -2097,6 +2096,13 @@ AM_CONDITIONAL([WITH_STORAGE_DISK], [test
"$with_storage_disk" = "yes"])
AC_SUBST([LIBPARTED_CFLAGS])
AC_SUBST([LIBPARTED_LIBS])
+if test "$with_libarchive" = "yes" || test "$with_libarchive" = "check"; then
+ PKG_CHECK_MODULES([LIBARCHIVE], [libarchive >= 3.1.2], [],
[LIBARCHIVE_FOUND=no])
+fi
+AC_SUBST([LIBARCHIVE_CFLAGS])
+AC_SUBST([LIBARCHIVE_LIBS])
+
+
if test "$with_storage_mpath" = "yes" ||
test "$with_storage_disk" = "yes"; then
DEVMAPPER_CFLAGS=
@@ -2892,6 +2898,7 @@ LIBVIRT_RESULT_SANLOCK
LIBVIRT_RESULT_SASL
LIBVIRT_RESULT_SELINUX
LIBVIRT_RESULT_SSH2
+LIBVIRT_RESULT_LIBARCHIVE
LIBVIRT_RESULT_SYSTEMD_DAEMON
LIBVIRT_RESULT_UDEV
LIBVIRT_RESULT_WIRESHARK
diff --git a/include/libvirt/libvirt-stream.h b/include/libvirt/libvirt-stream.h
index 831640d..ac48fba 100644
--- a/include/libvirt/libvirt-stream.h
+++ b/include/libvirt/libvirt-stream.h
@@ -35,6 +35,12 @@ typedef enum {
virStreamPtr virStreamNew(virConnectPtr conn,
unsigned int flags);
+virStreamPtr virStreamNewLz4(virConnectPtr conn,
+ unsigned int flags);
+virStreamPtr virStreamNewGzip(virConnectPtr conn,
+ unsigned int flags);
+virStreamPtr virStreamNewXz(virConnectPtr conn,
+ unsigned int flags);
int virStreamRef(virStreamPtr st);
int virStreamSend(virStreamPtr st,
diff --git a/m4/virt-archive.m4 b/m4/virt-archive.m4
new file mode 100644
index 000..b550c41
--- /dev/null
+++ b/m4/virt-archive.m4
@@ -0,0 +1,26 @@
+dnl The libarchive.so library
+dnl
+dnl Copyright (C) 2012-2013 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library. If not, see
+dnl
Re: [libvirt] [PATCH] add compress stream support
2015-09-22 17:10 GMT+03:00 Vasiliy Tolstov : > use libarchive for compressed stream support > > Signed-off-by: Vasiliy Tolstov this is test patch, because libvirt not build with this - struct virFDStreamData is internally used by fdstream.c, but i need to acces private data from libvirt-stream.c via virStreamNewLz4 and so. Can somebody helps me and say, how the best to integrate compressed stream support to libvirt? I plan to use this stream when upload/download volume via libvirt. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
