[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]