On 20.05.2015 07:19, Martin Kletzander wrote:
> Initial scratch of the admin library. It has its own virAdmConnectPtr
> that inherits from virAbstractConnectPtr and thus trivially supports
> error reporting.
>
> There's pkg-config file added and spec-file adjusted as well.
>
> Since the library should be "minimalistic" and not depend on any other
> library, the list of files is especially crafted for it. Most of them
> could've been put to it's own sub-libraries that would be LIBADD'd to
> libvirt_util, libvirt_net_rpc and libvirt_setuid_rpc_client to minimize
> the number of object files being built, but that's a refactoring that
> isn't the orginal aim of this commit.
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> cfg.mk | 1 +
> configure.ac | 4 +-
> include/libvirt/Makefile.am | 4 +-
> include/libvirt/libvirt-admin.h | 62 +++++++
> libvirt-admin.pc.in | 13 ++
> libvirt.spec.in | 7 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 106 ++++++++++-
> src/datatypes.c | 30 ++++
> src/datatypes.h | 37 ++++
> src/internal.h | 1 +
> src/libvirt-admin.c | 386 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_admin.syms | 18 ++
> src/rpc/gendispatch.pl | 4 +-
> 14 files changed, 669 insertions(+), 5 deletions(-)
> create mode 100644 include/libvirt/libvirt-admin.h
> create mode 100644 libvirt-admin.pc.in
> create mode 100644 src/libvirt-admin.c
> create mode 100644 src/libvirt_admin.syms
>
> diff --git a/libvirt-admin.pc.in b/libvirt-admin.pc.in
> new file mode 100644
> index 0000000..76126ae
> --- /dev/null
> +++ b/libvirt-admin.pc.in
> @@ -0,0 +1,13 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +datarootdir=@datarootdir@
> +
> +libvirt_admin_api=@datadir@/libvirt/api/libvirt-admin-api.xml
> +
> +Name: libvirt-admin
> +Version: @VERSION@
> +Description: libvirt admin library
> +Libs: -L${libdir} -lvirt-admin
> +Cflags: -I${includedir}
This file should be put into AC_CONFIG_FILES() too. And into EXTRA_DIST
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 4195518..afcfe31 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -2206,6 +2206,12 @@ exit 0
> %attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper
> %endif
>
> +%if %{with_admin}
This doesn't make much sense. libvirt-admin is build unconditionally.
Moreover, the macro {with_admin} is never defined.
> +%files admin
> +%defattr(-, root, root)
> +%{_libdir}/%{name}/libvirt_admin.so
Nope. It's installed under different path.
> +%endif
Unfortunately, you haven't defined admin package. So this won't fly.
> +
> %files client -f %{name}.lang
> %defattr(-, root, root)
> %doc COPYING COPYING.LESSER
> @@ -2298,6 +2304,7 @@ exit 0
> %{_includedir}/libvirt/libvirt-stream.h
> %{_includedir}/libvirt/libvirt-qemu.h
> %{_includedir}/libvirt/libvirt-lxc.h
> +%{_includedir}/libvirt/libvirt-admin.h
> %{_libdir}/pkgconfig/libvirt.pc
> %{_libdir}/pkgconfig/libvirt-qemu.pc
> %{_libdir}/pkgconfig/libvirt-lxc.pc
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index ebb0482..4afa2f9 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -59,6 +59,7 @@ src/interface/interface_backend_netcf.c
> src/interface/interface_backend_udev.c
> src/internal.h
> src/libvirt.c
> +src/libvirt-admin.c
> src/libvirt-domain.c
> src/libvirt-domain-snapshot.c
> src/libvirt-host.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index e8dce78..1241d6d 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -501,6 +501,7 @@ PROTOCOL_STRUCTS = \
> $(srcdir)/virkeepaliveprotocol-structs \
> $(srcdir)/lxc_monitor_protocol-structs \
> $(srcdir)/lock_protocol-structs \
> + $(srcdir)/admin_protocol-structs \
> $(NULL)
>
> if WITH_REMOTE
> @@ -522,6 +523,9 @@ $(srcdir)/lxc_monitor_protocol-struct: \
> $(srcdir)/lock_protocol-struct: \
> $(srcdir)/%-struct: locking/lockd_la-%.lo
> $(PDWTAGS)
> +$(srcdir)/admin_protocol-struct: \
> + $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo
> + $(PDWTAGS)
>
> else !WITH_REMOTE
> # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be
> @@ -534,6 +538,7 @@ check-drivername:
> $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \
> $(srcdir)/driver.h \
> $(srcdir)/libvirt_public.syms \
> + $(srcdir)/libvirt_admin.syms \
> $(srcdir)/libvirt_qemu.syms \
> $(srcdir)/libvirt_lxc.syms
>
> @@ -1092,6 +1097,7 @@ USED_SYM_FILES = $(srcdir)/libvirt_private.syms
> GENERATED_SYM_FILES = \
> $(ACCESS_DRIVER_SYM_FILES) \
> libvirt.syms libvirt.def libvirt_qemu.def libvirt_lxc.def \
> + libvirt_admin.def \
> $(NULL)
>
> if WITH_TEST
> @@ -1803,7 +1809,8 @@ EXTRA_DIST += \
> $(VBOX_DRIVER_EXTRA_DIST) \
> $(VMWARE_DRIVER_SOURCES) \
> $(XENCONFIG_SOURCES) \
> - $(ACCESS_DRIVER_POLKIT_POLICY)
> + $(ACCESS_DRIVER_POLKIT_POLICY) \
> + $(libvirt_admin_la_SOURCES)
This is not necessary. libvirt-admin is build unconditionally.
>
> check-local: check-augeas
>
> @@ -2000,6 +2007,7 @@ EXTRA_DIST += \
> libvirt_public.syms \
> libvirt_lxc.syms \
> libvirt_qemu.syms \
> + libvirt_admin.syms \
> $(SYM_FILES) \
> $(NULL)
>
> @@ -2043,6 +2051,102 @@ libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms
> chmod a-w $@-tmp && \
> mv $@-tmp libvirt_lxc.def
>
> +libvirt_admin.def: $(srcdir)/libvirt_admin.syms
> + $(AM_V_GEN)rm -f -- $@-tmp $@ ; \
> + printf 'EXPORTS\n' > $@-tmp && \
> + sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d' \
> + -e 's/[ ]*\(.*\)\;/ \1/g' $^ >> $@-tmp && \
> + chmod a-w $@-tmp && \
> + mv $@-tmp libvirt_admin.def
This pattern repeats itself already. Maybe one day we can turn it into
a general rule how to make .def from .syms.
> +
> +lib_LTLIBRARIES += libvirt-admin.la
> +libvirt_admin_la_SOURCES = \
> + libvirt-admin.c \
> + $(ADMIN_PROTOCOL_GENERATED)
> +
> +libvirt_admin_la_SOURCES += \
Spurious space after =.
> + datatypes.c \
> + util/viralloc.c \
> + util/viratomic.c \
> + util/virauth.c \
> + util/virauthconfig.c \
> + util/virbitmap.c \
> + util/virbuffer.c \
> + util/vircommand.c \
> + util/virerror.c \
> + util/virevent.c \
> + util/vireventpoll.c \
> + util/virfile.c \
> + util/virhash.c \
> + util/virhashcode.c \
> + util/virjson.c \
> + util/virkeyfile.c \
> + util/virlog.c \
> + util/virobject.c \
> + util/virpidfile.c \
> + util/virprocess.c \
> + util/virrandom.c \
> + util/virseclabel.c \
> + util/virsocketaddr.c \
> + util/virstorageencryption.c \
> + util/virstoragefile.c \
> + util/virstring.c \
> + util/virthread.c \
> + util/virtime.c \
> + util/virtypedparam.c \
> + util/viruri.c \
> + util/virutil.c \
> + util/viruuid.c \
> + util/virxml.c \
> + remote/remote_protocol.c \
This drags in (de)serializers for all the public APIs we have. I guess
you have it here just becase of serializers for some basic types (e.g.
string). Well, if we introduce a separate libvirt_admin.x file, rpcgen
will generate the serializers again, and just for the types we need. So
I think it's safe to drop this line (and libvirt-admin.so will lose
some weight).
Then, I wonder if we need to recompile nearly all utils/ again. Can't
we just link libvirt_utils.so in?
> + rpc/virnetmessage.h \
> + rpc/virnetmessage.c \
> + rpc/virnetsocket.c \
> + rpc/virnetsshsession.c \
> + rpc/virkeepalive.c \
> + rpc/virnetclient.c \
> + rpc/virnetclientprogram.c \
> + rpc/virnetclientstream.c \
> + rpc/virnetprotocol.c \
> + rpc/virnettlscontext.c \
> + rpc/virnetsaslcontext.c
SSH, TLS and SASL? It's going to be a local socket only. I guess we
don't need them. Or is it some kind of black magic of dependencies?
> +
> +libvirt_admin_la_LDFLAGS = \
> + $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_ADMIN_SYMBOL_FILE) \
> + -version-info $(LIBVIRT_VERSION_INFO) \
> + $(AM_LDFLAGS) \
> + $(CYGWIN_EXTRA_LDFLAGS) \
> + $(MINGW_EXTRA_LDFLAGS)
> +
> +libvirt_admin_la_LIBADD = \
> + $(CYGWIN_EXTRA_LIBADD)
> +
> +libvirt_admin_la_CFLAGS = \
> + $(AM_CFLAGS) \
> + -I$(srcdir)/remote \
> + -I$(srcdir)/rpc \
> + -I$(srcdir)/admin
> +
> +libvirt_admin_la_CFLAGS += \
> + $(CAPNG_CFLAGS) \
> + $(YAJL_CFLAGS) \
> + $(SSH2_CFLAGS) \
> + $(SASL_CFLAGS) \
> + $(GNUTLS_CFLAGS)
> +
> +libvirt_admin_la_LIBADD += \
> + $(CAPNG_LIBS) \
> + $(YAJL_LIBS) \
> + $(DEVMAPPER_LIBS) \
> + $(LIBXML_LIBS) \
> + $(SSH2_LIBS) \
> + $(SASL_LIBS) \
> + $(GNUTLS_LIBS)
> +
> diff --git a/src/libvirt_admin.syms b/src/libvirt_admin.syms
> new file mode 100644
> index 0000000..ea6a8cc
> --- /dev/null
> +++ b/src/libvirt_admin.syms
> @@ -0,0 +1,18 @@
> +#
> +# Officially exported symbols, for which header
> +# file definitions are installed in /usr/include/libvirt
> +# from libvirt-admin.h
> +#
> +# Versions here are *fixed* to match the libvirt version
> +# at which the symbol was introduced. This ensures that
> +# a new client app requiring symbol foo() can't accidentally
> +# run with old libvirt-admin.so not providing foo() - the global
> +# soname version info can't enforce this since we never
> +# change the soname
> +#
> +LIBVIRT_ADMIN_1.3.0 {
> + global:
> + virAdmInitialize;
> + virAdmConnectOpen;
> + virAdmConnectClose;
I wonder if we should introduce (and implement) virAdmConnectRef. For
instance, if you have a multithreaded application, you open the
connection, and then spawn threads. But for some reason, you want to
have virAdmConnectClose in threads. Therefore each thread should
increase the reference counter on the connection objects, so the first
one to call the close() won't close the connection for the others.
> +};
> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index 5097e13..dda04a9 100755
> --- a/src/rpc/gendispatch.pl
> +++ b/src/rpc/gendispatch.pl
> @@ -1255,8 +1255,8 @@ elsif ($mode eq "client") {
> }
> }
>
> - if (($structprefix ne "admin") && !@args_list) {
> - push(@args_list, "virConnectPtr conn");
> + if (!@args_list) {
> + push(@args_list, "$connect_ptr conn");
> }
>
> # handle return values of the function
>
This needs to be squashed in at least:
diff --git a/Makefile.am b/Makefile.am
index 4aafe94..c4178d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,6 +38,7 @@ EXTRA_DIST = \
libvirt.spec libvirt.spec.in \
mingw-libvirt.spec.in \
libvirt.pc.in \
+ libvirt-admin.pc.in \
libvirt-qemu.pc.in \
libvirt-lxc.pc.in \
autobuild.sh \
diff --git a/configure.ac b/configure.ac
index 0409f9d..af1fa82 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2793,6 +2793,7 @@ AC_CONFIG_FILES([\
gnulib/lib/Makefile \
gnulib/tests/Makefile \
libvirt.pc \
+ libvirt-admin.pc \
libvirt-qemu.pc \
libvirt-lxc.pc \
src/libvirt.pc \
diff --git a/libvirt.spec.in b/libvirt.spec.in
index afcfe31..10010cf 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1216,6 +1216,15 @@ Includes the Sanlock lock manager plugin for the QEMU
driver
%endif
+%package admin
+Summary: Plugin to control daemon itself
+Group: Development/Libraries
+
+%description admin
+This package contains set of APIs for runtime daemon
+administration, for instance adjustment of worker pool size,
+logging etc.
+
%prep
%setup -q
@@ -2206,11 +2215,9 @@ exit 0
%attr(0755, root, root) %{_libexecdir}/libvirt_sanlock_helper
%endif
-%if %{with_admin}
%files admin
%defattr(-, root, root)
-%{_libdir}/%{name}/libvirt_admin.so
-%endif
+%{_libdir}/libvirt-admin.so.*
%files client -f %{name}.lang
%defattr(-, root, root)
@@ -2286,6 +2293,7 @@ exit 0
%defattr(-, root, root)
%{_libdir}/libvirt.so
+%{_libdir}/libvirt-admin.so
%{_libdir}/libvirt-qemu.so
%{_libdir}/libvirt-lxc.so
%dir %{_includedir}/libvirt
diff --git a/src/Makefile.am b/src/Makefile.am
index 1241d6d..7e4554d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1809,8 +1809,7 @@ EXTRA_DIST +=
\
$(VBOX_DRIVER_EXTRA_DIST) \
$(VMWARE_DRIVER_SOURCES) \
$(XENCONFIG_SOURCES) \
- $(ACCESS_DRIVER_POLKIT_POLICY) \
- $(libvirt_admin_la_SOURCES)
+ $(ACCESS_DRIVER_POLKIT_POLICY)
check-local: check-augeas
@@ -2064,7 +2063,7 @@ libvirt_admin_la_SOURCES = \
libvirt-admin.c \
$(ADMIN_PROTOCOL_GENERATED)
-libvirt_admin_la_SOURCES += \
+libvirt_admin_la_SOURCES += \
datatypes.c \
util/viralloc.c \
util/viratomic.c \
@@ -2098,7 +2097,6 @@ libvirt_admin_la_SOURCES += \
util/virutil.c \
util/viruuid.c \
util/virxml.c \
- remote/remote_protocol.c \
rpc/virnetmessage.h \
rpc/virnetmessage.c \
rpc/virnetsocket.c \
Otherwise looking good.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list