diff mbox series

[v12,09/19] multi-process: Initialize message handler in remote device

Message ID 32c713a44d3514b4f0edcd23195e25a10153c347.1606853298.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 Dec. 1, 2020, 8:22 p.m. UTC
Initializes the message handler function in the remote process. It is
called whenever there's an event pending on QIOChannel that registers
this function.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/remote/machine.h |  9 +++++++
 hw/remote/message.c         | 61 +++++++++++++++++++++++++++++++++++++++++++++
 MAINTAINERS                 |  1 +
 hw/remote/meson.build       |  1 +
 4 files changed, 72 insertions(+)
 create mode 100644 hw/remote/message.c

Comments

Marc-André Lureau Dec. 7, 2020, 1:33 p.m. UTC | #1
Hi

On Wed, Dec 2, 2020 at 12:23 AM Jagannathan Raman <jag.raman@oracle.com>
wrote:

> Initializes the message handler function in the remote process. It is
> called whenever there's an event pending on QIOChannel that registers
> this function.
>
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/hw/remote/machine.h |  9 +++++++
>  hw/remote/message.c         | 61
> +++++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS                 |  1 +
>  hw/remote/meson.build       |  1 +
>  4 files changed, 72 insertions(+)
>  create mode 100644 hw/remote/message.c
>
> diff --git a/include/hw/remote/machine.h b/include/hw/remote/machine.h
> index d312972..3073db6 100644
> --- a/include/hw/remote/machine.h
> +++ b/include/hw/remote/machine.h
> @@ -14,6 +14,7 @@
>  #include "qom/object.h"
>  #include "hw/boards.h"
>  #include "hw/pci-host/remote.h"
> +#include "io/channel.h"
>
>  typedef struct RemoteMachineState {
>      MachineState parent_obj;
> @@ -21,8 +22,16 @@ typedef struct RemoteMachineState {
>      RemotePCIHost *host;
>  } RemoteMachineState;
>
> +/* Used to pass to co-routine device and ioc. */
> +typedef struct RemoteCommDev {
> +    PCIDevice *dev;
> +    QIOChannel *ioc;
> +} RemoteCommDev;
> +
>  #define TYPE_REMOTE_MACHINE "x-remote-machine"
>  #define REMOTE_MACHINE(obj) \
>      OBJECT_CHECK(RemoteMachineState, (obj), TYPE_REMOTE_MACHINE)
>
> +void coroutine_fn mpqemu_remote_msg_loop_co(void *data);
> +
>  #endif
> diff --git a/hw/remote/message.c b/hw/remote/message.c
> new file mode 100644
> index 0000000..5d87bf4
> --- /dev/null
> +++ b/hw/remote/message.c
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright © 2020 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or
> later.
> + *
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +
> +#include "hw/remote/machine.h"
> +#include "io/channel.h"
> +#include "hw/remote/mpqemu-link.h"
> +#include "qapi/error.h"
> +#include "sysemu/runstate.h"
> +
> +void coroutine_fn mpqemu_remote_msg_loop_co(void *data)
> +{
> +    RemoteCommDev *com = (RemoteCommDev *)data;
> +    PCIDevice *pci_dev = NULL;
> +
> +    pci_dev = com->dev;
> +    for (;;) {
> +        MPQemuMsg msg = {0};
> +        Error *local_err = NULL;
> +
> +        if (!com->ioc) {
> +            error_report("ERROR: No channel available");
> +            break;
> +        }
>

Shouldn't this be assert() at the top?


> +        mpqemu_msg_recv(&msg, com->ioc, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +            break;
>

Error handling is not consistent in this function. Could you cleanup error
code paths so error handling & reporting is done in one place?

+        }
> +
> +        if (!mpqemu_msg_valid(&msg)) {
> +            error_report("Received invalid message from proxy"
> +                         "in remote process pid=%d", getpid());
> +            break;
> +        }
> +
> +        switch (msg.cmd) {
> +        default:
> +            error_setg(&local_err,
> +                       "Unknown command (%d) received for device %s
> (pid=%d)",
> +                       msg.cmd, DEVICE(pci_dev)->id, getpid());
> +        }
> +
> +        if (local_err) {
> +            error_report_err(local_err);
> +            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>

Presumably that error handling should be done outside of the for(;;) loop.

SHUTDOWN_CAUSE_HOST_ERROR might be more appropriate in this case, or
perhaps introduce a new ShutdownCause?

+            break;
> +        }
> +    }
> +    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +
> +    return;
>

needless return statement

+}
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d0c891a..b64e4b8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3143,6 +3143,7 @@ F: hw/remote/machine.c
>  F: include/hw/remote/machine.h
>  F: hw/remote/mpqemu-link.c
>  F: include/hw/remote/mpqemu-link.h
> +F: hw/remote/message.c
>
>  Build and test automation
>  -------------------------
> diff --git a/hw/remote/meson.build b/hw/remote/meson.build
> index a2b2fc0..9f5c57f 100644
> --- a/hw/remote/meson.build
> +++ b/hw/remote/meson.build
> @@ -2,5 +2,6 @@ remote_ss = ss.source_set()
>
>  remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('machine.c'))
>  remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true:
> files('mpqemu-link.c'))
> +remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c'))
>
>  softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss)
> --
> 1.8.3.1
>
>
diff mbox series

