Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1453290

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

Show all headers | View raw

On Mon, Aug 01, 2016 at 10:24:39AM +0100, Daniel P. Berrange wrote:
> On Sat, Jul 30, 2016 at 05:57:02PM +0900, Namhyung Kim wrote:
> > On Thu, Jul 28, 2016 at 02:22:39PM +0100, Daniel P. Berrange wrote:
> > > > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> > > > +                                        char *buf, size_t sz,
> > > > +                                        struct virtio_pstore_fileinfo *info)
> > > > +{
> > > > +    snprintf(buf, sz, "%s/%s", s->directory, name);
> > > > +
> > > > +    if (g_str_has_prefix(name, "dmesg-")) {
> > > > +        info->type = VIRTIO_PSTORE_TYPE_DMESG;
> > > > +        name += strlen("dmesg-");
> > > > +    } else if (g_str_has_prefix(name, "console-")) {
> > > > +        info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
> > > > +        name += strlen("console-");
> > > > +    } else if (g_str_has_prefix(name, "unknown-")) {
> > > > +        info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> > > > +        name += strlen("unknown-");
> > > > +    }
> 
> [snip]
> 
> > > > +    struct virtio_pstore_fileinfo info;
> > > > +    size_t offset = sizeof(*res) + sizeof(info);
> > > > +
> > > > +    if (s->dirp == NULL) {
> > > > +        return -1;
> > > > +    }
> > > > +
> > > > +    dent = readdir(s->dirp);
> > > > +    while (dent) {
> > > > +        if (dent->d_name[0] != '.') {
> > > > +            break;
> > > > +        }
> > > > +        dent = readdir(s->dirp);
> > > > +    }
> > > > +
> > > > +    if (dent == NULL) {
> > > > +        return 0;
> > > > +    }
> > > 
> > > So this seems to just be picking the first filename reported by
> > > readdir that isn't starting with '.'. Surely this can't the right
> > > logic when your corresponding do_write method can pick several
> > > different filenames, its potluck which do_read will give back.
> > 
> > Do you mean that it'd be better to check a list of known filenames and
> > fail if not?
> 
> No, I mean that you have several different VIRTIO_PSTORE_TYPE_nnn and
> use a different file for each constant. When reading this directory
> though you're not looking for the file corresponding to any given
> VIRTIO_PSTORE_TYPE_nnn - you're simply reading whichever file is found
> first. So you might have just read a TYPE_CONSOLE or have read a
> TYPE_DMESG - it surely doesn't make sense to randonly read either
> TYPE_CONSOLE or TYPE_DMESG based on whatever order readdir() lists
> the files.

In fact, each VIRTIO_PSTORE_TYPE_xxx can have more than one files and
I don't care about the ordering between them.  They will be fed to the
pstore filesystem on the guest.

But I also think it'd be better to scan files of known types only
rather than read out whatever readdir() gives.

Thanks,
Namhyung

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2) Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:10 +0200
  [PATCH 2/7] pstore/ram: Set pstore flags dynamically Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  [PATCH 4/7] virtio: Basic implementation of virtio pstore driver Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
    Re: [PATCH 6/7] qemu: Implement virtio-pstore device "Michael S. Tsirkin" <mst@redhat.com> - 2016-07-28 02:10 +0200
      Re: [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-07-28 07:50 +0200
        Re: [PATCH 6/7] qemu: Implement virtio-pstore device Stefan Hajnoczi <stefanha@gmail.com> - 2016-07-28 15:00 +0200
          Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device "Daniel P. Berrange" <berrange@redhat.com> - 2016-07-28 15:10 +0200
            Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-07-30 10:50 +0200
              Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device "Daniel P. Berrange" <berrange@redhat.com> - 2016-08-01 11:50 +0200
                Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-08-01 17:40 +0200
        Re: [PATCH 6/7] qemu: Implement virtio-pstore device Steven Rostedt <rostedt@goodmis.org> - 2016-07-28 16:40 +0200
    Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device "Daniel P. Berrange" <berrange@redhat.com> - 2016-07-28 15:30 +0200
      Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-07-30 11:00 +0200
        Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device "Daniel P. Berrange" <berrange@redhat.com> - 2016-08-01 11:30 +0200
          Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-08-01 18:00 +0200
  [PATCH 3/7] pstore: Manage buffer position for async write Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  [PATCH 1/7] pstore: Split pstore fragile flags Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  [PATCH 5/7] virtio-pstore: Support PSTORE_TYPE_CONSOLE Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  [PATCH 7/7] kvmtool: Implement virtio-pstore device Namhyung Kim <namhyung@kernel.org> - 2016-07-27 17:20 +0200
  Re: [RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2) "Michael S. Tsirkin" <mst@redhat.com> - 2016-07-28 00:20 +0200
    Re: [RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2) Namhyung Kim <namhyung@kernel.org> - 2016-07-28 04:50 +0200

csiph-web