[PATCH 0/1] dirty-bitmaps: remove deprecated autoload parameter

[PATCH 0/1] dirty-bitmaps: remove deprecated autoload parameter

[John Snow]

I'm going to be honest, here. There's actually no real reason to remove
this now, but we could, so I'm going to.

Also, in terms of the API serving as documentation, it's nicer to not
pretend this is an option that does anything, so out it goes.

This will serve as a little smoke test to see what happens if we
actually stop dropping features we claimed were deprecated.

John Snow (1):
  dirty-bitmaps: remove deprecated autoload parameter

 qemu-deprecated.texi | 20 +++++++++++++++-----
 qapi/block-core.json |  6 +-----
 blockdev.c           |  6 ------
 3 files changed, 16 insertions(+), 16 deletions(-)

-- 
2.21.0

[PATCH 1/1] dirty-bitmaps: remove deprecated autoload parameter

[John Snow]

This parameter has been deprecated since 2.12.0 and is eligible for
removal. Remove this parameter as it is actually completely ignored;
let's not give false hope.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qemu-deprecated.texi | 20 +++++++++++++++-----
 qapi/block-core.json |  6 +-----
 blockdev.c           |  6 ------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 01245e0b1c..d60246d5d6 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -149,11 +149,6 @@ QEMU 4.1 has three options, please migrate to one of these three:
 
 @section QEMU Machine Protocol (QMP) commands
 
-@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
-
-"autoload" parameter is now ignored. All bitmaps are automatically loaded
-from qcow2 images.
-
 @subsection query-block result field dirty-bitmaps[i].status (since 4.0)
 
 The ``status'' field of the ``BlockDirtyInfo'' structure, returned by
@@ -356,3 +351,18 @@ existing CPU models.  Management software that needs runnability
 guarantees must resolve the CPU model aliases using te
 ``alias-of'' field returned by the ``query-cpu-definitions'' QMP
 command.
+
+
+@node Recently removed features
+@appendix Recently removed features
+
+What follows is a record of recently removed, formerly deprecated
+features that serves as a record for users who have encountered
+trouble after a recent upgrade.
+
+@section QEMU Machine Protocol (QMP) commands
+
+@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
+
+"autoload" parameter is now ignored. All bitmaps are automatically loaded
+from qcow2 images.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e6edd641f1..e4975ece4a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1987,10 +1987,6 @@
 #              Qcow2 disks support persistent bitmaps. Default is false for
 #              block-dirty-bitmap-add. (Since: 2.10)
 #
-# @autoload: ignored and deprecated since 2.12.
-#            Currently, all dirty tracking bitmaps are loaded from Qcow2 on
-#            open.
-#
 # @disabled: the bitmap is created in the disabled state, which means that
 #            it will not track drive changes. The bitmap may be enabled with
 #            block-dirty-bitmap-enable. Default is false. (Since: 4.0)
@@ -1999,7 +1995,7 @@
 ##
 { 'struct': 'BlockDirtyBitmapAdd',
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
-            '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
+            '*persistent': 'bool', '*disabled': 'bool' } }
 
 ##
 # @BlockDirtyBitmapMergeSource:
diff --git a/blockdev.c b/blockdev.c
index fbef6845c8..93804da840 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1966,7 +1966,6 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
     qmp_block_dirty_bitmap_add(action->node, action->name,
                                action->has_granularity, action->granularity,
                                action->has_persistent, action->persistent,
-                               action->has_autoload, action->autoload,
                                action->has_disabled, action->disabled,
                                &local_err);
 
@@ -2858,7 +2857,6 @@ out:
 void qmp_block_dirty_bitmap_add(const char *node, const char *name,
                                 bool has_granularity, uint32_t granularity,
                                 bool has_persistent, bool persistent,
-                                bool has_autoload, bool autoload,
                                 bool has_disabled, bool disabled,
                                 Error **errp)
 {
@@ -2890,10 +2888,6 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
         persistent = false;
     }
 
-    if (has_autoload) {
-        warn_report("Autoload option is deprecated and its value is ignored");
-    }
-
     if (!has_disabled) {
         disabled = false;
     }
-- 
2.21.0

Re: [PATCH 1/1] dirty-bitmaps: remove deprecated autoload parameter

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1273 bytes --]

