[PATCH 0/4] Remove the deprecated pc-1.x machine types and related stuff
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
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
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 :|
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
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
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
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>
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
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
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
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
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
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
participants (6)
-
Daniel P. Berrangé -
David Hildenbrand -
Gerd Hoffmann -
John Snow -
Michael S. Tsirkin -
Thomas Huth