[libvirt] [PATCH RFC 00/22] Move process code to qemu_process

[Michal Privoznik]

On 11/11/2018 08:59 PM, Chris Venteicher wrote:
> Make process code usable outside qemu_capabilities by moving code
> from qemu_capabilities to qemu_process and exposing public functions.
> 
> The process code is used to activate a non domain QEMU process for QMP
> message exchanges.
> 
> This patch set modifies capabilities to use the new public functions.
> 
> --
> 
> The process code is being decoupled from qemu_capabilities now to
> support hypervisor baseline and comparison using QMP commands.
> 
> This patch set was originally submitted as part of the baseline patch set:
>   [libvirt] [PATCH v4 00/37] BaselineHypervisorCPU using QEMU QMP exchanges
>   https://www.redhat.com/archives/libvir-list/2018-November/msg00091.html

Okay, so you want to implement cpu-baseline for s390. But that doesn't
really explain the code movement. Also, somehow the code movement makes
the code bigger? I guess what I am saying is that I don't see much
justification for these patches.

> 
> The baseline and comparison requirements are described here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1511999
> https://bugzilla.redhat.com/show_bug.cgi?id=1511996
> 
> 
> I am extracting and resubmitting just the process changes as a stand
> alone series to try to make review easier.
> 
> The patch set shows capabilities using the public functions.
> To see baseline using the public functions...
> Look at the "qemu_driver:" patches at the end of
> https://www.redhat.com/archives/libvir-list/2018-November/msg00091.html
> 
> Also,
> The "qemu_driver: Support feature expansion via QEMU when baselining cpu"
> patch might be of particular interest because the same QEMU process is
> used for both baseline and expansion using QMP commands.
> 
> --
> 
> Many patches were used to isolate code moves and name changes from other
> actual implementation changes.
> 
> The patches reuse the pattern of public qemuProcess{Start,Stop} functions
> and internal static qemuProcess{Init,Launch,ConnectMonitor} functions
> but adds a "Qmp" suffix to make them unique.
> 
> A number of patches are about re-partitioning the code into static
> functions for initialization, process launch and connection monitor
> stuff.  This matches the established pattern in qemu_process and seemed
> to make sense to do.
> 
> For concurrency...
> A thread safe library function creates a unique directory under libDir for each QEMU
> process (for QMP messaging) to decouple processes in terms of sockets and
> file system footprint.
> 
> Every patch should compile independently if applied in sequence.

Oh, but it doesn't. I'm running 'make -j10 all syntax-check check' and I
am hitting compilation/syntax error occasionally.


> 
> 
> Chris Venteicher (22):
>   qemu_process: Move process code from qemu_capabilities to qemu_process
>   qemu_process: Use qemuProcess prefix
>   qemu_process: Limit qemuProcessNew to const input strings
>   qemu_process: Refer to proc not cmd in process code
>   qemu_process: Use consistent name for stop process function
>   qemu_capabilities: Stop QEMU process before freeing
>   qemu_process: Use qemuProcess struct for a single process
>   qemu_process: Persist stderr in qemuProcess struct
>   qemu_capabilities: Detect caps probe failure by checking monitor ptr
>   qemu_process: Introduce qemuProcessStartQmp
>   qemu_process: Collect monitor code in single function
>   qemu_process: Store libDir in qemuProcess struct
>   qemu_process: Setup paths within qemuProcessInitQmp
>   qemu_process: Stop retaining Qemu Monitor config in qemuProcess
>   qemu_process: Don't open monitor if process failed
>   qemu_process: Cleanup qemuProcess alloc function
>   qemu_process: Cleanup qemuProcessStopQmp function
>   qemu_process: Catch process free before process stop
>   qemu_monitor: Make monitor callbacks optional
>   qemu_process: Enter QMP command mode when starting QEMU Process
>   qemu_process: Use unique directories for QMP processes
>   qemu_process: Stop locking QMP process monitor immediately
> 
>  src/qemu/qemu_capabilities.c | 300 +++++------------------------
>  src/qemu/qemu_monitor.c      |   4 +-
>  src/qemu/qemu_process.c      | 356 +++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_process.h      |  37 ++++
>  tests/qemucapabilitiestest.c |   7 +
>  5 files changed, 444 insertions(+), 260 deletions(-)
> 

Michal