On Fri, 2017-06-02 at 04:13 -0400, Frediano Ziglio wrote: > Not much related to other patches a bit, by location :) I can send it separately, I wanted to avoid merge conflicts. > > > > > 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 I'd go for this > > > + 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 cannot break the old client or new clients. they are ready to receive the status error client should send the "transfer start message" and wait for the "can send data" > send back an error for every discarded data. I can store an id of transfer somewhere a avoid discarding the same transfer more than once. imo it is overkill, client must survive xfer error status. > Maybe the client will continue > to send data (for instance for a file transfer) It should not, once it gets the error status it cancels the transfer Thanks, Pavel > generating lot of go and > back? > > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel