| Message ID | 1494431442-25163-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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.
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
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
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) {
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(+)