Skip to contentSkip to conversation

block: Remove deprecated -drive options

PATCH Superseded Kevin Wolf 4 participants 11 messages Jun 13, 2018 Block layer

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Kevin Wolf

author
#0 lore raw

cover block: Remove deprecated -drive options

We deprecated a bunch of -drive options in 2.10, so let's remove them for 3.0.

Kevin Wolf (3):
block: Remove deprecated -drive geometry options
block: Remove deprecated -drive option addr
block: Remove deprecated -drive option serial

 include/hw/block/block.h  |   1 -
 include/sysemu/blockdev.h |   3 --
 block/block-backend.c     |   1 -
 blockdev.c                | 110 ----------------------------------------------
 device-hotplug.c          |   4 --
 hw/block/block.c          |  27 ------------
 hw/block/nvme.c           |   1 -
 hw/block/virtio-blk.c     |   1 -
 hw/ide/qdev.c             |   1 -
 hw/scsi/scsi-disk.c       |   1 -
 hw/usb/dev-storage.c      |   1 -
 tests/ahci-test.c         |   6 +--
 tests/hd-geo-test.c       |  18 --------
 tests/ide-test.c          |   8 ++--
 hmp-commands.hx           |   1 -
 qemu-doc.texi             |  15 -------
 qemu-options.hx           |  14 +-----
 17 files changed, 8 insertions(+), 205 deletions(-)
Signature · 1 line
2.13.6
author
#1 lore raw

PATCH 1/3 block: Remove deprecated -drive geometry options

The -drive options cyls, heads, secs and trans were deprecated in QEMU 2.10. It's time to remove them.

hd-geo-test tested both the old version with geometry options in -drive and the new one with -device. Therefore the code using -drive doesn't have to be replaced there, we just need to remove the -drive test cases.

---
 include/sysemu/blockdev.h |  1 -
 blockdev.c                | 75 +----------------------------------------------
 hw/block/block.c          | 14 ---------
 tests/hd-geo-test.c       | 18 ------------
 hmp-commands.hx           |  1 -
 qemu-doc.texi             |  5 ----
 qemu-options.hx           |  7 +----
 7 files changed, 2 insertions(+), 119 deletions(-)
include/sysemu/blockdev.h +0 -1
index ac22f2ae1f..37ea39719e 100644
@@ -35,7 +35,6 @@ struct DriveInfo {
     int auto_del;               /* see blockdev_mark_auto_del() */
     bool is_default;            /* Added by default_drive() ?  */
     int media_cd;
-    int cyls, heads, secs, trans;
     QemuOpts *opts;
     char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
blockdev.c +1 -74

105 lines not on this page · raw message

hw/block/block.c +0 -14
index b91e2b6d7e..b6c80ab0b7 100644
@@ -108,20 +108,6 @@ bool blkconf_geometry(BlockConf *conf, int *ptrans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp)
 {
-    DriveInfo *dinfo;
-
-    if (!conf->cyls && !conf->heads && !conf->secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = blk_legacy_dinfo(conf->blk);
-        if (dinfo) {
-            conf->cyls  = dinfo->cyls;
-            conf->heads = dinfo->heads;
-            conf->secs  = dinfo->secs;
-            if (ptrans) {
-                *ptrans = dinfo->trans;
-            }
-        }
-    }
     if (!conf->cyls && !conf->heads && !conf->secs) {
         hd_geometry_guess(conf->blk,
                           &conf->cyls, &conf->heads, &conf->secs,
tests/hd-geo-test.c +0 -18
index 24870b38f4..d263a74ec0 100644
@@ -347,22 +347,6 @@ static void test_ide_drive_user(const char *dev, bool trans)
 }
 
 /*
- * Test case: IDE device (if=ide) with explicit CHS
- */
-static void test_ide_drive_user_chs(void)
-{
-    test_ide_drive_user(NULL, false);
-}
-
-/*
- * Test case: IDE device (if=ide) with explicit CHS and translation
- */
-static void test_ide_drive_user_chst(void)
-{
-    test_ide_drive_user(NULL, true);
-}
-
-/*
  * Test case: IDE device (if=none) with explicit CHS
  */
 static void test_ide_device_user_chs(void)
@@ -422,8 +406,6 @@ int main(int argc, char **argv)
     qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
     qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
     qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
     qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
     qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
     qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
hmp-commands.hx +0 -1
index 0734fea931..0de7c4c29e 100644
@@ -1283,7 +1283,6 @@ ETEXI
         .params     = "[-n] [[<domain>:]<bus>:]<slot>\n"
                       "[file=file][,if=type][,bus=n]\n"
                       "[,unit=m][,media=d][,index=i]\n"
-                      "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
                       "[,snapshot=on|off][,cache=on|off]\n"
                       "[,readonly=on|off][,copy-on-read=on|off]",
         .help       = "add drive to PCI storage controller",
qemu-doc.texi +0 -5
index cd05760cac..ab95bffc74 100644
@@ -2850,11 +2850,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
 (for embedded NICs). The new syntax allows different settings to be
 provided per NIC.
 
-@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0)
-
-The drive geometry arguments are replaced by the the geometry arguments
-that can be specified with the ``-device'' parameter.
-
 @subsection -drive serial=... (since 2.10.0)
 
 The drive serial argument is replaced by the the serial argument
qemu-options.hx +1 -6
index c0d3951e9f..a14b9655c5 100644
@@ -804,9 +804,8 @@ ETEXI
 
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
-    "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
@@ -847,10 +846,6 @@ This option defines where is connected the drive by using an index in the list
 of available connectors of a given interface type.
 @item media=@var{media}
 This option defines the type of the media: disk or cdrom.
-@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
-Force disk physical geometry and the optional BIOS translation (trans=none or
-lba). 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}).
Signature · 1 line
2.13.6
Reviewed-by on v1 1/3
#2 lore raw

