From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40622) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9Ss-0004Dn-DB for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bU9Sn-0004U1-W5 for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9Sn-0004Tv-NM for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:45 -0400 Date: Mon, 1 Aug 2016 10:24:39 +0100 From: "Daniel P. Berrange" Message-ID: <20160801092439.GF6455@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> <20160728132239.GM22677@redhat.com> <20160730085702.GB26275@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160730085702.GB26275@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: kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, Tony Luck , Radim Kr??m???? , Kees Cook , "Michael S. Tsirkin" , Anton Vorontsov , LKML , Steven Rostedt , Minchan Kim , Anthony Liguori , Colin Cross , Paolo Bonzini , Ingo Molnar 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. 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 :|