qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 4/5] block: Drop BlockDriverState.filename


From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH v3 4/5] block: Drop BlockDriverState.filename
Date: Thu, 13 Aug 2015 17:28:12 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 12.08.2015 um 17:48 hat Max Reitz geschrieben:
> That field is now only used during initialization of BlockDriverStates
> (opening images) and for error or warning messages. Performance is not
> that much of an issue here, so we can drop the field and replace its use
> by a call to bdrv_filename() or bdrv_filename_alloc(). By doing so we
> can ensure the result always to be recent, whereas the contents of
> BlockDriverState.filename may have been obsoleted by manipulations of
> single BlockDriverStates or of the BDS graph.
> 
> The users of the BDS filename field were changed as follows:
> - copying the filename into another buffer is trivially replaced by
>   using bdrv_filename() instead of the copy function
> - strdup() on the filename is replaced by a call to
>   bdrv_filename_alloc()
> - bdrv_filename(bs, bs->filename, sizeof(bs->filename)) is replaced by
>   bdrv_refresh_filename(bs)
>   (these were introduced by the previous patch)
> - anywhere else bdrv_filename_alloc() is used, any access to
>   BlockDriverState.filename is then replaced by the buffer returned, and
>   it is freed when it is no longer needed
> 
> Signed-off-by: Max Reitz <address@hidden>

> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 10f8f62..17daf06 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -512,12 +512,14 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> *options,
>          goto fail;
>      }
>      if (!s->use_aio && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
> +        char *filename = bdrv_filename_alloc(bs);
>          error_printf("WARNING: aio=native was specified for '%s', but "
>                       "it requires cache.direct=on, which was not "
>                       "specified. Falling back to aio=threads.\n"
>                       "         This will become an error condition in "
>                       "future QEMU versions.\n",
> -                     bs->filename);
> +                     filename);
> +        g_free(filename);
>      }
>  #endif

This variable shadows the existing local variable filename, which
already contains the right string (if we want the normalised name in the
error message, which seems to make sense to me).

> @@ -2100,9 +2102,13 @@ static bool hdev_is_sg(BlockDriverState *bs)
>  
>      struct stat st;
>      struct sg_scsi_id scsiid;
> -    int sg_version;
> +    int sg_version, ret;
> +    char *filename = bdrv_filename_alloc(bs);
>  
> -    if (stat(bs->filename, &st) >= 0 && S_ISCHR(st.st_mode) &&
> +    ret = stat(filename, &st);
> +    g_free(filename);
> +
> +    if (ret >= 0 && S_ISCHR(st.st_mode) &&
>          !bdrv_ioctl(bs, SG_GET_VERSION_NUM, &sg_version) &&
>          !bdrv_ioctl(bs, SG_GET_SCSI_ID, &scsiid)) {
>          DPRINTF("SG device found: type=%d, version=%d\n",

Here we don't have the filename in a local string, but for raw-posix, I
think bs->exact_filename should be reliably set. This is also another
option above if you don't want to print the normalised filename.

> diff --git a/block/vpc.c b/block/vpc.c
> index 3e385d9..064b2f8 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -204,9 +204,12 @@ static int vpc_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>      checksum = be32_to_cpu(footer->checksum);
>      footer->checksum = 0;
> -    if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum)
> +    if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum) {
> +        char *filename = bdrv_filename_alloc(bs);
>          fprintf(stderr, "block-vpc: The header checksum of '%s' is "
> -            "incorrect.\n", bs->filename);
> +                "incorrect.\n", filename);
> +        g_free(filename);
> +    }

Here the filename shouldn't be included in the error message in the
first place, but that's a separate patch on top.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]