Re: [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options

Quoted 9 lines · Kevin Wolf

The -drive options cyls, heads, secs and trans were deprecated in QEMU 2.10. It's time to remove them.

hd-geo-test tested both the old version with geometry options in -drive and the new one with -device. Therefore the code using -drive doesn't have to be replaced there, we just need to remove the -drive test cases.

---

[...]

tests/hd-geo-test.c static void test_ide_drive_user(const char *dev, bool trans)

Quoted 5 lines
tests/hd-geo-test.c +0 -0
index 24870b38f4..d263a74ec0 100644
@@ -347,22 +347,6 @@ static void test_ide_drive_user(const char *dev, bool trans)

static void test_ide_drive_user(const char *dev, bool trans)

  {
      char **argv = g_new0(char *, ARGV_SIZE);
      char *args, *opts;
      int argc;
      int secs = img_secs[backend_small];
      const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans };

      argc = setup_common(argv, ARGV_SIZE);
      opts = g_strdup_printf("%s,%s%scyls=%d,heads=%d,secs=%d",
                             dev ?: "",
                             trans && dev ? "bios-chs-" : "",
                             trans ? "trans=lba," : "",
                             expected_chst.cyls, expected_chst.heads,
                             expected_chst.secs);
      cur_ide[0] = &expected_chst;
      argc = setup_ide(argc, argv, ARGV_SIZE,
                       0, dev ? opts : NULL, backend_small, mbr_chs,
                       dev ? "" : opts);
      g_free(opts);
      args = g_strjoinv(" ", argv);
      qtest_start(args);
      g_strfreev(argv);
      g_free(args);
      test_cmos();
      qtest_end();
Quoted 7 lines
tests/hd-geo-test.c +0 -4
 }
 
 /*
- * Test case: IDE device (if=ide) with explicit CHS
- */
-static void test_ide_drive_user_chs(void)
-{

tests/hd-geo-test.c static void test_ide_drive_user(const char *dev, bool trans) · line 363

Quoted 10 lines
tests/hd-geo-test.c +0 -10
-    test_ide_drive_user(NULL, false);
-}
-
-/*
- * Test case: IDE device (if=ide) with explicit CHS and translation
- */
-static void test_ide_drive_user_chst(void)
-{
-    test_ide_drive_user(NULL, true);
-}

All remaining calls of test_ide_drive_user() pass non-null @dev. Please simplify test_ide_drive_user() accordingly.

Quoted 6 lines
tests/hd-geo-test.c +0 -2
-
-/*
  * Test case: IDE device (if=none) with explicit CHS
  */
 static void test_ide_device_user_chs(void)

tests/hd-geo-test.c int main(int argc, char **argv) · line 411

Quoted 8 lines
tests/hd-geo-test.c +0 -2
@@ -422,8 +406,6 @@ int main(int argc, char **argv)
     qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
     qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
     qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
     qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
     qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
     qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);

[...]

With that done:

Reviewed-by on v1 1/3
#3 lore raw

Re: [Qemu-devel] [Qemu-block] [PATCH 1/3] block: Remove deprecated -drive geometry options

In reply to Kevin Wolf
Quoted 8 lines · Kevin Wolf

The -drive options cyls, heads, secs and trans were deprecated in QEMU 2.10. It's time to remove them.

hd-geo-test tested both the old version with geometry options in -drive and the new one with -device. Therefore the code using -drive doesn't have to be replaced there, we just need to remove the -drive test cases.

Quoted 255 lines
---
 include/sysemu/blockdev.h |  1 -
 blockdev.c                | 75 +----------------------------------------------
 hw/block/block.c          | 14 ---------
 tests/hd-geo-test.c       | 18 ------------
 hmp-commands.hx           |  1 -
 qemu-doc.texi             |  5 ----
 qemu-options.hx           |  7 +----
 7 files changed, 2 insertions(+), 119 deletions(-)
include/sysemu/blockdev.h +0 -1
index ac22f2ae1f..37ea39719e 100644
@@ -35,7 +35,6 @@ struct DriveInfo {
     int auto_del;               /* see blockdev_mark_auto_del() */
     bool is_default;            /* Added by default_drive() ?  */
     int media_cd;
-    int cyls, heads, secs, trans;
     QemuOpts *opts;
     char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
blockdev.c +1 -74
index 4862323012..9c891706ef 100644
@@ -730,22 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "cyls",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of cylinders (ide disk geometry)",
-        },{
-            .name = "heads",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of heads (ide disk geometry)",
-        },{
-            .name = "secs",
-            .type = QEMU_OPT_NUMBER,
-            .help = "number of sectors (ide disk geometry)",
-        },{
-            .name = "trans",
-            .type = QEMU_OPT_STRING,
-            .help = "chs translation (auto, lba, none)",
-        },{
             .name = "addr",
             .type = QEMU_OPT_STRING,
             .help = "pci address (virtio only)",
@@ -791,7 +775,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     QemuOpts *legacy_opts;
     DriveMediaType media = MEDIA_DISK;
     BlockInterfaceType type;
-    int cyls, heads, secs, translation;
     int max_devs, bus_id, unit_id, index;
     const char *devaddr;
     const char *werror, *rerror;
@@ -802,7 +785,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     Error *local_err = NULL;
     int i;
     const char *deprecated[] = {
-        "serial", "trans", "secs", "heads", "cyls", "addr"
+        "serial", "addr"
     };
 
     /* Change legacy command line options into QMP ones */
@@ -931,57 +914,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         type = block_default_type;
     }
 
-    /* Geometry */
-    cyls  = qemu_opt_get_number(legacy_opts, "cyls", 0);
-    heads = qemu_opt_get_number(legacy_opts, "heads", 0);
-    secs  = qemu_opt_get_number(legacy_opts, "secs", 0);
-
-    if (cyls || heads || secs) {
-        if (cyls < 1) {
-            error_report("invalid physical cyls number");
-            goto fail;
-        }
-        if (heads < 1) {
-            error_report("invalid physical heads number");
-            goto fail;
-        }
-        if (secs < 1) {
-            error_report("invalid physical secs number");
-            goto fail;
-        }
-    }
-
-    translation = BIOS_ATA_TRANSLATION_AUTO;
-    value = qemu_opt_get(legacy_opts, "trans");
-    if (value != NULL) {
-        if (!cyls) {
-            error_report("'%s' trans must be used with cyls, heads and secs",
-                         value);
-            goto fail;
-        }
-        if (!strcmp(value, "none")) {
-            translation = BIOS_ATA_TRANSLATION_NONE;
-        } else if (!strcmp(value, "lba")) {
-            translation = BIOS_ATA_TRANSLATION_LBA;
-        } else if (!strcmp(value, "large")) {
-            translation = BIOS_ATA_TRANSLATION_LARGE;
-        } else if (!strcmp(value, "rechs")) {
-            translation = BIOS_ATA_TRANSLATION_RECHS;
-        } else if (!strcmp(value, "auto")) {
-            translation = BIOS_ATA_TRANSLATION_AUTO;
-        } else {
-            error_report("'%s' invalid translation type", value);
-            goto fail;
-        }
-    }
-
-    if (media == MEDIA_CDROM) {
-        if (cyls || secs || heads) {
-            error_report("CHS can't be set with media=cdrom");
-            goto fail;
-        }
-    }
-
     /* Device address specified by bus/unit or index.
      * If none was specified, try to find the first free one. */
     bus_id  = qemu_opt_get_number(legacy_opts, "bus", 0);
