From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40124) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9Po-0001pb-QT for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:21:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bU9Pl-00041j-BC for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:21:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54384) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9Pl-00041Z-3N for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:21:37 -0400 Date: Mon, 1 Aug 2016 10:21:30 +0100 From: "Daniel P. Berrange" Message-ID: <20160801092130.GE6455@redhat.com> Reply-To: "Daniel P. Berrange" References: <1469632111-23260-1-git-send-email-namhyung@kernel.org> <1469632111-23260-7-git-send-email-namhyung@kernel.org> <20160728023254-mutt-send-email-mst@kernel.org> <20160728053953.GB4371@sejong> <20160728125607.GA31002@stefanha-x1.localdomain> <20160728130841.GL22677@redhat.com> <20160730083827.GA26275@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160730083827.GA26275@danjae.aot.lge.com> Subject: Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Namhyung Kim Cc: Stefan Hajnoczi , Tony Luck , Kees Cook , kvm@vger.kernel.org, Radim Kr??m???? , Anton Vorontsov , LKML , Steven Rostedt , qemu-devel@nongnu.org, Minchan Kim , "Michael S. Tsirkin" , Anthony Liguori , Colin Cross , Paolo Bonzini , virtualization@lists.linux-foundation.org, Ingo Molnar On Sat, Jul 30, 2016 at 05:38:27PM +0900, Namhyung Kim wrote: > Hello, > > On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote: > > On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote: > > > On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote: > > > > On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote: > > > > > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote: > > > > > > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg, > > > > > > + unsigned int out_num, > > > > > > + struct virtio_pstore_req *req) > > > > > > +{ > > > > > > + char path[PATH_MAX]; > > > > > > + int fd; > > > > > > + ssize_t len; > > > > > > + unsigned short type; > > > > > > + int flags = O_WRONLY | O_CREAT; > > > > > > + > > > > > > + /* we already consume the req */ > > > > > > + iov_discard_front(&out_sg, &out_num, sizeof(*req)); > > > > > > + > > > > > > + virtio_pstore_to_filename(s, path, sizeof(path), req); > > > > > > + > > > > > > + type = le16_to_cpu(req->type); > > > > > > + > > > > > > + if (type == VIRTIO_PSTORE_TYPE_DMESG) { > > > > > > + flags |= O_TRUNC; > > > > > > + } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) { > > > > > > + flags |= O_APPEND; > > > > > > + } > > > > > > + > > > > > > + fd = open(path, flags, 0644); > > > > > > + if (fd < 0) { > > > > > > + error_report("cannot open %s", path); > > > > > > + return -1; > > > > > > + } > > > > > > + len = writev(fd, out_sg, out_num); > > > > > > + close(fd); > > > > > > + > > > > > > + return len; > > > > > > > > > > All this is blocking VM until host io completes. > > > > > > > > Hmm.. I don't know about the internals of qemu. So does it make guest > > > > stop? If so, that's what I want to do for _DMESG. :) As it's called > > > > only on kernel oops I think it's admittable. But for _CONSOLE, it > > > > needs to do asynchronously. Maybe I can add a thread to do the work. > > > > > > Please look at include/io/channel.h. QEMU is event-driven and tends to > > > use asynchronous I/O instead of spawning threads. The include/io/ APIs > > > allow you to do asynchronous I/O in the event loop. > > > > That is true, except for I/O to/from plain files - the QIOChannelFile > > impl doesn't do anything special (yet) to make that work correctly in > > non-blocking mode. Of course that could be fixed... > > Yep, I don't know how I can use the QIOChannelFile for async IO. > AFAICS it's just a wrapper for normal readv/writev. Who is > responsible to do these IO when I use the IO channel API? Also does > it guarantee that all IOs are processed in order? I'd suggest just using QIOChannelFile regardless - we need to fix the blocking behaviour already for sake of the qemu chardev code, and your code just adds more pressure to fix it. I/O will be done in the order in which you make the calls, as with regular POSIX I/O funcs you're currently using. 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 :|