Hi Jes, you raise some good points and pitfalls with the current getfile approach. I've been thinking about an alternative and am wondering what you (and others) think...
First off, I think we should switch to a copyfile() API that allows us to avoid presenting the file contents to the user. Neither the human monitor nor the control monitor are designed to be file pagers. Let the user decide how to consume the data once it has been transferred. Now we don't need to care if the file is binary or text. The virtagent RPC protocol is bi-directional and supports asynchronous events. We can use these to implement a better copyfile RPC that can transfer larger files without wasting memory. The host issues a copyfile(<guest-path>, <host-path>) RPC. The immediate result of this call will indicate whether the guest is able to initiate the transfer. The guest will generate a series of events (<offset>, <size>, <payload>) until the entire contents has been transferred. The host and guest could negotiate the chunk size if necessary. Once the transfer is complete, the guest sends a final event to indicate this (<file-size>, 0). This interface could be integrated into the monitor with a pair of commands (va_copyfile and info va_copyfile), the former used to initiate transfers and the latter to check on the status. Thoughts on this? On Tue, 2010-12-07 at 15:18 +0100, Jes Sorensen wrote: > On 12/03/10 19:03, Michael Roth wrote: > > Add RPC to retrieve a guest file. This interface is intended > > for smaller reads like peeking at logs and /proc and such. > > I think you need to redesign your approach here..... see below. > > In 06/21 you had: > > +#define VA_GETFILE_MAX 1 << 30 > > > + while ((ret = read(fd, buf, VA_FILEBUF_LEN)) > 0) { > > + file_contents = qemu_realloc(file_contents, count + > > VA_FILEBUF_LEN); > > + memcpy(file_contents + count, buf, ret); > > UH OH! > > realloc will do a malloc and a memcpy of the data, this is going to turn > into a really nasty malloc memcpy loop if someone tries to transfer a > large file using this method. You could end up with almost 4GB of > parallel allocations for a guest that might have been configured as a > 1GB guest. This would allow the guest to effectively blow the expected > memory consumption out of the water. It's not exactly going to be fast > either :( > > Maybe use a tmp file, and write data out to that as you receive it to > avoid the malloc ballooning. > > Jes -- Thanks, Adam