Patch

diff --git a/include/hw/remote/machine.h b/include/hw/remote/machine.h
index d312972..3073db6 100644
--- a/include/hw/remote/machine.h
+++ b/include/hw/remote/machine.h
@@ -14,6 +14,7 @@ 
 #include "qom/object.h"
 #include "hw/boards.h"
 #include "hw/pci-host/remote.h"
+#include "io/channel.h"
 
 typedef struct RemoteMachineState {
     MachineState parent_obj;
@@ -21,8 +22,16 @@  typedef struct RemoteMachineState {
     RemotePCIHost *host;
 } RemoteMachineState;
 
+/* Used to pass to co-routine device and ioc. */
+typedef struct RemoteCommDev {
+    PCIDevice *dev;
+    QIOChannel *ioc;
+} RemoteCommDev;
+
 #define TYPE_REMOTE_MACHINE "x-remote-machine"
 #define REMOTE_MACHINE(obj) \
     OBJECT_CHECK(RemoteMachineState, (obj), TYPE_REMOTE_MACHINE)
 
+void coroutine_fn mpqemu_remote_msg_loop_co(void *data);
+
 #endif
diff --git a/hw/remote/message.c b/hw/remote/message.c
new file mode 100644
index 0000000..5d87bf4
--- /dev/null
+++ b/hw/remote/message.c
@@ -0,0 +1,61 @@ 
+/*
+ * Copyright © 2020 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL-v2, version 2 or later.
+ *
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+
+#include "hw/remote/machine.h"
+#include "io/channel.h"
+#include "hw/remote/mpqemu-link.h"
+#include "qapi/error.h"
+#include "sysemu/runstate.h"
+
+void coroutine_fn mpqemu_remote_msg_loop_co(void *data)
+{
+    RemoteCommDev *com = (RemoteCommDev *)data;
+    PCIDevice *pci_dev = NULL;
+
+    pci_dev = com->dev;
+    for (;;) {
+        MPQemuMsg msg = {0};
+        Error *local_err = NULL;
+
+        if (!com->ioc) {
+            error_report("ERROR: No channel available");
+            break;
+        }
+        mpqemu_msg_recv(&msg, com->ioc, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            break;
+        }
+
+        if (!mpqemu_msg_valid(&msg)) {
+            error_report("Received invalid message from proxy"
+                         "in remote process pid=%d", getpid());
+            break;
+        }
+
+        switch (msg.cmd) {
+        default:
+            error_setg(&local_err,
+                       "Unknown command (%d) received for device %s (pid=%d)",
+                       msg.cmd, DEVICE(pci_dev)->id, getpid());
+        }
+
+        if (local_err) {
+            error_report_err(local_err);
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+            break;
+        }
+    }
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+
+    return;
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index d0c891a..b64e4b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3143,6 +3143,7 @@  F: hw/remote/machine.c
 F: include/hw/remote/machine.h
 F: hw/remote/mpqemu-link.c
 F: include/hw/remote/mpqemu-link.h
+F: hw/remote/message.c
 
 Build and test automation
 -------------------------
diff --git a/hw/remote/meson.build b/hw/remote/meson.build
index a2b2fc0..9f5c57f 100644
--- a/hw/remote/meson.build
+++ b/hw/remote/meson.build
@@ -2,5 +2,6 @@  remote_ss = ss.source_set()
 
 remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('machine.c'))
 remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('mpqemu-link.c'))
+remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c'))
 
 softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss)