diff mbox series

[v5,12/50] multi-process: remote process initialization

Message ID 264de034fcdc1000bada4a93ac7e0856fef591f2.1582576372.git.jag.raman@oracle.com (mailing list archive)
State New, archived
Headers show
Series Initial support for multi-process qemu | expand

Commit Message

Jag Raman Feb. 24, 2020, 8:55 p.m. UTC
Adds the handler to process message from QEMU,
Initialize remote process main loop, handles SYNC_SYSMEM
message by updating its "system_memory" container using
shared file descriptors received from QEMU.

Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
---
 v4 -> v5:
  - We checked if we could use functions already defined in
    util/main-loop.c instead of using g_main_loop_run. However,
    we couldn't find a suitable function that's generic enough
    to do this. All of them have emulator code embedded in them
    which is not used by the remote process. We are therefore
    not making any change to this patch

 remote/remote-main.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

Comments

Dr. David Alan Gilbert March 4, 2020, 10:29 a.m. UTC | #1
* Jagannathan Raman (jag.raman@oracle.com) wrote:
> Adds the handler to process message from QEMU,
> Initialize remote process main loop, handles SYNC_SYSMEM
> message by updating its "system_memory" container using
> shared file descriptors received from QEMU.
> 
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  v4 -> v5:
>   - We checked if we could use functions already defined in
>     util/main-loop.c instead of using g_main_loop_run. However,
>     we couldn't find a suitable function that's generic enough
>     to do this. All of them have emulator code embedded in them
>     which is not used by the remote process. We are therefore
>     not making any change to this patch
> 
>  remote/remote-main.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/remote/remote-main.c b/remote/remote-main.c
> index ecf30e0..56315cd 100644
> --- a/remote/remote-main.c
> +++ b/remote/remote-main.c
> @@ -12,6 +12,7 @@
>  #include "qemu-common.h"
>  
>  #include <stdio.h>
> +#include <unistd.h>
>  
>  #include "qemu/module.h"
>  #include "remote/pcihost.h"
> @@ -19,12 +20,96 @@
>  #include "hw/boards.h"
>  #include "hw/qdev-core.h"
>  #include "qemu/main-loop.h"
> +#include "remote/memory.h"
> +#include "io/mpqemu-link.h"
> +#include "qapi/error.h"
> +#include "qemu/main-loop.h"
> +#include "sysemu/cpus.h"
> +#include "qemu-common.h"
> +#include "hw/pci/pci.h"
> +#include "qemu/thread.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/config-file.h"
> +#include "sysemu/sysemu.h"
> +#include "block/block.h"
> +#include "exec/ramlist.h"
> +
> +static MPQemuLinkState *mpqemu_link;
> +PCIDevice *remote_pci_dev;
> +
> +static void process_msg(GIOCondition cond, MPQemuChannel *chan)
> +{
> +    MPQemuMsg *msg = NULL;
> +    Error *err = NULL;
> +
> +    if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
> +        goto finalize_loop;
> +    }
> +
> +    msg = g_malloc0(sizeof(MPQemuMsg));
> +
> +    if (mpqemu_msg_recv(msg, chan) < 0) {
> +        error_setg(&err, "Failed to receive message");

Please give error messages more context, e.g. the device or process name
or something like that so that; it helps when we get a user
reporting a crash and they report 'Unknown command' in their log, but
then we have to figure out where to point them.

> +        goto finalize_loop;
> +    }
> +
> +    switch (msg->cmd) {
> +    case INIT:
> +        break;
> +    case PCI_CONFIG_WRITE:
> +        break;
> +    case PCI_CONFIG_READ:
> +        break;
> +    default:
> +        error_setg(&err, "Unknown command");
> +        goto finalize_loop;
> +    }
> +
> +    g_free(msg->data2);
> +    g_free(msg);
> +
> +    return;
> +
> +finalize_loop:
> +    if (err) {
> +        error_report_err(err);
> +    }
> +    g_free(msg);
> +    mpqemu_link_finalize(mpqemu_link);
> +    mpqemu_link = NULL;
> +}
>  
>  int main(int argc, char *argv[])
>  {
> +    Error *err = NULL;
> +
>      module_call_init(MODULE_INIT_QOM);
>  
> +    bdrv_init_with_whitelist();
> +
> +    if (qemu_init_main_loop(&err)) {
> +        error_report_err(err);
> +        return -EBUSY;
> +    }
> +
> +    qemu_init_cpu_loop();
> +
> +    page_size_init();
> +
> +    qemu_mutex_init(&ram_list.mutex);
> +

So these are some subset of the things from qemu_init; I guess
we'll have to be careful when we add stuff to vl.c to think what should
also be added here.

Dave

>      current_machine = MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
>  
> +    mpqemu_link = mpqemu_link_create();
> +    if (!mpqemu_link) {
> +        printf("Could not create MPQemu link\n");
> +        return -1;
> +    }
> +
> +    mpqemu_init_channel(mpqemu_link, &mpqemu_link->com, STDIN_FILENO);
> +    mpqemu_link_set_callback(mpqemu_link, process_msg);
> +
> +    mpqemu_start_coms(mpqemu_link);
> +
>      return 0;
>  }
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Jag Raman March 4, 2020, 6:45 p.m. UTC | #2
On 3/4/2020 5:29 AM, Dr. David Alan Gilbert wrote:
> * Jagannathan Raman (jag.raman@oracle.com) wrote:
>> Adds the handler to process message from QEMU,
>> Initialize remote process main loop, handles SYNC_SYSMEM
>> message by updating its "system_memory" container using
>> shared file descriptors received from QEMU.
>>
>> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
>> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
>> ---
>>   v4 -> v5:
>>    - We checked if we could use functions already defined in
>>      util/main-loop.c instead of using g_main_loop_run. However,
>>      we couldn't find a suitable function that's generic enough
>>      to do this. All of them have emulator code embedded in them
>>      which is not used by the remote process. We are therefore
>>      not making any change to this patch
>>
>>   remote/remote-main.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 85 insertions(+)
>>
>> diff --git a/remote/remote-main.c b/remote/remote-main.c
>> index ecf30e0..56315cd 100644
>> --- a/remote/remote-main.c
>> +++ b/remote/remote-main.c
>> @@ -12,6 +12,7 @@
>>   #include "qemu-common.h"
>>   
>>   #include <stdio.h>
>> +#include <unistd.h>
>>   
>>   #include "qemu/module.h"
>>   #include "remote/pcihost.h"
>> @@ -19,12 +20,96 @@
>>   #include "hw/boards.h"
>>   #include "hw/qdev-core.h"
>>   #include "qemu/main-loop.h"
>> +#include "remote/memory.h"
>> +#include "io/mpqemu-link.h"
>> +#include "qapi/error.h"
>> +#include "qemu/main-loop.h"
>> +#include "sysemu/cpus.h"
>> +#include "qemu-common.h"
>> +#include "hw/pci/pci.h"
>> +#include "qemu/thread.h"
>> +#include "qemu/main-loop.h"
>> +#include "qemu/config-file.h"
>> +#include "sysemu/sysemu.h"
>> +#include "block/block.h"
>> +#include "exec/ramlist.h"
>> +
>> +static MPQemuLinkState *mpqemu_link;
>> +PCIDevice *remote_pci_dev;
>> +
>> +static void process_msg(GIOCondition cond, MPQemuChannel *chan)
>> +{
>> +    MPQemuMsg *msg = NULL;
>> +    Error *err = NULL;
>> +
>> +    if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
>> +        goto finalize_loop;
>> +    }
>> +
>> +    msg = g_malloc0(sizeof(MPQemuMsg));
>> +
>> +    if (mpqemu_msg_recv(msg, chan) < 0) {
>> +        error_setg(&err, "Failed to receive message");
> 
> Please give error messages more context, e.g. the device or process name
> or something like that so that; it helps when we get a user
> reporting a crash and they report 'Unknown command' in their log, but
> then we have to figure out where to point them.

Ah OK, we'll add the exec name & PID of the QEMU process that spawned
this remote process in the error messages.

Thank you!
--
Jag

> 
>> +        goto finalize_loop;
>> +    }
>> +
>> +    switch (msg->cmd) {
>> +    case INIT:
>> +        break;
>> +    case PCI_CONFIG_WRITE:
>> +        break;
>> +    case PCI_CONFIG_READ:
>> +        break;
>> +    default:
>> +        error_setg(&err, "Unknown command");
>> +        goto finalize_loop;
>> +    }
>> +
>> +    g_free(msg->data2);
>> +    g_free(msg);
>> +
>> +    return;
>> +
>> +finalize_loop:
>> +    if (err) {
>> +        error_report_err(err);
>> +    }
>> +    g_free(msg);
>> +    mpqemu_link_finalize(mpqemu_link);
>> +    mpqemu_link = NULL;
>> +}
>>   
>>   int main(int argc, char *argv[])
>>   {
>> +    Error *err = NULL;
>> +
>>       module_call_init(MODULE_INIT_QOM);
>>   
>> +    bdrv_init_with_whitelist();
>> +
>> +    if (qemu_init_main_loop(&err)) {
>> +        error_report_err(err);
>> +        return -EBUSY;
>> +    }
>> +
>> +    qemu_init_cpu_loop();
>> +
>> +    page_size_init();
>> +
>> +    qemu_mutex_init(&ram_list.mutex);
>> +
> 
> So these are some subset of the things from qemu_init; I guess
> we'll have to be careful when we add stuff to vl.c to think what should
> also be added here.
> 
> Dave
> 
>>       current_machine = MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
>>   
>> +    mpqemu_link = mpqemu_link_create();
>> +    if (!mpqemu_link) {
>> +        printf("Could not create MPQemu link\n");
>> +        return -1;
>> +    }
>> +
>> +    mpqemu_init_channel(mpqemu_link, &mpqemu_link->com, STDIN_FILENO);
>> +    mpqemu_link_set_callback(mpqemu_link, process_msg);
>> +
>> +    mpqemu_start_coms(mpqemu_link);
>> +
>>       return 0;
>>   }
>> -- 
>> 1.8.3.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
Dr. David Alan Gilbert March 4, 2020, 7 p.m. UTC | #3
* Jag Raman (jag.raman@oracle.com) wrote:
> 
> 
> On 3/4/2020 5:29 AM, Dr. David Alan Gilbert wrote:
> > * Jagannathan Raman (jag.raman@oracle.com) wrote:
> > > Adds the handler to process message from QEMU,
> > > Initialize remote process main loop, handles SYNC_SYSMEM
> > > message by updating its "system_memory" container using
> > > shared file descriptors received from QEMU.
> > > 
> > > Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> > > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> > > ---
> > >   v4 -> v5:
> > >    - We checked if we could use functions already defined in
> > >      util/main-loop.c instead of using g_main_loop_run. However,
> > >      we couldn't find a suitable function that's generic enough
> > >      to do this. All of them have emulator code embedded in them
> > >      which is not used by the remote process. We are therefore
> > >      not making any change to this patch
> > > 
> > >   remote/remote-main.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 85 insertions(+)
> > > 
> > > diff --git a/remote/remote-main.c b/remote/remote-main.c
> > > index ecf30e0..56315cd 100644
> > > --- a/remote/remote-main.c
> > > +++ b/remote/remote-main.c
> > > @@ -12,6 +12,7 @@
> > >   #include "qemu-common.h"
> > >   #include <stdio.h>
> > > +#include <unistd.h>
> > >   #include "qemu/module.h"
> > >   #include "remote/pcihost.h"
> > > @@ -19,12 +20,96 @@
> > >   #include "hw/boards.h"
> > >   #include "hw/qdev-core.h"
> > >   #include "qemu/main-loop.h"
> > > +#include "remote/memory.h"
> > > +#include "io/mpqemu-link.h"
> > > +#include "qapi/error.h"
> > > +#include "qemu/main-loop.h"
> > > +#include "sysemu/cpus.h"
> > > +#include "qemu-common.h"
> > > +#include "hw/pci/pci.h"
> > > +#include "qemu/thread.h"
> > > +#include "qemu/main-loop.h"
> > > +#include "qemu/config-file.h"
> > > +#include "sysemu/sysemu.h"
> > > +#include "block/block.h"
> > > +#include "exec/ramlist.h"
> > > +
> > > +static MPQemuLinkState *mpqemu_link;
> > > +PCIDevice *remote_pci_dev;
> > > +
> > > +static void process_msg(GIOCondition cond, MPQemuChannel *chan)
> > > +{
> > > +    MPQemuMsg *msg = NULL;
> > > +    Error *err = NULL;
> > > +
> > > +    if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
> > > +        goto finalize_loop;
> > > +    }
> > > +
> > > +    msg = g_malloc0(sizeof(MPQemuMsg));
> > > +
> > > +    if (mpqemu_msg_recv(msg, chan) < 0) {
> > > +        error_setg(&err, "Failed to receive message");
> > 
> > Please give error messages more context, e.g. the device or process name
> > or something like that so that; it helps when we get a user
> > reporting a crash and they report 'Unknown command' in their log, but
> > then we have to figure out where to point them.
> 
> Ah OK, we'll add the exec name & PID of the QEMU process that spawned
> this remote process in the error messages.

Great, if you can get something to say which process this particular one
is it would be good; so if you've got multiple remote processes we know
which one spat out the error.

Dave

> Thank you!
> --
> Jag
> 
> > 
> > > +        goto finalize_loop;
> > > +    }
> > > +
> > > +    switch (msg->cmd) {
> > > +    case INIT:
> > > +        break;
> > > +    case PCI_CONFIG_WRITE:
> > > +        break;
> > > +    case PCI_CONFIG_READ:
> > > +        break;
> > > +    default:
> > > +        error_setg(&err, "Unknown command");
> > > +        goto finalize_loop;
> > > +    }
> > > +
> > > +    g_free(msg->data2);
> > > +    g_free(msg);
> > > +
> > > +    return;
> > > +
> > > +finalize_loop:
> > > +    if (err) {
> > > +        error_report_err(err);
> > > +    }
> > > +    g_free(msg);
> > > +    mpqemu_link_finalize(mpqemu_link);
> > > +    mpqemu_link = NULL;
> > > +}
> > >   int main(int argc, char *argv[])
> > >   {
> > > +    Error *err = NULL;
> > > +
> > >       module_call_init(MODULE_INIT_QOM);
> > > +    bdrv_init_with_whitelist();
> > > +
> > > +    if (qemu_init_main_loop(&err)) {
> > > +        error_report_err(err);
> > > +        return -EBUSY;
> > > +    }
> > > +
> > > +    qemu_init_cpu_loop();
> > > +
> > > +    page_size_init();
> > > +
> > > +    qemu_mutex_init(&ram_list.mutex);
> > > +
> > 
> > So these are some subset of the things from qemu_init; I guess
> > we'll have to be careful when we add stuff to vl.c to think what should
> > also be added here.
> > 
> > Dave
> > 
> > >       current_machine = MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
> > > +    mpqemu_link = mpqemu_link_create();
> > > +    if (!mpqemu_link) {
> > > +        printf("Could not create MPQemu link\n");
> > > +        return -1;
> > > +    }
> > > +
> > > +    mpqemu_init_channel(mpqemu_link, &mpqemu_link->com, STDIN_FILENO);
> > > +    mpqemu_link_set_callback(mpqemu_link, process_msg);
> > > +
> > > +    mpqemu_start_coms(mpqemu_link);
> > > +
> > >       return 0;
> > >   }
> > > -- 
> > > 1.8.3.1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/remote/remote-main.c b/remote/remote-main.c
index ecf30e0..56315cd 100644
--- a/remote/remote-main.c
+++ b/remote/remote-main.c
@@ -12,6 +12,7 @@ 
 #include "qemu-common.h"
 
 #include <stdio.h>
