Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
On Fri, Oct 24, 2014 at 10:44:33AM -0400, Marc-André Lureau wrote: - Original Message - Message will be used by client for requesting an information about a version of the agent running on the guest side. --- v2: - removed 'type' field from VDAgentInformation - VD_AGENT_INFORMATION changed to VD_AGENT_GUEST_VERSION VDAgentInformation changed to VDAgentGuestVersion - added VD_AGENT_CAP_GUEST_VERSION looks good; but I still worry about use fulness of this message. Having only the agent version is really a small part of what is the guest actually running, and it might be misleading (missing configure options, patches, dependencies etc) The sent data is a string, we can always stick more than a simple version number in it. If we go that way, a more generic name would be better though. Christophe pgp7dodc6PxOW.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
- Mail original - On Fri, Oct 24, 2014 at 10:44:33AM -0400, Marc-André Lureau wrote: - Original Message - Message will be used by client for requesting an information about a version of the agent running on the guest side. --- v2: - removed 'type' field from VDAgentInformation - VD_AGENT_INFORMATION changed to VD_AGENT_GUEST_VERSION VDAgentInformation changed to VDAgentGuestVersion - added VD_AGENT_CAP_GUEST_VERSION looks good; but I still worry about use fulness of this message. Having only the agent version is really a small part of what is the guest actually running, and it might be misleading (missing configure options, patches, dependencies etc) The sent data is a string, we can always stick more than a simple version number in it. If we go that way, a more generic name would be better though. If this message is only meant for user display only (which I think it should), then it could be defined as free-form text containing various guest details, in which case I would also propose to rename GUEST_VERSION - GUEST_INFOS. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
On Wed, 2014-10-29 at 09:48 -0400, Marc-André Lureau wrote: - Mail original - On Fri, Oct 24, 2014 at 10:44:33AM -0400, Marc-André Lureau wrote: - Original Message - Message will be used by client for requesting an information about a version of the agent running on the guest side. nitpicks: - requesting information, not requesting an information - the version, not a version --- v2: - removed 'type' field from VDAgentInformation - VD_AGENT_INFORMATION changed to VD_AGENT_GUEST_VERSION VDAgentInformation changed to VDAgentGuestVersion - added VD_AGENT_CAP_GUEST_VERSION looks good; but I still worry about use fulness of this message. Having only the agent version is really a small part of what is the guest actually running, and it might be misleading (missing configure options, patches, dependencies etc) The sent data is a string, we can always stick more than a simple version number in it. If we go that way, a more generic name would be better though. If this message is only meant for user display only (which I think it should), then it could be defined as free-form text containing various guest details, in which case I would also propose to rename GUEST_VERSION - GUEST_INFOS. I'm not sure that I'm a big fan of a free-form field. It would be nice if it had at least *some* structure and you could expert certain information to be there... Regarding the name, it should be GUEST_INFO rather than GUEST_INFOS (if we go that route) since 'informations' is not really a valid word. jonathon ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
On Wed, Oct 29, 2014 at 3:34 PM, Jonathon Jongsma jjong...@redhat.com wrote: I'm not sure that I'm a big fan of a free-form field. It would be nice if it had at least *some* structure and you could expert certain information to be there... Then I would have a defined structured message in the protocol rather than a simple text field. I would also state clearly the informative only purpose. Regarding the name, it should be GUEST_INFO rather than GUEST_INFOS (if we go that route) since 'informations' is not really a valid word. right! thanks -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
On Wed, Oct 29, 2014 at 4:31 PM, Pavel Grunt pgr...@redhat.com wrote: What do you think about using the message structure that was suggested in v1 (with the type field specifying what type of information is provided) ? Please detail how that would help: to me it is introducing an agent message subtype, and I don't see the need, you can simply use the agent message type (which is already a subtype of spice main messages) -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
Please detail how that would help: to me it is introducing an agent message subtype, and I don't see the need, you can simply use the agent message type (which is already a subtype of spice main messages) I wanted to use the type field to determine the type of information of the text field. I take your point, the agent message type can be used for that. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-protocol v2] Add agent information message
Message will be used by client for requesting an information about a version of the agent running on the guest side. --- v2: - removed 'type' field from VDAgentInformation - VD_AGENT_INFORMATION changed to VD_AGENT_GUEST_VERSION VDAgentInformation changed to VDAgentGuestVersion - added VD_AGENT_CAP_GUEST_VERSION --- spice/vd_agent.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/spice/vd_agent.h b/spice/vd_agent.h index 7464661..31356a8 100644 --- a/spice/vd_agent.h +++ b/spice/vd_agent.h @@ -77,6 +77,7 @@ enum { VD_AGENT_FILE_XFER_DATA, VD_AGENT_CLIENT_DISCONNECTED, VD_AGENT_MAX_CLIPBOARD, +VD_AGENT_GUEST_VERSION, VD_AGENT_END_MESSAGE, }; @@ -218,6 +219,7 @@ enum { VD_AGENT_CAP_GUEST_LINEEND_LF, VD_AGENT_CAP_GUEST_LINEEND_CRLF, VD_AGENT_CAP_MAX_CLIPBOARD, +VD_AGENT_CAP_GUEST_VERSION, VD_AGENT_END_CAP, }; @@ -245,6 +247,10 @@ typedef struct SPICE_ATTR_PACKED VDAgentAnnounceCapabilities { #define VD_AGENT_SET_CAPABILITY(caps, index) \ { (caps)[(index) / 32] |= (1 ((index) % 32)); } +typedef struct SPICE_ATTR_PACKED VDAgentGuestVersion { +uint8_t data[0]; +} VDAgentGuestVersion; + #include spice/end-packed.h #endif /* _H_VD_AGENT */ -- 1.9.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol v2] Add agent information message
- Original Message - Message will be used by client for requesting an information about a version of the agent running on the guest side. --- v2: - removed 'type' field from VDAgentInformation - VD_AGENT_INFORMATION changed to VD_AGENT_GUEST_VERSION VDAgentInformation changed to VDAgentGuestVersion - added VD_AGENT_CAP_GUEST_VERSION looks good; but I still worry about use fulness of this message. Having only the agent version is really a small part of what is the guest actually running, and it might be misleading (missing configure options, patches, dependencies etc) --- spice/vd_agent.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/spice/vd_agent.h b/spice/vd_agent.h index 7464661..31356a8 100644 --- a/spice/vd_agent.h +++ b/spice/vd_agent.h @@ -77,6 +77,7 @@ enum { VD_AGENT_FILE_XFER_DATA, VD_AGENT_CLIENT_DISCONNECTED, VD_AGENT_MAX_CLIPBOARD, +VD_AGENT_GUEST_VERSION, VD_AGENT_END_MESSAGE, }; @@ -218,6 +219,7 @@ enum { VD_AGENT_CAP_GUEST_LINEEND_LF, VD_AGENT_CAP_GUEST_LINEEND_CRLF, VD_AGENT_CAP_MAX_CLIPBOARD, +VD_AGENT_CAP_GUEST_VERSION, VD_AGENT_END_CAP, }; @@ -245,6 +247,10 @@ typedef struct SPICE_ATTR_PACKED VDAgentAnnounceCapabilities { #define VD_AGENT_SET_CAPABILITY(caps, index) \ { (caps)[(index) / 32] |= (1 ((index) % 32)); } +typedef struct SPICE_ATTR_PACKED VDAgentGuestVersion { +uint8_t data[0]; +} VDAgentGuestVersion; + #include spice/end-packed.h #endif /* _H_VD_AGENT */ -- 1.9.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel