[Top][All Lists]

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

Re: [Qemu-block] [PATCH 5/5] include/block/block_int: Document protocol

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH 5/5] include/block/block_int: Document protocol related functions
Date: Mon, 12 Mar 2018 14:50:50 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 2018-03-09 19:22, Fabiano Rosas wrote:
> Clarify that for protocols the brdv_file_open function is used instead
> of bdrv_open and that protocol_name is expected to be set.
> Signed-off-by: Fabiano Rosas <address@hidden>
> ---
>  include/block/block_int.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 64a5700f2b..d5e864c2dc 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -126,6 +126,8 @@ struct BlockDriver {
>      int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
>                       Error **errp);
> +
> +    /* Protocol drivers should implement this instead of bdrv_open */
>      int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
>                            Error **errp);
>      void (*bdrv_close)(BlockDriverState *bs);
> @@ -247,6 +249,10 @@ struct BlockDriver {
>       */
>      int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
> +    /*
> +     * Drivers that set this field should also provide a
> +     * bdrv_file_open implementation

Well...  In a sense.

Drivers that implement this should be expected to be given only a
filename and no other options.  Therefore, they should generally
implement .bdrv_parse_filename() so they can extract options from it
(exception: gluster, which parses the whole filename in its
.bdrv_file_open() implementation).

(Or maybe they can just accept a URL and don't need to extract options
from it, like curl or file.)

So the stress lies on "Drivers setting this field must be able to work
with just a plain filename with this prefix, and no other options."
(Note that all of the drivers you removed this field from in this series
violated this.  They expect some other options and cannot work without.)

A driver doesn't need to be a protocol driver for this, and technically
a protocol driver doesn't need to set this.  Maybe we should rename it
to "filename_prefix"...?


> +     */
>      const char *protocol_name;
>      int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
>                           PreallocMode prealloc, Error **errp);

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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