+#include <unistd.h>
 
 #include "qemu/module.h"
 #include "remote/pcihost.h"
@@ -19,12 +20,96 @@ 
 #include "hw/boards.h"
 #include "hw/qdev-core.h"
 #include "qemu/main-loop.h"
+#include "remote/memory.h"
+#include "io/mpqemu-link.h"
+#include "qapi/error.h"
+#include "qemu/main-loop.h"
+#include "sysemu/cpus.h"
+#include "qemu-common.h"
+#include "hw/pci/pci.h"
+#include "qemu/thread.h"
+#include "qemu/main-loop.h"
+#include "qemu/config-file.h"
+#include "sysemu/sysemu.h"
+#include "block/block.h"
+#include "exec/ramlist.h"
+
+static MPQemuLinkState *mpqemu_link;
+PCIDevice *remote_pci_dev;
+
+static void process_msg(GIOCondition cond, MPQemuChannel *chan)
+{
+    MPQemuMsg *msg = NULL;
+    Error *err = NULL;
+
+    if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
+        goto finalize_loop;
+    }
+
+    msg = g_malloc0(sizeof(MPQemuMsg));
+
+    if (mpqemu_msg_recv(msg, chan) < 0) {
+        error_setg(&err, "Failed to receive message");
+        goto finalize_loop;
+    }
+
+    switch (msg->cmd) {
+    case INIT:
+        break;
+    case PCI_CONFIG_WRITE:
+        break;
+    case PCI_CONFIG_READ:
+        break;
+    default:
+        error_setg(&err, "Unknown command");
+        goto finalize_loop;
+    }
+
+    g_free(msg->data2);
+    g_free(msg);
+
+    return;
+
+finalize_loop:
+    if (err) {
+        error_report_err(err);
+    }
+    g_free(msg);
+    mpqemu_link_finalize(mpqemu_link);
+    mpqemu_link = NULL;
+}
 
 int main(int argc, char *argv[])
 {
+    Error *err = NULL;
+
     module_call_init(MODULE_INIT_QOM);
 
+    bdrv_init_with_whitelist();
+
+    if (qemu_init_main_loop(&err)) {
+        error_report_err(err);
+        return -EBUSY;
+    }
+
+    qemu_init_cpu_loop();
+
+    page_size_init();
+
+    qemu_mutex_init(&ram_list.mutex);
+
     current_machine = MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
 
+    mpqemu_link = mpqemu_link_create();
+    if (!mpqemu_link) {
+        printf("Could not create MPQemu link\n");
+        return -1;
+    }
+
+    mpqemu_init_channel(mpqemu_link, &mpqemu_link->com, STDIN_FILENO);
+    mpqemu_link_set_callback(mpqemu_link, process_msg);
+
+    mpqemu_start_coms(mpqemu_link);
+
     return 0;
 }