On Fri, Oct 02, 2015 at 05:32:18PM +0100, Dr. David Alan Gilbert wrote: > * Daniel P. Berrange (berra...@redhat.com) wrote: > > On Tue, Sep 29, 2015 at 09:37:40AM +0100, Dr. David Alan Gilbert (git) > > wrote: > > > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > > > > > > Postcopy needs a method to send messages from the destination back to > > > the source, this is the 'return path'. > > > > > > Wire it up for 'socket' QEMUFile's. > > > > I find this to be a pretty wierd approach to the problem. THe underlying > > transport is bi-directional, so I would expect to have a single QEMUFile > > object that allowed bi-directional I/O on it, rather than creating a > > second QEMUFile for the back channel, which was forbidden from closing > > the shared FD. > > > > I can understand why you've done this though - since we only have a > > single buffer embedded in QEMUFile. I wonder though if we'd be better > > off changing QEMUFile to have a 'inbuf' and 'outbuf' intead of just > > 'buf' and likewise iniov & outiov. Then we can allow bi-directional > > I/O on the single QEMUFile object which is a more natural fit. > > The 'c' FILE* is one directional, and I just took it that the QEMUFile* is > like that; i.e. a buffered layer on top of an underlying one directional > transport. stdin,stdout are two separate FILE*'s.
Yep, QEMUFile was really designed as a FILE* alternative, so makes sense from that POV. > Your iniov, outiov would be basically the same, so you'd end up duplicating > code for the in and out parts; where as what you really have is two of the > same > thing wired up in opposite directions. I don't think it'd actually end up duplicating any code - mostly just updating which variable was accessed in each existing method, depending on whether it was a read or write related method. > Having said that, for things like RDMA, they have to do special stuff for > each direction and the QEMUFile is really a shim on top of that. Similarly when we add TLS into the mix, there is a single shared TLS session context that is used by both I/O directionals. Now this would not be visible to the QEMUFile regardless, since its hidden in the QIOChannel object I'm defining, so its not a show stopper either but I guess my general thought is that there is a mixture of state that we maintain some different for read vs write and some shared. You workaround the fact that the FD is shared by having a comment saying we should not call close() on the FD kept by the QEMUFile for the return path. All that said, I don't think it is too critical to change this right now. It would be fine to leave it to a later date, unless there's a more pressing reason. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
