Re: [Qemu-devel] [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 

> 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:) 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 
> ---
>  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

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

[Fabiano Rosas]

The replication driver is only selected explicitly (via
driver=replication,mode=primary,...) so it is not a protocol driver.

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:) 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 
---
 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,
-- 
2.13.6