On 9/24/19 6:01 PM, John Snow wrote:
> This parameter has been deprecated since 2.12.0 and is eligible for
> removal. Remove this parameter as it is actually completely ignored;
> let's not give false hope.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qemu-deprecated.texi | 20 +++++++++++++++-----
>  qapi/block-core.json |  6 +-----
>  blockdev.c           |  6 ------
>  3 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 01245e0b1c..d60246d5d6 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi

> +@node Recently removed features
> +@appendix Recently removed features
> +
> +What follows is a record of recently removed, formerly deprecated
> +features that serves as a record for users who have encountered
> +trouble after a recent upgrade.
> +
> +@section QEMU Machine Protocol (QMP) commands
> +
> +@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)

Do we want to state 'since 4.2' here, as the point where it was removed?

Otherwise,

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [PATCH 1/1] dirty-bitmaps: remove deprecated autoload parameter

[Vladimir Sementsov-Ogievskiy]

25.09.2019 2:01, John Snow wrote:
> This parameter has been deprecated since 2.12.0 and is eligible for
> removal. Remove this parameter as it is actually completely ignored;
> let's not give false hope.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   qemu-deprecated.texi | 20 +++++++++++++++-----
>   qapi/block-core.json |  6 +-----
>   blockdev.c           |  6 ------
>   3 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 01245e0b1c..d60246d5d6 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -149,11 +149,6 @@ QEMU 4.1 has three options, please migrate to one of these three:
>   
>   @section QEMU Machine Protocol (QMP) commands
>   
> -@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> -
> -"autoload" parameter is now ignored. All bitmaps are automatically loaded
> -from qcow2 images.
> -
>   @subsection query-block result field dirty-bitmaps[i].status (since 4.0)
>   
>   The ``status'' field of the ``BlockDirtyInfo'' structure, returned by
> @@ -356,3 +351,18 @@ existing CPU models.  Management software that needs runnability
>   guarantees must resolve the CPU model aliases using te
>   ``alias-of'' field returned by the ``query-cpu-definitions'' QMP
>   command.
> +
> +
> +@node Recently removed features
> +@appendix Recently removed features
> +
> +What follows is a record of recently removed, formerly deprecated
> +features that serves as a record for users who have encountered
> +trouble after a recent upgrade.
> +
> +@section QEMU Machine Protocol (QMP) commands
> +
> +@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)

Agree with Eric that it should be 4.2 - as this section is about removing

> +
> +"autoload" parameter is now ignored. All bitmaps are automatically loaded
> +from qcow2 images.

Maybe, rephrase it as s/is now ignored/is now removed (ignored since 2.12.0)/ ,
so that this paragraph don't mislead without a context.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

(Yay, deprecation works!)

-- 
Best regards,
Vladimir

Re: [PATCH 1/1] dirty-bitmaps: remove deprecated autoload parameter

[John Snow]

On 9/25/19 3:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> 25.09.2019 2:01, John Snow wrote:
>> This parameter has been deprecated since 2.12.0 and is eligible for
>> removal. Remove this parameter as it is actually completely ignored;
>> let's not give false hope.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   qemu-deprecated.texi | 20 +++++++++++++++-----
>>   qapi/block-core.json |  6 +-----
>>   blockdev.c           |  6 ------
>>   3 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>> index 01245e0b1c..d60246d5d6 100644
>> --- a/qemu-deprecated.texi
>> +++ b/qemu-deprecated.texi
>> @@ -149,11 +149,6 @@ QEMU 4.1 has three options, please migrate to one of these three:
>>   
>>   @section QEMU Machine Protocol (QMP) commands
>>   
>> -@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
>> -
>> -"autoload" parameter is now ignored. All bitmaps are automatically loaded
>> -from qcow2 images.
>> -
>>   @subsection query-block result field dirty-bitmaps[i].status (since 4.0)
>>   
>>   The ``status'' field of the ``BlockDirtyInfo'' structure, returned by
>> @@ -356,3 +351,18 @@ existing CPU models.  Management software that needs runnability
>>   guarantees must resolve the CPU model aliases using te
>>   ``alias-of'' field returned by the ``query-cpu-definitions'' QMP
>>   command.
>> +
>> +
>> +@node Recently removed features
>> +@appendix Recently removed features
>> +
>> +What follows is a record of recently removed, formerly deprecated
>> +features that serves as a record for users who have encountered
>> +trouble after a recent upgrade.
>> +
>> +@section QEMU Machine Protocol (QMP) commands
>> +
>> +@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> 
> Agree with Eric that it should be 4.2 - as this section is about removing
> 