@@ -1104,11 +1036,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo = g_malloc0(sizeof(*dinfo));
     dinfo->opts = all_opts;
 
-    dinfo->cyls = cyls;
-    dinfo->heads = heads;
-    dinfo->secs = secs;
-    dinfo->trans = translation;
-
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
hw/block/block.c +0 -14
index b91e2b6d7e..b6c80ab0b7 100644
@@ -108,20 +108,6 @@ bool blkconf_geometry(BlockConf *conf, int *ptrans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp)
 {
-    DriveInfo *dinfo;
-
-    if (!conf->cyls && !conf->heads && !conf->secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = blk_legacy_dinfo(conf->blk);
-        if (dinfo) {
-            conf->cyls  = dinfo->cyls;
-            conf->heads = dinfo->heads;
-            conf->secs  = dinfo->secs;
-            if (ptrans) {
-                *ptrans = dinfo->trans;
-            }
-        }
-    }
     if (!conf->cyls && !conf->heads && !conf->secs) {
         hd_geometry_guess(conf->blk,
                           &conf->cyls, &conf->heads, &conf->secs,
tests/hd-geo-test.c +0 -18
index 24870b38f4..d263a74ec0 100644
@@ -347,22 +347,6 @@ static void test_ide_drive_user(const char *dev, bool trans)
 }
 
 /*
- * Test case: IDE device (if=ide) with explicit CHS
- */
-static void test_ide_drive_user_chs(void)
-{
-    test_ide_drive_user(NULL, false);
-}
-
-/*
- * Test case: IDE device (if=ide) with explicit CHS and translation
- */
-static void test_ide_drive_user_chst(void)
-{
-    test_ide_drive_user(NULL, true);
-}
-
-/*
  * Test case: IDE device (if=none) with explicit CHS
  */
 static void test_ide_device_user_chs(void)
@@ -422,8 +406,6 @@ int main(int argc, char **argv)
     qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
     qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
     qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
-    qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
     qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
     qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
     qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
hmp-commands.hx +0 -1
index 0734fea931..0de7c4c29e 100644
@@ -1283,7 +1283,6 @@ ETEXI
         .params     = "[-n] [[<domain>:]<bus>:]<slot>\n"
                       "[file=file][,if=type][,bus=n]\n"
                       "[,unit=m][,media=d][,index=i]\n"
-                      "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
                       "[,snapshot=on|off][,cache=on|off]\n"
                       "[,readonly=on|off][,copy-on-read=on|off]",
         .help       = "add drive to PCI storage controller",
qemu-doc.texi +0 -5
index cd05760cac..ab95bffc74 100644
@@ -2850,11 +2850,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
 (for embedded NICs). The new syntax allows different settings to be
 provided per NIC.
 
-@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0)
-
-The drive geometry arguments are replaced by the the geometry arguments
-that can be specified with the ``-device'' parameter.
-
 @subsection -drive serial=... (since 2.10.0)
 
 The drive serial argument is replaced by the the serial argument
qemu-options.hx +1 -6
index c0d3951e9f..a14b9655c5 100644
@@ -804,9 +804,8 @@ ETEXI
 
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
-    "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
@@ -847,10 +846,6 @@ This option defines where is connected the drive by using an index in the list
 of available connectors of a given interface type.
 @item media=@var{media}
 This option defines the type of the media: disk or cdrom.
-@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
-Force disk physical geometry and the optional BIOS translation (trans=none or
-lba). 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}).
Signature · 1 line
2.13.6
author
#4 lore raw

PATCH 2/3 block: Remove deprecated -drive option addr

In reply to Kevin Wolf

The -drive option addr was deprecated in QEMU 2.10. It's time to remove it.

---
 include/sysemu/blockdev.h |  1 -
 blockdev.c                | 17 +----------------
 device-hotplug.c          |  4 ----
 qemu-doc.texi             |  5 -----
 qemu-options.hx           |  5 +----
 5 files changed, 2 insertions(+), 30 deletions(-)
include/sysemu/blockdev.h +0 -1
index 37ea39719e..c0ae3700ec 100644
@@ -28,7 +28,6 @@ typedef enum {
 } BlockInterfaceType;
 
 struct DriveInfo {
-    const char *devaddr;
     BlockInterfaceType type;
     int bus;
     int unit;
blockdev.c +1 -16
index 9c891706ef..83b3cc12e9 100644
@@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "addr",
-            .type = QEMU_OPT_STRING,
-            .help = "pci address (virtio only)",
-        },{
             .name = "serial",
             .type = QEMU_OPT_STRING,
             .help = "disk serial number",
@@ -776,7 +772,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     DriveMediaType media = MEDIA_DISK;
     BlockInterfaceType type;
     int max_devs, bus_id, unit_id, index;
-    const char *devaddr;
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
@@ -785,7 +780,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     Error *local_err = NULL;
     int i;
     const char *deprecated[] = {
-        "serial", "addr"
+        "serial"
     };
 
     /* Change legacy command line options into QMP ones */
@@ -975,12 +970,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     }
 
     /* Add virtio block device */
-    devaddr = qemu_opt_get(legacy_opts, "addr");
-    if (devaddr && type != IF_VIRTIO) {
-        error_report("addr is not supported by this bus type");
-        goto fail;
-    }
-
     if (type == IF_VIRTIO) {
         QemuOpts *devopts;
         devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
@@ -992,9 +981,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         }
         qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
                      &error_abort);
-        if (devaddr) {
-            qemu_opt_set(devopts, "addr", devaddr, &error_abort);
-        }
     }
 
     filename = qemu_opt_get(legacy_opts, "file");
