Re: [RFC v4 PATCH 10/49] multi-process: setup a machine object for remote device process

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

On Thu, Oct 24, 2019 at 05:08:51AM -0400, Jagannathan Raman wrote:
> +static NotifierList machine_init_done_notifiers =
> +    NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
> +
> +bool machine_init_done;
> +
> +void qemu_add_machine_init_done_notifier(Notifier *notify)
> +{
> +    notifier_list_add(&machine_init_done_notifiers, notify);
> +    if (machine_init_done) {
> +        notify->notify(notify, NULL);
> +    }
> +}
> +
> +void qemu_remove_machine_init_done_notifier(Notifier *notify)
> +{
> +    notifier_remove(notify);
> +}
> +
> +void qemu_run_machine_init_done_notifiers(void)
> +{
> +    machine_init_done = true;
> +    notifier_list_notify(&machine_init_done_notifiers, NULL);
> +}

qemu_add_machine_init_done_notifier() is already defined in vl.c.
Please share the implementation instead of duplicating it into the
remote program.

> +
> +static void remote_machine_init(Object *obj)
> +{
> +    RemMachineState *s = REMOTE_MACHINE(obj);
> +    RemPCIHost *rem_host;
> +    MemoryRegion *system_memory, *system_io, *pci_memory;
> +
> +    Error *error_abort = NULL;
> +
> +    qemu_mutex_init(&ram_list.mutex);

Please keep global initialization separate from RemMachineState (e.g. do
it in main() or a function called by main()).  This function should only
initialize RemMachineState.

> +
> +    object_property_add_child(object_get_root(), "machine", obj, &error_abort);
> +    if (error_abort) {
> +        error_report_err(error_abort);
> +    }
> +
> +    memory_map_init();

This is global init, please move it elsewhere.

> +
> +    system_memory = get_system_memory();
> +    system_io = get_system_io();
> +
> +    pci_memory = g_new(MemoryRegion, 1);
> +    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> +
> +    rem_host = REMOTE_HOST_DEVICE(qdev_create(NULL, TYPE_REMOTE_HOST_DEVICE));
> +
> +    rem_host->mr_pci_mem = pci_memory;
> +    rem_host->mr_sys_mem = system_memory;
> +    rem_host->mr_sys_io = system_io;
> +
> +    s->host = rem_host;

Both s and rem_host are QOM objects.  There should be a child property
relationship between them here.  It will ensure that rem_host is cleaned
up when s is cleaned up.  Please use that instead of a regular C
pointer.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]