Re: [Qemu-block] [PATCH 1/5] block/replication: Remove protocol_name field

[Max Reitz]

On 2018-03-09 19:21, Fabiano Rosas wrote:
> The replication driver is only selected explicitly (via
> driver=replication,mode=primary,...) so it is not a protocol driver.

That's not really the point.  It isn't a protocol driver because it has
a "file" child (opened in replication_open).  All in all it's a filter
driver.

Apart from that, having a protocol_name might still be OK even for
non-protocol drivers; for instance, blkdebug and blkverify are both
filter drivers, but since they implement bdrv_parse_filename() it's OK
for them to specify protocol_name.

So your sentence is half correct.  Because it can only be selected
explicitly (for several reasons), giving a protocol_name doesn't make
sense.  (Independently of whether it is a protocol driver or not.)

But anyway:

Reviewed-by: Max Reitz <address@hidden>

> This patch removes the protocol_name field from the brdv_replication
> structure so that attempts to invoke this driver using protocol
> syntax (i.e. replication:<filename:options:...>) will fail gracefully:
> 
>   $ qemu-img info replication:foo
>   qemu-img: Could not open 'replication:': Unknown protocol 'replication'
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1726733
> Signed-off-by: Fabiano Rosas <address@hidden>
> ---
>  block/replication.c | 1 -
>  replication.h       | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/block/replication.c b/block/replication.c
> index f98ef094b9..6c0c7186d9 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -703,7 +703,6 @@ static void replication_stop(ReplicationState *rs, bool 
> failover, Error **errp)
>  
>  BlockDriver bdrv_replication = {
>      .format_name                = "replication",
> -    .protocol_name              = "replication",
>      .instance_size              = sizeof(BDRVReplicationState),
>  
>      .bdrv_open                  = replication_open,
> diff --git a/replication.h b/replication.h
> index 8faefe005f..4c8354de23 100644
> --- a/replication.h
> +++ b/replication.h
> @@ -67,7 +67,6 @@ typedef struct ReplicationState ReplicationState;
>   *
>   * BlockDriver bdrv_replication = {
>   *     .format_name                = "replication",
> - *     .protocol_name              = "replication",
>   *     .instance_size              = sizeof(BDRVReplicationState),
>   *
>   *     .bdrv_open                  = replication_open,
>

signature.asc
Description: OpenPGP digital signature