@@ -1039,7 +1025,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->devaddr = devaddr;
     dinfo->serial = g_strdup(serial);
 
     blk_set_legacy_dinfo(blk, dinfo);
device-hotplug.c +0 -4
index 23fd6656f1..cd427e2c76 100644
@@ -69,10 +69,6 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
     if (!dinfo) {
         goto err;
     }
-    if (dinfo->devaddr) {
-        monitor_printf(mon, "Parameter addr not supported\n");
-        goto err;
-    }
 
     switch (dinfo->type) {
     case IF_NONE:
qemu-doc.texi +0 -5
index ab95bffc74..338477725f 100644
@@ -2855,11 +2855,6 @@ provided per NIC.
 The drive serial argument is replaced by the the serial argument
 that can be specified with the ``-device'' parameter.
 
-@subsection -drive addr=... (since 2.10.0)
-
-The drive addr argument is replaced by the the addr argument
-that can be specified with the ``-device'' parameter.
-
 @subsection -usbdevice (since 2.10.0)
 
 The ``-usbdevice DEV'' argument is now a synonym for setting
qemu-options.hx +1 -4
index a14b9655c5..c2531e2f3c 100644
@@ -805,7 +805,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
@@ -883,9 +883,6 @@ an untrusted format header.
 This option specifies the serial number to assign to the device. This
 parameter is deprecated, use the corresponding parameter of @code{-device}
 instead.
-@item addr=@var{addr}
-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),
Signature · 1 line
2.13.6
Reviewed-by on v1 2/3
#6 lore raw

Re: [Qemu-devel] [Qemu-block] [PATCH 2/3] block: Remove deprecated -drive option addr

In reply to Kevin Wolf
Quoted 4 lines · Kevin Wolf

The -drive option addr was deprecated in QEMU 2.10. It's time to remove it.

Quoted 141 lines
---
 include/sysemu/blockdev.h |  1 -
 blockdev.c                | 17 +----------------
 device-hotplug.c          |  4 ----
 qemu-doc.texi             |  5 -----
 qemu-options.hx           |  5 +----
 5 files changed, 2 insertions(+), 30 deletions(-)
include/sysemu/blockdev.h +0 -1
index 37ea39719e..c0ae3700ec 100644
@@ -28,7 +28,6 @@ typedef enum {
 } BlockInterfaceType;
 
 struct DriveInfo {
-    const char *devaddr;
     BlockInterfaceType type;
     int bus;
     int unit;
blockdev.c +1 -16
index 9c891706ef..83b3cc12e9 100644
@@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "addr",
-            .type = QEMU_OPT_STRING,
-            .help = "pci address (virtio only)",
-        },{
             .name = "serial",
             .type = QEMU_OPT_STRING,
             .help = "disk serial number",
@@ -776,7 +772,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     DriveMediaType media = MEDIA_DISK;
     BlockInterfaceType type;
     int max_devs, bus_id, unit_id, index;
-    const char *devaddr;
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
@@ -785,7 +780,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     Error *local_err = NULL;
     int i;
     const char *deprecated[] = {
-        "serial", "addr"
+        "serial"
     };
 
     /* Change legacy command line options into QMP ones */
@@ -975,12 +970,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     }
 
     /* Add virtio block device */
-    devaddr = qemu_opt_get(legacy_opts, "addr");
-    if (devaddr && type != IF_VIRTIO) {
-        error_report("addr is not supported by this bus type");
-        goto fail;
-    }
-
     if (type == IF_VIRTIO) {
         QemuOpts *devopts;
         devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
@@ -992,9 +981,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         }
         qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
                      &error_abort);
-        if (devaddr) {
-            qemu_opt_set(devopts, "addr", devaddr, &error_abort);
-        }
     }
 
     filename = qemu_opt_get(legacy_opts, "file");
@@ -1039,7 +1025,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->devaddr = devaddr;
     dinfo->serial = g_strdup(serial);
 
     blk_set_legacy_dinfo(blk, dinfo);
device-hotplug.c +0 -4
index 23fd6656f1..cd427e2c76 100644
@@ -69,10 +69,6 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
     if (!dinfo) {
         goto err;
     }
-    if (dinfo->devaddr) {
-        monitor_printf(mon, "Parameter addr not supported\n");
-        goto err;
-    }
 
     switch (dinfo->type) {
     case IF_NONE:
qemu-doc.texi +0 -5
index ab95bffc74..338477725f 100644
@@ -2855,11 +2855,6 @@ provided per NIC.
 The drive serial argument is replaced by the the serial argument
 that can be specified with the ``-device'' parameter.
 
-@subsection -drive addr=... (since 2.10.0)
-
-The drive addr argument is replaced by the the addr argument
-that can be specified with the ``-device'' parameter.
-
 @subsection -usbdevice (since 2.10.0)
 
 The ``-usbdevice DEV'' argument is now a synonym for setting
qemu-options.hx +1 -4
index a14b9655c5..c2531e2f3c 100644
@@ -805,7 +805,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
@@ -883,9 +883,6 @@ an untrusted format header.
 This option specifies the serial number to assign to the device. This
 parameter is deprecated, use the corresponding parameter of @code{-device}
 instead.
-@item addr=@var{addr}
-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),
Signature · 1 line
2.13.6
author
#7 lore raw

PATCH 3/3 block: Remove deprecated -drive option serial

In reply to Kevin Wolf

The -drive option serial was deprecated in QEMU 2.10. It's time to remove it.

Tests need to be updated to set the serial number with -global instead of using the -drive option.

---
 include/hw/block/block.h  |  1 -
 include/sysemu/blockdev.h |  1 -
 block/block-backend.c     |  1 -
 blockdev.c                | 22 ----------------------
 hw/block/block.c          | 13 -------------
 hw/block/nvme.c           |  1 -
 hw/block/virtio-blk.c     |  1 -
 hw/ide/qdev.c             |  1 -
 hw/scsi/scsi-disk.c       |  1 -
 hw/usb/dev-storage.c      |  1 -
 tests/ahci-test.c         |  6 +++---
 tests/ide-test.c          |  8 ++++----
 qemu-doc.texi             |  5 -----
 qemu-options.hx           |  6 +-----
 14 files changed, 8 insertions(+), 60 deletions(-)
