Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver

On Tue, Jul 19, 2016 at 10:43 PM, Namhyung Kim [off-list ref] wrote:

Hi Kees,

On Mon, Jul 18, 2016 at 10:50:06AM -0700, Kees Cook wrote:

quoted

On Sun, Jul 17, 2016 at 10:50 PM, Namhyung Kim [off-list ref] wrote:

quoted

Hello,

On Sun, Jul 17, 2016 at 10:12:26PM -0700, Kees Cook wrote:

quoted

On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim [off-list ref] wrote:

[SNIP]

quoted

quoted

+static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type)
+{
+       u16 ret;
+
+       switch (type) {
+       case PSTORE_TYPE_DMESG:
+               ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_DMESG);
+               break;
+       default:
+               ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
+               break;
+       }

I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
relatively easy to add: I think it'd just be another virtio command?

Do you want to append the data to the host file as guest does
printk()?  I think it needs some kind of buffer management, but it's
not hard to add IMHO.

Well, with most pstore backends, the buffer size is limited, so it
tends to be a circular buffer of some sort. I think whatever you
choose to do is fine (I saw the various mentions of resource limits in
the qemu part of this thread), as long as the last N bytes of console
can be seen on the host side, where N is some portion of the memory
set aside for the log. (I don't mind the idea of an unlimited console
log either, but I suspect that will not be accepted on the qemu
side...)

I think it needs two kinds of buffer management.

The first one is the psinfo->buf (or something similar).  IIUC the
PSTORE_TYPE_CONSOLE is different than PSTORE_TYPE_DMESG as it is
emitted every time printk() sends messages to console.  So I think the
it should remain in async mode due to performance reason.  To do that,
the message should be copied to psinfo->buf and then sent via virtio.
Then it needs to keep track of the available buffer position IMHO.

Looking at the code, I found myself confused with the PSTORE_TYPE_FTRACE
and PSTORE_TYPE_CONSOLE.  It seems that handling of
PSTORE_TYPE_CONSOLE is basically same as PSTORE_TYPE_DMESG..

Thanks,
Namhyung