All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly
@ 2018-03-09 18:21 Fabiano Rosas
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field Fabiano Rosas
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz

Block drivers can be selected by either protocol syntax:

  <protocol>:<filename>[:options]

or explicitly:

  driver=<driver>[,option=<opt1>...]

For the protocol syntax to work, drivers should set the protocol_name
field of the BlockDriver structure and provide bdrv_file_open and
bdrv_parse_filename implementations.

Conversely, block drivers that do not support the protocol syntax
should instead implement bdrv_open and not have a protocol_name field.

Some drivers do not currently adhere to this and errors arise when
trying to select them using the protocol syntax. For instance:

  $ qemu-img info replication:foo
  qemu-img: block.c:2401: bdrv_open_inherit: \
  Assertion `!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open' failed.
  Aborted (core dumped)

This patch-set ensures that the following drivers are meeting the
above criteria:
- blkreplay
- quorum
- replication
- throttle

Aside from that, documentation was added to make the above more
explicit.


Fabiano Rosas (5):
  block/replication: Remove protocol_name field
  block/quorum: Remove protocol-related fields
  block/throttle: Remove protocol-related fields
  block/blkreplay: Remove protocol-related fields
  include/block/block_int: Document protocol related functions

 block/blkreplay.c         | 3 +--
 block/quorum.c            | 3 +--
 block/replication.c       | 1 -
 block/throttle.c          | 3 +--
 include/block/block_int.h | 6 ++++++
 replication.h             | 1 -
 6 files changed, 9 insertions(+), 8 deletions(-)

--
2.13.6

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field
  2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
@ 2018-03-09 18:21 ` Fabiano Rosas
  2018-03-12 13:41   ` Max Reitz
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields Fabiano Rosas
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, Wen Congyang, Xie Changlong

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:<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 <farosas@linux.vnet.ibm.com>
---
 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields
  2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field Fabiano Rosas
@ 2018-03-09 18:21 ` Fabiano Rosas
  2018-03-12 13:44   ` Max Reitz
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 3/5] block/throttle: " Fabiano Rosas
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, Alberto Garcia

The quorum driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. quorum:<filename:options:...>) will now fail gracefully:

  $ qemu-img info quorum:foo
  qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum'

Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
---
 block/quorum.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 14333c18aa..cfe484a945 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1098,11 +1098,10 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
 
 static BlockDriver bdrv_quorum = {
     .format_name                        = "quorum",
-    .protocol_name                      = "quorum",
 
     .instance_size                      = sizeof(BDRVQuorumState),
 
-    .bdrv_file_open                     = quorum_open,
+    .bdrv_open                          = quorum_open,
     .bdrv_close                         = quorum_close,
     .bdrv_refresh_filename              = quorum_refresh_filename,
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 3/5] block/throttle: Remove protocol-related fields
  2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field Fabiano Rosas
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields Fabiano Rosas
@ 2018-03-09 18:21 ` Fabiano Rosas
  2018-03-12 13:44   ` Max Reitz
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 4/5] block/blkreplay: " Fabiano Rosas
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions Fabiano Rosas
  4 siblings, 1 reply; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz

The throttle driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. throttle:<filename:options:...>) will now fail gracefully:

  $ qemu-img info throttle:foo
  qemu-img: Could not open 'throttle:foo': Unknown protocol 'throttle'

Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
---
 block/throttle.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/throttle.c b/block/throttle.c
index 5f4d43d0fc..95ed06acd8 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -215,10 +215,9 @@ static void coroutine_fn throttle_co_drain_end(BlockDriverState *bs)
 
 static BlockDriver bdrv_throttle = {
     .format_name                        =   "throttle",
-    .protocol_name                      =   "throttle",
     .instance_size                      =   sizeof(ThrottleGroupMember),
 
-    .bdrv_file_open                     =   throttle_open,
+    .bdrv_open                          =   throttle_open,
     .bdrv_close                         =   throttle_close,
     .bdrv_co_flush                      =   throttle_co_flush,
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/5] block/blkreplay: Remove protocol-related fields
  2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
                   ` (2 preceding siblings ...)
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 3/5] block/throttle: " Fabiano Rosas
@ 2018-03-09 18:22 ` Fabiano Rosas
  2018-03-12  8:05   ` Pavel Dovgalyuk
  2018-03-12 13:45   ` Max Reitz
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions Fabiano Rosas
  4 siblings, 2 replies; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, Pavel Dovgalyuk, Paolo Bonzini

The blkreplay driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. blkreplay:<filename:options:...>) will now fail gracefully:

  $ qemu-img info blkreplay:foo
  qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay'

Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
---
 block/blkreplay.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index 61e44a1949..fe5a9b4a98 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -129,10 +129,9 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
 
 static BlockDriver bdrv_blkreplay = {
     .format_name            = "blkreplay",
-    .protocol_name          = "blkreplay",
     .instance_size          = 0,
 
-    .bdrv_file_open         = blkreplay_open,
+    .bdrv_open              = blkreplay_open,
     .bdrv_close             = blkreplay_close,
     .bdrv_child_perm        = bdrv_filter_default_perms,
     .bdrv_getlength         = blkreplay_getlength,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions
  2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
                   ` (3 preceding siblings ...)
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 4/5] block/blkreplay: " Fabiano Rosas
@ 2018-03-09 18:22 ` Fabiano Rosas
  2018-03-12 13:50   ` Max Reitz
  4 siblings, 1 reply; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-09 18:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, mreitz

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 <farosas@linux.vnet.ibm.com>
---
 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