include/hw/block/block.h +0 -1
index d4f4dfffab..e9f9e2223f 100644
@@ -72,7 +72,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 
 /* Configuration helpers */
 
-void blkconf_serial(BlockConf *conf, char **serial);
 bool blkconf_geometry(BlockConf *conf, int *trans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp);
include/sysemu/blockdev.h +0 -1
index c0ae3700ec..24954b94e0 100644
@@ -35,7 +35,6 @@ struct DriveInfo {
     bool is_default;            /* Added by default_drive() ?  */
     int media_cd;
     QemuOpts *opts;
-    char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
 };
 
block/block-backend.c +0 -1
index d55c328736..2d1a3463e8 100644
@@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
         return;
     }
     qemu_opts_del(dinfo->opts);
-    g_free(dinfo->serial);
     g_free(dinfo);
 }
 
blockdev.c +0 -22
index 83b3cc12e9..4ab3d6ec0b 100644
@@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "serial",
-            .type = QEMU_OPT_STRING,
-            .help = "disk serial number",
-        },{
             .name = "file",
             .type = QEMU_OPT_STRING,
             .help = "file name",
@@ -775,13 +771,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
-    const char *serial;
     const char *filename;
     Error *local_err = NULL;
     int i;
-    const char *deprecated[] = {
-        "serial"
-    };
 
     /* Change legacy command line options into QMP ones */
     static const struct {
@@ -858,16 +850,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* 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) {
@@ -948,9 +930,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* Serial number */
-    serial = qemu_opt_get(legacy_opts, "serial");
-
     /* no id supplied -> create one */
     if (qemu_opts_id(all_opts) == NULL) {
         char *new_id;
@@ -1025,7 +1004,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->serial = g_strdup(serial);
 
     blk_set_legacy_dinfo(blk, dinfo);
 
hw/block/block.c +0 -13
index b6c80ab0b7..cf0eb826f1 100644
@@ -15,19 +15,6 @@
 #include "qapi/qapi-types-block.h"
 #include "qemu/error-report.h"
 
-void blkconf_serial(BlockConf *conf, char **serial)
-{
-    DriveInfo *dinfo;
-
-    if (!*serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = blk_legacy_dinfo(conf->blk);
-        if (dinfo) {
-            *serial = g_strdup(dinfo->serial);
-        }
-    }
-}
-
 void blkconf_blocksizes(BlockConf *conf)
 {
     BlockBackend *blk = conf->blk;
hw/block/nvme.c +0 -1
index 811084b6a7..d5bf95b79b 100644
@@ -1215,7 +1215,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&n->conf, &n->serial);
     if (!n->serial) {
         error_setg(errp, "serial property not set");
         return;
hw/block/virtio-blk.c +0 -1
index 50b5c869e3..225fe44b7a 100644
@@ -935,7 +935,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&conf->conf, &conf->serial);
     if (!blkconf_apply_backend_options(&conf->conf,
                                        blk_is_read_only(conf->conf.blk), true,
                                        errp)) {
hw/ide/qdev.c +0 -1
index f395d24592..573b022e1e 100644
@@ -188,7 +188,6 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         return;
     }
 
-    blkconf_serial(&dev->conf, &dev->serial);
     if (kind != IDE_CD) {
         if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
                               errp)) {
hw/scsi/scsi-disk.c +0 -1
index 9f43583ea0..920850a1c1 100644
@@ -2372,7 +2372,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->qdev.conf, &s->serial);
     blkconf_blocksizes(&s->qdev.conf);
 
     if (s->qdev.conf.logical_block_size >
hw/usb/dev-storage.c +0 -1
index 481694a473..47b992f403 100644
@@ -606,7 +606,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->conf, &dev->serial);
     blkconf_blocksizes(&s->conf);
     if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
                                        errp)) {
tests/ahci-test.c +3 -3
index 1a7b761304..937ed2f910 100644
@@ -180,12 +180,12 @@ static AHCIQState *ahci_boot(const char *cli, ...)
         s = ahci_vboot(cli, ap);
         va_end(ap);
     } else {
-        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-            ",format=%s"
+        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,format=%s"
             " -M q35 "
             "-device ide-hd,drive=drive0 "
+            "-global ide-hd.serial=%s "
             "-global ide-hd.ver=%s";
-        s = ahci_boot(cli, tmp_path, "testdisk", imgfmt, "version");
+        s = ahci_boot(cli, tmp_path, imgfmt, "testdisk", "version");
     }
 
     return s;
tests/ide-test.c +4 -4
index 2384c2c3e2..f39431b1a9 100644
@@ -529,8 +529,8 @@ static void test_bmdma_no_busmaster(void)
 static void test_bmdma_setup(void)
 {
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
     qtest_irq_intercept_in(global_qtest, "ioapic");
 }
@@ -561,8 +561,8 @@ static void test_identify(void)
     int ret;
 
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
 
     dev = get_pci_device(&bmdma_bar, &ide_bar);
qemu-doc.texi +0 -5
index 338477725f..282bc3dc35 100644
@@ -2850,11 +2850,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
 (for embedded NICs). The new syntax allows different settings to be
 provided per NIC.
 
-@subsection -drive serial=... (since 2.10.0)
-
-The drive serial argument is replaced by the the serial argument
-that can be specified with the ``-device'' parameter.
-
 @subsection -usbdevice (since 2.10.0)
 
 The ``-usbdevice DEV'' argument is now a synonym for setting
qemu-options.hx +1 -5
index c2531e2f3c..d5b0c26e8e 100644
@@ -805,7 +805,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
@@ -879,10 +879,6 @@ The default mode is @option{cache=writeback}.
 Specify which disk @var{format} will be used rather than detecting
 the format.  Can be used to specify format=raw to avoid interpreting
 an untrusted format header.
-@item serial=@var{serial}
-This option specifies the serial number to assign to the device. 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),
Signature · 1 line
2.13.6
Reviewed-by on v1 3/3
#8 lore raw

