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

[Markus Armbruster]

Thomas Huth <address@hidden> 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 <address@hidden>
>>> ---
>>>  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.

Feel free to pick the knee-jerk solution for now.