Re: [PATCH spice 3/3] reds: Cancel unexpected xfer messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Not much related to other patches

> 
> Inform client that transfer will not work, instead of only
> dropping the agent messages.
> 
> The file transfer messages can be disabled using qemu cli option:
>  "disable-agent-file-xfer"
> 
> Resolves:
> https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> 
> Signed-off-by: Pavel Grunt <pgrunt@xxxxxxxxxx>
> ---
> Can be extended to use a detailed status message introduced recently in
> protocol
> ---
>  server/reds.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 8abeb3fbd..38d34c827 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1110,6 +1110,39 @@ static void
> reds_on_main_agent_monitors_config(RedsState *reds, const void *mess
>      reds_client_monitors_config_cleanup(reds);
>  }
>  
> +static void agent_data_free(uint8_t *data, void *opaque G_GNUC_UNUSED)
> +{
> +    free(data);
> +}
> +
> +/*
> + * Inform client about discarded file transfer
> + * https://bugzilla.redhat.com/show_bug.cgi?id=1373725
> + */
> +static void agent_xfer_error(MainChannelClient *mcc, const VDAgentMessage
> *message)
> +{
> +    VDAgentMessage *xfer_msg;
> +    VDAgentFileXferStatusMessage *xfer_status;
> +    size_t len;
> +
> +    if (message->type < VD_AGENT_FILE_XFER_START || message->type >
> VD_AGENT_FILE_XFER_DATA) {
> +        return;
> +    }
> +
> +    len = sizeof(VDAgentMessage) + sizeof(VDAgentFileXferStatusMessage);
> +    xfer_msg = spice_malloc0(len);
> +    xfer_msg->protocol = VD_AGENT_PROTOCOL;
> +    xfer_msg->type = VD_AGENT_FILE_XFER_STATUS;
> +    xfer_msg->size = sizeof(VDAgentFileXferStatusMessage);
> +    xfer_status = (VDAgentFileXferStatusMessage *) xfer_msg->data;
> +    xfer_status->id = *((uint32_t *) message->data);

This is not really portable. See also patches from Christophe D
for clang.

I would personally define some integers like (not tested..
just did some test with smaller definitions)


#include <spice/start-packed.h>

#define PACKET_INT_DEFINITION(type) \
typedef struct SPICE_ATTR_PACKED type ## _packed_t { \
    type ## _t value; \
} type ## _packed_t

PACKET_INT_DEFINITION(int8);
PACKET_INT_DEFINITION(int16);
PACKET_INT_DEFINITION(int32);
PACKET_INT_DEFINITION(int64);

PACKET_INT_DEFINITION(uint8);
PACKET_INT_DEFINITION(uint16);
PACKET_INT_DEFINITION(uint32);
PACKET_INT_DEFINITION(uint64);

#undef PACKET_INT_DEFINITION

#include <spice/end-packed.h>


and write something like

   xfer_status->id = ((uint32_packed_t *) message->data)->value;

unfortunately the structure is required as packed attribute
applied to a pointer is useless (gcc just ignore it).
But using this code the compiler is able to generate good code
for whatever platform (tested on sparc and other architecture
which are not really unaligned friendly).

Maybe TYPE_unaligned_t is a better name?

In this case also a

   xfer_status->id = ((VDAgentFileXferStartMessage *) message->data)->id;

would be fine

> +    xfer_status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
> +
> +    spice_debug("canceling file transfer %u", xfer_status->id);
> +
> +    main_channel_client_push_agent_data(mcc, (uint8_t *) xfer_msg, len,
> agent_data_free, NULL);
> +}
> +
>  void reds_on_main_agent_data(RedsState *reds, MainChannelClient *mcc, const
>  void *message,
>                               size_t size)
>  {
> @@ -1123,6 +1156,7 @@ void reds_on_main_agent_data(RedsState *reds,
> MainChannelClient *mcc, const void
>      case AGENT_MSG_FILTER_OK:
>          break;
>      case AGENT_MSG_FILTER_DISCARD:
> +        agent_xfer_error(mcc, message);
>          return;
>      case AGENT_MSG_FILTER_MONITORS_CONFIG:
>          reds_on_main_agent_monitors_config(reds, message, size);

Not tested, not sure what happen for old clients and if is correct to
send back an error for every discarded data. Maybe the client will continue
to send data (for instance for a file transfer) generating lot of go and
back?

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]