Message ID | 1438807969-9723-3-git-send-email-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
On 08/06/2015 04:52 AM, Max Reitz wrote: > In places which directly pass a filename to the OS, we should not use > the filename field at all but exact_filename instead (although the > former currently equals the latter if that is set). > > In qemu-img's map command, we should be using the filename field; but > since this commit prepares to remove that field, using exact_filename is > fine, too (this is the only user of BlockDriverState.filename which > frequently queries that field). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 4 ++-- > block/gluster.c | 2 +- > block/raw-posix.c | 8 ++++---- > block/raw-win32.c | 4 ++-- > qemu-img.c | 2 +- > 5 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/block.c b/block.c > index e7dd2f1..5a1dc16 100644 > --- a/block.c > +++ b/block.c > @@ -918,8 +918,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > if (ret < 0) { > if (local_err) { > error_propagate(errp, local_err); > - } else if (bs->filename[0]) { > - error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename); > + } else if (bs->exact_filename[0]) { > + error_setg_errno(errp, -ret, "Could not open '%s'", bs->exact_filename); > } else { > error_setg_errno(errp, -ret, "Could not open image"); > } > diff --git a/block/gluster.c b/block/gluster.c > index 1eb3a8c..176682b 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -358,7 +358,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, > > gconf = g_new0(GlusterConf, 1); > > - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp); > + reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, errp); > if (reop_s->glfs == NULL) { > ret = -errno; > goto exit; > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 855febe..b61c11f 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -671,7 +671,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, > /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */ > if (raw_s->fd == -1) { > assert(!(raw_s->open_flags & O_CREAT)); > - raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags); > + raw_s->fd = qemu_open(state->bs->exact_filename, raw_s->open_flags); In raw_open_common(), we call raw_normalize_devicepath(). Why don't we call it here? Thanks Wen Congyang > if (raw_s->fd == -1) { > error_setg_errno(errp, errno, "Could not reopen file"); > ret = -1; > @@ -2195,7 +2195,7 @@ static int fd_open(BlockDriverState *bs) > DPRINTF("No floppy (open delayed)\n"); > return -EIO; > } > - s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK); > + s->fd = qemu_open(bs->exact_filename, s->open_flags & ~O_NONBLOCK); > if (s->fd < 0) { > s->fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > s->fd_got_error = 1; > @@ -2486,7 +2486,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) > qemu_close(s->fd); > s->fd = -1; > } > - fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); > + fd = qemu_open(bs->exact_filename, s->open_flags | O_NONBLOCK); > if (fd >= 0) { > if (ioctl(fd, FDEJECT, 0) < 0) > perror("FDEJECT"); > @@ -2710,7 +2710,7 @@ static int cdrom_reopen(BlockDriverState *bs) > */ > if (s->fd >= 0) > qemu_close(s->fd); > - fd = qemu_open(bs->filename, s->open_flags, 0644); > + fd = qemu_open(bs->exact_filename, s->open_flags, 0644); > if (fd < 0) { > s->fd = -1; > return -EIO; > diff --git a/block/raw-win32.c b/block/raw-win32.c > index 68f2338..5c8a894 100644 > --- a/block/raw-win32.c > +++ b/block/raw-win32.c > @@ -417,7 +417,7 @@ static void raw_close(BlockDriverState *bs) > > CloseHandle(s->hfile); > if (bs->open_flags & BDRV_O_TEMPORARY) { > - unlink(bs->filename); > + unlink(bs->exact_filename); > } > } > > @@ -485,7 +485,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) > DWORD * high); > get_compressed_t get_compressed; > struct _stati64 st; > - const char *filename = bs->filename; > + const char *filename = bs->exact_filename; > /* WinNT support GetCompressedFileSize to determine allocate size */ > get_compressed = > (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), > diff --git a/qemu-img.c b/qemu-img.c > index 75f4ee4..3d5587a 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -2155,7 +2155,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e, > } > if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) { > printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", > - e->start, e->length, e->offset, e->bs->filename); > + e->start, e->length, e->offset, e->bs->exact_filename); > } > /* This format ignores the distinction between 0, ZERO and ZERO|DATA. > * Modify the flags here to allow more coalescing. >
On 06.08.2015 04:27, Wen Congyang wrote: > On 08/06/2015 04:52 AM, Max Reitz wrote: >> In places which directly pass a filename to the OS, we should not use >> the filename field at all but exact_filename instead (although the >> former currently equals the latter if that is set). >> >> In qemu-img's map command, we should be using the filename field; but >> since this commit prepares to remove that field, using exact_filename is >> fine, too (this is the only user of BlockDriverState.filename which >> frequently queries that field). >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block.c | 4 ++-- >> block/gluster.c | 2 +- >> block/raw-posix.c | 8 ++++---- >> block/raw-win32.c | 4 ++-- >> qemu-img.c | 2 +- >> 5 files changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/block.c b/block.c >> index e7dd2f1..5a1dc16 100644 >> --- a/block.c >> +++ b/block.c >> @@ -918,8 +918,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, >> if (ret < 0) { >> if (local_err) { >> error_propagate(errp, local_err); >> - } else if (bs->filename[0]) { >> - error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename); >> + } else if (bs->exact_filename[0]) { >> + error_setg_errno(errp, -ret, "Could not open '%s'", bs->exact_filename); >> } else { >> error_setg_errno(errp, -ret, "Could not open image"); >> } >> diff --git a/block/gluster.c b/block/gluster.c >> index 1eb3a8c..176682b 100644 >> --- a/block/gluster.c >> +++ b/block/gluster.c >> @@ -358,7 +358,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, >> >> gconf = g_new0(GlusterConf, 1); >> >> - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp); >> + reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, errp); >> if (reop_s->glfs == NULL) { >> ret = -errno; >> goto exit; >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 855febe..b61c11f 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -671,7 +671,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, >> /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */ >> if (raw_s->fd == -1) { >> assert(!(raw_s->open_flags & O_CREAT)); >> - raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags); >> + raw_s->fd = qemu_open(state->bs->exact_filename, raw_s->open_flags); > > In raw_open_common(), we call raw_normalize_devicepath(). Why don't we call it > here? Ideally because bs->exact_filename is normalized already, whereas raw_open_common() takes the filename from the options QDict (which was specified by the user). The question now is whether bs->exact_filename is indeed normalized. Looking into bdrv_open_common(), where that field is initially set, it doesn't look that way. One way to fix this would be to implement bdrv_refresh_filename() for raw-posix. However, then we still have the issue of bs->filename usage in raw_open_common() (for an error message, although in reality, that message can only appear on Linux where raw_normalize_devicepath() won't do anything). So the easy way to fix it would be to copy the normalized filename into bs->filename at the beginning of raw_open_common(). I'll think about it, but in any case, I think this issue is not related to this series and the fix will work independently. I'll work something out, thanks for spotting this! Max > > Thanks > Wen Congyang > >> if (raw_s->fd == -1) { >> error_setg_errno(errp, errno, "Could not reopen file"); >> ret = -1; >> @@ -2195,7 +2195,7 @@ static int fd_open(BlockDriverState *bs) >> DPRINTF("No floppy (open delayed)\n"); >> return -EIO; >> } >> - s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK); >> + s->fd = qemu_open(bs->exact_filename, s->open_flags & ~O_NONBLOCK); >> if (s->fd < 0) { >> s->fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); >> s->fd_got_error = 1; >> @@ -2486,7 +2486,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) >> qemu_close(s->fd); >> s->fd = -1; >> } >> - fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); >> + fd = qemu_open(bs->exact_filename, s->open_flags | O_NONBLOCK); >> if (fd >= 0) { >> if (ioctl(fd, FDEJECT, 0) < 0) >> perror("FDEJECT"); >> @@ -2710,7 +2710,7 @@ static int cdrom_reopen(BlockDriverState *bs) >> */ >> if (s->fd >= 0) >> qemu_close(s->fd); >> - fd = qemu_open(bs->filename, s->open_flags, 0644); >> + fd = qemu_open(bs->exact_filename, s->open_flags, 0644); >> if (fd < 0) { >> s->fd = -1; >> return -EIO; >> diff --git a/block/raw-win32.c b/block/raw-win32.c >> index 68f2338..5c8a894 100644 >> --- a/block/raw-win32.c >> +++ b/block/raw-win32.c >> @@ -417,7 +417,7 @@ static void raw_close(BlockDriverState *bs) >> >> CloseHandle(s->hfile); >> if (bs->open_flags & BDRV_O_TEMPORARY) { >> - unlink(bs->filename); >> + unlink(bs->exact_filename); >> } >> } >> >> @@ -485,7 +485,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) >> DWORD * high); >> get_compressed_t get_compressed; >> struct _stati64 st; >> - const char *filename = bs->filename; >> + const char *filename = bs->exact_filename; >> /* WinNT support GetCompressedFileSize to determine allocate size */ >> get_compressed = >> (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), >> diff --git a/qemu-img.c b/qemu-img.c >> index 75f4ee4..3d5587a 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -2155,7 +2155,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e, >> } >> if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) { >> printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", >> - e->start, e->length, e->offset, e->bs->filename); >> + e->start, e->length, e->offset, e->bs->exact_filename); >> } >> /* This format ignores the distinction between 0, ZERO and ZERO|DATA. >> * Modify the flags here to allow more coalescing. >> >
diff --git a/block.c b/block.c index e7dd2f1..5a1dc16 100644 --- a/block.c +++ b/block.c @@ -918,8 +918,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, if (ret < 0) { if (local_err) { error_propagate(errp, local_err); - } else if (bs->filename[0]) { - error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename); + } else if (bs->exact_filename[0]) { + error_setg_errno(errp, -ret, "Could not open '%s'", bs->exact_filename); } else { error_setg_errno(errp, -ret, "Could not open image"); } diff --git a/block/gluster.c b/block/gluster.c index 1eb3a8c..176682b 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -358,7 +358,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, gconf = g_new0(GlusterConf, 1); - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp); + reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, errp); if (reop_s->glfs == NULL) { ret = -errno; goto exit; diff --git a/block/raw-posix.c b/block/raw-posix.c index 855febe..b61c11f 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -671,7 +671,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */ if (raw_s->fd == -1) { assert(!(raw_s->open_flags & O_CREAT)); - raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags); + raw_s->fd = qemu_open(state->bs->exact_filename, raw_s->open_flags); if (raw_s->fd == -1) { error_setg_errno(errp, errno, "Could not reopen file"); ret = -1; @@ -2195,7 +2195,7 @@ static int fd_open(BlockDriverState *bs) DPRINTF("No floppy (open delayed)\n"); return -EIO; } - s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK); + s->fd = qemu_open(bs->exact_filename, s->open_flags & ~O_NONBLOCK); if (s->fd < 0) { s->fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); s->fd_got_error = 1; @@ -2486,7 +2486,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) qemu_close(s->fd); s->fd = -1; } - fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); + fd = qemu_open(bs->exact_filename, s->open_flags | O_NONBLOCK); if (fd >= 0) { if (ioctl(fd, FDEJECT, 0) < 0) perror("FDEJECT"); @@ -2710,7 +2710,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s->fd >= 0) qemu_close(s->fd); - fd = qemu_open(bs->filename, s->open_flags, 0644); + fd = qemu_open(bs->exact_filename, s->open_flags, 0644); if (fd < 0) { s->fd = -1; return -EIO; diff --git a/block/raw-win32.c b/block/raw-win32.c index 68f2338..5c8a894 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -417,7 +417,7 @@ static void raw_close(BlockDriverState *bs) CloseHandle(s->hfile); if (bs->open_flags & BDRV_O_TEMPORARY) { - unlink(bs->filename); + unlink(bs->exact_filename); } } @@ -485,7 +485,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) DWORD * high); get_compressed_t get_compressed; struct _stati64 st; - const char *filename = bs->filename; + const char *filename = bs->exact_filename; /* WinNT support GetCompressedFileSize to determine allocate size */ get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), diff --git a/qemu-img.c b/qemu-img.c index 75f4ee4..3d5587a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2155,7 +2155,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e, } if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) { printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", - e->start, e->length, e->offset, e->bs->filename); + e->start, e->length, e->offset, e->bs->exact_filename); } /* This format ignores the distinction between 0, ZERO and ZERO|DATA. * Modify the flags here to allow more coalescing.
In places which directly pass a filename to the OS, we should not use the filename field at all but exact_filename instead (although the former currently equals the latter if that is set). In qemu-img's map command, we should be using the filename field; but since this commit prepares to remove that field, using exact_filename is fine, too (this is the only user of BlockDriverState.filename which frequently queries that field). Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 4 ++-- block/gluster.c | 2 +- block/raw-posix.c | 8 ++++---- block/raw-win32.c | 4 ++-- qemu-img.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-)