* [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
@ 2017-05-10 15:50 Thomas Huth
2017-05-10 15:55 ` Paolo Bonzini
2017-05-11 7:09 ` Thomas Huth
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Huth @ 2017-05-10 15:50 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf, Max Reitz; +Cc: qemu-block, Markus Armbruster
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) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
2017-05-10 15:50 [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device Thomas Huth
@ 2017-05-10 15:55 ` Paolo Bonzini
2017-05-10 16:12 ` Thomas Huth
2017-05-11 7:09 ` Thomas Huth
1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2017-05-10 15:55 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Kevin Wolf, Max Reitz
Cc: Markus Armbruster, qemu-block
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).
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
2017-05-10 15:55 ` Paolo Bonzini
@ 2017-05-10 16:12 ` Thomas Huth
2017-05-11 6:45 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2017-05-10 16:12 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel, Kevin Wolf, Max Reitz
Cc: Markus Armbruster, qemu-block
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).
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
2017-05-10 16:12 ` Thomas Huth
@ 2017-05-11 6:45 ` Markus Armbruster
2017-05-11 6:59 ` Thomas Huth
0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2017-05-11 6:45 UTC (permalink / raw)
To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Kevin Wolf, Max Reitz, qemu-block
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.
Feel free to pick the knee-jerk solution for now.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
2017-05-11 6:45 ` Markus Armbruster
@ 2017-05-11 6:59 ` Thomas Huth
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2017-05-11 6:59 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, qemu-devel, Kevin Wolf, Max Reitz, qemu-block
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device
2017-05-10 15:50 [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device Thomas Huth
2017-05-10 15:55 ` Paolo Bonzini
@ 2017-05-11 7:09 ` Thomas Huth
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2017-05-11 7:09 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf, Max Reitz; +Cc: Markus Armbruster, qemu-block
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) {
>
Self-NAK to this version of the patch ... looks like tests/hd-geo-test
is still using these parameters and now spills out these warning
messages. Need to clean up the test first...
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-11 7:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-10 15:50 [Qemu-devel] [PATCH] blockdev: Print a warning for legacy drive options that belong to -device Thomas Huth
2017-05-10 15:55 ` Paolo Bonzini
2017-05-10 16:12 ` Thomas Huth
2017-05-11 6:45 ` Markus Armbruster
2017-05-11 6:59 ` Thomas Huth
2017-05-11 7:09 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).