From 3b19f4506901ecce25ff36cf62353a2b4bfe4f2b Mon Sep 17 00:00:00 2001 From: Daniel P. Berrange Date: Wed, 4 Oct 2017 12:40:08 +0100 Subject: ide: support reporting of rotation rate The Linux kernel will query the ATA IDENTITY DEVICE data, word 217 to determine the rotations per minute of the disk. If this has the value 1, it is taken to be an SSD and so Linux sets the 'rotational' flag to 0 for the I/O queue and will stop using that disk as a source of random entropy. Other operating systems may also take into account rotation rate when setting up default behaviour. Mgmt apps should be able to set the rotation rate for virtualized block devices, based on characteristics of the host storage in use, so that the guest OS gets sensible behaviour out of the box. This patch thus adds a 'rotation-rate' parameter for 'ide-hd' device types. Signed-off-by: Daniel P. Berrange Message-Id: <20171004114008.14849-3-berrange@redhat.com> Reviewed-by: John Snow Signed-off-by: Paolo Bonzini --- include/hw/ide/internal.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'include') diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h index e641012b48..31851b44d1 100644 --- a/include/hw/ide/internal.h +++ b/include/hw/ide/internal.h @@ -508,6 +508,14 @@ struct IDEDevice { char *serial; char *model; uint64_t wwn; + /* + * 0x0000 - rotation rate not reported + * 0x0001 - non-rotating medium (SSD) + * 0x0002-0x0400 - reserved + * 0x0401-0xffe - rotations per minute + * 0xffff - reserved + */ + uint16_t rotation_rate; }; /* These are used for the error_status field of IDEBus */ -- cgit v1.2.3-55-g7522 From eb584b401fdc0866d2ff0c03ab8b09d2ba04a49b Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 27 Sep 2017 16:58:33 +0200 Subject: disas: Always initialize read_memory_inner_func properly I've recently seen this with valgrind while running the HMP tester: ==22373== Conditional jump or move depends on uninitialised value(s) ==22373== at 0x4A41FD: arm_disas_set_info (cpu.c:504) ==22373== by 0x3867A7: monitor_disas (disas.c:390) ==22373== by 0x38E80E: memory_dump (monitor.c:1339) ==22373== by 0x38FA43: handle_hmp_command (monitor.c:3123) ==22373== by 0x38FB9E: qmp_human_monitor_command (monitor.c:613) ==22373== by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736) ==22373== by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104) ==22373== by 0x769678: qmp_dispatch (qmp-dispatch.c:131) ==22373== by 0x38B734: handle_qmp_command (monitor.c:3853) ==22373== by 0x76ED07: json_message_process_token (json-streamer.c:105) ==22373== by 0x78D40A: json_lexer_feed_char (json-lexer.c:323) ==22373== by 0x78D4CD: json_lexer_feed (json-lexer.c:373) ==22373== by 0x38A08D: monitor_qmp_read (monitor.c:3895) And indeed, in monitor_disas, the read_memory_inner_func variable was not initialized, but arm_disas_set_info() expects this to be NULL or a valid pointer. Let's properly set this to NULL in the INIT_DISASSEMBLE_INFO to fix it in all functions that use the disassemble_info struct. Fixes: f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution") Signed-off-by: Thomas Huth Message-Id: <1506524313-20037-1-git-send-email-thuth@redhat.com> --- disas.c | 1 - include/disas/bfd.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/disas.c b/disas.c index d6a1eb9c8e..54eea3f9c9 100644 --- a/disas.c +++ b/disas.c @@ -190,7 +190,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code, s.cpu = cpu; s.info.read_memory_func = target_read_memory; - s.info.read_memory_inner_func = NULL; s.info.buffer_vma = code; s.info.buffer_length = size; s.info.print_address_func = generic_print_address; diff --git a/include/disas/bfd.h b/include/disas/bfd.h index b01e002b4c..d99da68267 100644 --- a/include/disas/bfd.h +++ b/include/disas/bfd.h @@ -479,6 +479,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *); (INFO).buffer_vma = 0, \ (INFO).buffer_length = 0, \ (INFO).read_memory_func = buffer_read_memory, \ + (INFO).read_memory_inner_func = NULL, \ (INFO).memory_error_func = perror_memory, \ (INFO).print_address_func = generic_print_address, \ (INFO).print_insn = NULL, \ -- cgit v1.2.3-55-g7522 From 04162f8f4bcf8c9ae2422def4357289b44208c8c Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Mon, 16 Oct 2017 17:23:13 -0500 Subject: qdev: store DeviceState's canonical path to use when unparenting device_unparent(dev, ...) is called when a device is unparented, either directly, or as a result of a parent device being finalized, and handles some final cleanup for the device. Part of this includes emiting a DEVICE_DELETED QMP event to notify management, which includes the device's path in the composition tree as provided by object_get_canonical_path(). object_get_canonical_path() assumes the device is still connected to the machine/root container, and will assert otherwise, but in some situations this isn't the case: If the parent is finalized as a result of object_unparent(), it will still be attached to the composition tree at the time any children are unparented as a result of that same call to object_unparent(). However, in some cases, object_unparent() will complete without finalizing the parent device, due to lingering references that won't be released till some time later. One such example is if the parent has MemoryRegion children (which take a ref on their parent), who in turn have AddressSpace's (which take a ref on their regions), since those AddressSpaces get cleaned up asynchronously by the RCU thread. In this case qdev:device_unparent() may be called for a child Device that no longer has a path to the root/machine container, causing object_get_canonical_path() to assert. Fix this by storing the canonical path during realize() so the information will still be available for device_unparent() in such cases. Cc: Michael S. Tsirkin Cc: Paolo Bonzini Signed-off-by: Michael Roth Signed-off-by: Greg Kurz Signed-off-by: Michael Roth Tested-by: Eric Auger Reviewed-by: David Gibson Message-Id: <20171016222315.407-2-mdroth@linux.vnet.ibm.com> [Clear dev->canonical_path at the post_realize_fail label, which is cleaner. Suggested by David Gibson. - Paolo] Signed-off-by: Paolo Bonzini --- hw/core/qdev.c | 17 ++++++++++++++--- include/hw/qdev-core.h | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 606ab53c42..0019a49503 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -928,6 +928,13 @@ static void device_set_realized(Object *obj, bool value, Error **errp) goto post_realize_fail; } + /* + * always free/re-initialize here since the value cannot be cleaned up + * in device_unrealize due to its usage later on in the unplug path + */ + g_free(dev->canonical_path); + dev->canonical_path = object_get_canonical_path(OBJECT(dev)); + if (qdev_get_vmsd(dev)) { if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev, dev->instance_id_alias, @@ -984,6 +991,8 @@ child_realize_fail: } post_realize_fail: + g_free(dev->canonical_path); + dev->canonical_path = NULL; if (dc->unrealize) { dc->unrealize(dev, NULL); } @@ -1102,10 +1111,12 @@ static void device_unparent(Object *obj) /* Only send event if the device had been completely realized */ if (dev->pending_deleted_event) { - gchar *path = object_get_canonical_path(OBJECT(dev)); + g_assert(dev->canonical_path); - qapi_event_send_device_deleted(!!dev->id, dev->id, path, &error_abort); - g_free(path); + qapi_event_send_device_deleted(!!dev->id, dev->id, dev->canonical_path, + &error_abort); + g_free(dev->canonical_path); + dev->canonical_path = NULL; } qemu_opts_del(dev->opts); diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 089146197f..0a71bf83f0 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -153,6 +153,7 @@ struct DeviceState { /*< public >*/ const char *id; + char *canonical_path; bool realized; bool pending_deleted_event; QemuOpts *opts; -- cgit v1.2.3-55-g7522