Re: [libvirt] [PATCHv2 09/11] qemu_capabilities: Persist QEMU instance over multiple QMP Cmds
Quoting Jiri Denemark (2018-07-13 10:14:22)
On Mon, Jul 09, 2018 at 22:56:53 -0500, Chris Venteicher wrote:
> Commit makes starting a single persistent QEMU instance possible for use
> over multiple independent QMP commands without starting and stopping
> QEMU for each QMP command or command type.
>
> Commit allows functions outside qemu_capabilities
> (ex. qemuConnectBaselineHypervisorCPU in qemu_driver)
> requiring multiple different QMP messages to be sent to QEMU to issue
> those requests over a single QMP Command Channel without starting and
> stopping QEMU for each independent QMP Command usage.
>
> Commit moves following to global scope so parent function can
> maintain QEMU instance over multiple QMP commands / command types:
> virQEMUCapsInitQMPCommand
> virQEMUCapsInitQMPCommandFree
>
> Commit Introduces virQEMUCapsNewQMPCommandConnection to Start and
> connect to QEMU so QMP commands can be performed.
>
> The new reusable function isolates code for starting QEMU and
> establishing Monitor connections from code for obtaining capabilities so
> that arbitrary QMP commands can be exchanged with QEMU.
> ---
> src/qemu/qemu_capabilities.c | 61 +++++++++++++++++++++++-------------
> src/qemu/qemu_capabilities.h | 26 +++++++++++++++
> 2 files changed, 66 insertions(+), 21 deletions(-)
Just a high level review since this patch will require some significant
changes...
Your approach will not work because all callers would end up using the
same monitor socket for to the QEMU process. And there might be several
concurrent callers which would want to call some QMP command to do the
job, each of them spawning their own QEMU process. You'd need to give
each caller a unique monitor socket path.
I think this should be reworked into a general code (ideally placed in
qemu_process.c), which would then be called by the QEMU capabilities
code and from APIs which need to call QEMU.
Just want to make sure I am on the same page on how to do this.
The path I am on seems useful but not trivial so I want
to make sure I am going where you want.
I think this means I should be moving the Qemu process code out of
qemu_capabilities.c and into qemu_process.c for general use.
There is an established function structure in qemu_process for
starting/stoping qemu processes for VM's.
The existing functions like qemuProcessStart/Stop are too VM specific
(too complex) for the non VM, QMP only, case so I should fold the
existing code from qemu_capabilities into functions in qemu_process
of the form qemuProcessStartNoVM(...).
I also think I need to change the unix sockets to be process specific so
multiple concurrent processes for simultaneous clients can be supported.
More locking might be needed but I am not sure about this yet.
Here are the specific functions and structs I am talking about:
Remove from qemu_capabilities.c:
struct _virQEMUCapsInitQMPCommand {}
virQEMUCapsInitQMPCommandNew(...)
virQEMUCapsInitQMPCommandRun(...)
virQEMUCapsInitQMPCommandAbort(...)
virQEMUCapsInitQMPCommandFree(...)
Create in qemu_process.c/h:
;
struct _qemuProcessNoVMDomain {}
qemuProcessStartNoVM(...) {
/* Static internal functions */
qemuProcessInitNoVM(...);
qemuProcessLaunchNoVM(...);
qemuConnectMonitorNoVM(...);
}
void qemuProcessStopNoVM(...)
Seem like I am going in the right direction?
Chris
Jirka