Re: [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial

Quoted 96 lines · Kevin Wolf

The -drive option serial was deprecated in QEMU 2.10. It's time to remove it.

Tests need to be updated to set the serial number with -global instead of using the -drive option.

---
 include/hw/block/block.h  |  1 -
 include/sysemu/blockdev.h |  1 -
 block/block-backend.c     |  1 -
 blockdev.c                | 22 ----------------------
 hw/block/block.c          | 13 -------------
 hw/block/nvme.c           |  1 -
 hw/block/virtio-blk.c     |  1 -
 hw/ide/qdev.c             |  1 -
 hw/scsi/scsi-disk.c       |  1 -
 hw/usb/dev-storage.c      |  1 -
 tests/ahci-test.c         |  6 +++---
 tests/ide-test.c          |  8 ++++----
 qemu-doc.texi             |  5 -----
 qemu-options.hx           |  6 +-----
 14 files changed, 8 insertions(+), 60 deletions(-)
include/hw/block/block.h +0 -1
index d4f4dfffab..e9f9e2223f 100644
@@ -72,7 +72,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 
 /* Configuration helpers */
 
-void blkconf_serial(BlockConf *conf, char **serial);
 bool blkconf_geometry(BlockConf *conf, int *trans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp);
include/sysemu/blockdev.h +0 -1
index c0ae3700ec..24954b94e0 100644
@@ -35,7 +35,6 @@ struct DriveInfo {
     bool is_default;            /* Added by default_drive() ?  */
     int media_cd;
     QemuOpts *opts;
-    char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
 };
 
block/block-backend.c +0 -1
index d55c328736..2d1a3463e8 100644
@@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
         return;
     }
     qemu_opts_del(dinfo->opts);
-    g_free(dinfo->serial);
     g_free(dinfo);
 }
 
blockdev.c +0 -11
index 83b3cc12e9..4ab3d6ec0b 100644
@@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "serial",
-            .type = QEMU_OPT_STRING,
-            .help = "disk serial number",
-        },{
             .name = "file",
             .type = QEMU_OPT_STRING,
             .help = "file name",
@@ -775,13 +771,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
-    const char *serial;
     const char *filename;
     Error *local_err = NULL;
     int i;
-    const char *deprecated[] = {
-        "serial"
-    };
 
     /* Change legacy command line options into QMP ones */
     static const struct {
@@ -858,16 +850,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* Other deprecated options */
-    if (!qtest_enabled()) {
-        for (i = 0; i < ARRAY_SIZE(deprecated); i++) {

blockdev.c DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) · line 855

Quoted 10 lines
blockdev.c +0 -7
-            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) {

I guess I'd remove this in a separate patch, to make bringing it back easier in case we need it again.

Quoted 109 lines
blockdev.c +0 -4
@@ -948,9 +930,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* Serial number */
-    serial = qemu_opt_get(legacy_opts, "serial");
-
     /* no id supplied -> create one */
     if (qemu_opts_id(all_opts) == NULL) {
         char *new_id;
@@ -1025,7 +1004,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->serial = g_strdup(serial);
 
     blk_set_legacy_dinfo(blk, dinfo);
 
hw/block/block.c +0 -13
index b6c80ab0b7..cf0eb826f1 100644
@@ -15,19 +15,6 @@
 #include "qapi/qapi-types-block.h"
 #include "qemu/error-report.h"
 
-void blkconf_serial(BlockConf *conf, char **serial)
-{
-    DriveInfo *dinfo;
-
-    if (!*serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = blk_legacy_dinfo(conf->blk);
-        if (dinfo) {
-            *serial = g_strdup(dinfo->serial);
-        }
-    }
-}
-
 void blkconf_blocksizes(BlockConf *conf)
 {
     BlockBackend *blk = conf->blk;
hw/block/nvme.c +0 -1
index 811084b6a7..d5bf95b79b 100644
@@ -1215,7 +1215,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&n->conf, &n->serial);
     if (!n->serial) {
         error_setg(errp, "serial property not set");
         return;
hw/block/virtio-blk.c +0 -1
index 50b5c869e3..225fe44b7a 100644
@@ -935,7 +935,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&conf->conf, &conf->serial);
     if (!blkconf_apply_backend_options(&conf->conf,
                                        blk_is_read_only(conf->conf.blk), true,
                                        errp)) {
hw/ide/qdev.c +0 -1
index f395d24592..573b022e1e 100644
@@ -188,7 +188,6 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         return;
     }
 
-    blkconf_serial(&dev->conf, &dev->serial);
     if (kind != IDE_CD) {
         if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
                               errp)) {
hw/scsi/scsi-disk.c +0 -1
index 9f43583ea0..920850a1c1 100644
@@ -2372,7 +2372,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->qdev.conf, &s->serial);
     blkconf_blocksizes(&s->qdev.conf);
 
     if (s->qdev.conf.logical_block_size >
hw/usb/dev-storage.c +0 -1
index 481694a473..47b992f403 100644
@@ -606,7 +606,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->conf, &dev->serial);
     blkconf_blocksizes(&s->conf);
     if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
                                        errp)) {
tests/ahci-test.c +0 -0
index 1a7b761304..937ed2f910 100644
@@ -180,12 +180,12 @@ static AHCIQState *ahci_boot(const char *cli, ...)
         s = ahci_vboot(cli, ap);
         va_end(ap);

tests/ahci-test.c static AHCIQState *ahci_boot(const char *cli, ...) · line 188

Quoted 10 lines
tests/ahci-test.c +3 -3
     } else {
-        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-            ",format=%s"
+        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,format=%s"
             " -M q35 "
             "-device ide-hd,drive=drive0 "
+            "-global ide-hd.serial=%s "
             "-global ide-hd.ver=%s";
-        s = ahci_boot(cli, tmp_path, "testdisk", imgfmt, "version");
+        s = ahci_boot(cli, tmp_path, imgfmt, "testdisk", "version");

Not this patch's problem: ahci_boot() and ahci_boot_and_enable() lack GCC_FMT_ATTR().

This patch's problem, sort of: once they have it, we'll have to eliminate @cli here.

I figure adding GCC_FMT_ATTR() will require us to clean up the ahci_boot(NULL) silliness:

   static AHCIQState *ahci_boot(const char *cli, ...)
   {
       AHCIQState *s;
       va_list ap;
       va_start(ap, cli);
       s = ahci_vboot(cli, ap);
       va_end(ap);

       return s;

}

   static AHCIQState *ahci_boot_defaults(void)
   {
       return ahci_boot("-drive if=none,id=drive0,file=%s,"
                        "cache=writeback,serial=%s,format=%s"
                        " -M q35 "
                        "-device ide-hd,drive=drive0 "
                        "-global ide-hd.ver=%s",
                        tmp_path, "testdisk", imgfmt, "version");
   }

Anyway, you decide whether you want to do anything here now.

Quoted 59 lines
tests/ahci-test.c +0 -0
     }
 
     return s;
tests/ide-test.c +4 -4
index 2384c2c3e2..f39431b1a9 100644
@@ -529,8 +529,8 @@ static void test_bmdma_no_busmaster(void)
 static void test_bmdma_setup(void)
 {
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
     qtest_irq_intercept_in(global_qtest, "ioapic");
 }
@@ -561,8 +561,8 @@ static void test_identify(void)
     int ret;
 
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
 
     dev = get_pci_device(&bmdma_bar, &ide_bar);
qemu-doc.texi +0 -5
index 338477725f..282bc3dc35 100644
@@ -2850,11 +2850,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
 (for embedded NICs). The new syntax allows different settings to be
 provided per NIC.
 
-@subsection -drive serial=... (since 2.10.0)
-
-The drive serial argument is replaced by the the serial argument
-that can be specified with the ``-device'' parameter.
-
 @subsection -usbdevice (since 2.10.0)
 
 The ``-usbdevice DEV'' argument is now a synonym for setting
qemu-options.hx +1 -1
index c2531e2f3c..d5b0c26e8e 100644
@@ -805,7 +805,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"

qemu-options.hx The default mode is @option{cache=writeback}. · line 884

Quoted 10 lines
qemu-options.hx +0 -4
@@ -879,10 +879,6 @@ The default mode is @option{cache=writeback}.
 Specify which disk @var{format} will be used rather than detecting
 the format.  Can be used to specify format=raw to avoid interpreting
 an untrusted format header.
-@item serial=@var{serial}
-This option specifies the serial number to assign to the device. 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),
Reviewed-by
#9 lore raw

