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

Re: [libvirt] [PATCH 07/13] Add libvirt-admin library



On 22.05.2015 07:22, Martin Kletzander wrote:
> On Thu, May 21, 2015 at 05:46:59PM +0200, Michal Privoznik wrote:
>> 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 redhat com>
>>> ---
>>>  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
>>>
>>


>>> +
>>> +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).
>>
> 
> Yes, but I have to rename that to something else than remote_string
> because that would collide in the libvirt daemon.  That would mean I
> have to add each of the new names to gendispatch.pl.  And so on and so
> on, just to get rid of some (de)serializers in the client library.
> 
> I'll do that for remote_string for now, but if there are more than
> that, we should probably put those into another file.  Well, you'll
> see how much bigger the diff will be even now.

I see, so you need it for remote_string. I did what I suggested and saw
a linking error later in the series (when the remote_string was added).
So I guess we can split remote_protocol into two pieces, one containing
just general type definitions, other containing libvirt APIs. Although
this would mean yet another commits in this already big series. So I'm
okay to save it to a follow up patches.

Michal


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