+     */
     const char *protocol_name;
     int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
                          PreallocMode prealloc, Error **errp);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] block/blkreplay: Remove protocol-related fields
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 4/5] block/blkreplay: " Fabiano Rosas
@ 2018-03-12  8:05   ` Pavel Dovgalyuk
  2018-03-12 13:45   ` Max Reitz
  1 sibling, 0 replies; 13+ messages in thread
From: Pavel Dovgalyuk @ 2018-03-12  8:05 UTC (permalink / raw)
  To: 'Fabiano Rosas', qemu-devel
  Cc: qemu-block, kwolf, mreitz, 'Pavel Dovgalyuk',
	'Paolo Bonzini'

> From: Fabiano Rosas [mailto:farosas@linux.vnet.ibm.com]
> The blkreplay driver is not a protocol so it should implement bdrv_open
> instead of bdrv_file_open and not provide a protocol_name.
> 
> Attempts to invoke this driver using protocol syntax
> (i.e. blkreplay:<filename:options:...>) will now fail gracefully:
> 
>   $ qemu-img info blkreplay:foo
>   qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay'
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
> ---
>  block/blkreplay.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index 61e44a1949..fe5a9b4a98 100755
> --- a/block/blkreplay.c
> +++ b/block/blkreplay.c
> @@ -129,10 +129,9 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
> 
>  static BlockDriver bdrv_blkreplay = {
>      .format_name            = "blkreplay",
> -    .protocol_name          = "blkreplay",
>      .instance_size          = 0,
> 
> -    .bdrv_file_open         = blkreplay_open,
> +    .bdrv_open              = blkreplay_open,
>      .bdrv_close             = blkreplay_close,
>      .bdrv_child_perm        = bdrv_filter_default_perms,
>      .bdrv_getlength         = blkreplay_getlength,
> --
> 2.13.6

Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

Pavel Dovgalyuk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field Fabiano Rosas
@ 2018-03-12 13:41   ` Max Reitz
  0 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2018-03-12 13:41 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel; +Cc: qemu-block, kwolf, Wen Congyang, Xie Changlong

[-- Attachment #1: Type: text/plain, Size: 2450 bytes --]

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 <mreitz@redhat.com>

> 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 <farosas@linux.vnet.ibm.com>
> ---
>  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,
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields Fabiano Rosas
@ 2018-03-12 13:44   ` Max Reitz
  0 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2018-03-12 13:44 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel; +Cc: qemu-block, kwolf, Alberto Garcia

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On 2018-03-09 19:21, Fabiano Rosas wrote:
> The quorum driver is not a protocol so it should implement bdrv_open
> instead of bdrv_file_open and not provide a protocol_name.
> 
> Attempts to invoke this driver using protocol syntax
> (i.e. quorum:<filename:options:...>) will now fail gracefully:
> 
>   $ qemu-img info quorum:foo
>   qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum'
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
> ---
>  block/quorum.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] block/throttle: Remove protocol-related fields
  2018-03-09 18:21 ` [Qemu-devel] [PATCH 3/5] block/throttle: " Fabiano Rosas
@ 2018-03-12 13:44   ` Max Reitz
  0 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2018-03-12 13:44 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel; +Cc: qemu-block, kwolf

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On 2018-03-09 19:21, Fabiano Rosas wrote:
> The throttle driver is not a protocol so it should implement bdrv_open
> instead of bdrv_file_open and not provide a protocol_name.
> 
> Attempts to invoke this driver using protocol syntax
> (i.e. throttle:<filename:options:...>) will now fail gracefully:
> 
>   $ qemu-img info throttle:foo
>   qemu-img: Could not open 'throttle:foo': Unknown protocol 'throttle'
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
> ---
>  block/throttle.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] block/blkreplay: Remove protocol-related fields
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 4/5] block/blkreplay: " Fabiano Rosas
  2018-03-12  8:05   ` Pavel Dovgalyuk
@ 2018-03-12 13:45   ` Max Reitz
  1 sibling, 0 replies; 13+ messages in thread