Re: [Qemu-devel] [Qemu-block] [PATCH 3/3] block: Remove deprecated -drive option serial

Quoted 9 lines · Markus Armbruster
Quoted 7 lines · Kevin Wolf

The -drive option serial was deprecated in QEMU 2.10. It's time to remove it.

Tests need to be updated to set the serial number with -global instead of using the -drive option.

Quoted 328 lines
Quoted 89 lines
---
 include/hw/block/block.h  |  1 -
 include/sysemu/blockdev.h |  1 -
 block/block-backend.c     |  1 -
 blockdev.c                | 22 ----------------------
 hw/block/block.c          | 13 -------------
 hw/block/nvme.c           |  1 -
 hw/block/virtio-blk.c     |  1 -
 hw/ide/qdev.c             |  1 -
 hw/scsi/scsi-disk.c       |  1 -
 hw/usb/dev-storage.c      |  1 -
 tests/ahci-test.c         |  6 +++---
 tests/ide-test.c          |  8 ++++----
 qemu-doc.texi             |  5 -----
 qemu-options.hx           |  6 +-----
 14 files changed, 8 insertions(+), 60 deletions(-)
include/hw/block/block.h +0 -1
index d4f4dfffab..e9f9e2223f 100644
@@ -72,7 +72,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 
 /* Configuration helpers */
 
-void blkconf_serial(BlockConf *conf, char **serial);
 bool blkconf_geometry(BlockConf *conf, int *trans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp);
include/sysemu/blockdev.h +0 -1
index c0ae3700ec..24954b94e0 100644
@@ -35,7 +35,6 @@ struct DriveInfo {
     bool is_default;            /* Added by default_drive() ?  */
     int media_cd;
     QemuOpts *opts;
-    char *serial;
     QTAILQ_ENTRY(DriveInfo) next;
 };
 
block/block-backend.c +0 -1
index d55c328736..2d1a3463e8 100644
@@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
         return;
     }
     qemu_opts_del(dinfo->opts);
-    g_free(dinfo->serial);
     g_free(dinfo);
 }
 
