Re: [PATCH] add compress stream support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 22, 2015 at 02:10:41PM +0000, 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 <v.tolstov@xxxxxxxxx>
> ---
>  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]