[Spice-devel] [PATCH spice-common] proto: Add agent features message

[Marc-André Lureau]

Hi

----- Original Message -----
> Hi Marc-André,
> 
> On Thu, 2016-09-15 at 06:28 -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > Hi
> > > 
> > > ----- Original Message -----
> > > > The message is sent by server to the client to indicate
> > > > which agent features are enabled or disabled by using flags.
> > > > 
> > > > If a flag is set, then the corresponding feature is enabled.
> > > > 
> > > > The message currently supports info about copy & paste and file
> > > > transfer.
> > > 
> > > 
> > > Have you considered using channel capabilities? it seems to me it
> > > would fit
> > > nicely, beside there is already APIs, and the flag are not limited
> > > to 32
> > > bits.
> > > 
> 
> Yes, I have, but the channel caps are only sent once, no? The filter
> can be changed on the fly by calling
> spice_server_set_agent_file_xfer() etc. and client should be informed
> about that.
> 

ok

> > 
> > 
> > Actually, there is agent capabilities, I think that's what the
> > server should be overriding instead.
> 
> I know that is possible but imo it is hack. It would be needed to
> filter VD_AGENT_ANNOUNCE_CAPABILITIES from the agent, insert something
> like VD_AGENT_CAP_FILE_XFER_DISABLED, VD_AGENT_CAP_COPY_PASTE_DISABLED
> and also in the case that the filter is changed on the fly, it would
> be needed to generate complete VD_AGENT_ANNOUNCE_CAPABILITIES (or
> request agent to send them). I think it would be more complicated...

I don't think it is so complicated, but I might be wrong. The server already parses some agents messages for filtering.

At least I think it would be cleaner from the protocol POV. I don't see much benefit for the client to know that the server disabled something explicitely vs the agent not having the capability.

> 
> Thanks,
> Pavel
> 
> > 
> > > > 
> > > > Related:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> > > > ---
> > > >  common/messages.h | 4 ++++
> > > >  configure.ac      | 2 +-
> > > >  spice.proto       | 9 +++++++++
> > > >  3 files changed, 14 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/common/messages.h b/common/messages.h
> > > > index 516a345..f8648b6 100644
> > > > --- a/common/messages.h
> > > > +++ b/common/messages.h
> > > > @@ -99,6 +99,10 @@ typedef struct
> > > > SpiceMsgMainMigrateBeginSeamless {
> > > >      uint32_t src_mig_version;
> > > >  } SpiceMsgMainMigrateBeginSeamless;
> > > >  
> > > > +typedef struct SpiceMsgMainAgentFeatures {
> > > > +    uint32_t flags;
> > > > +} SpiceMsgMainAgentFeatures;
> > > > +
> > > >  typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> > > >      uint32_t src_version;
> > > >  } SpiceMsgcMainMigrateDstDoSeamless;
> > > > diff --git a/configure.ac b/configure.ac
> > > > index c3ad5a4..c0e49b7 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -28,7 +28,7 @@ AM_PROG_CC_C_O
> > > >  SPICE_CHECK_SYSDEPS
> > > >  
> > > >  # Checks for libraries
> > > > -PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
> > > > +PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.13])
> > > >  
> > > >  SPICE_CHECK_PYTHON_MODULES()
> > > >  
> > > > diff --git a/spice.proto b/spice.proto
> > > > index 0bfc515..ea4dfc2 100644
> > > > --- a/spice.proto
> > > > +++ b/spice.proto
> > > > @@ -226,6 +226,11 @@ struct DstInfo {
> > > >  	uint8 *cert_subject_data[cert_subject_size]
> > > > @zero_terminated @marshall;
> > > >  } @ctype(SpiceMigrationDstInfo);
> > > >  
> > > > +flags32 agent_features_flags {
> > > > +    COPY_PASTE,
> > > > +    FILE_TRANSFER
> > > > +} @prefix(SPICE_AGENT_FEATURE_);
> > > > +
> > > >  channel MainChannel : BaseChannel {
> > > >   server:
> > > >       message {
> > > > @@ -303,6 +308,10 @@ channel MainChannel : BaseChannel {
> > > >      Empty migrate_dst_seamless_ack;
> > > >      Empty migrate_dst_seamless_nack;
> > > >  
> > > > +    message {
> > > > +        agent_features_flags flags;
> > > > +    } agent_features;
> > > > +
> > > >   client:
> > > >      message {
> > > >  	uint64 cache_size;
> > > > --
> > > > 2.10.0
> > > > 
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > 
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>