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 accordingly.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2:
 - Check for !qtest_enabled() since tests/hd-geo-test still uses these
 - Added "addr" to the list, too
 - Also mark the options as deprecated in the documentation

 blockdev.c      | 14 ++++++++++++++
 qemu-options.hx |  5 ++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 0b38c3d..aef38f0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -50,6 +50,7 @@
 #include "qmp-commands.h"
 #include "block/trace.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/qtest.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/throttle-options.h"
@@ -797,6 +798,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", "addr"
+    };
 
     /* Change legacy command line options into QMP ones */
     static const struct {
@@ -880,6 +884,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
                 "update your scripts.\n");
     }
 
+    /* Other deprecated options */
+    if (!qtest_enabled()) {
+        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) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 9d7964d..2f66f1a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -615,6 +615,8 @@ of available connectors of a given interface type.
 This option defines the type of the media: disk or cdrom.
 @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
 These options have the same definition as they have in @option{-hdachs}.
+These parameters are deprecated, use the corresponding parameters
+of @code{-device} instead.
 @item snapshot=@var{snapshot}
 @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive
 (see @option{-snapshot}).
@@ -631,7 +633,8 @@ an untrusted format header.
 @item serial=@var{serial}
 This option specifies the serial number to assign to the device.
 @item addr=@var{addr}
-Specify the controller's PCI address (if=virtio only).
+Specify the controller's PCI address (if=virtio only). This parameter is
+deprecated, use the corresponding parameter of @code{-device} instead.
 @item werror=@var{action},rerror=@var{action}
 Specify which @var{action} to take on write and read errors. Valid actions are:
 "ignore" (ignore the error and try to continue), "stop" (pause QEMU),
-- 
1.8.3.1

On 12.05.2017 12:33, 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 accordingly.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2:
>  - Check for !qtest_enabled() since tests/hd-geo-test still uses these
>  - Added "addr" to the list, too
>  - Also mark the options as deprecated in the documentation
> 
>  blockdev.c      | 14 ++++++++++++++
>  qemu-options.hx |  5 ++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0b38c3d..aef38f0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -50,6 +50,7 @@
>  #include "qmp-commands.h"
>  #include "block/trace.h"
>  #include "sysemu/arch_init.h"
> +#include "sysemu/qtest.h"
>  #include "qemu/cutils.h"
>  #include "qemu/help_option.h"
>  #include "qemu/throttle-options.h"
> @@ -797,6 +798,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", "addr"
> +    };
>  
>      /* Change legacy command line options into QMP ones */
>      static const struct {
> @@ -880,6 +884,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>                  "update your scripts.\n");
>      }
>  
> +    /* Other deprecated options */
> +    if (!qtest_enabled()) {
> +        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) {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9d7964d..2f66f1a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -615,6 +615,8 @@ of available connectors of a given interface type.
>  This option defines the type of the media: disk or cdrom.
>  @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
>  These options have the same definition as they have in @option{-hdachs}.
> +These parameters are deprecated, use the corresponding parameters
> +of @code{-device} instead.
>  @item snapshot=@var{snapshot}
>  @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive
>  (see @option{-snapshot}).
> @@ -631,7 +633,8 @@ an untrusted format header.
>  @item serial=@var{serial}
>  This option specifies the serial number to assign to the device.
>  @item addr=@var{addr}
> -Specify the controller's PCI address (if=virtio only).
> +Specify the controller's PCI address (if=virtio only). This parameter is
> +deprecated, use the corresponding parameter of @code{-device} instead.
>  @item werror=@var{action},rerror=@var{action}
>  Specify which @var{action} to take on write and read errors. Valid actions are:
>  "ignore" (ignore the error and try to continue), "stop" (pause QEMU),
> 

Ping ... any comments on this version of my patch?

 Thomas

On 12.05.2017 12:33, 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 accordingly.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2:
>  - Check for !qtest_enabled() since tests/hd-geo-test still uses these
>  - Added "addr" to the list, too
>  - Also mark the options as deprecated in the documentation
> 
>  blockdev.c      | 14 ++++++++++++++
>  qemu-options.hx |  5 ++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0b38c3d..aef38f0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -50,6 +50,7 @@
>  #include "qmp-commands.h"
>  #include "block/trace.h"
>  #include "sysemu/arch_init.h"
> +#include "sysemu/qtest.h"
>  #include "qemu/cutils.h"
>  #include "qemu/help_option.h"
>  #include "qemu/throttle-options.h"
> @@ -797,6 +798,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", "addr"
> +    };
>  
>      /* Change legacy command line options into QMP ones */
>      static const struct {
> @@ -880,6 +884,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>                  "update your scripts.\n");
>      }
>  
> +    /* Other deprecated options */
> +    if (!qtest_enabled()) {
> +        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) {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9d7964d..2f66f1a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -615,6 +615,8 @@ of available connectors of a given interface type.
>  This option defines the type of the media: disk or cdrom.
>  @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
>  These options have the same definition as they have in @option{-hdachs}.
> +These parameters are deprecated, use the corresponding parameters
> +of @code{-device} instead.
>  @item snapshot=@var{snapshot}
>  @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive
>  (see @option{-snapshot}).
> @@ -631,7 +633,8 @@ an untrusted format header.
>  @item serial=@var{serial}
>  This option specifies the serial number to assign to the device.
>  @item addr=@var{addr}
> -Specify the controller's PCI address (if=virtio only).
> +Specify the controller's PCI address (if=virtio only). This parameter is
> +deprecated, use the corresponding parameter of @code{-device} instead.
>  @item werror=@var{action},rerror=@var{action}
>  Specify which @var{action} to take on write and read errors. Valid actions are:
>  "ignore" (ignore the error and try to continue), "stop" (pause QEMU),
> 

ping²

Any takers?

 Thomas

Am 12.05.2017 um 12:33 hat Thomas Huth geschrieben:
> 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 accordingly.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2:
>  - Check for !qtest_enabled() since tests/hd-geo-test still uses these
>  - Added "addr" to the list, too
>  - Also mark the options as deprecated in the documentation

Thanks, added the missing deprecation note for 'serial' in the
documentation and applied to the block branch.

Kevin