* [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff
@ 2021-02-03 17:18 Thomas Huth
2021-02-03 17:18 ` [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types Thomas Huth
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Thomas Huth @ 2021-02-03 17:18 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann, John Snow
The pc-1.x machine types have been deprecated since QEMU v5.0 already, and
nobody complained, so they can now be removed. While we're at it, also
remove some compatibility switches and related code that are now not
necessary anymore after these machine types have been removed.
(We could maybe even remove more stuff like the various host_features
switches in the virtio devices, but they still might be useful in certain
cases, so I decided not to remove them yet)
Thomas Huth (4):
hw/i386: Remove the deprecated pc-1.x machine types
hw/block/fdc: Remove the check_media_rate property
hw/virtio/virtio-balloon: Remove the "class" property
hw/usb/bus: Remove the "full-path" property
docs/system/deprecated.rst | 6 --
docs/system/removed-features.rst | 6 ++
hw/block/fdc.c | 17 +-----
hw/i386/pc_piix.c | 94 --------------------------------
hw/usb/bus.c | 7 +--
hw/virtio/virtio-balloon-pci.c | 11 +---
include/hw/usb.h | 2 +-
tests/qemu-iotests/172.out | 35 ------------
8 files changed, 11 insertions(+), 167 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
@ 2021-02-03 17:18 ` Thomas Huth
2021-02-03 17:37 ` Daniel P. Berrangé
2021-02-03 17:18 ` [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property Thomas Huth
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-03 17:18 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann, John Snow
They have been deprecated since QEMU v5.0, time to remove them now.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
docs/system/deprecated.rst | 6 --
docs/system/removed-features.rst | 6 ++
hw/i386/pc_piix.c | 94 --------------------------------
3 files changed, 6 insertions(+), 100 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 6ac757ed9f..2fcac7861e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -322,12 +322,6 @@ The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or
System emulator machines
------------------------
-``pc-1.0``, ``pc-1.1``, ``pc-1.2`` and ``pc-1.3`` (since 5.0)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-These machine types are very old and likely can not be used for live migration
-from old QEMU versions anymore. A newer machine type should be used instead.
-
Raspberry Pi ``raspi2`` and ``raspi3`` machines (since 5.2)
'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 88b81a6156..c8481cafbd 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -136,6 +136,12 @@ mips ``fulong2e`` machine alias (removed in 6.0)
This machine has been renamed ``fuloong2e``.
+``pc-1.0``, ``pc-1.1``, ``pc-1.2`` and ``pc-1.3`` (removed in 6.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+These machine types were very old and likely could not be used for live
+migration from old QEMU versions anymore. Use a newer machine type instead.
+
Related binaries
----------------
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6188c3e97e..2904b40163 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -359,18 +359,6 @@ static void pc_compat_1_4_fn(MachineState *machine)
pc_compat_1_5_fn(machine);
}
-static void pc_compat_1_3(MachineState *machine)
-{
- pc_compat_1_4_fn(machine);
-}
-
-/* PC compat function for pc-1.0 to pc-1.2 */
-static void pc_compat_1_2(MachineState *machine)
-{
- pc_compat_1_3(machine);
- x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
-}
-
static void pc_init_isa(MachineState *machine)
{
pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
@@ -772,88 +760,6 @@ static void pc_i440fx_1_4_machine_options(MachineClass *m)
DEFINE_I440FX_MACHINE(v1_4, "pc-i440fx-1.4", pc_compat_1_4_fn,
pc_i440fx_1_4_machine_options);
-static void pc_i440fx_1_3_machine_options(MachineClass *m)
-{
- X86MachineClass *x86mc = X86_MACHINE_CLASS(m);
- static GlobalProperty compat[] = {
- PC_CPU_MODEL_IDS("1.3.0")
- { "usb-tablet", "usb_version", "1" },
- { "virtio-net-pci", "ctrl_mac_addr", "off" },
- { "virtio-net-pci", "mq", "off" },
- { "e1000", "autonegotiation", "off" },
- };
-
- pc_i440fx_1_4_machine_options(m);
- m->hw_version = "1.3.0";
- m->deprecation_reason = "use a newer machine type instead";
- x86mc->compat_apic_id_mode = true;
- compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_3, "pc-1.3", pc_compat_1_3,
- pc_i440fx_1_3_machine_options);
-
-
-static void pc_i440fx_1_2_machine_options(MachineClass *m)
-{
- static GlobalProperty compat[] = {
- PC_CPU_MODEL_IDS("1.2.0")
- { "nec-usb-xhci", "msi", "off" },
- { "nec-usb-xhci", "msix", "off" },
- { "qxl", "revision", "3" },
- { "qxl-vga", "revision", "3" },
- { "VGA", "mmio", "off" },
- };
-
- pc_i440fx_1_3_machine_options(m);
- m->hw_version = "1.2.0";
- compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_2, "pc-1.2", pc_compat_1_2,
- pc_i440fx_1_2_machine_options);
-
-
-static void pc_i440fx_1_1_machine_options(MachineClass *m)
-{
- static GlobalProperty compat[] = {
- PC_CPU_MODEL_IDS("1.1.0")
- { "virtio-scsi-pci", "hotplug", "off" },
- { "virtio-scsi-pci", "param_change", "off" },
- { "VGA", "vgamem_mb", "8" },
- { "vmware-svga", "vgamem_mb", "8" },
- { "qxl-vga", "vgamem_mb", "8" },
- { "qxl", "vgamem_mb", "8" },
- { "virtio-blk-pci", "config-wce", "off" },
- };
-
- pc_i440fx_1_2_machine_options(m);
- m->hw_version = "1.1.0";
- compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_1, "pc-1.1", pc_compat_1_2,
- pc_i440fx_1_1_machine_options);
-
-static void pc_i440fx_1_0_machine_options(MachineClass *m)
-{
- static GlobalProperty compat[] = {
- PC_CPU_MODEL_IDS("1.0")
- { TYPE_ISA_FDC, "check_media_rate", "off" },
- { "virtio-balloon-pci", "class", stringify(PCI_CLASS_MEMORY_RAM) },
- { "apic-common", "vapic", "off" },
- { TYPE_USB_DEVICE, "full-path", "no" },
- };
-
- pc_i440fx_1_1_machine_options(m);
- m->hw_version = "1.0";
- compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_0, "pc-1.0", pc_compat_1_2,
- pc_i440fx_1_0_machine_options);
-
-
typedef struct {
uint16_t gpu_device_id;
uint16_t pch_device_id;
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
2021-02-03 17:18 ` [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types Thomas Huth
@ 2021-02-03 17:18 ` Thomas Huth
2021-02-05 0:40 ` John Snow
2021-02-03 17:18 ` [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property Thomas Huth
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-03 17:18 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann, John Snow
This was only required for the pc-1.0 and earlier machine types.
Now that these have been removed, we can also drop the corresponding
code from the FDC device.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/block/fdc.c | 17 ++---------------
tests/qemu-iotests/172.out | 35 -----------------------------------
2 files changed, 2 insertions(+), 50 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 292ea87805..198940e737 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -874,7 +874,6 @@ struct FDCtrl {
FloppyDriveType type;
} qdev_for_drives[MAX_FD];
int reset_sensei;
- uint32_t check_media_rate;
FloppyDriveType fallback; /* type=auto failure fallback */
/* Timers state */
uint8_t timer0;
@@ -1021,18 +1020,10 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
}
};
-static bool fdrive_media_rate_needed(void *opaque)
-{
- FDrive *drive = opaque;
-
- return drive->fdctrl->check_media_rate;
-}
-
static const VMStateDescription vmstate_fdrive_media_rate = {
.name = "fdrive/media_rate",
.version_id = 1,
.minimum_version_id = 1,
- .needed = fdrive_media_rate_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT8(media_rate, FDrive),
VMSTATE_END_OF_LIST()
@@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
/* Check the data rate. If the programmed data rate does not match
* the currently inserted medium, the operation has to fail. */
- if (fdctrl->check_media_rate &&
- (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+ if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
@@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
}
/* READ_ID can't automatically succeed! */
- if (fdctrl->check_media_rate &&
- (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+ if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
@@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),
- DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
- 0, true),
DEFINE_PROP_SIGNED("fdtypeA", FDCtrlISABus, state.qdev_for_drives[0].type,
FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
FloppyDriveType),
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 2cd4a8fd83..349ae51d6c 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -14,7 +14,6 @@ Testing:
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -44,7 +43,6 @@ Testing: -fda TEST_DIR/t.qcow2
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -84,7 +82,6 @@ Testing: -fdb TEST_DIR/t.qcow2
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -139,7 +136,6 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -195,7 +191,6 @@ Testing: -fdb
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -236,7 +231,6 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -276,7 +270,6 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -331,7 +324,6 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -392,7 +384,6 @@ Use -device floppy,unit=0,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -434,7 +425,6 @@ Use -device floppy,unit=1,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -478,7 +468,6 @@ Use -device floppy,unit=1,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -537,7 +526,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -577,7 +565,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -617,7 +604,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -678,7 +664,6 @@ Use -device floppy,unit=1,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -736,7 +721,6 @@ Use -device floppy,unit=0,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -808,7 +792,6 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -864,7 +847,6 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -920,7 +902,6 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -976,7 +957,6 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1041,7 +1021,6 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1097,7 +1076,6 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1161,7 +1139,6 @@ Use -device floppy,unit=0,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1219,7 +1196,6 @@ Use -device floppy,unit=0,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1277,7 +1253,6 @@ Use -device floppy,unit=1,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1335,7 +1310,6 @@ Use -device floppy,unit=1,drive=... instead.
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1391,7 +1365,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1473,7 +1446,6 @@ Testing: -device floppy
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1500,7 +1472,6 @@ Testing: -device floppy,drive-type=120
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1527,7 +1498,6 @@ Testing: -device floppy,drive-type=144
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1554,7 +1524,6 @@ Testing: -device floppy,drive-type=288
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1584,7 +1553,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1624,7 +1592,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1667,7 +1634,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
@@ -1707,7 +1673,6 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica
dma = 2 (0x2)
driveA = ""
driveB = ""
- check_media_rate = true
fdtypeA = "auto"
fdtypeB = "auto"
fallback = "288"
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
2021-02-03 17:18 ` [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types Thomas Huth
2021-02-03 17:18 ` [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property Thomas Huth
@ 2021-02-03 17:18 ` Thomas Huth
2021-02-03 19:42 ` David Hildenbrand
2021-02-03 17:18 ` [PATCH 4/4] hw/usb/bus: Remove the "full-path" property Thomas Huth
2021-02-05 11:00 ` [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Michael S. Tsirkin
4 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-03 17:18 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann, John Snow
This property was only required for compatibility reasons in the
pc-1.0 machine type and earlier. Now that these machine types have
been removed, the property is not useful anymore.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/virtio/virtio-balloon-pci.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index a2c5cc7207..79a3ba979a 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -34,21 +34,13 @@ struct VirtIOBalloonPCI {
VirtIOPCIProxy parent_obj;
VirtIOBalloon vdev;
};
-static Property virtio_balloon_pci_properties[] = {
- DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
- DEFINE_PROP_END_OF_LIST(),
-};
static void virtio_balloon_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
{
VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
DeviceState *vdev = DEVICE(&dev->vdev);
- if (vpci_dev->class_code != PCI_CLASS_OTHERS &&
- vpci_dev->class_code != PCI_CLASS_MEMORY_RAM) { /* qemu < 1.1 */
- vpci_dev->class_code = PCI_CLASS_OTHERS;
- }
-
+ vpci_dev->class_code = PCI_CLASS_OTHERS;
qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
}
@@ -59,7 +51,6 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
k->realize = virtio_balloon_pci_realize;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
- device_class_set_props(dc, virtio_balloon_pci_properties);
pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] hw/usb/bus: Remove the "full-path" property
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
` (2 preceding siblings ...)
2021-02-03 17:18 ` [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property Thomas Huth
@ 2021-02-03 17:18 ` Thomas Huth
2021-02-04 8:36 ` Gerd Hoffmann
2021-02-05 11:00 ` [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Michael S. Tsirkin
4 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-03 17:18 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann, John Snow
This property was only required for the pc-1.0 and earlier machine
types. Since these have been removed now, we can delete the property
as well.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/usb/bus.c | 7 +------
include/hw/usb.h | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 064f94e9c3..df7411fea8 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -19,8 +19,6 @@ static void usb_qdev_unrealize(DeviceState *qdev);
static Property usb_props[] = {
DEFINE_PROP_STRING("port", USBDevice, port_path),
DEFINE_PROP_STRING("serial", USBDevice, serial),
- DEFINE_PROP_BIT("full-path", USBDevice, flags,
- USB_DEV_FLAG_FULL_PATH, true),
DEFINE_PROP_BIT("msos-desc", USBDevice, flags,
USB_DEV_FLAG_MSOS_DESC_ENABLE, true),
DEFINE_PROP_STRING("pcap", USBDevice, pcap_filename),
@@ -596,11 +594,8 @@ static char *usb_get_dev_path(DeviceState *qdev)
{
USBDevice *dev = USB_DEVICE(qdev);
DeviceState *hcd = qdev->parent_bus->parent;
- char *id = NULL;
+ char *id = qdev_get_dev_path(hcd);
- if (dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) {
- id = qdev_get_dev_path(hcd);
- }
if (id) {
char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
g_free(id);
diff --git a/include/hw/usb.h b/include/hw/usb.h
index abfbfc5284..c44b77dae0 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -216,7 +216,7 @@ struct USBEndpoint {
};
enum USBDeviceFlags {
- USB_DEV_FLAG_FULL_PATH,
+ USB_DEV_FLAG_FULL_PATH, /* unused since QEMU v6.0 */
USB_DEV_FLAG_IS_HOST,
USB_DEV_FLAG_MSOS_DESC_ENABLE,
USB_DEV_FLAG_MSOS_DESC_IN_USE,
--
2.27.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types
2021-02-03 17:18 ` [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types Thomas Huth
@ 2021-02-03 17:37 ` Daniel P. Berrangé
0 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2021-02-03 17:37 UTC (permalink / raw)
To: Thomas Huth
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, qemu-devel, Gerd Hoffmann, Paolo Bonzini,
John Snow
On Wed, Feb 03, 2021 at 06:18:29PM +0100, Thomas Huth wrote:
> They have been deprecated since QEMU v5.0, time to remove them now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> docs/system/deprecated.rst | 6 --
> docs/system/removed-features.rst | 6 ++
> hw/i386/pc_piix.c | 94 --------------------------------
> 3 files changed, 6 insertions(+), 100 deletions(-)
Ack from libvirt side,
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property
2021-02-03 17:18 ` [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property Thomas Huth
@ 2021-02-03 19:42 ` David Hildenbrand
0 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand @ 2021-02-03 19:42 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin, libvir-list,
Gerd Hoffmann, John Snow
On 03.02.21 18:18, Thomas Huth wrote:
> This property was only required for compatibility reasons in the
> pc-1.0 machine type and earlier. Now that these machine types have
> been removed, the property is not useful anymore.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/virtio/virtio-balloon-pci.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
> index a2c5cc7207..79a3ba979a 100644
> --- a/hw/virtio/virtio-balloon-pci.c
> +++ b/hw/virtio/virtio-balloon-pci.c
> @@ -34,21 +34,13 @@ struct VirtIOBalloonPCI {
> VirtIOPCIProxy parent_obj;
> VirtIOBalloon vdev;
> };
> -static Property virtio_balloon_pci_properties[] = {
> - DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
> - DEFINE_PROP_END_OF_LIST(),
> -};
>
> static void virtio_balloon_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> {
> VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
> DeviceState *vdev = DEVICE(&dev->vdev);
>
> - if (vpci_dev->class_code != PCI_CLASS_OTHERS &&
> - vpci_dev->class_code != PCI_CLASS_MEMORY_RAM) { /* qemu < 1.1 */
> - vpci_dev->class_code = PCI_CLASS_OTHERS;
> - }
> -
> + vpci_dev->class_code = PCI_CLASS_OTHERS;
> qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> }
>
> @@ -59,7 +51,6 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
> PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> k->realize = virtio_balloon_pci_realize;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> - device_class_set_props(dc, virtio_balloon_pci_properties);
> pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
> pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] hw/usb/bus: Remove the "full-path" property
2021-02-03 17:18 ` [PATCH 4/4] hw/usb/bus: Remove the "full-path" property Thomas Huth
@ 2021-02-04 8:36 ` Gerd Hoffmann
2021-02-04 15:51 ` Thomas Huth
0 siblings, 1 reply; 17+ messages in thread
From: Gerd Hoffmann @ 2021-02-04 8:36 UTC (permalink / raw)
To: Thomas Huth
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, qemu-devel, Paolo Bonzini, John Snow
Hi,
> enum USBDeviceFlags {
> - USB_DEV_FLAG_FULL_PATH,
> + USB_DEV_FLAG_FULL_PATH, /* unused since QEMU v6.0 */
Why not just drop it? Any remaining users?
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] hw/usb/bus: Remove the "full-path" property
2021-02-04 8:36 ` Gerd Hoffmann
@ 2021-02-04 15:51 ` Thomas Huth
2021-02-05 9:09 ` Gerd Hoffmann
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-04 15:51 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, qemu-devel, Paolo Bonzini, John Snow
On 04/02/2021 09.36, Gerd Hoffmann wrote:
> Hi,
>
>> enum USBDeviceFlags {
>> - USB_DEV_FLAG_FULL_PATH,
>> + USB_DEV_FLAG_FULL_PATH, /* unused since QEMU v6.0 */
>
> Why not just drop it? Any remaining users?
I didn't want to change the values of the other members of the enum ... but
if you prefer, I can also a "= 1" after the next member of the enum and
remove the USB_DEV_FLAG_FULL_PATH instead.
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-03 17:18 ` [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property Thomas Huth
@ 2021-02-05 0:40 ` John Snow
2021-02-05 6:37 ` Thomas Huth
0 siblings, 1 reply; 17+ messages in thread
From: John Snow @ 2021-02-05 0:40 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann
On 2/3/21 12:18 PM, Thomas Huth wrote:
> This was only required for the pc-1.0 and earlier machine types.
> Now that these have been removed, we can also drop the corresponding
> code from the FDC device.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/block/fdc.c | 17 ++---------------
> tests/qemu-iotests/172.out | 35 -----------------------------------
> 2 files changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 292ea87805..198940e737 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -874,7 +874,6 @@ struct FDCtrl {
> FloppyDriveType type;
> } qdev_for_drives[MAX_FD];
> int reset_sensei;
> - uint32_t check_media_rate;
I am a bit of a dunce when it comes to the compatibility properties...
does this mess with the migration format?
I guess it doesn't, since it's not in the VMSTATE declaration.
Hmmmm, alright.
> FloppyDriveType fallback; /* type=auto failure fallback */
> /* Timers state */
> uint8_t timer0;
> @@ -1021,18 +1020,10 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
> }
> };
>
> -static bool fdrive_media_rate_needed(void *opaque)
> -{
> - FDrive *drive = opaque;
> -
> - return drive->fdctrl->check_media_rate;
> -}
> -
> static const VMStateDescription vmstate_fdrive_media_rate = {
> .name = "fdrive/media_rate",
> .version_id = 1,
> .minimum_version_id = 1,
> - .needed = fdrive_media_rate_needed,
> .fields = (VMStateField[]) {
> VMSTATE_UINT8(media_rate, FDrive),
> VMSTATE_END_OF_LIST()
> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>
> /* Check the data rate. If the programmed data rate does not match
> * the currently inserted medium, the operation has to fail. */
> - if (fdctrl->check_media_rate &&
> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
> }
> /* READ_ID can't automatically succeed! */
> - if (fdctrl->check_media_rate &&
> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),
> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
> - 0, true),
Could you theoretically set this via QOM commands in QMP, and claim that
this is a break in behavior?
Though, it's ENTIRELY undocumented, so ... it's probably fine, I think.
Probably. (Please soothe my troubled mind.)
--js
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-05 0:40 ` John Snow
@ 2021-02-05 6:37 ` Thomas Huth
2021-02-05 20:15 ` John Snow
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-05 6:37 UTC (permalink / raw)
To: John Snow, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann
On 05/02/2021 01.40, John Snow wrote:
> On 2/3/21 12:18 PM, Thomas Huth wrote:
>> This was only required for the pc-1.0 and earlier machine types.
>> Now that these have been removed, we can also drop the corresponding
>> code from the FDC device.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/block/fdc.c | 17 ++---------------
>> tests/qemu-iotests/172.out | 35 -----------------------------------
>> 2 files changed, 2 insertions(+), 50 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index 292ea87805..198940e737 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -874,7 +874,6 @@ struct FDCtrl {
>> FloppyDriveType type;
>> } qdev_for_drives[MAX_FD];
>> int reset_sensei;
>> - uint32_t check_media_rate;
>
> I am a bit of a dunce when it comes to the compatibility properties... does
> this mess with the migration format?
>
> I guess it doesn't, since it's not in the VMSTATE declaration.
>
> Hmmmm, alright.
I think that should be fine, yes.
>> FloppyDriveType fallback; /* type=auto failure fallback */
>> /* Timers state */
>> uint8_t timer0;
>> @@ -1021,18 +1020,10 @@ static const VMStateDescription
>> vmstate_fdrive_media_changed = {
>> }
>> };
>> -static bool fdrive_media_rate_needed(void *opaque)
>> -{
>> - FDrive *drive = opaque;
>> -
>> - return drive->fdctrl->check_media_rate;
>> -}
>> -
>> static const VMStateDescription vmstate_fdrive_media_rate = {
>> .name = "fdrive/media_rate",
>> .version_id = 1,
>> .minimum_version_id = 1,
>> - .needed = fdrive_media_rate_needed,
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT8(media_rate, FDrive),
>> VMSTATE_END_OF_LIST()
>> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl,
>> int direction)
>> /* Check the data rate. If the programmed data rate does not match
>> * the currently inserted medium, the operation has to fail. */
>> - if (fdctrl->check_media_rate &&
>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
>> fdctrl->dsr & FD_DSR_DRATEMASK,
>> cur_drv->media_rate);
>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
>> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
>> }
>> /* READ_ID can't automatically succeed! */
>> - if (fdctrl->check_media_rate &&
>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
>> fdctrl->dsr & FD_DSR_DRATEMASK,
>> cur_drv->media_rate);
>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
>> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
>> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus,
>> state.qdev_for_drives[0].blk),
>> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus,
>> state.qdev_for_drives[1].blk),
>> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus,
>> state.check_media_rate,
>> - 0, true),
>
> Could you theoretically set this via QOM commands in QMP, and claim that
> this is a break in behavior?
>
> Though, it's ENTIRELY undocumented, so ... it's probably fine, I think.
> Probably. (Please soothe my troubled mind.)
A user actually could mess with this property even on the command line, e.g.
by using:
qemu-system-x86_64 -global isa-fdc.check_media_rate=false
... but, as you said, it's completely undocumented, the property is really
just there for the internal use of machine type compatibility. We've done
such clean-ups in the past already, see e.g. c6026998eef382d7ad76 or
2a4dbaf1c0db2453ab78f, so I think this should be fine. But if you disagree,
I could replace this by a patch that adds this property to the list of
deprecated features instead, so we could at least remove it after it has
been deprecated for two releases?
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] hw/usb/bus: Remove the "full-path" property
2021-02-04 15:51 ` Thomas Huth
@ 2021-02-05 9:09 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2021-02-05 9:09 UTC (permalink / raw)
To: Thomas Huth
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, qemu-devel, Paolo Bonzini, John Snow
On Thu, Feb 04, 2021 at 04:51:39PM +0100, Thomas Huth wrote:
> On 04/02/2021 09.36, Gerd Hoffmann wrote:
> > Hi,
> >
> > > enum USBDeviceFlags {
> > > - USB_DEV_FLAG_FULL_PATH,
> > > + USB_DEV_FLAG_FULL_PATH, /* unused since QEMU v6.0 */
> >
> > Why not just drop it? Any remaining users?
>
> I didn't want to change the values of the other members of the enum ...
This should be purely internal to qemu hw/usb and not some kind of abi,
so changing the values shouldn't break anything ...
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
` (3 preceding siblings ...)
2021-02-03 17:18 ` [PATCH 4/4] hw/usb/bus: Remove the "full-path" property Thomas Huth
@ 2021-02-05 11:00 ` Michael S. Tsirkin
4 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-02-05 11:00 UTC (permalink / raw)
To: Thomas Huth
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
qemu-devel, Gerd Hoffmann, Paolo Bonzini, John Snow
On Wed, Feb 03, 2021 at 06:18:28PM +0100, Thomas Huth wrote:
> The pc-1.x machine types have been deprecated since QEMU v5.0 already, and
> nobody complained, so they can now be removed. While we're at it, also
> remove some compatibility switches and related code that are now not
> necessary anymore after these machine types have been removed.
> (We could maybe even remove more stuff like the various host_features
> switches in the virtio devices, but they still might be useful in certain
> cases, so I decided not to remove them yet)
I queued patches 1 and 3. Thanks!
> Thomas Huth (4):
> hw/i386: Remove the deprecated pc-1.x machine types
> hw/block/fdc: Remove the check_media_rate property
> hw/virtio/virtio-balloon: Remove the "class" property
> hw/usb/bus: Remove the "full-path" property
>
> docs/system/deprecated.rst | 6 --
> docs/system/removed-features.rst | 6 ++
> hw/block/fdc.c | 17 +-----
> hw/i386/pc_piix.c | 94 --------------------------------
> hw/usb/bus.c | 7 +--
> hw/virtio/virtio-balloon-pci.c | 11 +---
> include/hw/usb.h | 2 +-
> tests/qemu-iotests/172.out | 35 ------------
> 8 files changed, 11 insertions(+), 167 deletions(-)
>
> --
> 2.27.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-05 6:37 ` Thomas Huth
@ 2021-02-05 20:15 ` John Snow
2021-02-08 7:05 ` Thomas Huth
0 siblings, 1 reply; 17+ messages in thread
From: John Snow @ 2021-02-05 20:15 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, libvir-list,
Michael S. Tsirkin, Gerd Hoffmann
On 2/5/21 1:37 AM, Thomas Huth wrote:
> On 05/02/2021 01.40, John Snow wrote:
>> On 2/3/21 12:18 PM, Thomas Huth wrote:
>>> This was only required for the pc-1.0 and earlier machine types.
>>> Now that these have been removed, we can also drop the corresponding
>>> code from the FDC device.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> hw/block/fdc.c | 17 ++---------------
>>> tests/qemu-iotests/172.out | 35 -----------------------------------
>>> 2 files changed, 2 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>>> index 292ea87805..198940e737 100644
>>> --- a/hw/block/fdc.c
>>> +++ b/hw/block/fdc.c
>>> @@ -874,7 +874,6 @@ struct FDCtrl {
>>> FloppyDriveType type;
>>> } qdev_for_drives[MAX_FD];
>>> int reset_sensei;
>>> - uint32_t check_media_rate;
>>
>> I am a bit of a dunce when it comes to the compatibility properties...
>> does this mess with the migration format?
>>
>> I guess it doesn't, since it's not in the VMSTATE declaration.
>>
>> Hmmmm, alright.
>
> I think that should be fine, yes.
>
>>> FloppyDriveType fallback; /* type=auto failure fallback */
>>> /* Timers state */
>>> uint8_t timer0;
>>> @@ -1021,18 +1020,10 @@ static const VMStateDescription
>>> vmstate_fdrive_media_changed = {
>>> }
>>> };
>>> -static bool fdrive_media_rate_needed(void *opaque)
>>> -{
>>> - FDrive *drive = opaque;
>>> -
>>> - return drive->fdctrl->check_media_rate;
>>> -}
>>> -
>>> static const VMStateDescription vmstate_fdrive_media_rate = {
>>> .name = "fdrive/media_rate",
>>> .version_id = 1,
>>> .minimum_version_id = 1,
>>> - .needed = fdrive_media_rate_needed,
>>> .fields = (VMStateField[]) {
>>> VMSTATE_UINT8(media_rate, FDrive),
>>> VMSTATE_END_OF_LIST()
>>> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl
>>> *fdctrl, int direction)
>>> /* Check the data rate. If the programmed data rate does not match
>>> * the currently inserted medium, the operation has to fail. */
>>> - if (fdctrl->check_media_rate &&
>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
>>> fdctrl->dsr & FD_DSR_DRATEMASK,
>>> cur_drv->media_rate);
>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
>>> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
>>> }
>>> /* READ_ID can't automatically succeed! */
>>> - if (fdctrl->check_media_rate &&
>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
>>> fdctrl->dsr & FD_DSR_DRATEMASK,
>>> cur_drv->media_rate);
>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
>>> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
>>> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus,
>>> state.qdev_for_drives[0].blk),
>>> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus,
>>> state.qdev_for_drives[1].blk),
>>> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus,
>>> state.check_media_rate,
>>> - 0, true),
>>
>> Could you theoretically set this via QOM commands in QMP, and claim
>> that this is a break in behavior?
>>
>> Though, it's ENTIRELY undocumented, so ... it's probably fine, I
>> think. Probably. (Please soothe my troubled mind.)
>
> A user actually could mess with this property even on the command line,
> e.g. by using:
>
> qemu-system-x86_64 -global isa-fdc.check_media_rate=false
>
> ... but, as you said, it's completely undocumented, the property is
> really just there for the internal use of machine type compatibility.
> We've done such clean-ups in the past already, see e.g.
> c6026998eef382d7ad76 or 2a4dbaf1c0db2453ab78f, so I think this should be
> fine. But if you disagree, I could replace this by a patch that adds
> this property to the list of deprecated features instead, so we could at
> least remove it after it has been deprecated for two releases?
>
I don't think it's necessary, personally -- just wanted to make sure I
knew the exact stakes here.
Reviewed-by: John Snow <jsnow@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-05 20:15 ` John Snow
@ 2021-02-08 7:05 ` Thomas Huth
2021-02-13 22:41 ` Laurent Vivier
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Huth @ 2021-02-08 7:05 UTC (permalink / raw)
To: John Snow, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, David Hildenbrand, QEMU Trivial,
Michael S. Tsirkin, Gerd Hoffmann
On 05/02/2021 21.15, John Snow wrote:
> On 2/5/21 1:37 AM, Thomas Huth wrote:
>> On 05/02/2021 01.40, John Snow wrote:
>>> On 2/3/21 12:18 PM, Thomas Huth wrote:
>>>> This was only required for the pc-1.0 and earlier machine types.
>>>> Now that these have been removed, we can also drop the corresponding
>>>> code from the FDC device.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> hw/block/fdc.c | 17 ++---------------
>>>> tests/qemu-iotests/172.out | 35 -----------------------------------
>>>> 2 files changed, 2 insertions(+), 50 deletions(-)
>>>>
>>>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>>>> index 292ea87805..198940e737 100644
>>>> --- a/hw/block/fdc.c
>>>> +++ b/hw/block/fdc.c
>>>> @@ -874,7 +874,6 @@ struct FDCtrl {
>>>> FloppyDriveType type;
>>>> } qdev_for_drives[MAX_FD];
>>>> int reset_sensei;
>>>> - uint32_t check_media_rate;
>>>
>>> I am a bit of a dunce when it comes to the compatibility properties...
>>> does this mess with the migration format?
>>>
>>> I guess it doesn't, since it's not in the VMSTATE declaration.
>>>
>>> Hmmmm, alright.
>>
>> I think that should be fine, yes.
>>
>>>> FloppyDriveType fallback; /* type=auto failure fallback */
>>>> /* Timers state */
>>>> uint8_t timer0;
>>>> @@ -1021,18 +1020,10 @@ static const VMStateDescription
>>>> vmstate_fdrive_media_changed = {
>>>> }
>>>> };
>>>> -static bool fdrive_media_rate_needed(void *opaque)
>>>> -{
>>>> - FDrive *drive = opaque;
>>>> -
>>>> - return drive->fdctrl->check_media_rate;
>>>> -}
>>>> -
>>>> static const VMStateDescription vmstate_fdrive_media_rate = {
>>>> .name = "fdrive/media_rate",
>>>> .version_id = 1,
>>>> .minimum_version_id = 1,
>>>> - .needed = fdrive_media_rate_needed,
>>>> .fields = (VMStateField[]) {
>>>> VMSTATE_UINT8(media_rate, FDrive),
>>>> VMSTATE_END_OF_LIST()
>>>> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl,
>>>> int direction)
>>>> /* Check the data rate. If the programmed data rate does not match
>>>> * the currently inserted medium, the operation has to fail. */
>>>> - if (fdctrl->check_media_rate &&
>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
>>>> fdctrl->dsr & FD_DSR_DRATEMASK,
>>>> cur_drv->media_rate);
>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
>>>> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
>>>> }
>>>> /* READ_ID can't automatically succeed! */
>>>> - if (fdctrl->check_media_rate &&
>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
>>>> fdctrl->dsr & FD_DSR_DRATEMASK,
>>>> cur_drv->media_rate);
>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
>>>> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
>>>> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus,
>>>> state.qdev_for_drives[0].blk),
>>>> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus,
>>>> state.qdev_for_drives[1].blk),
>>>> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus,
>>>> state.check_media_rate,
>>>> - 0, true),
>>>
>>> Could you theoretically set this via QOM commands in QMP, and claim that
>>> this is a break in behavior?
>>>
>>> Though, it's ENTIRELY undocumented, so ... it's probably fine, I think.
>>> Probably. (Please soothe my troubled mind.)
>>
>> A user actually could mess with this property even on the command line,
>> e.g. by using:
>>
>> qemu-system-x86_64 -global isa-fdc.check_media_rate=false
>>
>> ... but, as you said, it's completely undocumented, the property is really
>> just there for the internal use of machine type compatibility. We've done
>> such clean-ups in the past already, see e.g. c6026998eef382d7ad76 or
>> 2a4dbaf1c0db2453ab78f, so I think this should be fine. But if you
>> disagree, I could replace this by a patch that adds this property to the
>> list of deprecated features instead, so we could at least remove it after
>> it has been deprecated for two releases?
>>
>
> I don't think it's necessary, personally -- just wanted to make sure I knew
> the exact stakes here.
>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Acked-by: John Snow <jsnow@redhat.com>
Thanks! ... since the first patch has already been merged through Michael's
tree, could you then please take this patch here through your floppy / block
branch, John? Or maybe it could also go via qemu-trivial?
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-08 7:05 ` Thomas Huth
@ 2021-02-13 22:41 ` Laurent Vivier
2021-02-15 18:11 ` John Snow
0 siblings, 1 reply; 17+ messages in thread
From: Laurent Vivier @ 2021-02-13 22:41 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin, QEMU Trivial,
David Hildenbrand, Gerd Hoffmann
Le 08/02/2021 à 08:05, Thomas Huth a écrit :
> On 05/02/2021 21.15, John Snow wrote:
>> On 2/5/21 1:37 AM, Thomas Huth wrote:
>>> On 05/02/2021 01.40, John Snow wrote:
>>>> On 2/3/21 12:18 PM, Thomas Huth wrote:
>>>>> This was only required for the pc-1.0 and earlier machine types.
>>>>> Now that these have been removed, we can also drop the corresponding
>>>>> code from the FDC device.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>> hw/block/fdc.c | 17 ++---------------
>>>>> tests/qemu-iotests/172.out | 35 -----------------------------------
>>>>> 2 files changed, 2 insertions(+), 50 deletions(-)
>>>>>
>>>>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>>>>> index 292ea87805..198940e737 100644
>>>>> --- a/hw/block/fdc.c
>>>>> +++ b/hw/block/fdc.c
>>>>> @@ -874,7 +874,6 @@ struct FDCtrl {
>>>>> FloppyDriveType type;
>>>>> } qdev_for_drives[MAX_FD];
>>>>> int reset_sensei;
>>>>> - uint32_t check_media_rate;
>>>>
>>>> I am a bit of a dunce when it comes to the compatibility properties... does this mess with the
>>>> migration format?
>>>>
>>>> I guess it doesn't, since it's not in the VMSTATE declaration.
>>>>
>>>> Hmmmm, alright.
>>>
>>> I think that should be fine, yes.
>>>
>>>>> FloppyDriveType fallback; /* type=auto failure fallback */
>>>>> /* Timers state */
>>>>> uint8_t timer0;
>>>>> @@ -1021,18 +1020,10 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
>>>>> }
>>>>> };
>>>>> -static bool fdrive_media_rate_needed(void *opaque)
>>>>> -{
>>>>> - FDrive *drive = opaque;
>>>>> -
>>>>> - return drive->fdctrl->check_media_rate;
>>>>> -}
>>>>> -
>>>>> static const VMStateDescription vmstate_fdrive_media_rate = {
>>>>> .name = "fdrive/media_rate",
>>>>> .version_id = 1,
>>>>> .minimum_version_id = 1,
>>>>> - .needed = fdrive_media_rate_needed,
>>>>> .fields = (VMStateField[]) {
>>>>> VMSTATE_UINT8(media_rate, FDrive),
>>>>> VMSTATE_END_OF_LIST()
>>>>> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>>>>> /* Check the data rate. If the programmed data rate does not match
>>>>> * the currently inserted medium, the operation has to fail. */
>>>>> - if (fdctrl->check_media_rate &&
>>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
>>>>> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
>>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>>> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
>>>>> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
>>>>> }
>>>>> /* READ_ID can't automatically succeed! */
>>>>> - if (fdctrl->check_media_rate &&
>>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
>>>>> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
>>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>>> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
>>>>> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
>>>>> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
>>>>> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),
>>>>> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
>>>>> - 0, true),
>>>>
>>>> Could you theoretically set this via QOM commands in QMP, and claim that this is a break in
>>>> behavior?
>>>>
>>>> Though, it's ENTIRELY undocumented, so ... it's probably fine, I think. Probably. (Please soothe
>>>> my troubled mind.)
>>>
>>> A user actually could mess with this property even on the command line, e.g. by using:
>>>
>>> qemu-system-x86_64 -global isa-fdc.check_media_rate=false
>>>
>>> ... but, as you said, it's completely undocumented, the property is really just there for the
>>> internal use of machine type compatibility. We've done such clean-ups in the past already, see
>>> e.g. c6026998eef382d7ad76 or 2a4dbaf1c0db2453ab78f, so I think this should be fine. But if you
>>> disagree, I could replace this by a patch that adds this property to the list of deprecated
>>> features instead, so we could at least remove it after it has been deprecated for two releases?
>>>
>>
>> I don't think it's necessary, personally -- just wanted to make sure I knew the exact stakes here.
>>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>> Acked-by: John Snow <jsnow@redhat.com>
>
> Thanks! ... since the first patch has already been merged through Michael's tree, could you then
> please take this patch here through your floppy / block branch, John? Or maybe it could also go via
> qemu-trivial?
Applied to my trivial-patches branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
2021-02-13 22:41 ` Laurent Vivier
@ 2021-02-15 18:11 ` John Snow
0 siblings, 0 replies; 17+ messages in thread
From: John Snow @ 2021-02-15 18:11 UTC (permalink / raw)
To: Laurent Vivier, Thomas Huth, qemu-devel, Paolo Bonzini
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin, QEMU Trivial,
David Hildenbrand, Gerd Hoffmann
On 2/13/21 5:41 PM, Laurent Vivier wrote:
> Le 08/02/2021 à 08:05, Thomas Huth a écrit :
>> On 05/02/2021 21.15, John Snow wrote:
>>> On 2/5/21 1:37 AM, Thomas Huth wrote:
>>>> On 05/02/2021 01.40, John Snow wrote:
>>>>> On 2/3/21 12:18 PM, Thomas Huth wrote:
>>>>>> This was only required for the pc-1.0 and earlier machine types.
>>>>>> Now that these have been removed, we can also drop the corresponding
>>>>>> code from the FDC device.
>>>>>>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>> hw/block/fdc.c | 17 ++---------------
>>>>>> tests/qemu-iotests/172.out | 35 -----------------------------------
>>>>>> 2 files changed, 2 insertions(+), 50 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>>>>>> index 292ea87805..198940e737 100644
>>>>>> --- a/hw/block/fdc.c
>>>>>> +++ b/hw/block/fdc.c
>>>>>> @@ -874,7 +874,6 @@ struct FDCtrl {
>>>>>> FloppyDriveType type;
>>>>>> } qdev_for_drives[MAX_FD];
>>>>>> int reset_sensei;
>>>>>> - uint32_t check_media_rate;
>>>>>
>>>>> I am a bit of a dunce when it comes to the compatibility properties... does this mess with the
>>>>> migration format?
>>>>>
>>>>> I guess it doesn't, since it's not in the VMSTATE declaration.
>>>>>
>>>>> Hmmmm, alright.
>>>>
>>>> I think that should be fine, yes.
>>>>
>>>>>> FloppyDriveType fallback; /* type=auto failure fallback */
>>>>>> /* Timers state */
>>>>>> uint8_t timer0;
>>>>>> @@ -1021,18 +1020,10 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
>>>>>> }
>>>>>> };
>>>>>> -static bool fdrive_media_rate_needed(void *opaque)
>>>>>> -{
>>>>>> - FDrive *drive = opaque;
>>>>>> -
>>>>>> - return drive->fdctrl->check_media_rate;
>>>>>> -}
>>>>>> -
>>>>>> static const VMStateDescription vmstate_fdrive_media_rate = {
>>>>>> .name = "fdrive/media_rate",
>>>>>> .version_id = 1,
>>>>>> .minimum_version_id = 1,
>>>>>> - .needed = fdrive_media_rate_needed,
>>>>>> .fields = (VMStateField[]) {
>>>>>> VMSTATE_UINT8(media_rate, FDrive),
>>>>>> VMSTATE_END_OF_LIST()
>>>>>> @@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>>>>>> /* Check the data rate. If the programmed data rate does not match
>>>>>> * the currently inserted medium, the operation has to fail. */
>>>>>> - if (fdctrl->check_media_rate &&
>>>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>>> FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
>>>>>> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
>>>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>>>> @@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
>>>>>> cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
>>>>>> }
>>>>>> /* READ_ID can't automatically succeed! */
>>>>>> - if (fdctrl->check_media_rate &&
>>>>>> - (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>>> + if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
>>>>>> FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
>>>>>> fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
>>>>>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
>>>>>> @@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
>>>>>> DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
>>>>>> DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
>>>>>> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),
>>>>>> - DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
>>>>>> - 0, true),
>>>>>
>>>>> Could you theoretically set this via QOM commands in QMP, and claim that this is a break in
>>>>> behavior?
>>>>>
>>>>> Though, it's ENTIRELY undocumented, so ... it's probably fine, I think. Probably. (Please soothe
>>>>> my troubled mind.)
>>>>
>>>> A user actually could mess with this property even on the command line, e.g. by using:
>>>>
>>>> qemu-system-x86_64 -global isa-fdc.check_media_rate=false
>>>>
>>>> ... but, as you said, it's completely undocumented, the property is really just there for the
>>>> internal use of machine type compatibility. We've done such clean-ups in the past already, see
>>>> e.g. c6026998eef382d7ad76 or 2a4dbaf1c0db2453ab78f, so I think this should be fine. But if you
>>>> disagree, I could replace this by a patch that adds this property to the list of deprecated
>>>> features instead, so we could at least remove it after it has been deprecated for two releases?
>>>>
>>>
>>> I don't think it's necessary, personally -- just wanted to make sure I knew the exact stakes here.
>>>
>>> Reviewed-by: John Snow <jsnow@redhat.com>
>>> Acked-by: John Snow <jsnow@redhat.com>
>>
>> Thanks! ... since the first patch has already been merged through Michael's tree, could you then
>> please take this patch here through your floppy / block branch, John? Or maybe it could also go via
>> qemu-trivial?
>
> Applied to my trivial-patches branch.
>
> Thanks,
> Laurent
>
Thank you, Laurent!
--js
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-02-15 18:13 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 17:18 [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Thomas Huth
2021-02-03 17:18 ` [PATCH 1/4] hw/i386: Remove the deprecated pc-1.x machine types Thomas Huth
2021-02-03 17:37 ` Daniel P. Berrangé
2021-02-03 17:18 ` [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property Thomas Huth
2021-02-05 0:40 ` John Snow
2021-02-05 6:37 ` Thomas Huth
2021-02-05 20:15 ` John Snow
2021-02-08 7:05 ` Thomas Huth
2021-02-13 22:41 ` Laurent Vivier
2021-02-15 18:11 ` John Snow
2021-02-03 17:18 ` [PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property Thomas Huth
2021-02-03 19:42 ` David Hildenbrand
2021-02-03 17:18 ` [PATCH 4/4] hw/usb/bus: Remove the "full-path" property Thomas Huth
2021-02-04 8:36 ` Gerd Hoffmann
2021-02-04 15:51 ` Thomas Huth
2021-02-05 9:09 ` Gerd Hoffmann
2021-02-05 11:00 ` [PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.