On 23.07.2018 16:33, Kevin Wolf wrote: > Am 18.07.2018 um 17:28 hat Thomas Huth geschrieben: >> When using the vvfat driver with a directory that contains too many files, >> QEMU currently crashes. We are trying to print the wrong path variable here. >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> block/vvfat.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/block/vvfat.c b/block/vvfat.c >> index fc41841..6ae7458 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -975,8 +975,7 @@ static int init_directories(BDRVVVFATState* s, >> if (mapping->mode & MODE_DIRECTORY) { >> mapping->begin = cluster; >> if(read_directory(s, i)) { >> - error_setg(errp, "Could not read directory %s", >> - mapping->path); >> + error_setg(errp, "Could not read directory \"%s\"", >> s->path); > > Hm, I'm not sure if that's right. Before this patch we were printing > the name of the subdirectory that couldn't be loaded, now it's the > parent directory. > > My test case where this difference is visible is a subdirectory with > chmod 000.
Right. >> return -1; >> } >> mapping = array_get(&(s->mapping), i); > > Maybe the right solution would be moving the reloading of mapping to > between the read_directory() call and the error path? No, that does not work either. The problem seems to be that read_directory() is changing the mapping->path pointer to something invalid in between, but I've been unable to track it down where it happens. This patch here seems to work for me, though: diff --git a/block/vvfat.c b/block/vvfat.c index fc41841..f2e7d50 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -973,10 +973,10 @@ static int init_directories(BDRVVVFATState* s, mapping = array_get(&(s->mapping), i); if (mapping->mode & MODE_DIRECTORY) { + char *path = mapping->path; mapping->begin = cluster; if(read_directory(s, i)) { - error_setg(errp, "Could not read directory %s", - mapping->path); + error_setg(errp, "Could not read directory %s", path); return -1; } mapping = array_get(&(s->mapping), i); Does this look reasonable for you, too? Thomas