[Qemu-devel] [PATCH 0/2] Remove unsupported / empty .bdrv_make_empty functions

[Qemu-devel] [PATCH 0/2] Remove unsupported / empty .bdrv_make_empty functions

[Jeff Cody]

QED and QCOW2 both have .bdrv_make_empty stubs that do nothing.  In the 
case of QED, it returns an error (-ENOTSUP), which causes problems with
bdrv_commit().

This removes those stubs.

Jeff Cody (2):
  block: remove QED .bdrv_make_empty implementation
  block: remove qcow2 .bdrv_make_empty implementation

 block/qcow2.c | 21 ---------------------
 block/qed.c   |  6 ------
 2 files changed, 27 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] block: remove QED .bdrv_make_empty implementation

[Jeff Cody]

The QED .bdrv_make_empty() implementation does nothing but return
-ENOTSUP, which causes problems in bdrv_commit().  Since the function
stub exists for QED, it is called, which then always returns an error.

The proper way to not support an optional driver function stub is to
just not implement it, so let's remove the stub.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/qed.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 694e6e2..b9ca7ac 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -731,11 +731,6 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
     return cb.status;
 }
 
-static int bdrv_qed_make_empty(BlockDriverState *bs)
-{
-    return -ENOTSUP;
-}
-
 static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
 {
     return acb->common.bs->opaque;
@@ -1617,7 +1612,6 @@ static BlockDriver bdrv_qed = {
     .bdrv_create              = bdrv_qed_create,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
-    .bdrv_make_empty          = bdrv_qed_make_empty,
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
     .bdrv_aio_writev          = bdrv_qed_aio_writev,
     .bdrv_co_write_zeroes     = bdrv_qed_co_write_zeroes,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] block: remove QED .bdrv_make_empty implementation

[Benoît Canet]

Le Monday 27 Jan 2014 à 16:09:12 (-0500), Jeff Cody a écrit :
> The QED .bdrv_make_empty() implementation does nothing but return
> -ENOTSUP, which causes problems in bdrv_commit().  Since the function
> stub exists for QED, it is called, which then always returns an error.
> 
> The proper way to not support an optional driver function stub is to
> just not implement it, so let's remove the stub.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/qed.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/block/qed.c b/block/qed.c
> index 694e6e2..b9ca7ac 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -731,11 +731,6 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
>      return cb.status;
>  }
>  
> -static int bdrv_qed_make_empty(BlockDriverState *bs)
> -{
> -    return -ENOTSUP;
> -}
> -
>  static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
>  {
>      return acb->common.bs->opaque;
> @@ -1617,7 +1612,6 @@ static BlockDriver bdrv_qed = {
>      .bdrv_create              = bdrv_qed_create,
>      .bdrv_has_zero_init       = bdrv_has_zero_init_1,
>      .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
> -    .bdrv_make_empty          = bdrv_qed_make_empty,
>      .bdrv_aio_readv           = bdrv_qed_aio_readv,
>      .bdrv_aio_writev          = bdrv_qed_aio_writev,
>      .bdrv_co_write_zeroes     = bdrv_qed_co_write_zeroes,
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH 2/2] block: remove qcow2 .bdrv_make_empty implementation

[Jeff Cody]

The QCOW2 .bdrv_make_empty implementation always returns 0 for success,
but does not actually do anything.

The proper way to not support an optional driver function stub is to
just not implement it, so let's remove the stub.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/qcow2.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2da62b8..99a1ad1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1689,26 +1689,6 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     return ret;
 }
 
-static int qcow2_make_empty(BlockDriverState *bs)
-{
-#if 0
-    /* XXX: not correct */
-    BDRVQcowState *s = bs->opaque;
-    uint32_t l1_length = s->l1_size * sizeof(uint64_t);
-    int ret;
-
-    memset(s->l1_table, 0, l1_length);
-    if (bdrv_pwrite(bs->file, s->l1_table_offset, s->l1_table, l1_length) < 0)
-        return -1;
-    ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length);
-    if (ret < 0)
-        return ret;
-
-    l2_cache_reset(bs);
-#endif
-    return 0;
-}
-
 static coroutine_fn int qcow2_co_write_zeroes(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
 {
@@ -2252,7 +2232,6 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,
     .bdrv_set_key       = qcow2_set_key,
-    .bdrv_make_empty    = qcow2_make_empty,
 
     .bdrv_co_readv          = qcow2_co_readv,
     .bdrv_co_writev         = qcow2_co_writev,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] block: remove qcow2 .bdrv_make_empty implementation

[Benoît Canet]

Le Monday 27 Jan 2014 à 16:09:13 (-0500), Jeff Cody a écrit :
> The QCOW2 .bdrv_make_empty implementation always returns 0 for success,
> but does not actually do anything.
> 
> The proper way to not support an optional driver function stub is to
> just not implement it, so let's remove the stub.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/qcow2.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2da62b8..99a1ad1 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1689,26 +1689,6 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
>      return ret;
>  }
>  
> -static int qcow2_make_empty(BlockDriverState *bs)
> -{
> -#if 0
> -    /* XXX: not correct */
> -    BDRVQcowState *s = bs->opaque;
> -    uint32_t l1_length = s->l1_size * sizeof(uint64_t);
> -    int ret;
> -
> -    memset(s->l1_table, 0, l1_length);
> -    if (bdrv_pwrite(bs->file, s->l1_table_offset, s->l1_table, l1_length) < 0)
> -        return -1;
> -    ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length);
> -    if (ret < 0)
> -        return ret;
> -
> -    l2_cache_reset(bs);
> -#endif
> -    return 0;
> -}
> -
>  static coroutine_fn int qcow2_co_write_zeroes(BlockDriverState *bs,
>      int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
>  {
> @@ -2252,7 +2232,6 @@ static BlockDriver bdrv_qcow2 = {
>      .bdrv_has_zero_init = bdrv_has_zero_init_1,
>      .bdrv_co_get_block_status = qcow2_co_get_block_status,
>      .bdrv_set_key       = qcow2_set_key,
> -    .bdrv_make_empty    = qcow2_make_empty,
>  
>      .bdrv_co_readv          = qcow2_co_readv,
>      .bdrv_co_writev         = qcow2_co_writev,
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH 0/2] Remove unsupported / empty .bdrv_make_empty functions

[Eric Blake]

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

On 01/27/2014 02:09 PM, Jeff Cody wrote:
> QED and QCOW2 both have .bdrv_make_empty stubs that do nothing.  In the 
> case of QED, it returns an error (-ENOTSUP), which causes problems with
> bdrv_commit().
> 
> This removes those stubs.
> 
> Jeff Cody (2):
>   block: remove QED .bdrv_make_empty implementation
>   block: remove qcow2 .bdrv_make_empty implementation
> 
>  block/qcow2.c | 21 ---------------------
>  block/qed.c   |  6 ------
>  2 files changed, 27 deletions(-)

Always fun when you can fix bugs by pure deletion :)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 0/2] Remove unsupported / empty .bdrv_make_empty functions

[Kevin Wolf]

Am 27.01.2014 um 22:09 hat Jeff Cody geschrieben:
> QED and QCOW2 both have .bdrv_make_empty stubs that do nothing.  In the 
> case of QED, it returns an error (-ENOTSUP), which causes problems with
> bdrv_commit().
> 
> This removes those stubs.
> 
> Jeff Cody (2):
>   block: remove QED .bdrv_make_empty implementation
>   block: remove qcow2 .bdrv_make_empty implementation

Reviewed-by: Kevin Wolf <kwolf@redhat.com>