From a0c688a68e732bc895ea4fde3cd8e899d922e6fa Mon Sep 17 00:00:00 2001
From: Ilya Verbitskiy <ilya@verbit.io>
Date: Fri, 16 Aug 2024 11:36:32 +0200
Subject: [PATCH] qmp: simplify design to a single gen_server

fixes #156
---
 apps/virtuerl/src/virtuerl_qemu.erl |   3 +-
 apps/virtuerl/src/virtuerl_qmp.erl  | 100 ++++++++--------------------
 2 files changed, 31 insertions(+), 72 deletions(-)

diff --git a/apps/virtuerl/src/virtuerl_qemu.erl b/apps/virtuerl/src/virtuerl_qemu.erl
index 60af797..187d89d 100644
--- a/apps/virtuerl/src/virtuerl_qemu.erl
+++ b/apps/virtuerl/src/virtuerl_qemu.erl
@@ -93,7 +93,8 @@ handle_continue(setup_qmp, #{id := ID} = State) ->
     %%  {ok, _} = exec:run(iolist_to_binary(["inotifywait -e create --include 'qmp\\.sock' ", ID]), [sync]),
     case wait_for_socket(QmpSocketPath) of
         ok ->
-            {ok, QmpPid} = virtuerl_qmp:start_link(QmpSocketPath, self()),
+            % TODO: start_link might be too strong here, consider attaching it to a supervisor instead (or sth like gen_tcp's design)
+            {ok, QmpPid} = virtuerl_qmp:start_link(QmpSocketPath),
             virtuerl_qmp:exec(QmpPid, cont),
             {noreply, State#{qmp_pid => QmpPid}, {continue, setup_serial}};
         timeout ->
diff --git a/apps/virtuerl/src/virtuerl_qmp.erl b/apps/virtuerl/src/virtuerl_qmp.erl
index 5b761ac..1a7188c 100644
--- a/apps/virtuerl/src/virtuerl_qmp.erl
+++ b/apps/virtuerl/src/virtuerl_qmp.erl
@@ -2,102 +2,60 @@
 
 -behaviour(gen_server).
 
--export([start_link/2, exec/2, stop/1]).
+-export([start_link/1, exec/2, stop/1]).
 -export([init/1, terminate/2, handle_cast/2, handle_call/3, handle_info/2]).
 
 -include_lib("kernel/include/logger.hrl").
 
 
 exec(Pid, Command) ->
-    gen_server:call(Pid, Command).
-
-
-stop(Pid) ->
-    gen_server:stop(Pid).
-
-
-%%%===================================================================
-%%% Spawning and gen_server implementation
-%%%===================================================================
-
-
-start_link(QmpSocketPath, Receiver) ->
-    gen_server:start_link(?MODULE, {QmpSocketPath, Receiver}, []).
+    Pid ! {qmp, Command},
+    receive
+        {qmp, #{<<"return">> := #{}}} -> ok
+    end.
 
 
-init({QmpSocketPath, Receiver}) ->
-    Self = self(),
-    Pid = spawn_link(fun() -> qmp_translator(QmpSocketPath, Self) end),
-    io:format("virtuerl_qmp: init~n"),
+start_link(QmpSocketPath) ->
+    {ok, Pid} = gen_server:start_link(?MODULE, {QmpSocketPath, self()}, []),
     receive
         {qmp, #{<<"QMP">> := #{}}} -> ok
     after
         1000 ->
             exit(qmp_not_responding)
     end,
-    io:format("virtuerl_qmp: init after~n"),
-    execute(Pid, qmp_capabilities),
-    io:format("virtuerl_qmp: qmp caps~n"),
-    {ok, {Pid, Receiver}}.
+    exec(Pid, qmp_capabilities),
+    {ok, Pid}.
 
 
-terminate(_Reason, {Pid, _}) ->
-    Ref = monitor(process, Pid),
-    exit(Pid, normal),
-    receive
-        {'DOWN', Ref, process, Pid, normal} -> ok
-    end,
-    true = demonitor(Ref),
-    io:format("exiting QMP server~n").
+stop(Pid) ->
+    gen_server:stop(Pid).
 
 
-handle_call(Command, _From, {Pid, _} = State) ->
-    execute(Pid, Command),
-    {reply, ok, State}.
+init({QmpSocketPath, Receiver}) ->
+    {ok, QmpSocket} = gen_tcp:connect({local, QmpSocketPath}, 0, [local, {active, true}]),
+    {ok, {QmpSocket, Receiver}}.
 
 
-handle_cast(Request, State) ->
+handle_call(_Request, _From, _State) ->
     erlang:error(not_implemented).
 
 
-handle_info({qmp, #{<<"event">> := _} = Event}, {_, Receiver} = State) ->
-    Receiver ! {qmp, Event},
-    {noreply, State}.
-
-
-execute(Pid, Command) when is_pid(Pid) ->
-    Pid ! {qmp, Command},
-    receive
-        {qmp, #{<<"return">> := #{}}} -> ok
-    end.
-
+handle_cast(_Request, _State) ->
+    erlang:error(not_implemented).
 
-%%%===================================================================
-%%% Internal functions
-%%%===================================================================
 
+handle_info({tcp, _Socket, RawData}, {_QmpSocket, Receiver} = State) ->
+    Lines = re:split(RawData, "\r?\n", [trim]),
+    Jsons = lists:map(fun(Line) -> {ok, Json} = thoas:decode(Line), Json end, Lines),
+    ?LOG_DEBUG(#{qmp_raw => RawData, qmp_parsed => Jsons}),
+    [ Receiver ! {qmp, Json} || Json <- Jsons ],
+    {noreply, State};
 
-qmp_translator(QmpSocketPath, Receiver) ->
-    process_flag(trap_exit, true),
-    io:format("starting QMP translator (~s)!~n", [QmpSocketPath]),
-    {ok, QmpSocket} = gen_tcp:connect({local, QmpSocketPath}, 0, [local, {active, true}]),
-    qmp_loop(QmpSocket, Receiver).
+handle_info({qmp, Command}, {QmpSocket, _Receiver} = State) ->
+    ?LOG_INFO(#{who => virtuerl_qmp, command => Command}),
+    ok = gen_tcp:send(QmpSocket, thoas:encode(#{execute => Command})),
+    {noreply, State}.
 
 
-qmp_loop(QmpSocket, Receiver) ->
-    receive
-        {tcp, _Socket, RawData} ->
-            Lines = re:split(RawData, "\r?\n", [trim]),
-            Jsons = lists:map(fun(Line) -> {ok, Json} = thoas:decode(Line), Json end, Lines),
-            ?LOG_DEBUG(#{qmp_raw => RawData, qmp_parsed => Jsons}),
-            [ Receiver ! {qmp, Json} || Json <- Jsons ],
-            qmp_loop(QmpSocket, Receiver);
-        {qmp, Command} when is_atom(Command) ->
-            io:format("qmp_loop/qmp: ~p~n", [Command]),
-            ok = gen_tcp:send(QmpSocket, thoas:encode(#{execute => Command})),
-            qmp_loop(QmpSocket, Receiver);
-        {'EXIT', _SenderID, Reason} ->
-            io:format("closing QMP socket (~p)!~n", [Reason]),
-            ok = gen_tcp:close(QmpSocket),
-            exit(Reason)
-    end.
+terminate(_Reason, {QmpSocket, _Receiver}) ->
+    gen_tcp:close(QmpSocket).