[Qemu-commits] [qemu/qemu] 12b2e9: qdev/core: fix qbus_is_full()

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 12b2e9f30f978f26f35f9df5c2ef96fbc019bab6
      
https://github.com/qemu/qemu/commit/12b2e9f30f978f26f35f9df5c2ef96fbc019bab6
  Author: Tony Krowiak <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M hw/core/qdev.c
    M include/hw/qdev-core.h
    M qdev-monitor.c

  Log Message:
  -----------
  qdev/core: fix qbus_is_full()

The qbus_is_full(BusState *bus) function (qdev_monitor.c) compares the max_index
value of the BusState structure with the max_dev value of the BusClass structure
to determine whether the maximum number of children has been reached for the
bus. The problem is, the max_index field of the BusState structure does not
necessarily reflect the number of devices that have been plugged into
the bus.

Whenever a child device is plugged into the bus, the bus's max_index value is
assigned to the child device and then incremented. If the child is subsequently
unplugged, the value of the max_index does not change and no longer reflects the
number of children.

When the bus's max_index value reaches the maximum number of devices
allowed for the bus (i.e., the max_dev field in the BusClass structure),
attempts to plug another device will be rejected claiming that the bus is
full -- even if the bus is actually empty.

To resolve the problem, a new 'num_children' field is being added to the
BusState structure to keep track of the number of children plugged into the
bus. It will be incremented when a child is plugged, and decremented when a
child is unplugged.

Signed-off-by: Tony Krowiak <address@hidden>
Reviewed-by: Pierre Morel<address@hidden>
Reviewed-by: Halil Pasic <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Igor Mammedov <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>


  Commit: 15160ab72ca48c86800b9227dfe806f27f7caf08
      
https://github.com/qemu/qemu/commit/15160ab72ca48c86800b9227dfe806f27f7caf08
  Author: Igor Mammedov <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M backends/hostmem.c

  Log Message:
  -----------
  hostmem: fix crash when querying empty host-nodes property via QMP

QEMU will crashes with
 qapi/qobject-output-visitor.c:210: qobject_output_complete: Assertion 
