With the right patch attached this time.. ;) I've only compile-tested this as this really is just a proof of concept at this point.
Christophe
On Mon, Mar 26, 2018 at 06:47:08PM +0200, Christophe Fergeau wrote:
> On Mon, Mar 26, 2018 at 11:27:19AM +0200, Christophe de Dinechin wrote:
> >
> >
> > > On 26 Mar 2018, at 11:25, Christophe Fergeau <cferg...@redhat.com> wrote:
> > >
> > > Hey,
> > >
> > > On Fri, Mar 23, 2018 at 01:05:22PM +0100, Christophe de Dinechin wrote:
> > >> From: Christophe de Dinechin <dinec...@redhat.com>
> > >>
> > >> Ensure that plugin version checking cannot be bypassed.
> > >> Implement version checking without violating the C++ ODR.
> > >> Improve ABI version numbering when incompatibilites are detected
> > >> post-release.
> > >> Add macro to make it easier to generate the ABI interface.
> > >>
> > >> Christophe de Dinechin (2):
> > >> Ensure that plugins cannot bypass version check
> > >> Add a macro to deal with the boilerplate of writing a streaming agent
> > >> plugin
> > >
> > > Could this be split in 4 patches each implementing one of these changes?
> >
> > I don’t see how. Any idea on how this could be done?
>
> I would at least split the change regarding how the versioning is
> managed, see attached patch for a proof of concept (most likely needs
> some adjustment, no proper logs, ..). The ODR change/ensure that plugin
> cannot bypass version check can probably be considered two faces of the
> same coin).
>
> Christophe
> From f3b8d49752949c8b24efb3bc6515cb31cc81ebb0 Mon Sep 17 00:00:00 2001
> From: Frediano Ziglio <fzig...@redhat.com>
> Date: Fri, 16 Mar 2018 11:31:41 +0100
> Subject: [spice-server 1/3] Preparation work
>
> ---
> server/stream-channel.c | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/server/stream-channel.c b/server/stream-channel.c
> index 88f859f6d..3fab28ae1 100644
> --- a/server/stream-channel.c
> +++ b/server/stream-channel.c
> @@ -508,17 +508,10 @@ data_item_free(RedPipeItem *base)
> g_free(pipe_item);
> }
>
> -void
> -stream_channel_send_data(StreamChannel *channel, const void *data, size_t
> size, uint32_t mm_time)
> +static StreamDataItem*
> +stream_channel_new_data_item(StreamChannel *channel, size_t size, uint32_t
> mm_time)
> {
> - if (channel->stream_id < 0) {
> - // this condition can happen if the guest didn't handle
> - // the format stop that we send so think the stream is still
> - // started
> - return;
> - }
> -
> - RedChannel *red_channel = RED_CHANNEL(channel);
> + stream_channel_unref_data_item(channel);
>
> StreamDataItem *item = g_malloc(sizeof(*item) + size);
> red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_STREAM_DATA,
> @@ -527,9 +520,29 @@ stream_channel_send_data(StreamChannel *channel, const
> void *data, size_t size,
> item->data.base.multi_media_time = mm_time;
> item->data.data_size = size;
> item->channel = channel;
> - stream_channel_update_queue_stat(channel, 1, size);
> +
> + return item;
> +}
> +
> +void
> +stream_channel_send_data(StreamChannel *channel, const void *data, size_t
> size, uint32_t mm_time)
> +{
> + RedChannel *red_channel = RED_CHANNEL(channel);
> +
> + if (channel->stream_id < 0) {
> + // this condition can happen if the guest didn't handle
> + // the format stop that we send so think the stream is still
> + // started
> + return;
> + }
> +
> + StreamDataItem *item;
> +
> + item = stream_channel_new_data_item(channel, size, mm_time);
> +
> // TODO try to optimize avoiding the copy
> memcpy(item->data.data, data, size);
> + stream_channel_update_queue_stat(channel, 1, size);
> red_channel_pipes_add(red_channel, &item->base);
> }
>
> --
> 2.14.3
>
>
> From c8c204c5482e3bfa0038ea9829fb96923274ee29 Mon Sep 17 00:00:00 2001
> From: Frediano Ziglio <fzig...@redhat.com>
> Date: Fri, 16 Mar 2018 11:44:21 +0100
> Subject: [spice-server 2/3] more preparation work
>
> ---
> server/stream-channel.c | 57
> +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/server/stream-channel.c b/server/stream-channel.c
> index 3fab28ae1..b5652e5ff 100644
> --- a/server/stream-channel.c
> +++ b/server/stream-channel.c
> @@ -44,6 +44,7 @@
>
> typedef struct StreamChannelClient StreamChannelClient;
> typedef struct StreamChannelClientClass StreamChannelClientClass;
> +typedef struct StreamDataItem StreamDataItem;
>
> /* we need to inherit from CommonGraphicsChannelClient
> * to get buffer handling */
> @@ -74,6 +75,10 @@ struct StreamChannel {
>
> StreamQueueStat queue_stat;
>
> + /* pending partial data item */
> + StreamDataItem *data_item;
> + uint32_t data_item_pos;
> +
> /* callback to notify when a stream should be started or stopped */
> stream_channel_start_proc start_cb;
> void *start_opaque;
> @@ -104,12 +109,12 @@ typedef struct StreamCreateItem {
> SpiceMsgDisplayStreamCreate stream_create;
> } StreamCreateItem;
>
> -typedef struct StreamDataItem {
> +struct StreamDataItem {
> RedPipeItem base;
> StreamChannel *channel;
> // NOTE: this must be the last field in the structure
> SpiceMsgDisplayStreamData data;
> -} StreamDataItem;
> +};
>
> #define PRIMARY_SURFACE_ID 0
>
> @@ -129,6 +134,16 @@ stream_channel_client_init(StreamChannelClient *client)
> client->stream_id = -1;
> }
>
> +static void
> +stream_channel_unref_data_item(StreamChannel *channel)
> +{
> + if (channel->data_item) {
> + red_pipe_item_unref(&channel->data_item->base);
> + channel->data_item = NULL;
> + channel->data_item_pos = 0;
> + }
> +}
> +
> static void
> request_new_stream(StreamChannel *channel, StreamMsgStartStop *start)
> {
> @@ -152,6 +167,7 @@ stream_channel_client_on_disconnect(RedChannelClient *rcc)
> channel->stream_id = -1;
> channel->width = 0;
> channel->height = 0;
> + stream_channel_unref_data_item(channel);
>
> // send stream stop to device
> StreamMsgStartStop stop = { 0, };
> @@ -424,6 +440,16 @@ stream_channel_constructed(GObject *object)
> reds_register_channel(reds, red_channel);
> }
>
> +static void
> +stream_channel_finalize(GObject *object)
> +{
> + StreamChannel *channel = STREAM_CHANNEL(object);
> +
> + stream_channel_unref_data_item(channel);
> +
> + G_OBJECT_CLASS(stream_channel_parent_class)->finalize(object);
> +}
> +
> static void
> stream_channel_class_init(StreamChannelClass *klass)
> {
> @@ -431,6 +457,7 @@ stream_channel_class_init(StreamChannelClass *klass)
> RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>
> object_class->constructed = stream_channel_constructed;
> + object_class->finalize = stream_channel_finalize;
>
> channel_class->parser =
> spice_get_client_channel_parser(SPICE_CHANNEL_DISPLAY, NULL);
> channel_class->handle_message = handle_message;
> @@ -503,7 +530,9 @@ data_item_free(RedPipeItem *base)
> {
> StreamDataItem *pipe_item = SPICE_UPCAST(StreamDataItem, base);
>
> - stream_channel_update_queue_stat(pipe_item->channel, -1,
> -pipe_item->data.data_size);
> + if (pipe_item->channel->data_item != pipe_item) {
> + stream_channel_update_queue_stat(pipe_item->channel, -1,
> -pipe_item->data.data_size);
> + }
>
> g_free(pipe_item);
> }
> @@ -521,6 +550,9 @@ stream_channel_new_data_item(StreamChannel *channel,
> size_t size, uint32_t mm_ti
> item->data.data_size = size;
> item->channel = channel;
>
> + channel->data_item = item;
> + channel->data_item_pos = 0;
> +
> return item;
> }
>
> @@ -536,14 +568,20 @@ stream_channel_send_data(StreamChannel *channel, const
> void *data, size_t size,
> return;
> }
>
> - StreamDataItem *item;
> + while (size) {
> + StreamDataItem *item = channel->data_item;
>
> - item = stream_channel_new_data_item(channel, size, mm_time);
> + if (!item) {
> + item = stream_channel_new_data_item(channel, size, mm_time);
> + }
>
> - // TODO try to optimize avoiding the copy
> - memcpy(item->data.data, data, size);
> - stream_channel_update_queue_stat(channel, 1, size);
> - red_channel_pipes_add(red_channel, &item->base);
> + size_t copy_size = size;
> + // TODO try to optimize avoiding the copy
> + memcpy(item->data.data, data, copy_size);
> + size -= copy_size;
> + stream_channel_update_queue_stat(channel, 1, size);
> + red_channel_pipes_add(red_channel, &item->base);
> + }
> }
>
> void
> @@ -583,6 +621,7 @@ stream_channel_reset(StreamChannel *channel)
> channel->stream_id = -1;
> channel->width = 0;
> channel->height = 0;
> + stream_channel_unref_data_item(channel);
>
> if (!red_channel_is_connected(red_channel)) {
> return;
> --
> 2.14.3
>
>
> From 43b50f11428313d395c7c01bb3c93356c76f9ac8 Mon Sep 17 00:00:00 2001
> From: Frediano Ziglio <fzig...@redhat.com>
> Date: Wed, 7 Mar 2018 12:01:49 +0000
> Subject: [spice-server 3/3] stream-channel: Send the full frame in a single
> message
>
> The current implementation of server and client assumes that a single
> data message contains an encoded frame.
> This is not a problem for most encoding but for MJPEG this causes
> the client to fail decoding.
> Collapse frame data into a single message before sending to the client.
> This is done in the channel as the channel code is responsible to take care
> of client protocol details. This allows for instance to support chunked
> transfer to client if implemented.
>
> Signed-off-by: Frediano Ziglio <fzig...@redhat.com>
> ---
> server/red-stream-device.c | 10 ++++++----
> server/stream-channel.c | 27 +++++++++++++++++++++++----
> server/stream-channel.h | 12 ++++++++++++
> 3 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index c87c4899d..81336e549 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -271,11 +271,13 @@ handle_msg_data(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> if (n <= 0) {
> break;
> }
> - // TODO collect all message ??
> - // up: we send a single frame together
> - // down: guest can cause a crash
> - stream_channel_send_data(dev->stream_channel, buf, n,
> reds_get_mm_time());
> + uint32_t mm_time = reds_get_mm_time();
> + if (dev->msg_pos == 0) {
> + stream_channel_start_data(dev->stream_channel, dev->hdr.size,
> mm_time);
> + }
> + stream_channel_send_data(dev->stream_channel, buf, n, mm_time);
> dev->hdr.size -= n;
> + dev->msg_pos += n;
> }
>
> return dev->hdr.size == 0;
> diff --git a/server/stream-channel.c b/server/stream-channel.c
> index b5652e5ff..75dd1f867 100644
> --- a/server/stream-channel.c
> +++ b/server/stream-channel.c
> @@ -556,6 +556,20 @@ stream_channel_new_data_item(StreamChannel *channel,
> size_t size, uint32_t mm_ti
> return item;
> }
>
> +void
> +stream_channel_start_data(StreamChannel *channel, size_t size, uint32_t
> mm_time)
> +{
> + // see stream_channel_send_data comment
> + if (channel->stream_id < 0) {
> + return;
> + }
> +
> + // TODO this collects all chunks in a single message
> + // up: we send a single frame together (more compatible)
> + // down: guest can cause a crash due to DoS. As a safe measure we limit
> the maximum message
> + stream_channel_new_data_item(channel, MIN(size, 32*1024*1024), mm_time);
> +}
> +
> void
> stream_channel_send_data(StreamChannel *channel, const void *data, size_t
> size, uint32_t mm_time)
> {
> @@ -575,12 +589,17 @@ stream_channel_send_data(StreamChannel *channel, const
> void *data, size_t size,
> item = stream_channel_new_data_item(channel, size, mm_time);
> }
>
> - size_t copy_size = size;
> + size_t copy_size = item->data.data_size - channel->data_item_pos;
> + copy_size = MIN(copy_size, size);
> // TODO try to optimize avoiding the copy
> - memcpy(item->data.data, data, copy_size);
> + memcpy(item->data.data + channel->data_item_pos, data, copy_size);
> size -= copy_size;
> - stream_channel_update_queue_stat(channel, 1, size);
> - red_channel_pipes_add(red_channel, &item->base);
> + channel->data_item_pos += copy_size;
> + if (channel->data_item_pos == item->data.data_size) {
> + channel->data_item = NULL;
> + stream_channel_update_queue_stat(channel, 1,
> item->data.data_size);
> + red_channel_pipes_add(red_channel, &item->base);
> + }
> }
> }
>
> diff --git a/server/stream-channel.h b/server/stream-channel.h
> index e8bec80bd..18a1bdead 100644
> --- a/server/stream-channel.h
> +++ b/server/stream-channel.h
> @@ -60,6 +60,18 @@ struct StreamMsgStartStop;
>
> void stream_channel_change_format(StreamChannel *channel,
> const struct StreamMsgFormat *fmt);
> +
> +/**
> + * Tell the channel that a new data packet is starting.
> + * This can be used to group all chunks together.
> + */
> +void stream_channel_start_data(StreamChannel *channel,
> + size_t size,
> + uint32_t mm_time);
> +
> +/**
> + * Send to channel a chunk of data.
> + */
> void stream_channel_send_data(StreamChannel *channel,
> const void *data, size_t size,
> uint32_t mm_time);
> --
> 2.14.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
From 758edbedb6a8b771ee0c45fca522c2b510d58784 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cferg...@redhat.com>
Date: Mon, 26 Mar 2018 14:46:46 +0200
Subject: [spice-streaming-agent 1/3] mjpeg: Remove obsolete version check
This plugin code is statically linked with the agent, so having a
version check in it is not useful.
---
src/mjpeg-fallback.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
index 68c282f..605a4b3 100644
--- a/src/mjpeg-fallback.cpp
+++ b/src/mjpeg-fallback.cpp
@@ -180,9 +180,6 @@ SpiceVideoCodecType MjpegPlugin::VideoCodecType() const {
bool MjpegPlugin::Register(Agent* agent)
{
- if (agent->Version() != PluginVersion)
- return false;
-
std::unique_ptr<MjpegPlugin> plugin(new MjpegPlugin());
try {
--
2.14.3
From 5948c5840c50beb780cc5ef38a35fef2e2a340ad Mon Sep 17 00:00:00 2001
From: Christophe de Dinechin <dinec...@redhat.com>
Date: Fri, 23 Mar 2018 13:05:23 +0100
Subject: [spice-streaming-agent 2/3] Ensure that plugins cannot bypass version
check
This change addresses three issues related to plugin version checking:
1. It is possible for plugins to bypass version checking or do it wrong
(as a matter of fact, the mjpeg fallback sets a bad example)
2. The current plugin version check violates the C++ ODR, i.e.
it relies on undefined behaviors when the header used to compile
the plugin and to compile the agent are not identical,
[More info]
1. Make it impossible to bypass version checking
The current code depends on the plugin implementing the version check
correctly by calling PluginVersionIsCompatible. To make things worse,
the only publicly available example gets this wrong and uses an
ad-hoc version check, so anybody copy-pasting this code will get it
wrong.
It is more robust to do the version check in the agent before calling
any method in the plugin. It ensures that version checking cannot be
bypassed, is done consistently and generates consistent error messages.
As an indication that the aproach is robust, the new check correctly
refuses to load older plugins that use the old version checking method:
spice-streaming-agent[5167]:
error loading plugin [...].so: no version information
2. ODR-related problems
The C++ One Definition Rule (ODR) states that all translation units
must see the same definitions. In the current code, when we call
Agent::PluginVersionIsCompatible from the plugin, it is an ODR
violation as soon as we have made any change in the Agent class
compared to what the plugin was compiled against.
The current code therefore relies on implementation dependent knowlege
of how virtual functions are laid out in the vtable, and puts
unnecessary constraints on the changes allowed in the classes
(e.g. it's not allowed to put anything before some member functions)
Signed-off-by: Christophe de Dinechin <dinec...@redhat.com>
---
include/spice-streaming-agent/plugin.hpp | 39 ++++++++++++++++----------------
src/concrete-agent.cpp | 22 +++++++++++++++---
src/concrete-agent.hpp | 5 +---
3 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/include/spice-streaming-agent/plugin.hpp
b/include/spice-streaming-agent/plugin.hpp
index e08e3a6..142689f 100644
--- a/include/spice-streaming-agent/plugin.hpp
+++ b/include/spice-streaming-agent/plugin.hpp
@@ -102,20 +102,6 @@ public:
class Agent
{
public:
- /*!
- * Get agent version.
- * Plugin should check the version for compatibility before doing
- * everything.
- * \return version specified like PluginVersion
- */
- virtual unsigned Version() const = 0;
-
- /*!
- * Check if a given plugin version is compatible with this agent
- * \return true is compatible
- */
- virtual bool PluginVersionIsCompatible(unsigned pluginVersion) const = 0;
-
/*!
* Register a plugin in the system.
*/
@@ -135,19 +121,34 @@ typedef bool
PluginInitFunc(spice::streaming_agent::Agent* agent);
}} // namespace spice::streaming_agent
#ifndef SPICE_STREAMING_AGENT_PROGRAM
+/*!
+ * Plugin interface version
+ * Each plugin should define this variable and set it to PluginInterfaceVersion
+ * That version will be checked by the agent before executing any plugin code
+ */
+extern "C" unsigned spice_streaming_agent_plugin_interface_version;
+
/*!
* Plugin main entry point.
- * Plugins should check if the version of the agent is compatible.
- * If is compatible should register itself to the agent and return
- * true.
- * If is not compatible can decide to stay in memory or not returning
- * true (do not unload) or false (safe to unload). This is necessary
+ * This entry point is only called if the version check passed.
+ * It should return true if it loaded and initialized successfully.
+ * If the plugin does not initialize and does not want to be unloaded,
+ * it may still return true on failure. This is necessary
* if the plugin uses some library which are not safe to be unloaded.
* This public interface is also designed to avoid exporting data from
* the plugin which could be a problem in some systems.
* \return true if plugin should stay loaded, false otherwise
*/
extern "C" spice::streaming_agent::PluginInitFunc
spice_streaming_agent_plugin_init;
+
+#define SPICE_STREAMING_AGENT_PLUGIN(agent) \
+ __attribute__ ((visibility ("default"))) \
+ unsigned spice_streaming_agent_plugin_version = \
+ spice::streaming_agent::PluginVersion; \
+ \
+ __attribute__ ((visibility ("default"))) \
+ bool spice_streaming_agent_plugin_init(spice::streaming_agent::Agent*
agent)
+
#endif
#endif // SPICE_STREAMING_AGENT_PLUGIN_HPP
diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
index 4cf70e7..e7f7457 100644
--- a/src/concrete-agent.cpp
+++ b/src/concrete-agent.cpp
@@ -32,9 +32,9 @@ ConcreteAgent::ConcreteAgent()
bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const
{
- unsigned version = Version();
- return MajorVersion(version) == MajorVersion(pluginVersion) &&
- MinorVersion(version) >= MinorVersion(pluginVersion);
+ unsigned current_agent_abi_version = PluginVersion;
+ return MajorVersion(current_agent_abi_version) ==
MajorVersion(pluginVersion) &&
+ MinorVersion(current_agent_abi_version) >= MinorVersion(pluginVersion);
}
void ConcreteAgent::Register(Plugin& plugin)
@@ -83,6 +83,22 @@ void ConcreteAgent::LoadPlugin(const std::string
&plugin_filename)
return;
}
+ unsigned *version =
+ (unsigned *) dlsym(dl, "spice_streaming_agent_plugin_version");
+ if (!version) {
+ syslog(LOG_ERR, "error loading plugin %s: no version information",
+ plugin_filename.c_str());
+ return;
+ }
+ if (!PluginVersionIsCompatible(*version)) {
+ syslog(LOG_ERR,
+ "error loading plugin %s: plugin version %u, "
+ "agent version is %u",
+ plugin_filename.c_str(), *version,
+ PluginVersion);
+ return;
+ }
+
try {
PluginInitFunc* init_func =
(PluginInitFunc *) dlsym(dl, "spice_streaming_agent_plugin_init");
diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
index 5bca23b..c0cf9ba 100644
--- a/src/concrete-agent.hpp
+++ b/src/concrete-agent.hpp
@@ -27,17 +27,14 @@ class ConcreteAgent final : public Agent
{
public:
ConcreteAgent();
- unsigned Version() const override {
- return PluginVersion;
- }
void Register(Plugin& plugin) override;
const ConfigureOption* Options() const override;
void LoadPlugins(const std::string &directory);
// pointer must remain valid
void AddOption(const char *name, const char *value);
FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>&
codecs);
- bool PluginVersionIsCompatible(unsigned pluginVersion) const override;
private:
+ bool PluginVersionIsCompatible(unsigned pluginVersion) const;
void LoadPlugin(const std::string &plugin_filename);
std::vector<std::shared_ptr<Plugin>> plugins;
std::vector<ConcreteConfigureOption> options;
--
2.14.3
From 250b62be1150b8c252f81cb4a61c4338c1f080fe Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cferg...@redhat.com>
Date: Mon, 26 Mar 2018 14:38:13 +0200
Subject: [spice-streaming-agent 3/3] Versioning changes
---
include/spice-streaming-agent/plugin.hpp | 23 +++++++++++++++++------
src/concrete-agent.cpp | 29 +++++++----------------------
src/concrete-agent.hpp | 1 -
3 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/include/spice-streaming-agent/plugin.hpp
b/include/spice-streaming-agent/plugin.hpp
index 142689f..b01cd82 100644
--- a/include/spice-streaming-agent/plugin.hpp
+++ b/include/spice-streaming-agent/plugin.hpp
@@ -23,11 +23,22 @@ namespace streaming_agent {
class FrameCapture;
/*!
- * Plugin version, only using few bits, schema is 0xMMmm
- * where MM is major and mm is the minor, can be easily expanded
- * using more bits in the future
+ * Plugins use a versioning system similar to that implemented by libtool
+ * http://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
+ * Update the version information as follows:
+ * [ANY CHANGE] If any interfaces have been added, removed, or changed since
the last update,
+ * increment PluginInterfaceVersion.
+ * [COMPATIBLE CHANGE] If interfaces have only been added since the last
public release,
+ * leave PluginInterfaceOldestCompatibleVersion identical.
+ * [INCOMPATIBLE CHANGE] If any interfaces have been removed or changed since
the last release,
+ * set PluginInterfaceOldestCompatibleVersion to PluginInterfaceVersion.
+ * [DETECTED INCOMPATIBILITY]: If an incompatibility is detected after a
release,
+ * set PluginInterfaceOldestCompatibleVersion to the last known compatible
version.
*/
-enum Constants : unsigned { PluginVersion = 0x100u };
+enum Constants : unsigned {
+ PluginInterfaceVersion = 1,
+ PluginInterfaceOldestCompatibleVersion = 1
+};
enum Ranks : unsigned {
/// this plugin should not be used
@@ -143,8 +154,8 @@ extern "C" spice::streaming_agent::PluginInitFunc
spice_streaming_agent_plugin_i
#define SPICE_STREAMING_AGENT_PLUGIN(agent) \
__attribute__ ((visibility ("default"))) \
- unsigned spice_streaming_agent_plugin_version = \
- spice::streaming_agent::PluginVersion; \
+ unsigned spice_streaming_agent_plugin_interface_version = \
+ spice::streaming_agent::PluginInterfaceVersion; \
\
__attribute__ ((visibility ("default"))) \
bool spice_streaming_agent_plugin_init(spice::streaming_agent::Agent*
agent)
diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
index e7f7457..eb4f333 100644
--- a/src/concrete-agent.cpp
+++ b/src/concrete-agent.cpp
@@ -15,28 +15,11 @@
using namespace spice::streaming_agent;
-static inline unsigned MajorVersion(unsigned version)
-{
- return version >> 8;
-}
-
-static inline unsigned MinorVersion(unsigned version)
-{
- return version & 0xffu;
-}
-
ConcreteAgent::ConcreteAgent()
{
options.push_back(ConcreteConfigureOption(nullptr, nullptr));
}
-bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const
-{
- unsigned current_agent_abi_version = PluginVersion;
- return MajorVersion(current_agent_abi_version) ==
MajorVersion(pluginVersion) &&
- MinorVersion(current_agent_abi_version) >= MinorVersion(pluginVersion);
-}
-
void ConcreteAgent::Register(Plugin& plugin)
{
plugins.push_back(std::shared_ptr<Plugin>(&plugin));
@@ -84,18 +67,20 @@ void ConcreteAgent::LoadPlugin(const std::string
&plugin_filename)
}
unsigned *version =
- (unsigned *) dlsym(dl, "spice_streaming_agent_plugin_version");
+ (unsigned *) dlsym(dl,
"spice_streaming_agent_plugin_interface_version");
if (!version) {
syslog(LOG_ERR, "error loading plugin %s: no version information",
plugin_filename.c_str());
return;
}
- if (!PluginVersionIsCompatible(*version)) {
+ if (*version < PluginInterfaceOldestCompatibleVersion ||
+ *version > PluginInterfaceVersion) {
syslog(LOG_ERR,
- "error loading plugin %s: plugin version %u, "
- "agent version is %u",
+ "error loading plugin %s: plugin interface version %u, "
+ "agent accepts version %u...%u",
plugin_filename.c_str(), *version,
- PluginVersion);
+ PluginInterfaceOldestCompatibleVersion,
+ PluginInterfaceVersion);
return;
}
diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
index c0cf9ba..c631916 100644
--- a/src/concrete-agent.hpp
+++ b/src/concrete-agent.hpp
@@ -34,7 +34,6 @@ public:
void AddOption(const char *name, const char *value);
FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>&
codecs);
private:
- bool PluginVersionIsCompatible(unsigned pluginVersion) const;
void LoadPlugin(const std::string &plugin_filename);
std::vector<std::shared_ptr<Plugin>> plugins;
std::vector<ConcreteConfigureOption> options;
--
2.14.3
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
