>
> Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
> On Wed, 2017-08-30 at 16:28 +0100, Frediano Ziglio wrote:
> > This patch allocates VMC IDs by finding the first ID not used
> > instead of using a global variable and incrementing the value
> > for each channel created.
> > This solves some potential issues:
> > - remove the global state potentially making possible
> > to use multiple SpiceServer on the same process;
> > - don't potentially overflow the variable. This can happen if
> > channels are allocated/deallocated multiple times
> > (currently not done by Qemu).
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/reds.c | 32 ++++++++++++++++++++++++++++++++
> > server/reds.h | 1 +
> > server/spicevmc.c | 10 ++++++++--
> > 3 files changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/server/reds.c b/server/reds.c
> > index a738afc3..b6ecc6c6 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -408,6 +408,38 @@ RedChannel *reds_find_channel(RedsState *reds,
> > uint32_t type, uint32_t id)
> > return NULL;
> > }
> >
> > +/* Search for first free channel id for a specific channel type.
> > + * Return first id free or <0 if not found. */
> > +int reds_get_free_channel_id(RedsState *reds, uint32_t type)
> > +{
> > + RedChannel *channel;
> > +
> > + // this mark if some IDs are used.
> > + // The size of the array limits the possible id returned but
> > + // usually the IDs used for a channel type are not much.
> > + bool used_ids[256];
> > +
I just realized due to protocol implementation that this
size is actually not a restriction, we are limited to
256 IDs in any case (a uint8 is used).
> > + unsigned n;
> > +
> > + // mark id used for the specific channel type
> > + memset(used_ids, 0, sizeof(used_ids));
> > + GLIST_FOREACH(reds->channels, RedChannel, channel) {
> > + uint32_t this_type, this_id;
> > + g_object_get(channel, "channel-type", &this_type, "id",
> > &this_id, NULL);
> > + if (this_type == type && this_id <
> > SPICE_N_ELEMENTS(used_ids)) {
> > + used_ids[this_id] = true;
> > + }
> > + }
> > +
> > + // find first ID not marked as used
> > + for (n = 0; n < SPICE_N_ELEMENTS(used_ids); ++n) {
> > + if (!used_ids[n]) {
> > + return n;
> > + }
> > + }
> > + return -1;
> > +}
> > +
> > static void reds_mig_cleanup(RedsState *reds)
> > {
> > if (reds->mig_inprogress) {
> > diff --git a/server/reds.h b/server/reds.h
> > index ee5a46c0..4f5fc28c 100644
> > --- a/server/reds.h
> > +++ b/server/reds.h
> > @@ -48,6 +48,7 @@ uint32_t reds_get_mm_time(void);
> > void reds_register_channel(RedsState *reds, RedChannel *channel);
> > void reds_unregister_channel(RedsState *reds, RedChannel *channel);
> > RedChannel *reds_find_channel(RedsState *reds, uint32_t type,
> > uint32_t id);
> > +int reds_get_free_channel_id(RedsState *reds, uint32_t type);
> > SpiceMouseMode reds_get_mouse_mode(RedsState *reds); // used by
> > inputs_channel
> > gboolean reds_config_get_agent_mouse(const RedsState *reds); // used
> > by inputs_channel
> > int reds_has_vdagent(RedsState *reds); // used by inputs channel
> > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > index 9305c9b4..08d9954b 100644
> > --- a/server/spicevmc.c
> > +++ b/server/spicevmc.c
> > @@ -272,7 +272,6 @@ red_vmc_channel_finalize(GObject *object)
> > static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t
> > channel_type)
> > {
> > GType gtype = G_TYPE_NONE;
> > - static uint8_t id[SPICE_END_CHANNEL] = { 0, };
> >
> > switch (channel_type) {
> > case SPICE_CHANNEL_USBREDIR:
> > @@ -288,11 +287,18 @@ static RedVmcChannel
> > *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
> > g_error("Unsupported channel_type for
> > red_vmc_channel_new(): %u", channel_type);
> > return NULL;
> > }
> > +
> > + int id = reds_get_free_channel_id(reds, channel_type);
> > + if (id < 0) {
> > + g_warning("Free ID not found creating new VMC channel");
> > + return NULL;
> > + }
> > +
> > return g_object_new(gtype,
> > "spice-server", reds,
> > "core-interface",
> > reds_get_core_interface(reds),
> > "channel-type", channel_type,
> > - "id", id[channel_type]++,
> > + "id", id,
> > "handle-acks", FALSE,
> > "migration-flags",
> > (SPICE_MIGRATE_NEED_FLUSH |
> > SPICE_MIGRATE_NEED_DATA_TRANSFER),
>
2 missing global variables to remove!
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel