Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device

[Thomas Huth <thuth@redhat.com>]

On 11.05.2017 08:45, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 10.05.2017 17:55, Paolo Bonzini wrote:
>>>
>>>
>>> On 10/05/2017 17:50, Thomas Huth wrote:
>>>> We likely do not want to carry these legacy -drive options along forever.
>>>> Let's emit a deprecation warning for the -drive options that have a
>>>> replacement with the -device option, so that the (hopefully few) remaining
>>>> users are aware of this and can adapt their scripts / behaviour.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  blockdev.c | 11 +++++++++++
>>>>  1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index 4d8cded..87a025a 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -797,6 +797,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>>>>      const char *filename;
>>>>      Error *local_err = NULL;
>>>>      int i;
>>>> +    const char *deprecated[] = {
>>>> +        "serial", "trans", "secs", "heads", "cyls"
>>>> +    };
>>>>  
>>>>      /* Change legacy command line options into QMP ones */
>>>>      static const struct {
>>>> @@ -880,6 +883,14 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>>>>                  "update your scripts.\n");
>>>>      }
>>>>  
>>>> +    /* Other deprecated options */
>>>> +    for (i = 0; i < ARRAY_SIZE(deprecated); i++) {
>>>> +        if (qemu_opt_get(legacy_opts, deprecated[i]) != NULL) {
>>>> +            error_report("'%s' is deprecated, please use the corresponding "
>>>> +                         "option of '-device' instead", deprecated[i]);
>>>> +        }
>>>> +    }
>>>> +
>>>>      /* Media type */
>>>>      value = qemu_opt_get(legacy_opts, "media");
>>>>      if (value) {
>>>
>>> This one should be deprecated too (separate patch if you prefer).
>>
>> I tried to add it, but then the deprecation message is always shown. I
>> think this happens because of the default CD-ROM drive is create in vl.c
>> with CDROM_OPTS (that is defined to "media=cdrom").
>> So no clue how to properly print a deprecation option for "media" here
>> right now (and it should likely be in a separate patch if there is a
>> solution).
> 
> The knee-jerk solution is to suppress the warning just for default
> drives, like we do in drive_check_orphaned().
> 
> The proper solution is to desugar default drives into non-legacy form.

"grep -r media= *" also shows that this option is heavily used in the
qemu-iotests ... so there is even more to be done here. Since I'm really
no block layer expert at all, I'd prefer if someone who is more
experienced in this area could clean up this legacy mess...

 Thomas