qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@gmail.com>
To: Josh Durgin <josh.durgin@inktank.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 2/2] rbd: link and load librbd dynamically
Date: Wed, 10 Apr 2013 10:09:54 +0200	[thread overview]
Message-ID: <CAJSP0QWy1CUOX+BVB2kfXfRpcdPuwbywZ3tReo83zK23+B4vbQ@mail.gmail.com> (raw)
In-Reply-To: <1365552342-22840-1-git-send-email-josh.durgin@inktank.com>

On Wed, Apr 10, 2013 at 2:05 AM, Josh Durgin <josh.durgin@inktank.com> wrote:
> This allows the rbd block driver to detect symbols in the installed
> version of librbd, and enable or disable features appropriately.  This
> obviates the #ifdefs regarding librbd versions.
>
> Loading librbd dynamically also makes the rbd block driver easier to
> install and package, since it removes the dependency on librbd at
> build time.
>
> Add structures containing the necessary function pointer signatures
> and types from librbd, and fill them in the first time the rbd module
> is used. Use glib's g_module interface so we don't preclude future
> portability, and don't have to deal with odd dlopen behavior directly.
>
> Internally, librbd and some libraries it depends on use C++ templates,
> which mean that they each contain a defined weak symbol for their
> definition.  Due to the way the linker resolves duplicate symbols, the
> libraries loaded by librbd end up using the template definitions from
> librbd, creating a circular dependency. This means that once librbd is
> loaded, it won't be unloaded. Changing this behavior might work with a
> Sun ld, but there doesn't seem to be a portable (or even working with
> GNU ld) way to hide these C++ symbols correctly. Instead, never unload
> librbd, and explicitly make it resident.
>
> Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
> ---
>  block/Makefile.objs |    3 +-
>  block/rbd.c         |  292 ++++++++++++++++++++++++++++++++++++---------------
>  block/rbd_types.h   |   95 +++++++++++++++++
>  configure           |   41 +-------
>  4 files changed, 309 insertions(+), 122 deletions(-)
>  create mode 100644 block/rbd_types.h

NACK

I think we're solving the problem at the wrong level.  Writing our own
dynamic linker and adding boilerplate to juggle function pointers
every time we use a library dependency is ugly.

There are two related problems here:

1. Packagers do not want to enable niche dependencies since users will
complain that the package is bloated and pulls in too much stuff.

2. QEMU linked against a newer library version fails to run on hosts
that have an older library.

Problem #1 has several solutions:

1. Let packagers take care of it.  For example, vim is often shipped
in several packages that have various amounts of dependencies
(vim-tiny, vim-gtk, etc).  Packagers create the specialized packages
for specific groups of users to meet their demands without dragging in
too many dependencies.

2. Make QEMU modular - host devices should be shared libraries that
are loaded at runtime.  There should be no stable API so that
development stays flexible and we discourage binary-only modules.
This lets packagers easily ship a qemu-rbd package, for example, that
drops in a .so file that QEMU can load at runtime.

Problem #2 is already solved:

The dynamic linker will refuse to load the program if there are
missing symbols.  It's not possible to mix and match binaries across
environments while downgrading their library dependencies.  With
effort, this could be doable but it's not an interesting use case that
many users care about - they get their binaries from a distro or build
them from source with correct dependencies.

Maybe it's time to move block drivers and other components into modules?

Stefan

  reply	other threads:[~2013-04-10  8:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-29  7:59 [Qemu-devel] [PATCH] rbd: add an asynchronous flush Josh Durgin
2013-03-29 20:03 ` [Qemu-devel] [PATCH v2] " Josh Durgin
2013-04-02 14:10   ` Kevin Wolf
2013-04-04  8:35     ` [Qemu-devel] [PATCH v3 1/2] " Josh Durgin
2013-04-04  8:35     ` [Qemu-devel] [PATCH 2/2] rbd: disable unsupported librbd functions at runtime Josh Durgin
2013-04-04 10:10       ` Kevin Wolf
2013-04-04 16:50         ` Josh Durgin
2013-04-05  9:31           ` Kevin Wolf
2013-04-10  0:05             ` [Qemu-devel] [PATCH v3 2/2] rbd: link and load librbd dynamically Josh Durgin
2013-04-10  8:09               ` Stefan Hajnoczi [this message]
2013-04-10 14:52                 ` [Qemu-devel] runtime Block driver modules (was Re: [PATCH v3 2/2] rbd: link and load librbd dynamically) Josh Durgin
2013-04-10 15:08                 ` [Qemu-devel] [PATCH v3 2/2] rbd: link and load librbd dynamically Anthony Liguori
2013-04-10 21:19                   ` Paolo Bonzini
2013-04-11  8:04                     ` Stefan Hajnoczi
2013-04-11  7:59                   ` Stefan Hajnoczi
2013-06-21 18:42                   ` Sage Weil
2013-06-21 19:08                     ` Alex Bligh
2013-06-21 19:13                     ` Anthony Liguori
2013-04-10 14:03     ` [Qemu-devel] [PATCH v2] rbd: add an asynchronous flush Josh Durgin
2013-04-11  8:02       ` Stefan Hajnoczi
2013-04-11  8:48         ` Kevin Wolf
2013-04-11 17:19           ` Josh Durgin
2013-04-12  6:50             ` Kevin Wolf
2013-04-12  7:42               ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJSP0QWy1CUOX+BVB2kfXfRpcdPuwbywZ3tReo83zK23+B4vbQ@mail.gmail.com \
    --to=stefanha@gmail.com \
    --cc=josh.durgin@inktank.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).