On Fri, Sep 25, 2009 at 02:09:54PM +0100, Daniel P. Berrange wrote: > On Fri, Sep 25, 2009 at 02:32:39PM +0200, Daniel Veillard wrote: > > Hum, this would then raise the signal that stream can be used > > both ways, do we really want to suggest this at the API level, > > I can see how we're gonna use this internally, but aren't we opening > > the door to much complexity ? > > Yeah, that's more or less why I left it out so far - I've not > yet found a case where I absolutely needed the WRITE/READ > flags to be set explicitly by apps. Okay, let's keep it that way. Ultimately we could add another API to create the Stream. > > > If we wanted to mandate use of READ/WRITE flags for stream > > > creation, we'd obviously need todo it from teh start, since > > > we couldn't add that as a mandatory flag once the API is > > > released & in use by apps. > > > > yes that's a good point, a design issue too. If you really expect API > > usage for both read and write, then I would say we should make those > > flags mandatory. The only point is that our existing flags use in APIs > > are just fine with 0 as being the 'default', and we would break this > > but it's not a big deal IMHO, that will be caught immediately. > > There is one likely API where we'd have a full read+write stream. > I've thought about adding ability to tunnel a serial port PTY > over libvirt, so 'virsh console' could be made to work remotely. > eg, something like > > virDomainOpenConsole(virDomainPtr dom, virStreamPtr stream > const char *consolename); > > In this case you'd be reading & writing from /to the same > stream. It still wouldn't really require that we set the > READ+WRITE flags when doing virStreamNew() okay > > > > > +typedef int (*virStreamSinkFunc)(virStreamPtr st, > > > > > + const char *data, > > > > > + size_t nbytes, > > > > > + void *opaque); > > > > > > > > Same thing do we allow a sink function to be called repeatedly ? > > > > If we want to allow this in some ways we will need an extra argument > > > > to indicate the end of the stream. Even if we don't plan this yet, I > > > > would suggest to add a flags to allow for this possibility in the > > > > future. With a chunk size of 256K at the protocol level it may not > > > > be a good idea to keep the full data in memory, so I would allow > > > > for this interface to call the sink multiple times. And IMHO it's best > > > > to pass the indication of end of transfer directly at the sink level > > > > rather than wait for the virStreamFree() coming from the user. > > > > > > > > > +int virStreamRecvAll(virStreamPtr st, > > > > > + virStreamSinkFunc handler, > > > > > + void *opaque); > > > > > > > > Okay > > > > > > Same as for SendAll, this API will invoke the handler multilpe > > > times to write out data that is being received. In both cases > > > the implementation is invoking the handler with 64kb buffers > > > to avoid pulling lots of data into memory. > > > > Okay, but I think being able to indicate there that a packet is the > > last one may be important, for example if the application design prefer > > to initiate the closure of the transfer (close/sync/...) as soon as > > possible. > > Actually in the case of the 'source' function, the app already > knows when its got to the end, because its source funtion will > be returning '0' for end-of-file. yes for read it's not a problem > For the 'sink' function we'd have to make sure we called it once > at the end with a length of '0' to indicate EOF in that direction. > I can't remember offhand if we'll do that already or not. ah write 0 lenght, yes that's another way to pass the signal. > > Also how flexible are we in the design with callbacks taling a long > > time to complete, for example reads crossing the network, or slow > > output devices ? Maybe this should be hinted in the callback > > descriptions. > > These callbacks are the app's responsibility & execute in its > context, so libvirt doesn't care whether they are slow or fast > to execute. Everything internal to libvirt relating to streams > is non-blocking & fast. okidoc, I just wanted to doub;e-check :-) > > > The sink/source callbacks do not need to close the FD since this is easily > > > done with RecvAll/SendAll returns control to the application. THis is > > > in fact important, because it is not until RecvAll/SendAll returns that > > > you can call virStreamFinish to check for success. If it did not suceed > > > then you may want do other cleanup before closing the FD, such as > > > unlinking the file > > > > Hum, the two example for RecvAll and SendAll don't suggest > > virStreamFinish() to be called to get the status, I would expect error > > reporting to show up as the result code from RecvAll and SendAll. > > Actually these 2 code examples are wrong. There should be a call > to virStreamFinish in there, before the virStreamFree. This was > not required in an earlier version of my patch, because SendAll > would call Finish for you, but I realized this made it impossible > for callers to detect certain error conditions. So the app should > always call either Finish or Abort once they're done with I/O. > I'll update the example Ah, okay ! ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list