[Spice-devel] [PATCH spice v2 1/2] agent-filter: Differentiate xfer messages

[Frediano Ziglio]

> 
> To be able to inform the sender about the cancelled file transfer
> 
> Related:
> https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> ---
>  server/agent-msg-filter.c            | 12 ++++++++++--
>  server/agent-msg-filter.h            |  1 +
>  server/reds.c                        |  6 ++++++
>  server/tests/test-agent-msg-filter.c | 10 ++++++++--
>  4 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
> index 17f8e889..a83dee97 100644
> --- a/server/agent-msg-filter.c
> +++ b/server/agent-msg-filter.c
> @@ -80,7 +80,15 @@ data_to_read:
>      }
>  
>      if (filter->discard_all) {
> -        filter->result = AGENT_MSG_FILTER_DISCARD;
> +        switch (msg_header.type) {
> +        case VD_AGENT_FILE_XFER_START:
> +        case VD_AGENT_FILE_XFER_STATUS:
> +        case VD_AGENT_FILE_XFER_DATA:
> +            filter->result = AGENT_MSG_FILTER_DISCARD_XFER;
> +            break;
> +        default:
> +            filter->result = AGENT_MSG_FILTER_DISCARD;
> +        }
>      } else {
>          switch (msg_header.type) {
>          case VD_AGENT_CLIPBOARD:
> @@ -99,7 +107,7 @@ data_to_read:
>              if (filter->file_xfer_enabled) {
>                  filter->result = AGENT_MSG_FILTER_OK;
>              } else {
> -                filter->result = AGENT_MSG_FILTER_DISCARD;
> +                filter->result = AGENT_MSG_FILTER_DISCARD_XFER;
>              }
>              break;
>          case VD_AGENT_MONITORS_CONFIG:
> diff --git a/server/agent-msg-filter.h b/server/agent-msg-filter.h
> index b4d8e720..acbeaaaf 100644
> --- a/server/agent-msg-filter.h
> +++ b/server/agent-msg-filter.h
> @@ -28,6 +28,7 @@
>  typedef enum {
>      AGENT_MSG_FILTER_OK,
>      AGENT_MSG_FILTER_DISCARD,
> +    AGENT_MSG_FILTER_DISCARD_XFER,
>      AGENT_MSG_FILTER_PROTO_ERROR,
>      AGENT_MSG_FILTER_MONITORS_CONFIG,
>  } AgentMsgFilterResult;

I don't know why but I feel that the filter should just
filter but looks like it starts classifying the messages
too.
It's not clear the responsibility of this filter code but
if you ask me I would attempt to remove AGENT_MSG_FILTER_MONITORS_CONFIG
instead of adding other classifications.

> diff --git a/server/reds.c b/server/reds.c
> index 3b30928a..b9f13e2e 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -803,6 +803,8 @@ static RedPipeItem
> *vdi_port_read_one_msg_from_device(RedCharDevice *self,
>              case AGENT_MSG_FILTER_MONITORS_CONFIG:
>                  /* fall through */
>              case AGENT_MSG_FILTER_DISCARD:
> +                /* fall through */
> +            case AGENT_MSG_FILTER_DISCARD_XFER:
>                  red_pipe_item_unref(&dispatch_buf->base);
>              }
>          }
> @@ -1111,6 +1113,8 @@ void reds_on_main_agent_data(RedsState *reds,
> MainChannelClient *mcc, void *mess
>      case AGENT_MSG_FILTER_OK:
>          break;
>      case AGENT_MSG_FILTER_DISCARD:
> +        /* fall through */
> +    case AGENT_MSG_FILTER_DISCARD_XFER:
>          return;
>      case AGENT_MSG_FILTER_MONITORS_CONFIG:
>          reds_on_main_agent_monitors_config(reds, mcc, message, size);
> @@ -1198,6 +1202,8 @@ void reds_on_main_channel_migrate(RedsState *reds,
> MainChannelClient *mcc)
>          case AGENT_MSG_FILTER_MONITORS_CONFIG:
>              /* fall through */
>          case AGENT_MSG_FILTER_DISCARD:
> +            /* fall through */
> +        case AGENT_MSG_FILTER_DISCARD_XFER:
>              red_pipe_item_unref(&read_buf->base);
>          }
>  
> diff --git a/server/tests/test-agent-msg-filter.c
> b/server/tests/test-agent-msg-filter.c
> index 2f5568a6..ea8b9363 100644
> --- a/server/tests/test-agent-msg-filter.c
> +++ b/server/tests/test-agent-msg-filter.c
> @@ -83,8 +83,12 @@ static void test_agent_msg_filter_run(void)
>      msg.msg_header.protocol = VD_AGENT_PROTOCOL;
>      for (type = VD_AGENT_MOUSE_STATE; type < VD_AGENT_END_MESSAGE; type++) {
>          msg.msg_header.type = type;
> +        AgentMsgFilterResult filter_result = AGENT_MSG_FILTER_DISCARD;
> +        if (type >= VD_AGENT_FILE_XFER_START && type <=
> VD_AGENT_FILE_XFER_DATA) {
> +            filter_result = AGENT_MSG_FILTER_DISCARD_XFER;
> +        }
>          g_assert_cmpint(agent_msg_filter_process_data(&filter, msg.data,
>          len), ==,
> -                        AGENT_MSG_FILTER_DISCARD);
> +                        filter_result);
>      }
>  
>      /* data exceeds size from header */
> @@ -113,10 +117,12 @@ static void test_agent_msg_filter_run(void)
>          case VD_AGENT_CLIPBOARD_GRAB:
>          case VD_AGENT_CLIPBOARD_REQUEST:
>          case VD_AGENT_CLIPBOARD_RELEASE:
> +            result = AGENT_MSG_FILTER_DISCARD;
> +            break;
>          case VD_AGENT_FILE_XFER_START:
>          case VD_AGENT_FILE_XFER_STATUS:
>          case VD_AGENT_FILE_XFER_DATA:
> -            result = AGENT_MSG_FILTER_DISCARD;
> +            result = AGENT_MSG_FILTER_DISCARD_XFER;
>              break;
>          case VD_AGENT_MONITORS_CONFIG:
>              result = AGENT_MSG_FILTER_MONITORS_CONFIG;

Frediano