`qov->root && ((&qov->stack)->slh_first == ((void *)0))' failed
when trying to get value of not set hostmem's "host-nodes"
property, HostMemoryBackend::host_nodes bitmap doesn't have
any bits set in it, which leads to find_first_bit() returning
MAX_NODES and consequently to an early return from
host_memory_backend_get_host_nodes() without calling visitor.

Fix it by calling visitor even if "host-nodes" property wasn't
set before exiting from property getter to return valid empty
list.

Signed-off-by: Igor Mammedov <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Stefano Garzarella <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>


  Commit: 07578b0ad6a6792fbe23bcd8477f3331b78f6027
      
https://github.com/qemu/qemu/commit/07578b0ad6a6792fbe23bcd8477f3331b78f6027
  Author: David Hildenbrand <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M hw/acpi/cpu.c
    M hw/acpi/memory_hotplug.c
    M hw/acpi/pcihp.c
    M hw/core/qdev.c
    M hw/i386/pc.c
    M hw/pci/pci.c
    M hw/pci/pcie.c
    M hw/pci/shpc.c
    M hw/ppc/spapr.c
    M hw/ppc/spapr_pci.c
    M hw/s390x/css-bridge.c
    M hw/s390x/s390-pci-bus.c
    M qdev-monitor.c

  Log Message:
  -----------
  qdev: Let the hotplug_handler_unplug() caller delete the device

When unplugging a device, at one point the device will be destroyed
via object_unparent(). This will, one the one hand, unrealize the
removed device hierarchy, and on the other hand, destroy/free the
device hierarchy.

When chaining hotplug handlers, we want to overwrite a bus hotplug
handler by the machine hotplug handler, to be able to perform
some part of the plug/unplug and to forward the calls to the bus hotplug
handler.

For now, the bus hotplug handler would trigger an object_unparent(), not
allowing us to perform some unplug action on a device after we forwarded
the call to the bus hotplug handler. The device would be gone at that
point.

machine_unplug_handler(dev)
    /* eventually do unplug stuff */
    bus_unplug_handler(dev)
    /* dev is gone, we can't do more unplug stuff */

So move the object_unparent() to the original caller of the unplug. For
now, keep the unrealize() at the original places of the
object_unparent(). For implicitly chained hotplug handlers (e.g. pc
code calling acpi hotplug handlers), the object_unparent() has to be
done by the outermost caller. So when calling hotplug_handler_unplug()
from inside an unplug handler, nothing is to be done.

hotplug_handler_unplug(dev) -> calls machine_unplug_handler()
    machine_unplug_handler(dev) {
        /* eventually do unplug stuff */
        bus_unplug_handler(dev) -> calls unrealize(dev)
        /* we can do more unplug stuff but device already unrealized */
    }
object_unparent(dev)

In the long run, every unplug action should be factored out of the
unrealize() function into the unplug handler (especially for PCI). Then
we can get rid of the additonal unrealize() calls and object_unparent()
will properly unrealize the device hierarchy after the device has been
unplugged.

hotplug_handler_unplug(dev) -> calls machine_unplug_handler()
    machine_unplug_handler(dev) {
        /* eventually do unplug stuff */
        bus_unplug_handler(dev) -> only unplugs, does not unrealize
        /* we can do more unplug stuff */
    }
object_unparent(dev) -> will unrealize

The original approach was suggested by Igor Mammedov for the PCI
part, but I extended it to all hotplug handlers. I consider this one
step into the right direction.

To summarize:
- object_unparent() on synchronous unplugs is done by common code
-- "Caller of hotplug_handler_unplug"
- object_unparent() on asynchronous unplugs ("unplug requests") has to
  be done manually
-- "Caller of hotplug_handler_unplug"

Reviewed-by: Igor Mammedov <address@hidden>
Acked-by: Cornelia Huck <address@hidden>
Signed-off-by: David Hildenbrand <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Greg Kurz <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>


  Commit: 17cc0128da3d4a211731a79853ea81c7159b26af
      
https://github.com/qemu/qemu/commit/17cc0128da3d4a211731a79853ea81c7159b26af
  Author: Igor Mammedov <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M hw/core/qdev.c
    M include/hw/qdev-core.h

  Log Message:
  -----------
  qdev: Let machine hotplug handler to override bus hotplug handler

it will allow to return another hotplug handler than the default
one for a specific bus based device type. Which is needed to handle
non trivial plug/unplug sequences that need the access to resources
configured outside of bus where device is attached.

That will allow for returned hotplug handler to orchestrate wiring
in arbitrary order, by chaining other hotplug handlers when
it's needed.

PS:
It could be used for hybrid virtio-mem and virtio-pmem devices
where it will return machine as hotplug handler which will do
necessary wiring at machine level and then pass control down
the chain to bus specific hotplug handler.

Example of top level hotplug handler override and custom plug sequence:

  some_machine_get_hotplug_handler(machine){
      if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) {
          return HOTPLUG_HANDLER(machine);
      }
      return NULL;
  }

  some_machine_device_plug(hotplug_dev, dev) {
      if (object_dynamic_cast(OBJECT(dev), TYPE_SOME_BUS_DEVICE)) {
          /* do machine specific initialization */
          some_machine_init_special_device(dev)

          /* pass control to bus specific handler */
          hotplug_handler_plug(dev->parent_bus->hotplug_handler, dev)
      }
  }

Reviewed-by: David Gibson <address@hidden>
Signed-off-by: Igor Mammedov <address@hidden>
Signed-off-by: David Hildenbrand <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>


  Commit: 14405c274e86e993e90198a49eecab3ca0ded8db
      
https://github.com/qemu/qemu/commit/14405c274e86e993e90198a49eecab3ca0ded8db
  Author: David Hildenbrand <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M hw/core/qdev.c
    M include/hw/qdev-core.h

  Log Message:
  -----------
  qdev: Provide qdev_get_bus_hotplug_handler()

Let's use a wrapper instead of looking it up manually. This function can
than be reused when we explicitly want to have the bus hotplug handler
(e.g. when the bus hotplug handler was overwritten by the machine
hotplug handler).

Reviewed-by: Igor Mammedov <address@hidden>
Signed-off-by: David Hildenbrand <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>


  Commit: 32694e98b8d7a246345448a8f707d2e11d6c65e2
      
https://github.com/qemu/qemu/commit/32694e98b8d7a246345448a8f707d2e11d6c65e2
  Author: Peter Maydell <address@hidden>
  Date:   2019-03-06 (Wed, 06 Mar 2019)

  Changed paths:
    M backends/hostmem.c
    M hw/acpi/cpu.c
    M hw/acpi/memory_hotplug.c
    M hw/acpi/pcihp.c
    M hw/core/qdev.c
    M hw/i386/pc.c
    M hw/pci/pci.c
    M hw/pci/pcie.c
    M hw/pci/shpc.c
    M hw/ppc/spapr.c
    M hw/ppc/spapr_pci.c
    M hw/s390x/css-bridge.c
    M hw/s390x/s390-pci-bus.c
    M include/hw/qdev-core.h
    M qdev-monitor.c

  Log Message:
  -----------
  Merge remote-tracking branch 
'remotes/ehabkost/tags/machine-next-pull-request' into staging

Machine queue, 2019-03-06

* qdev: Hotplug handler chaining (David Hildenbrand)
* qdev: fix qbus_is_full() (Tony Krowiak)
* hostmem: fix crash when querying empty host-nodes property via
  QMP (Igor Mammedov)

# gpg: Signature made Wed 06 Mar 2019 18:39:29 GMT
# gpg:                using RSA key 2807936F984DC5A6
# gpg: Good signature from "Eduardo Habkost <address@hidden>" [full]
# Primary key fingerprint: 5A32 2FD5 ABC4 D3DB ACCF  D1AA 2807 936F 984D C5A6

* remotes/ehabkost/tags/machine-next-pull-request:
  qdev: Provide qdev_get_bus_hotplug_handler()
  qdev: Let machine hotplug handler to override bus hotplug handler
  qdev: Let the hotplug_handler_unplug() caller delete the device
  hostmem: fix crash when querying empty host-nodes property via QMP
  qdev/core: fix qbus_is_full()

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/c557a8c7b755...32694e98b8d7