Yes, shame on me. I spent about three seconds on this patch and should
have spent four.

>> +
>> +"autoload" parameter is now ignored. All bitmaps are automatically loaded
>> +from qcow2 images.
> 
> Maybe, rephrase it as s/is now ignored/is now removed (ignored since 2.12.0)/ ,
> so that this paragraph don't mislead without a context.
> 

Also a good idea.

'The "autoload" parameter has been ignored since 2.12.0. All bitmaps are
automatically loaded from qcow2 images.'

> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> (Yay, deprecation works!)
> 

Thanks, and I'll get to the rest of your pending bitmap patches and
cleanups soon.

--js

Re: [PATCH 1/1] dirty-bitmaps: remove deprecated autoload parameter

[Vladimir Sementsov-Ogievskiy]

25.09.2019 16:01, John Snow wrote:
> 
> 
> On 9/25/19 3:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 25.09.2019 2:01, John Snow wrote:
>>> This parameter has been deprecated since 2.12.0 and is eligible for
>>> removal. Remove this parameter as it is actually completely ignored;
>>> let's not give false hope.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>    qemu-deprecated.texi | 20 +++++++++++++++-----
>>>    qapi/block-core.json |  6 +-----
>>>    blockdev.c           |  6 ------
>>>    3 files changed, 16 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>>> index 01245e0b1c..d60246d5d6 100644
>>> --- a/qemu-deprecated.texi
>>> +++ b/qemu-deprecated.texi
>>> @@ -149,11 +149,6 @@ QEMU 4.1 has three options, please migrate to one of these three:
>>>    
>>>    @section QEMU Machine Protocol (QMP) commands
>>>    
>>> -@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
>>> -
>>> -"autoload" parameter is now ignored. All bitmaps are automatically loaded
>>> -from qcow2 images.
>>> -
>>>    @subsection query-block result field dirty-bitmaps[i].status (since 4.0)
>>>    
>>>    The ``status'' field of the ``BlockDirtyInfo'' structure, returned by
>>> @@ -356,3 +351,18 @@ existing CPU models.  Management software that needs runnability
>>>    guarantees must resolve the CPU model aliases using te
>>>    ``alias-of'' field returned by the ``query-cpu-definitions'' QMP
>>>    command.
>>> +
>>> +
>>> +@node Recently removed features
>>> +@appendix Recently removed features
>>> +
>>> +What follows is a record of recently removed, formerly deprecated
>>> +features that serves as a record for users who have encountered
>>> +trouble after a recent upgrade.
>>> +
>>> +@section QEMU Machine Protocol (QMP) commands
>>> +
>>> +@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
>>
>> Agree with Eric that it should be 4.2 - as this section is about removing
>>
> 
> Yes, shame on me. I spent about three seconds on this patch and should
> have spent four.
> 
>>> +
>>> +"autoload" parameter is now ignored. All bitmaps are automatically loaded
>>> +from qcow2 images.
>>
>> Maybe, rephrase it as s/is now ignored/is now removed (ignored since 2.12.0)/ ,
>> so that this paragraph don't mislead without a context.
>>
> 
> Also a good idea.
> 
> 'The "autoload" parameter has been ignored since 2.12.0. All bitmaps are
> automatically loaded from qcow2 images.'

OK

> 
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> (Yay, deprecation works!)
>>
> 
> Thanks, and I'll get to the rest of your pending bitmap patches and
> cleanups soon.
> 


That's great!


-- 
Best regards,
Vladimir

Re: [PATCH 0/1] dirty-bitmaps: remove deprecated autoload parameter

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 811 bytes --]

On 25.09.19 01:01, John Snow wrote:
> I'm going to be honest, here. There's actually no real reason to remove
> this now, but we could, so I'm going to.
> 
> Also, in terms of the API serving as documentation, it's nicer to not
> pretend this is an option that does anything, so out it goes.
> 
> This will serve as a little smoke test to see what happens if we
> actually stop dropping features we claimed were deprecated.
> 
> John Snow (1):
>   dirty-bitmaps: remove deprecated autoload parameter
> 
>  qemu-deprecated.texi | 20 +++++++++++++++-----
>  qapi/block-core.json |  6 +-----
>  blockdev.c           |  6 ------
>  3 files changed, 16 insertions(+), 16 deletions(-)

Cleaning up my inbox...  Was it your intention for someone other than
yourself to take this patch? :S

Max


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