[Spice-devel] [PATCH spice-protocol] agent: Add support for reporting on free space

[Victor Toso]

Hi,

On Wed, May 03, 2017 at 04:28:36PM +0200, Jakub Janků wrote:
> Agent can send VDAgentFileXferStatusMessage with result
> VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE to indicate lack of free
> space. This enables more detailed error reporting, so the user knows
> why the file transfer has failed.
> ---
>  spice/vd_agent.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/spice/vd_agent.h b/spice/vd_agent.h
> index 3b1f183..3c4de05 100644
> --- a/spice/vd_agent.h
> +++ b/spice/vd_agent.h
> @@ -99,11 +99,16 @@ enum {
>      VD_AGENT_FILE_XFER_STATUS_CANCELLED,
>      VD_AGENT_FILE_XFER_STATUS_ERROR,
>      VD_AGENT_FILE_XFER_STATUS_SUCCESS,
> +    VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE,
>  };
>
>  typedef struct SPICE_ATTR_PACKED VDAgentFileXferStatusMessage {
>     uint32_t id;
>     uint32_t result;
> +   /* Used when result is VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE
> +    * to send available space on remote machine (bytes)
> +   */
> +   uint64_t data[0];
>  } VDAgentFileXferStatusMessage;

The VDAgentMessage is defined by:

 | typedef struct SPICE_ATTR_PACKED VDAgentMessage {
 |     uint32_t protocol;
 |     uint32_t type;
 |     uint64_t opaque;
 |     uint32_t size;
 |     uint8_t data[0];
 | } VDAgentMessage;

The type still is VD_AGENT_FILE_XFER_STATUS and size will correctly take
into account the right amount of bytes (including the error message) if
the client has set the capability VD_AGENT_CAP_FILE_XFER_FREE_SPACE that
you have introduced here.

So, the protocol might not be broken (is it?) but this change does not
seem right to me.

Maybe the main reason is that the capability name seems tricky to allow
you get that error message? We a bug open upstream [0] that can be
solved with something like this.

[0] https://bugs.freedesktop.org/show_bug.cgi?id=99982

At the very least I would change the uint64_t to uint8_t above and
rename the capability to be generic enough to cover file-transfer
errors.

Cheers and thanks for your interest in Spice!
    toso

>
>  typedef struct SPICE_ATTR_PACKED VDAgentFileXferStartMessage {
> @@ -248,6 +253,7 @@ enum {
>      VD_AGENT_CAP_AUDIO_VOLUME_SYNC,
>      VD_AGENT_CAP_MONITORS_CONFIG_POSITION,
>      VD_AGENT_CAP_FILE_XFER_DISABLED,
> +    VD_AGENT_CAP_FILE_XFER_FREE_SPACE,
>      VD_AGENT_END_CAP,
>  };
>  
> -- 
> 2.12.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170504/6fefd8a0/attachment-0001.sig>