These functions all return NULL rather than asserting, if the requested
type is not registered and also cannot be dynamically loaded.

In several cases their usage is pointless, since the caller then just
reports an error & exits anyway. Easier to just let qdev_new fail
with &error_fatal.

In other cases, the desired semantics are clearer to understand if the
caller directly checks module_object_class_by_name(), before calling
the regular qdev_new (or specialized equiv) function.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/core/qdev.c              |  9 ---------
 hw/i386/pc.c                | 22 ++++++++++------------
 hw/isa/isa-bus.c            |  5 -----
 hw/s390x/s390-pci-bus.c     |  6 +-----
 hw/usb/bus.c                |  6 ++----
 include/hw/isa/isa.h        |  1 -
 include/hw/net/ne2000-isa.h | 16 ++++++----------
 include/hw/qdev-core.h      | 12 ------------
 include/hw/usb.h            |  5 -----
 9 files changed, 19 insertions(+), 63 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5f13111b77..960a704a96 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -149,15 +149,6 @@ DeviceState *qdev_new(const char *name)
     return DEVICE(object_new(name));
 }
 
-DeviceState *qdev_try_new(const char *name)
-{
-    ObjectClass *oc = module_object_class_by_name(name);
-    if (!oc) {
-        return NULL;
-    }
-    return DEVICE(object_new_with_class(oc));
-}
-
 static QTAILQ_HEAD(, DeviceListener) device_listeners
     = QTAILQ_HEAD_INITIALIZER(device_listeners);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f9147fecbd..d668970bee 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -596,9 +596,11 @@ static gboolean pc_init_ne2k_isa(ISABus *bus, NICInfo *nd, Error **errp)
                    "maximum number of ISA NE2000 devices exceeded");
         return false;
     }
-    isa_ne2000_init(bus, ne2000_io[nb_ne2k],
-                    ne2000_irq[nb_ne2k], nd);
-    nb_ne2k++;
+    if (module_object_class_by_name(TYPE_ISA_NE2000)) {
+        isa_ne2000_init(bus, ne2000_io[nb_ne2k],
+                        ne2000_irq[nb_ne2k], nd);
+        nb_ne2k++;
+    }
     return true;
 }
 
@@ -1087,7 +1089,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
     int i;
     DriveInfo *fd[MAX_FD];
     qemu_irq *a20_line;
-    ISADevice *i8042, *port92, *vmmouse;
+    ISADevice *i8042, *port92, *vmmouse = NULL;
 
     serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS);
     parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
