Re: [PATCH] ioeventfd: Introduce KVM_IOEVENTFD_FLAG_PIPE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/04/2011 01:32 PM, Michael S. Tsirkin wrote:
On Sun, Jul 03, 2011 at 08:04:49PM +0300, Sasha Levin wrote:
>  The new flag allows passing a write side of a pipe instead of an
>  eventfd to be notified of writes to the specified memory region.
>
>  Instead of signaling an event, the value written to the memory region
>  is written to the pipe.
>
>  Using a pipe instead of an eventfd is usefull when any value can be
>  written to the memory region but we're interested in recieving the
>  actual value instead of just a notification.
>
>  A simple example for practical use is the serial port. we are not
>  interested in an exit every time a char is written to the port, but
>  we do need to know what was written so we could handle it on the guest.

Looking at this example, how would you handle a pipe full condition?
We can't buffer unlimited amount of data in the host.

Stall.

>  +static ssize_t kernel_write(struct file *file, const char *buf, size_t count,
>  +			    loff_t pos)
>  +{
>  +	mm_segment_t old_fs;
>  +	ssize_t res;
>  +
>  +	old_fs = get_fs();
>  +	set_fs(get_ds());
>  +	/* The cast to a user pointer is valid due to the set_fs() */

Interesting. Is buf really always a user pointer?
Why don't we tag it __user then?

It's not a user pointer, this is just to get the tools happy. set_fs() makes it safe.

>  +	res = vfs_write(file, (const char __user *)buf, count,&pos);

If pipe is non-blocking, or if we get a signal,
this might fail or return a value<  len.
Data will be lost then, won't it?

Yes.  Need a loop-until-buffer-exhausted-or-error.

Error reporting is an interesting question. Typically we have KVM_RUN return the error, but if we use this facility to run something in a separate process, this can cause the device process crash to cause the guest to crash.

>  -	p->eventfd = eventfd;
>  +
>  +	if (args->flags&  KVM_IOEVENTFD_FLAG_PIPE)
>  +		p->pipe = fget(args->fd);

This really needs to check that the fd is a pipe.
Otherwise you can do weird things like pass in
the kvm device fd itself.

Eww, reference loop.  Good catch.

We should allow unix domain sockets as well. In fact, for read/write support, we need this to be a unix domain socket.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux