Thread (4 messages) 4 messages, 2 authors, 2016-08-01

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

From: Namhyung Kim <namhyung@kernel.org>
Date: 2016-08-01 15:34:10

Possibly related (same subject, not in this thread)

Hi Daniel,

On Mon, Aug 01, 2016 at 10:21:30AM +0100, Daniel P. Berrange wrote:
On Sat, Jul 30, 2016 at 05:38:27PM +0900, Namhyung Kim wrote:
quoted
Hello,

On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote:
quoted
On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote:
quoted
On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote:
quoted
On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote:
quoted
On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
quoted
+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.
Thanks for the info.

So can I simply replace regular IO funcs to QIOChannel API like below?

  ioc = qio_channel_file_new_path(path, flags, 0644, &err);
  qio_channel_set_blocking(ioc, false, &err);
  len = qio_channel_writev(ioc, out_sg, out_num, &err);
  qio_channel_file_close(ioc, &err);
  return len;

Or do I need to call qio_channel_add_watch() or something?

Thanks,
Namhyung
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help