* [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options
@ 2018-06-13 12:34 Kevin Wolf
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Kevin Wolf @ 2018-06-13 12:34 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, qemu-devel
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(-)
--
2.13.6
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options
2018-06-13 12:34 [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Kevin Wolf
@ 2018-06-13 12:34 ` Kevin Wolf
2018-06-13 14:00 ` Markus Armbruster
2018-06-13 15:36 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2018-06-13 12:34 ` [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr Kevin Wolf
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Kevin Wolf @ 2018-06-13 12:34 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, qemu-devel
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.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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(-)
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index ac22f2ae1f..37ea39719e 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -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;
diff --git a/blockdev.c b/blockdev.c
index 4862323012..9c891706ef 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hw/block/block.c b/hw/block/block.c
index b91e2b6d7e..b6c80ab0b7 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -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,
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 24870b38f4..d263a74ec0 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -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);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0734fea931..0de7c4c29e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -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",
diff --git a/qemu-doc.texi b/qemu-doc.texi
index cd05760cac..ab95bffc74 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -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
diff --git a/qemu-options.hx b/qemu-options.hx
index c0d3951e9f..a14b9655c5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -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}).
--
2.13.6
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr
2018-06-13 12:34 [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Kevin Wolf
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
@ 2018-06-13 12:34 ` Kevin Wolf
2018-06-13 14:04 ` Markus Armbruster
2018-06-13 15:38 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2018-06-13 12:34 ` [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial Kevin Wolf
2018-06-13 13:34 ` [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Cole Robinson
3 siblings, 2 replies; 11+ messages in thread
From: Kevin Wolf @ 2018-06-13 12:34 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, qemu-devel
The -drive option addr was deprecated in QEMU 2.10. It's time to remove
it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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(-)
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 37ea39719e..c0ae3700ec 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -28,7 +28,6 @@ typedef enum {
} BlockInterfaceType;
struct DriveInfo {
- const char *devaddr;
BlockInterfaceType type;
int bus;
int unit;
diff --git a/blockdev.c b/blockdev.c
index 9c891706ef..83b3cc12e9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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);
diff --git a/device-hotplug.c b/device-hotplug.c
index 23fd6656f1..cd427e2c76 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -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:
diff --git a/qemu-doc.texi b/qemu-doc.texi
index ab95bffc74..338477725f 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -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
diff --git a/qemu-options.hx b/qemu-options.hx
index a14b9655c5..c2531e2f3c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -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),
--
2.13.6
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial
2018-06-13 12:34 [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Kevin Wolf
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
2018-06-13 12:34 ` [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr Kevin Wolf
@ 2018-06-13 12:34 ` Kevin Wolf
2018-06-13 14:18 ` Markus Armbruster
2018-06-13 13:34 ` [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Cole Robinson
3 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2018-06-13 12:34 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, qemu-devel
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.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index d4f4dfffab..e9f9e2223f 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -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);
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index c0ae3700ec..24954b94e0 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,7 +35,6 @@ struct DriveInfo {
bool is_default; /* Added by default_drive() ? */
int media_cd;
QemuOpts *opts;
- char *serial;
QTAILQ_ENTRY(DriveInfo) next;
};
diff --git a/block/block-backend.c b/block/block-backend.c
index d55c328736..2d1a3463e8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
return;
}
qemu_opts_del(dinfo->opts);
- g_free(dinfo->serial);
g_free(dinfo);
}
diff --git a/blockdev.c b/blockdev.c
index 83b3cc12e9..4ab3d6ec0b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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);
diff --git a/hw/block/block.c b/hw/block/block.c
index b6c80ab0b7..cf0eb826f1 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -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;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 811084b6a7..d5bf95b79b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -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;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 50b5c869e3..225fe44b7a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -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)) {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index f395d24592..573b022e1e 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -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)) {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 9f43583ea0..920850a1c1 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -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 >
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 481694a473..47b992f403 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -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)) {
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 1a7b761304..937ed2f910 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -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;
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 2384c2c3e2..f39431b1a9 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -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);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 338477725f..282bc3dc35 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -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
diff --git a/qemu-options.hx b/qemu-options.hx
index c2531e2f3c..d5b0c26e8e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -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),
--
2.13.6
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options
2018-06-13 12:34 [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Kevin Wolf
` (2 preceding siblings ...)
2018-06-13 12:34 ` [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial Kevin Wolf
@ 2018-06-13 13:34 ` Cole Robinson
3 siblings, 0 replies; 11+ messages in thread
From: Cole Robinson @ 2018-06-13 13:34 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, mreitz, libvirt mailing list
On 06/13/2018 08:34 AM, Kevin Wolf wrote:
> 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(-)
>
I think libvirt needs to be adjusted to handle new style options for
patch #1 and patch #3, ccing
- Cole
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
@ 2018-06-13 14:00 ` Markus Armbruster
2018-06-13 15:36 ` [Qemu-devel] [Qemu-block] " Jeff Cody
1 sibling, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2018-06-13 14:00 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, mreitz
Kevin Wolf <kwolf@redhat.com> writes:
> 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.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
[...]
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 24870b38f4..d263a74ec0 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -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();
> }
>
> /*
> - * 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);
> -}
All remaining calls of test_ide_drive_user() pass non-null @dev. Please
simplify test_ide_drive_user() accordingly.
> -
> -/*
> * 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);
[...]
With that done:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr
2018-06-13 12:34 ` [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr Kevin Wolf
@ 2018-06-13 14:04 ` Markus Armbruster
2018-06-13 15:38 ` [Qemu-devel] [Qemu-block] " Jeff Cody
1 sibling, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2018-06-13 14:04 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, mreitz
Kevin Wolf <kwolf@redhat.com> writes:
> The -drive option addr was deprecated in QEMU 2.10. It's time to remove
> it.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
One of my first additions to QEMU... and a bit of an embarrassment.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial
2018-06-13 12:34 ` [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial Kevin Wolf
@ 2018-06-13 14:18 ` Markus Armbruster
2018-06-13 15:40 ` [Qemu-devel] [Qemu-block] " Jeff Cody
0 siblings, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2018-06-13 14:18 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, mreitz
Kevin Wolf <kwolf@redhat.com> writes:
> 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.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> 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(-)
>
> diff --git a/include/hw/block/block.h b/include/hw/block/block.h
> index d4f4dfffab..e9f9e2223f 100644
> --- a/include/hw/block/block.h
> +++ b/include/hw/block/block.h
> @@ -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);
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index c0ae3700ec..24954b94e0 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -35,7 +35,6 @@ struct DriveInfo {
> bool is_default; /* Added by default_drive() ? */
> int media_cd;
> QemuOpts *opts;
> - char *serial;
> QTAILQ_ENTRY(DriveInfo) next;
> };
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index d55c328736..2d1a3463e8 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
> return;
> }
> qemu_opts_del(dinfo->opts);
> - g_free(dinfo->serial);
> g_free(dinfo);
> }
>
> diff --git a/blockdev.c b/blockdev.c
> index 83b3cc12e9..4ab3d6ec0b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -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) {
I guess I'd remove this in a separate patch, to make bringing it back
easier in case we need it again.
> @@ -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);
>
> diff --git a/hw/block/block.c b/hw/block/block.c
> index b6c80ab0b7..cf0eb826f1 100644
> --- a/hw/block/block.c
> +++ b/hw/block/block.c
> @@ -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;
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 811084b6a7..d5bf95b79b 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -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;
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 50b5c869e3..225fe44b7a 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -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)) {
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index f395d24592..573b022e1e 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -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)) {
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 9f43583ea0..920850a1c1 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -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 >
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 481694a473..47b992f403 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -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)) {
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 1a7b761304..937ed2f910 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -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");
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.
> }
>
> return s;
> diff --git a/tests/ide-test.c b/tests/ide-test.c
> index 2384c2c3e2..f39431b1a9 100644
> --- a/tests/ide-test.c
> +++ b/tests/ide-test.c
> @@ -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);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 338477725f..282bc3dc35 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -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
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c2531e2f3c..d5b0c26e8e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -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),
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/3] block: Remove deprecated -drive geometry options
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
2018-06-13 14:00 ` Markus Armbruster
@ 2018-06-13 15:36 ` Jeff Cody
1 sibling, 0 replies; 11+ messages in thread
From: Jeff Cody @ 2018-06-13 15:36 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, mreitz
On Wed, Jun 13, 2018 at 02:34:56PM +0200, Kevin Wolf wrote:
> 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.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
> ---
> 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(-)
>
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index ac22f2ae1f..37ea39719e 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -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;
> diff --git a/blockdev.c b/blockdev.c
> index 4862323012..9c891706ef 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -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;
> diff --git a/hw/block/block.c b/hw/block/block.c
> index b91e2b6d7e..b6c80ab0b7 100644
> --- a/hw/block/block.c
> +++ b/hw/block/block.c
> @@ -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,
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 24870b38f4..d263a74ec0 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -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);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 0734fea931..0de7c4c29e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -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",
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index cd05760cac..ab95bffc74 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -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
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c0d3951e9f..a14b9655c5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -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}).
> --
> 2.13.6
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/3] block: Remove deprecated -drive option addr
2018-06-13 12:34 ` [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr Kevin Wolf
2018-06-13 14:04 ` Markus Armbruster
@ 2018-06-13 15:38 ` Jeff Cody
1 sibling, 0 replies; 11+ messages in thread
From: Jeff Cody @ 2018-06-13 15:38 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, mreitz
On Wed, Jun 13, 2018 at 02:34:57PM +0200, Kevin Wolf wrote:
> The -drive option addr was deprecated in QEMU 2.10. It's time to remove
> it.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
> ---
> 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(-)
>
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 37ea39719e..c0ae3700ec 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -28,7 +28,6 @@ typedef enum {
> } BlockInterfaceType;
>
> struct DriveInfo {
> - const char *devaddr;
> BlockInterfaceType type;
> int bus;
> int unit;
> diff --git a/blockdev.c b/blockdev.c
> index 9c891706ef..83b3cc12e9 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -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);
> diff --git a/device-hotplug.c b/device-hotplug.c
> index 23fd6656f1..cd427e2c76 100644
> --- a/device-hotplug.c
> +++ b/device-hotplug.c
> @@ -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:
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index ab95bffc74..338477725f 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -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
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a14b9655c5..c2531e2f3c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -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),
> --
> 2.13.6
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 3/3] block: Remove deprecated -drive option serial
2018-06-13 14:18 ` Markus Armbruster
@ 2018-06-13 15:40 ` Jeff Cody
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Cody @ 2018-06-13 15:40 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Kevin Wolf, qemu-devel, qemu-block, mreitz
On Wed, Jun 13, 2018 at 04:18:17PM +0200, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > 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.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
> > ---
> > 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(-)
> >
> > diff --git a/include/hw/block/block.h b/include/hw/block/block.h
> > index d4f4dfffab..e9f9e2223f 100644
> > --- a/include/hw/block/block.h
> > +++ b/include/hw/block/block.h
> > @@ -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);
> > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > index c0ae3700ec..24954b94e0 100644
> > --- a/include/sysemu/blockdev.h
> > +++ b/include/sysemu/blockdev.h
> > @@ -35,7 +35,6 @@ struct DriveInfo {
> > bool is_default; /* Added by default_drive() ? */
> > int media_cd;
> > QemuOpts *opts;
> > - char *serial;
> > QTAILQ_ENTRY(DriveInfo) next;
> > };
> >
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index d55c328736..2d1a3463e8 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo)
> > return;
> > }
> > qemu_opts_del(dinfo->opts);
> > - g_free(dinfo->serial);
> > g_free(dinfo);
> > }
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 83b3cc12e9..4ab3d6ec0b 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -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) {
>
> I guess I'd remove this in a separate patch, to make bringing it back
> easier in case we need it again.
>
> > @@ -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);
> >
> > diff --git a/hw/block/block.c b/hw/block/block.c
> > index b6c80ab0b7..cf0eb826f1 100644
> > --- a/hw/block/block.c
> > +++ b/hw/block/block.c
> > @@ -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;
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 811084b6a7..d5bf95b79b 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -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;
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 50b5c869e3..225fe44b7a 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -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)) {
> > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> > index f395d24592..573b022e1e 100644
> > --- a/hw/ide/qdev.c
> > +++ b/hw/ide/qdev.c
> > @@ -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)) {
> > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> > index 9f43583ea0..920850a1c1 100644
> > --- a/hw/scsi/scsi-disk.c
> > +++ b/hw/scsi/scsi-disk.c
> > @@ -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 >
> > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> > index 481694a473..47b992f403 100644
> > --- a/hw/usb/dev-storage.c
> > +++ b/hw/usb/dev-storage.c
> > @@ -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)) {
> > diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> > index 1a7b761304..937ed2f910 100644
> > --- a/tests/ahci-test.c
> > +++ b/tests/ahci-test.c
> > @@ -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");
>
> 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.
>
> > }
> >
> > return s;
> > diff --git a/tests/ide-test.c b/tests/ide-test.c
> > index 2384c2c3e2..f39431b1a9 100644
> > --- a/tests/ide-test.c
> > +++ b/tests/ide-test.c
> > @@ -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);
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 338477725f..282bc3dc35 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -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
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index c2531e2f3c..d5b0c26e8e 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -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),
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-13 15:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-13 12:34 [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Kevin Wolf
2018-06-13 12:34 ` [Qemu-devel] [PATCH 1/3] block: Remove deprecated -drive geometry options Kevin Wolf
2018-06-13 14:00 ` Markus Armbruster
2018-06-13 15:36 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2018-06-13 12:34 ` [Qemu-devel] [PATCH 2/3] block: Remove deprecated -drive option addr Kevin Wolf
2018-06-13 14:04 ` Markus Armbruster
2018-06-13 15:38 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2018-06-13 12:34 ` [Qemu-devel] [PATCH 3/3] block: Remove deprecated -drive option serial Kevin Wolf
2018-06-13 14:18 ` Markus Armbruster
2018-06-13 15:40 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2018-06-13 13:34 ` [Qemu-devel] [PATCH 0/3] block: Remove deprecated -drive options Cole Robinson
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).