@@ -1117,9 +1119,9 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
     i8042 = isa_create_simple(isa_bus, TYPE_I8042);
     if (!no_vmport) {
         isa_create_simple(isa_bus, TYPE_VMPORT);
-        vmmouse = isa_try_new("vmmouse");
-    } else {
-        vmmouse = NULL;
+        if (module_object_class_by_name("vmmouse")) {
+            vmmouse = isa_new("vmmouse");
+        }
     }
     if (vmmouse) {
         object_property_set_link(OBJECT(vmmouse), TYPE_I8042, OBJECT(i8042),
@@ -1163,11 +1165,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     if (pcms->hpet_enabled) {
         qemu_irq rtc_irq;
 
-        hpet = qdev_try_new(TYPE_HPET);
-        if (!hpet) {
-            error_report("couldn't create HPET device");
-            exit(1);
-        }
+        hpet = qdev_new(TYPE_HPET);
         /*
          * For pc-piix-*, hpet's intcap is always IRQ2. For pc-q35-*,
          * use IRQ16~23, IRQ8 and IRQ2.  If the user has already set
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index f1e0f14007..8aaf44a3ef 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -158,11 +158,6 @@ ISADevice *isa_new(const char *name)
     return ISA_DEVICE(qdev_new(name));
 }
 
-ISADevice *isa_try_new(const char *name)
-{
-    return ISA_DEVICE(qdev_try_new(name));
-}
-
 ISADevice *isa_create_simple(ISABus *bus, const char *name)
 {
     ISADevice *dev;
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 40b2567aa7..558f17d3ba 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -922,11 +922,7 @@ static S390PCIBusDevice *s390_pci_device_new(S390pciState *s,
     Error *local_err = NULL;
     DeviceState *dev;
 
-    dev = qdev_try_new(TYPE_S390_PCI_DEVICE);
-    if (!dev) {
-        error_setg(errp, "zPCI device could not be created");
-        return NULL;
-    }
+    dev = qdev_new(TYPE_S390_PCI_DEVICE);
 
     if (!object_property_set_str(OBJECT(dev), "target", target, &local_err)) {
         object_unparent(OBJECT(dev));
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index bfab2807d7..002f7737e4 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -394,7 +394,6 @@ void usb_claim_port(USBDevice *dev, Error **errp)
 {
     USBBus *bus = usb_bus_from_device(dev);
     USBPort *port;
-    USBDevice *hub;
 
     assert(dev->port == NULL);
 
@@ -412,9 +411,8 @@ void usb_claim_port(USBDevice *dev, Error **errp)
     } else {
         if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), "usb-hub") != 0) {
             /* Create a new hub and chain it on */
-            hub = usb_try_new("usb-hub");
-            if (hub) {
-                usb_realize_and_unref(hub, bus, NULL);
+            if (module_object_class_by_name("usb-hub")) {
+                usb_realize_and_unref(usb_new("usb-hub"), bus, NULL);
             }
         }
         if (bus->nfree == 0) {
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 40d6224a4e..8475120849 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -81,7 +81,6 @@ IsaDma *isa_bus_get_dma(ISABus *bus, int nchan);
  */
 qemu_irq isa_bus_get_irq(ISABus *bus, unsigned irqnum);
 ISADevice *isa_new(const char *name);
-ISADevice *isa_try_new(const char *name);
 bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
 
diff --git a/include/hw/net/ne2000-isa.h b/include/hw/net/ne2000-isa.h
index 73bae10ad1..aeb3e2bfd5 100644
--- a/include/hw/net/ne2000-isa.h
+++ b/include/hw/net/ne2000-isa.h
@@ -20,17 +20,13 @@
 static inline ISADevice *isa_ne2000_init(ISABus *bus, int base, int irq,
                                          NICInfo *nd)
 {
-    ISADevice *d;
+    ISADevice *d = isa_new(TYPE_ISA_NE2000);
+    DeviceState *dev = DEVICE(d);
 
-    d = isa_try_new(TYPE_ISA_NE2000);
-    if (d) {
-        DeviceState *dev = DEVICE(d);
-
-        qdev_prop_set_uint32(dev, "iobase", base);
-        qdev_prop_set_uint32(dev, "irq",    irq);
-        qdev_set_nic_properties(dev, nd);
-        isa_realize_and_unref(d, bus, &error_fatal);
-    }
+    qdev_prop_set_uint32(dev, "iobase", base);
+    qdev_prop_set_uint32(dev, "irq",    irq);
+    qdev_set_nic_properties(dev, nd);
+    isa_realize_and_unref(d, bus, &error_fatal);
     return d;
 }
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5be9844412..020417d027 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -443,18 +443,6 @@ compat_props_add(GPtrArray *arr,
  */
 DeviceState *qdev_new(const char *name);
 
-/**
- * qdev_try_new: Try to create a device on the heap
- * @name: device type to create
- *
- * This is like qdev_new(), except it returns %NULL when type @name
- * does not exist, rather than asserting.
- *
- * Return: a derived DeviceState object with a reference count of 1 or
- * NULL if type @name does not exist.
- */
-DeviceState *qdev_try_new(const char *name);
-
 /**
  * qdev_is_realized() - check if device is realized
  * @dev: The device to check.
diff --git a/include/hw/usb.h b/include/hw/usb.h
index d46d96779a..bb778cb844 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -584,11 +584,6 @@ static inline USBDevice *usb_new(const char *name)
     return USB_DEVICE(qdev_new(name));
 }
 
-static inline USBDevice *usb_try_new(const char *name)
-{
-    return USB_DEVICE(qdev_try_new(name));
-}
-
 static inline bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
 {
     return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
-- 
2.46.0

On Fri, Nov 15, 2024 at 05:25:13PM +0000, Daniel P. Berrangé wrote:
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 40b2567aa7..558f17d3ba 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -922,11 +922,7 @@ static S390PCIBusDevice *s390_pci_device_new(S390pciState *s,
>      Error *local_err = NULL;
>      DeviceState *dev;
>  
> -    dev = qdev_try_new(TYPE_S390_PCI_DEVICE);
> -    if (!dev) {
> -        error_setg(errp, "zPCI device could not be created");
> -        return NULL;
> -    }
> +    dev = qdev_new(TYPE_S390_PCI_DEVICE);

This one used to allow failures, but now it asserts.  Especially, see:

b6e67ecc7b ("s390x/pci: properly fail if the zPCI device cannot be created")

Would it be safer to use module_object_class_by_name() too here?

All the rest changes look sane.

>  
>      if (!object_property_set_str(OBJECT(dev), "target", target, &local_err)) {
>          object_unparent(OBJECT(dev));

-- 
Peter Xu

On Fri, Nov 15, 2024 at 12:54:10PM -0500, Peter Xu wrote:
> On Fri, Nov 15, 2024 at 05:25:13PM +0000, Daniel P. Berrangé wrote:
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > index 40b2567aa7..558f17d3ba 100644
> > --- a/hw/s390x/s390-pci-bus.c
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -922,11 +922,7 @@ static S390PCIBusDevice *s390_pci_device_new(S390pciState *s,
> >      Error *local_err = NULL;
> >      DeviceState *dev;
> >  
> > -    dev = qdev_try_new(TYPE_S390_PCI_DEVICE);
> > -    if (!dev) {
> > -        error_setg(errp, "zPCI device could not be created");
> > -        return NULL;
> > -    }
> > +    dev = qdev_new(TYPE_S390_PCI_DEVICE);
> 
> This one used to allow failures, but now it asserts.  Especially, see:
> 
> b6e67ecc7b ("s390x/pci: properly fail if the zPCI device cannot be created")
> 
> Would it be safer to use module_object_class_by_name() too here?

Yes, my bad. I was mixed up with the HPET case which did exit(), this
one must propagate the error.


With 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 :|