From: Max Reitz @ 2018-03-12 13:45 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel
  Cc: qemu-block, kwolf, Pavel Dovgalyuk, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On 2018-03-09 19:22, Fabiano Rosas wrote:
> The blkreplay driver is not a protocol so it should implement bdrv_open
> instead of bdrv_file_open and not provide a protocol_name.
> 
> Attempts to invoke this driver using protocol syntax
> (i.e. blkreplay:<filename:options:...>) will now fail gracefully:
> 
>   $ qemu-img info blkreplay:foo
>   qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay'
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com>
> ---
>  block/blkreplay.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions
  2018-03-09 18:22 ` [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions Fabiano Rosas
@ 2018-03-12 13:50   ` Max Reitz
  2018-03-12 22:26     ` Fabiano Rosas
  0 siblings, 1 reply; 13+ messages in thread
From: Max Reitz @ 2018-03-12 13:50 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel; +Cc: qemu-block, kwolf

[-- Attachment #1: Type: text/plain, Size: 2265 bytes --]

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 <farosas@linux.vnet.ibm.com>
> ---
>  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"...?

Max

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions
  2018-03-12 13:50   ` Max Reitz
@ 2018-03-12 22:26     ` Fabiano Rosas
  0 siblings, 0 replies; 13+ messages in thread
From: Fabiano Rosas @ 2018-03-12 22:26 UTC (permalink / raw)
  To: Max Reitz, qemu-devel; +Cc: kwolf, qemu-block

On 2018-03-12 10:50, Max Reitz wrote:

> 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"...?

Yes, something that is closer to the filename string and farther from
the notion of what defines a protocol would be good. I see "protocol
prefix" mentioned in block.c, maybe that would be a good middle
ground.

Cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-03-12 22:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 18:21 [Qemu-devel] [PATCH 0/5] block: Ensure non-protocol drivers can only be selected explicitly Fabiano Rosas
2018-03-09 18:21 ` [Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field Fabiano Rosas
2018-03-12 13:41   ` Max Reitz
2018-03-09 18:21 ` [Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields Fabiano Rosas
2018-03-12 13:44   ` Max Reitz
2018-03-09 18:21 ` [Qemu-devel] [PATCH 3/5] block/throttle: " Fabiano Rosas
2018-03-12 13:44   ` Max Reitz
2018-03-09 18:22 ` [Qemu-devel] [PATCH 4/5] block/blkreplay: " Fabiano Rosas
2018-03-12  8:05   ` Pavel Dovgalyuk
2018-03-12 13:45   ` Max Reitz
2018-03-09 18:22 ` [Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions Fabiano Rosas
2018-03-12 13:50   ` Max Reitz
2018-03-12 22:26     ` Fabiano Rosas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.