On Fri, Feb 04, 2011 at 10:11:59AM +0100, Hans de Goede wrote: > Hi, > > On 02/04/2011 09:39 AM, Alon Levy wrote: > > <snip> > > >>Hmm, > >> > >>This seems error prone. What if the other side is sending a success state > >>for some > >>other reason? Either error->code == VSC_SUCCESS can only be send for the > >>adding a reader case and keeping track of num_pending_reader_inserts is > >>not necessary or this seems racy to me. Maybe add a VCS_READER_ADD_SUCCESS > >>error code ? > > > >Ok, I admit this whole counter was a way to quickly fix the problem that I > >was getting a second VSC_SUCCESS. The idea was to have a single outstanding > >request that needs an answer, thereby removing ambiguity. But the > >implementation doesn't do that, hence the hack of > >_num_pending_reader_inserts. To keep the protocol as is (btw I just > >*removed* such a message because I wanted to simplify the protocol - > >ReaderAddResponse. equivalent to the suggested error code) I could just > >serialize to: > > client: request reader add > > client: wait for response > > server: send VSCError.code==VSC_SUCCESS > > client: report card insert > > cient: wait for response > > > > That is still racy. What if a VSCError.code==VSC_SUCCESS is already in the > pipe > from server to client for some other request send by the client earlier. Then > the client will report the card insert, when it receives the unrelated > VSCError.code==VSC_SUCCESS.
the connection is tcp, no drops, so if the client keeps track correctly it can't happen. put another way, assuming you are correct then there was a message earlier that the client sent and it assumed was responded to => client bug. q.e.d. > > The way I've fixed issues like this with the usb network redirection stuff > I'm doing is give the common header for all requests an id field and increment > that which each packet send. When the server (in your case) then sends a > response Yes, of course that's the canonical fix, also used in spice protocol as you know, I was trying to avoid that because, well, it adds code, but perhaps you are right.. > that includes the id of the request it is a response to so that requests and > responses can be mapped 1 on 1. > > Regards, > > Hans > _______________________________________________ > Spice-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
