diff mbox

block: use correct filename for error report

Message ID 1380017641-29820-1-git-send-email-riegamaths@gmail.com
State New
Headers show

Commit Message

dunrong huang Sept. 24, 2013, 10:14 a.m. UTC
The content filename point to will be erased by qemu_opts_absorb_qdict()
in raw_open_common() in drv->bdrv_file_open()

So it's better to use bs->filename.

Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
---
 block.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Max Reitz Sept. 24, 2013, 12:12 p.m. UTC | #1
On 2013-09-24 12:14, Dunrong Huang wrote:
> The content filename point to will be erased by qemu_opts_absorb_qdict()
> in raw_open_common() in drv->bdrv_file_open()
>
> So it's better to use bs->filename.
>
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> ---
>   block.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>
dunrong huang Sept. 30, 2013, 9:20 a.m. UTC | #2
ping?


On Tue, Sep 24, 2013 at 8:12 PM, Max Reitz <mreitz@redhat.com> wrote:

> On 2013-09-24 12:14, Dunrong Huang wrote:
>
>> The content filename point to will be erased by qemu_opts_absorb_qdict()
>> in raw_open_common() in drv->bdrv_file_open()
>>
>> So it's better to use bs->filename.
>>
>> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
>> ---
>>   block.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
Stefan Hajnoczi Oct. 2, 2013, 9:48 a.m. UTC | #3
On Tue, Sep 24, 2013 at 06:14:01PM +0800, Dunrong Huang wrote:
> The content filename point to will be erased by qemu_opts_absorb_qdict()
> in raw_open_common() in drv->bdrv_file_open()
> 
> So it's better to use bs->filename.
> 
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> ---
>  block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

The same issue affects the rest of the function:

#ifndef _WIN32
    if (bs->is_temporary) {
        assert(filename != NULL);
        unlink(filename);
    }
#endif

Do you want to send a separate patch to fix this?

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
dunrong huang Oct. 2, 2013, 1:39 p.m. UTC | #4
On Wed, Oct 2, 2013 at 5:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Sep 24, 2013 at 06:14:01PM +0800, Dunrong Huang wrote:
> > The content filename point to will be erased by qemu_opts_absorb_qdict()
> > in raw_open_common() in drv->bdrv_file_open()
> >
> > So it's better to use bs->filename.
> >
> > Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> > ---
> >  block.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> The same issue affects the rest of the function:
>
> #ifndef _WIN32
>     if (bs->is_temporary) {
>         assert(filename != NULL);
>         unlink(filename);
>     }
> #endif
>
> Do you want to send a separate patch to fix this?
>
Sure.

>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>
diff mbox

Patch

diff --git a/block.c b/block.c
index ea4956d..9cd78a1 100644
--- a/block.c
+++ b/block.c
@@ -808,8 +808,8 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     if (ret < 0) {
         if (error_is_set(&local_err)) {
             error_propagate(errp, local_err);
-        } else if (filename) {
-            error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+        } else if (bs->filename[0]) {
+            error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename);
         } else {
             error_setg_errno(errp, -ret, "Could not open image");
         }