1. 15 5月, 2020 20 次提交
    • P
      various: Remove unnecessary OBJECT() cast · 688ffbb4
      Philippe Mathieu-Daudé 提交于
      The OBJECT() macro is defined as:
      
        #define OBJECT(obj) ((Object *)(obj))
      
      Remove the unnecessary OBJECT() casts when we already know the
      pointer is of Object type.
      
      Patch created mechanically using spatch with this script:
      
        @@
        typedef Object;
        Object *o;
        @@
        -   OBJECT(o)
        +   o
      Acked-by: NCornelia Huck <cohuck@redhat.com>
      Acked-by: NCorey Minyard <cminyard@mvista.com>
      Acked-by: NJohn Snow <jsnow@redhat.com>
      Reviewed-by: NRichard Henderson <richard.henderson@linaro.org>
      Signed-off-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Message-Id: <20200512070020.22782-3-f4bug@amsat.org>
      [Trivial rebase conflict in hw/s390x/sclp.c resolved]
      688ffbb4
    • P
      target: Remove unnecessary CPU() cast · 96449e4a
      Philippe Mathieu-Daudé 提交于
      The CPU() macro is defined as:
      
        #define CPU(obj) ((CPUState *)(obj))
      
      which expands to:
      
        ((CPUState *)object_dynamic_cast_assert((Object *)(obj), (name),
                                                __FILE__, __LINE__, __func__))
      
      This assertion can only fail when @obj points to something other
      than its stated type, i.e. when we're in undefined behavior country.
      
      Remove the unnecessary CPU() casts when we already know the pointer
      is of CPUState type.
      
      Patch created mechanically using spatch with this script:
      
        @@
        typedef CPUState;
        CPUState *s;
        @@
        -   CPU(s)
        +   s
      Acked-by: NDavid Gibson <david@gibson.dropbear.id.au>
      Reviewed-by: NCédric Le Goater <clg@kaod.org>
      Reviewed-by: NRichard Henderson <richard.henderson@linaro.org>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Message-Id: <20200512070020.22782-2-f4bug@amsat.org>
      96449e4a
    • M
      qom: Drop @errp parameter of object_property_del() · df4fe0b2
      Markus Armbruster 提交于
      Same story as for object_property_add(): the only way
      object_property_del() can fail is when the property with this name
      does not exist.  Since our property names are all hardcoded, failure
      is a programming error, and the appropriate way to handle it is
      passing &error_abort.  Most callers do that, the commit before
      previous fixed one that didn't (and got the error handling wrong), and
      the two remaining exceptions ignore errors.
      
      Drop the @errp parameter.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-19-armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      df4fe0b2
    • M
      spapr_pci: Drop some dead error handling · 7ef1553d
      Markus Armbruster 提交于
      chassis_from_bus() uses object_property_get_uint() to get property
      "chassis_nr" of the bridge device.  Failure would be a programming
      error.  Pass &error_abort, and simplify its callers.
      
      Cc: David Gibson <david@gibson.dropbear.id.au>
      Cc: qemu-ppc@nongnu.org
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Acked-by: NDavid Gibson <david@gibson.dropbear.id.au>
      Reviewed-by: NGreg Kurz <groug@kaod.org>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-18-armbru@redhat.com>
      7ef1553d
    • M
      qdev: Unrealize must not fail · b69c3c21
      Markus Armbruster 提交于
      Devices may have component devices and buses.
      
      Device realization may fail.  Realization is recursive: a device's
      realize() method realizes its components, and device_set_realized()
      realizes its buses (which should in turn realize the devices on that
      bus, except bus_set_realized() doesn't implement that, yet).
      
      When realization of a component or bus fails, we need to roll back:
      unrealize everything we realized so far.  If any of these unrealizes
      failed, the device would be left in an inconsistent state.  Must not
      happen.
      
      device_set_realized() lets it happen: it ignores errors in the roll
      back code starting at label child_realize_fail.
      
      Since realization is recursive, unrealization must be recursive, too.
      But how could a partly failed unrealize be rolled back?  We'd have to
      re-realize, which can fail.  This design is fundamentally broken.
      
      device_set_realized() does not roll back at all.  Instead, it keeps
      unrealizing, ignoring further errors.
      
      It can screw up even for a device with no buses: if the lone
      dc->unrealize() fails, it still unregisters vmstate, and calls
      listeners' unrealize() callback.
      
      bus_set_realized() does not roll back either.  Instead, it stops
      unrealizing.
      
      Fortunately, no unrealize method can fail, as we'll see below.
      
      To fix the design error, drop parameter @errp from all the unrealize
      methods.
      
      Any unrealize method that uses @errp now needs an update.  This leads
      us to unrealize() methods that can fail.  Merely passing it to another
      unrealize method cannot cause failure, though.  Here are the ones that
      do other things with @errp:
      
      * virtio_serial_device_unrealize()
      
        Fails when qbus_set_hotplug_handler() fails, but still does all the
        other work.  On failure, the device would stay realized with its
        resources completely gone.  Oops.  Can't happen, because
        qbus_set_hotplug_handler() can't actually fail here.  Pass
        &error_abort to qbus_set_hotplug_handler() instead.
      
      * hw/ppc/spapr_drc.c's unrealize()
      
        Fails when object_property_del() fails, but all the other work is
        already done.  On failure, the device would stay realized with its
        vmstate registration gone.  Oops.  Can't happen, because
        object_property_del() can't actually fail here.  Pass &error_abort
        to object_property_del() instead.
      
      * spapr_phb_unrealize()
      
        Fails and bails out when remove_drcs() fails, but other work is
        already done.  On failure, the device would stay realized with some
        of its resources gone.  Oops.  remove_drcs() fails only when
        chassis_from_bus()'s object_property_get_uint() fails, and it can't
        here.  Pass &error_abort to remove_drcs() instead.
      
      Therefore, no unrealize method can fail before this patch.
      
      device_set_realized()'s recursive unrealization via bus uses
      object_property_set_bool().  Can't drop @errp there, so pass
      &error_abort.
      
      We similarly unrealize with object_property_set_bool() elsewhere,
      always ignoring errors.  Pass &error_abort instead.
      
      Several unrealize methods no longer handle errors from other unrealize
      methods: virtio_9p_device_unrealize(),
      virtio_input_device_unrealize(), scsi_qdev_unrealize(), ...
      Much of the deleted error handling looks wrong anyway.
      
      One unrealize methods no longer ignore such errors:
      usb_ehci_pci_exit().
      
      Several realize methods no longer ignore errors when rolling back:
      v9fs_device_realize_common(), pci_qdev_unrealize(),
      spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(),
      virtio_device_realize().
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-17-armbru@redhat.com>
      b69c3c21
    • M
      Drop more @errp parameters after previous commit · 40c2281c
      Markus Armbruster 提交于
      Several functions can't fail anymore: ich9_pm_add_properties(),
      device_add_bootindex_property(), ppc_compat_add_property(),
      spapr_caps_add_properties(), PropertyInfo.create().  Drop their @errp
      parameter.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-16-armbru@redhat.com>
      40c2281c
    • M
      qom: Drop parameter @errp of object_property_add() & friends · d2623129
      Markus Armbruster 提交于
      The only way object_property_add() can fail is when a property with
      the same name already exists.  Since our property names are all
      hardcoded, failure is a programming error, and the appropriate way to
      handle it is passing &error_abort.
      
      Same for its variants, except for object_property_add_child(), which
      additionally fails when the child already has a parent.  Parentage is
      also under program control, so this is a programming error, too.
      
      We have a bit over 500 callers.  Almost half of them pass
      &error_abort, slightly fewer ignore errors, one test case handles
      errors, and the remaining few callers pass them to their own callers.
      
      The previous few commits demonstrated once again that ignoring
      programming errors is a bad idea.
      
      Of the few ones that pass on errors, several violate the Error API.
      The Error ** argument must be NULL, &error_abort, &error_fatal, or a
      pointer to a variable containing NULL.  Passing an argument of the
      latter kind twice without clearing it in between is wrong: if the
      first call sets an error, it no longer points to NULL for the second
      call.  ich9_pm_add_properties(), sparc32_ledma_realize(),
      sparc32_dma_realize(), xilinx_axidma_realize(), xilinx_enet_realize()
      are wrong that way.
      
      When the one appropriate choice of argument is &error_abort, letting
      users pick the argument is a bad idea.
      
      Drop parameter @errp and assert the preconditions instead.
      
      There's one exception to "duplicate property name is a programming
      error": the way object_property_add() implements the magic (and
      undocumented) "automatic arrayification".  Don't drop @errp there.
      Instead, rename object_property_add() to object_property_try_add(),
      and add the obvious wrapper object_property_add().
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-15-armbru@redhat.com>
      [Two semantic rebase conflicts resolved]
      d2623129
    • M
      qdev: Clean up qdev_connect_gpio_out_named() · 9f742c28
      Markus Armbruster 提交于
      Both qdev_connect_gpio_out_named() and device_set_realized() put
      objects without a parent into the "/machine/unattached/" orphanage.
      
      qdev_connect_gpio_out_named() needs a lengthy comment to explain how
      it works.  It exploits that object_property_add_child() can fail only
      when we got a parent already, and ignoring that error does what we
      want.  True.  If it failed due to "duplicate property", we'd be in
      trouble, but that would be a programming error.
      
      device_set_realized() is cleaner: it checks whether we need a parent,
      then calls object_property_add_child(), aborting on failure.  No need
      for a comment, and programming errors get caught.
      
      Change qdev_connect_gpio_out_named() to match.
      
      Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Message-Id: <20200505152926.18877-14-armbru@redhat.com>
      9f742c28
    • M
      hw/arm/bcm2835: Drop futile attempts at QOM-adopting memory · 5462cc81
      Markus Armbruster 提交于
      The "bcm2835-peripherals" device's .instance_init() method
      bcm2835_peripherals_init() attempts to make two memory regions QOM
      children of the device.  This is futile, because memory_region_init()
      already did.  The errors are ignored (a later commit will change
      that).  Drop the useless calls.
      
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
      Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
      Cc: qemu-arm@nongnu.org
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Message-Id: <20200505152926.18877-13-armbru@redhat.com>
      5462cc81
    • M
      e1000: Don't run e1000_instance_init() twice · a13f2042
      Markus Armbruster 提交于
      QOM object initialization runs .instance_init() for the type and all
      its supertypes; see object_init_with_type().
      
      Both TYPE_E1000_BASE and its concrete subtypes set .instance_init() to
      e1000_instance_init().  For the concrete subtypes, it duly gets run
      twice.  The second run fails, but the error gets ignored (a later
      commit will change that).
      
      Remove it from the subtypes.
      
      Cc: Jason Wang <jasowang@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Acked-by: NJason Wang <jasowang@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-12-armbru@redhat.com>
      a13f2042
    • M
      hw/isa/superio: Make the components QOM children · e5084306
      Markus Armbruster 提交于
      isa_superio_realize() attempts to make isa-parallel and isa-serial QOM
      children, but this does not work, because it calls
      object_property_add_child() after realizing with qdev_init_nofail().
      Realizing a device without a parent gives it one: it gets put into the
      "/machine/unattached/" orphanage.  The extra
      object_property_add_child() fails, and isa_superio_realize() ignores
      the error.
      
      Move the object_property_add_child() before qdev_init_nofail(), and
      pass &error_abort.
      
      For the other components, isa_superio_realize() doesn't even try.  Add
      object_property_add_child() there.
      
      This affects machines 40p, clipper and fulong2e.
      
      For instance, fulong2e has its vt82c686b-superio (which is an
      isa-superio) at /machine/unattached/device[9].  Before the patch, its
      components are at /machine/unattached/device[10] .. [14].  Afterwards,
      they are at
      /machine/unattached/device[9]/{parallel0,serial0,serial1,isa-fdc,i8042}.
      
      Cc: Michael S. Tsirkin <mst@redhat.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Tested-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-11-armbru@redhat.com>
      e5084306
    • M
      s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256 · e274408c
      Markus Armbruster 提交于
      Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and
      s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is
      "pcc-cmac-eaes-256".  The former is obviously a pasto.
      
      Impact:
      
      * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256
        as "pcc-cmac-eaes-256".  Affects QMP commands query-cpu-definitions,
        query-cpu-model-expansion, query-cpu-model-baseline,
        query-cpu-model-comparison, and the error message when
        s390_realize_cpu_model() fails in check_compatibility().
      
      * s390_cpu_list() also misidentifies it.  Affects -cpu help.
      
      * s390_cpu_model_register_props() creates CPU property
        "pcc-cmac-eaes-256" twice.  The second one fails, but the error is
        ignored (a later commit will change that).  Results in a single
        property "pcc-cmac-eaes-256" with the description for
        S390_FEAT_PCC_CMAC_AES_256, and no property for
        S390_FEAT_PCC_CMAC_EAES_256.  CPU properties are visible in CLI -cpu
        and -device, QMP & HMP device_add, QMP device-list-properties, and
        QOM introspection.
      
      The two features are almost always used via their group msa4.  Such
      use is not affected by this bug.
      
      Fix by deleting the wayward 'e'.
      
      Fixes: 78241744 ("s390x/cpumodel: introduce CPU features")
      Cc: Halil Pasic <pasic@linux.ibm.com>
      Cc: Cornelia Huck <cohuck@redhat.com>
      Cc: Christian Borntraeger <borntraeger@de.ibm.com>
      Cc: Richard Henderson <rth@twiddle.net>
      Cc: David Hildenbrand <david@redhat.com>
      Cc: qemu-s390x@nongnu.org
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NDavid Hildenbrand <david@redhat.com>
      Tested-by: NChristian Borntraeger <borntraeger@de.ibm.com>
      Message-Id: <20200505152926.18877-10-armbru@redhat.com>
      Reviewed-by: NCornelia Huck <cohuck@redhat.com>
      [Lost paragraph in commit message restored, Fixes: tweaked]
      e274408c
    • M
      tests/check-qom-proplist: Improve iterator coverage · 48942138
      Markus Armbruster 提交于
      The tests' "qemu-dummy" device has only class properties.  Turn one of
      them into an instance property.  test_dummy_class_iterator() expects
      one fewer property than test_dummy_iterator().  Rewrite
      test_dummy_prop_iterator() to take expected properties as argument.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-9-armbru@redhat.com>
      48942138
    • M
      qom: Drop object_property_set_description() parameter @errp · 7eecec7d
      Markus Armbruster 提交于
      object_property_set_description() and
      object_class_property_set_description() fail only when property @name
      is not found.
      
      There are 85 calls of object_property_set_description() and
      object_class_property_set_description().  None of them can fail:
      
      * 84 immediately follow the creation of the property.
      
      * The one in spapr_rng_instance_init() refers to a property created in
        spapr_rng_class_init(), from spapr_rng_properties[].
      
      Every one of them still gets to decide what to pass for @errp.
      
      51 calls pass &error_abort, 32 calls pass NULL, one receives the error
      and propagates it to &error_abort, and one propagates it to
      &error_fatal.  I'm actually surprised none of them violates the Error
      API.
      
      What are we gaining by letting callers handle the "property not found"
      error?  Use when the property is not known to exist is simpler: you
      don't have to guard the call with a check.  We haven't found such a
      use in 5+ years.  Until we do, let's make life a bit simpler and drop
      the @errp parameter.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-8-armbru@redhat.com>
      [One semantic rebase conflict resolved]
      7eecec7d
    • M
      qom: Make all the object_property_add_FOO() return the property · 70251887
      Markus Armbruster 提交于
      Some object_property_add_FOO() return the newly added property, some
      don't.  Clean that up.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-7-armbru@redhat.com>
      70251887
    • M
      qom: Drop convenience method object_property_get_uint16List() · 44a17fe0
      Markus Armbruster 提交于
      qom/object.c provides object_property_get_TYPE() and
      object_property_set_TYPE() for a number of common types.  These are
      all convenience wrappers around object_property_get_qobject() and
      object_property_set_qobject().
      
      Except for object_property_get_uint16List(), which is unusual in two ways:
      
      * It bypasses object_property_get_qobject().  Fixable; the previous
        commit did it for object_property_get_enum())
      
      * It stores the value through a parameter.  Its contract claims it
        returns the value, like the other functions do.  Also fixable.
      
      Fixing is not worthwhile, though: object_property_get_uint16List() has
      seen exactly one user in six years.
      
      Convert the lone user to do its job with the generic
      object_property_get_qobject(), and drop object_property_get_uint16List().
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20200505152926.18877-6-armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      [Commit message typo fixed]
      44a17fe0
    • M
      qom: Simplify object_property_get_enum() · b555f89f
      Markus Armbruster 提交于
      Reuse object_property_get_str().  Switches from the string to the
      qobject visitor under the hood.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20200505152926.18877-5-armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      b555f89f
    • M
      qom: Drop object_property_del_child()'s unused parameter @errp · f73a32a5
      Markus Armbruster 提交于
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-4-armbru@redhat.com>
      f73a32a5
    • M
      qom: Clean up inconsistent use of gchar * vs. char * · ddfb0baa
      Markus Armbruster 提交于
      Uses of gchar * in qom/object.h:
      
      * ObjectProperty member @name
      
        Functions that take a property name argument all use char *.  Change
        the member to match.
      
      * ObjectProperty member @type
      
        Functions that take a property type argument or return it all use
        char *.  Change the member to match.
      
      * ObjectProperty member @description
      
        Functions that take a property description argument all use char *.
        Change the member to match.
      
      * object_resolve_path_component() parameter @part
      
        Path components are property names.  Most callers pass char *
        arguments.  Change the parameter to match.  Adjust the few callers
        that pass gchar * to pass char *.
      
      * Return value of object_get_canonical_path_component(),
        object_get_canonical_path()
      
        Most callers convert their return values right back to char *.
        Change the return value to match.  Adjust the few callers where that
        would add a conversion to gchar * to use char * instead.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-3-armbru@redhat.com>
      ddfb0baa
    • M
      qom: Clearer reference counting in object_initialize_childv() · 975ac455
      Markus Armbruster 提交于
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-2-armbru@redhat.com>
      975ac455
  2. 14 5月, 2020 20 次提交