On Fri, Jul 31, 2020 at 02:20:16PM -0400, Jagannathan Raman wrote: > +static void remote_object_set_devid(Object *obj, const char *str, Error **errp) > +{ > + RemoteObject *o = REMOTE_OBJECT(obj); > + > + o->devid = g_strdup(str); Setter functions can be called multiple times so g_free(o->devid) is needed to prevent memory leaks. > +} > + > +static void remote_object_machine_done(Notifier *notifier, void *data) > +{ Is UserCreatableClass->complete() called too early? If you can use it instead of the machine init done notifier then that would make error reporting cleaner. Both command-line and object_add monitor commands would fail as expected if an error occurs, whereas this patch prints an error and continues running. > + RemoteObject *o = container_of(notifier, RemoteObject, machine_done); > + DeviceState *dev = NULL; > + QIOChannel *ioc = NULL; > + Error *err = NULL; > + > + dev = qdev_find_recursive(sysbus_get_default(), o->devid); > + if (!dev || !object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + error_report("%s is not a PCI device", o->devid); > + return; > + } What happens when a "device_del" monitor command removes the device? I guess a use-after-free will occur since this object isn't aware that the device disappeared. QOM's Object has a reference count that you can use to keep the object alive, but that still might not play well with hot unplug where the device is being removed from its bus. One solution is to refuse hot unplugging the device while the remote object exists. > + > + ioc = qio_channel_new_fd(o->fd, &err); > + if (!ioc) { > + error_free(err); > + error_report("Failed to allocate IO channel"); Please print err, it contains a more detailed error message than "Failed to allocate IO channel".