blockdev.c +0 -11
index 83b3cc12e9..4ab3d6ec0b 100644
@@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
         },{
-            .name = "serial",
-            .type = QEMU_OPT_STRING,
-            .help = "disk serial number",
-        },{
             .name = "file",
             .type = QEMU_OPT_STRING,
             .help = "file name",
@@ -775,13 +771,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
-    const char *serial;
     const char *filename;
     Error *local_err = NULL;
     int i;
-    const char *deprecated[] = {
-        "serial"
-    };
 
     /* Change legacy command line options into QMP ones */
     static const struct {
@@ -858,16 +850,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* Other deprecated options */
-    if (!qtest_enabled()) {
-        for (i = 0; i < ARRAY_SIZE(deprecated); i++) {

blockdev.c DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) · line 855

Quoted 10 lines
blockdev.c +0 -7
-            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) {

I guess I'd remove this in a separate patch, to make bringing it back easier in case we need it again.

Quoted 109 lines
blockdev.c +0 -4
@@ -948,9 +930,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         goto fail;
     }
 
-    /* Serial number */
-    serial = qemu_opt_get(legacy_opts, "serial");
-
     /* no id supplied -> create one */
     if (qemu_opts_id(all_opts) == NULL) {
         char *new_id;
@@ -1025,7 +1004,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->serial = g_strdup(serial);
 
     blk_set_legacy_dinfo(blk, dinfo);
 
hw/block/block.c +0 -13
index b6c80ab0b7..cf0eb826f1 100644
@@ -15,19 +15,6 @@
 #include "qapi/qapi-types-block.h"
 #include "qemu/error-report.h"
 
-void blkconf_serial(BlockConf *conf, char **serial)
-{
-    DriveInfo *dinfo;
-
-    if (!*serial) {
-        /* try to fall back to value set with legacy -drive serial=... */
-        dinfo = blk_legacy_dinfo(conf->blk);
-        if (dinfo) {
-            *serial = g_strdup(dinfo->serial);
-        }
-    }
-}
-
 void blkconf_blocksizes(BlockConf *conf)
 {
     BlockBackend *blk = conf->blk;
hw/block/nvme.c +0 -1
index 811084b6a7..d5bf95b79b 100644
@@ -1215,7 +1215,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&n->conf, &n->serial);
     if (!n->serial) {
         error_setg(errp, "serial property not set");
         return;
hw/block/virtio-blk.c +0 -1
index 50b5c869e3..225fe44b7a 100644
@@ -935,7 +935,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&conf->conf, &conf->serial);
     if (!blkconf_apply_backend_options(&conf->conf,
                                        blk_is_read_only(conf->conf.blk), true,
                                        errp)) {
hw/ide/qdev.c +0 -1
index f395d24592..573b022e1e 100644
@@ -188,7 +188,6 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         return;
     }
 
-    blkconf_serial(&dev->conf, &dev->serial);
     if (kind != IDE_CD) {
         if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
                               errp)) {
hw/scsi/scsi-disk.c +0 -1
index 9f43583ea0..920850a1c1 100644
@@ -2372,7 +2372,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->qdev.conf, &s->serial);
     blkconf_blocksizes(&s->qdev.conf);
 
     if (s->qdev.conf.logical_block_size >
hw/usb/dev-storage.c +0 -1
index 481694a473..47b992f403 100644
@@ -606,7 +606,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
         return;
     }
 
-    blkconf_serial(&s->conf, &dev->serial);
     blkconf_blocksizes(&s->conf);
     if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
                                        errp)) {
tests/ahci-test.c +0 -0
index 1a7b761304..937ed2f910 100644
@@ -180,12 +180,12 @@ static AHCIQState *ahci_boot(const char *cli, ...)
         s = ahci_vboot(cli, ap);
         va_end(ap);

tests/ahci-test.c static AHCIQState *ahci_boot(const char *cli, ...) · line 188

Quoted 10 lines
tests/ahci-test.c +3 -3
     } else {
-        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-            ",format=%s"
+        cli = "-drive if=none,id=drive0,file=%s,cache=writeback,format=%s"
             " -M q35 "
             "-device ide-hd,drive=drive0 "
+            "-global ide-hd.serial=%s "
             "-global ide-hd.ver=%s";
-        s = ahci_boot(cli, tmp_path, "testdisk", imgfmt, "version");
+        s = ahci_boot(cli, tmp_path, imgfmt, "testdisk", "version");

Not this patch's problem: ahci_boot() and ahci_boot_and_enable() lack GCC_FMT_ATTR().

This patch's problem, sort of: once they have it, we'll have to eliminate @cli here.

I figure adding GCC_FMT_ATTR() will require us to clean up the ahci_boot(NULL) silliness:

   static AHCIQState *ahci_boot(const char *cli, ...)
   {
       AHCIQState *s;
       va_list ap;
       va_start(ap, cli);
       s = ahci_vboot(cli, ap);
       va_end(ap);

       return s;

}

   static AHCIQState *ahci_boot_defaults(void)
   {
       return ahci_boot("-drive if=none,id=drive0,file=%s,"
                        "cache=writeback,serial=%s,format=%s"
                        " -M q35 "
                        "-device ide-hd,drive=drive0 "
                        "-global ide-hd.ver=%s",
                        tmp_path, "testdisk", imgfmt, "version");
   }

Anyway, you decide whether you want to do anything here now.

Quoted 59 lines
tests/ahci-test.c +0 -0
     }
 
     return s;
tests/ide-test.c +4 -4
index 2384c2c3e2..f39431b1a9 100644
@@ -529,8 +529,8 @@ static void test_bmdma_no_busmaster(void)
 static void test_bmdma_setup(void)
 {
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
     qtest_irq_intercept_in(global_qtest, "ioapic");
 }
@@ -561,8 +561,8 @@ static void test_identify(void)
     int ret;
 
     ide_test_start(
-        "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
-        "-global ide-hd.ver=%s",
+        "-drive file=%s,if=ide,cache=writeback,format=raw "
+        "-global ide-hd.serial=%s -global ide-hd.ver=%s",
         tmp_path, "testdisk", "version");
 
     dev = get_pci_device(&bmdma_bar, &ide_bar);
qemu-doc.texi +0 -5
index 338477725f..282bc3dc35 100644
@@ -2850,11 +2850,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
 (for embedded NICs). The new syntax allows different settings to be
 provided per NIC.
 
-@subsection -drive serial=... (since 2.10.0)
-
-The drive serial argument is replaced by the the serial argument
-that can be specified with the ``-device'' parameter.
-
 @subsection -usbdevice (since 2.10.0)
 
 The ``-usbdevice DEV'' argument is now a synonym for setting
qemu-options.hx +1 -1
index c2531e2f3c..d5b0c26e8e 100644
@@ -805,7 +805,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n"
+    "       [,snapshot=on|off][,rerror=ignore|stop|report]\n"
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"

qemu-options.hx The default mode is @option{cache=writeback}. · line 884

Quoted 10 lines
qemu-options.hx +0 -4
@@ -879,10 +879,6 @@ The default mode is @option{cache=writeback}.
 Specify which disk @var{format} will be used rather than detecting
 the format.  Can be used to specify format=raw to avoid interpreting
 an untrusted format header.
-@item serial=@var{serial}
-This option specifies the serial number to assign to the device. 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),
#10 lore raw
In reply to Kevin Wolf
Quoted 18 lines · Kevin Wolf

We deprecated a bunch of -drive options in 2.10, so let's remove them for 3.0.

Kevin Wolf (3):
block: Remove deprecated -drive geometry options
block: Remove deprecated -drive option addr
block: Remove deprecated -drive option serial

 include/hw/block/block.h  |   1 -
 include/sysemu/blockdev.h |   3 --
 block/block-backend.c     |   1 -
 blockdev.c                | 110 ----------------------------------------------
 device-hotplug.c          |   4 --
 hw/block/block.c          |  27 ------------
 hw/block/nvme.c           |   1 -
 hw/block/virtio-blk.c     |   1 -
 hw/ide/qdev.c             |   1 -
 hw/scsi/scsi-disk.c       |   1 -
Quoted 9 lines
 hw/usb/dev-storage.c      |   1 -
 tests/ahci-test.c         |   6 +--
 tests/hd-geo-test.c       |  18 --------
 tests/ide-test.c          |   8 ++--
 hmp-commands.hx           |   1 -
 qemu-doc.texi             |  15 -------
 qemu-options.hx           |  14 +-----
 17 files changed, 8 insertions(+), 205 deletions(-)

I think libvirt needs to be adjusted to handle new style options for patch #1 and patch #3, ccing

- Cole