Move a step closer to the function structure used elsewhere in
qemu_process where qemuProcessStart and qemuProcessStop are the exposed
functions.

qemuProcessStartQmp mirrors qemuProcessStart in calling sub functions to
intialize, launch the process and connect the monitor to the QEMU
process.

static functions qemuProcessInitQmp, qemuProcessLaunchQmp and
qemuConnectMonitorQmp are also introduced.

qemuProcessLaunchQmp is just renamed from qemuProcessRun and
encapsulates all of the original code.

qemuProcessInitQmp and qemuProcessMonitorQmp are introduced as empty
wrappers into which subsequent patches will partition code from
qemuProcessLaunchQmp.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
---
 src/qemu/qemu_capabilities.c |  4 +-
 src/qemu/qemu_process.c      | 96 +++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_process.h      |  2 +-
 3 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fbb4336201..7168c470f6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4230,7 +4230,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
         goto cleanup;
 
 
-    if (qemuProcessRun(proc) < 0)
+    if (qemuProcessStartQmp(proc) < 0)
         goto cleanup;
 
     if (!(mon = QEMU_PROCESS_MONITOR(proc))) {
@@ -4249,7 +4249,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
         forceTCG = true;
         proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
 
-        if (qemuProcessRun(proc_kvm) < 0)
+        if (qemuProcessStartQmp(proc_kvm) < 0)
             goto cleanup;
 
         if (!(mon_kvm = QEMU_PROCESS_MONITOR(proc_kvm))) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2640ec2b32..b6aa3a9af3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8148,8 +8148,29 @@ qemuProcessNew(const char *binary,
 }
 
 
-int
-qemuProcessRun(qemuProcessPtr proc){
+/* Initialize configuration and paths prior to starting QEMU
+ */
+static int
+qemuProcessInitQmp(qemuProcessPtr proc)
+{
+    int ret = -1;
+
+    VIR_DEBUG("Beginning VM startup process"
+              "emulator=%s",
+              proc->binary);
+
+    ret = 0;
+
+    VIR_DEBUG("ret=%i", ret);
+    return ret;
+}
+
+
+/* Launch QEMU Process
+ */
+static int
+qemuProcessLaunchQmp(qemuProcessPtr proc)
+{
     virDomainXMLOptionPtr xmlopt = NULL;
     const char *machine;
     int status = 0;
@@ -8226,6 +8247,77 @@ qemuProcessRun(qemuProcessPtr proc){
 }
 
 
+/* Connect Monitor to QEMU Process
+ */
+static int
+qemuConnectMonitorQmp(qemuProcessPtr proc)
+{
+    int ret = -1;
+
+    VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
+              proc, NULLSTR(proc->binary), (long long) proc->pid);
+
+    ret = 0;
+
+    VIR_DEBUG("ret=%i", ret);
+    return ret;
+}
+
+
+/**
+ * qemuProcessStartQmp:
+ * @proc: Stores Process and Connection State
+ *
+ * Start and connect to QEMU binary so QMP queries can be made.
+ *
+ * Usage:
+ *   proc = qemuProcessNew(binary, forceTCG, libDir, runUid, runGid);
+ *   qemuProcessStartQmp(proc);
+ *   mon = QEMU_PROCESS_MONITOR(proc);
+ *   ** Send QMP Queries to QEMU using monitor **
+ *   qemuProcessStopQmp(proc);
+ *   qemuProcessFree(proc);
+ *
+ * Check monitor is not NULL before using.
+ *
+ * QEMU probing failure results in monitor being NULL but is not considered
+ * an error case and does not result in a negative return value.
+ *
+ * Both qemuProcessStopQmp and qemuProcessFree must be called to cleanup and
+ * free resources, even in activation failure cases.
+ *
+ * Process error output (proc->qmperr) remains available in qemuProcess struct
+ * until qemuProcessFree is called.
+ */
+int
+qemuProcessStartQmp(qemuProcessPtr proc)
+{
+    int ret = -1;
+
+    VIR_DEBUG("emulator=%s",
+              proc->binary);
+
+    if (qemuProcessInitQmp(proc) < 0)
+        goto cleanup;
+
+    if (qemuProcessLaunchQmp(proc) < 0)
+        goto stop;
+
+    if (qemuConnectMonitorQmp(proc) < 0)
+        goto stop;
+
+    ret = 0;
+
+ cleanup:
+    VIR_DEBUG("ret=%i", ret);
+    return ret;
+
+ stop:
+    qemuProcessStopQmp(proc);
+    goto cleanup;
+}
+
+
 void
 qemuProcessStopQmp(qemuProcessPtr proc)
 {
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 37194e2501..c34ca52ef5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -246,7 +246,7 @@ qemuProcessPtr qemuProcessNew(const char *binary,
 
 void qemuProcessFree(qemuProcessPtr proc);
 
-int qemuProcessRun(qemuProcessPtr proc);
+int qemuProcessStartQmp(qemuProcessPtr proc);
 
 void qemuProcessStopQmp(qemuProcessPtr proc);
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 11/11/2018 08:59 PM, Chris Venteicher wrote:
> Move a step closer to the function structure used elsewhere in
> qemu_process where qemuProcessStart and qemuProcessStop are the exposed
> functions.
> 
> qemuProcessStartQmp mirrors qemuProcessStart in calling sub functions to
> intialize, launch the process and connect the monitor to the QEMU
> process.
> 
> static functions qemuProcessInitQmp, qemuProcessLaunchQmp and
> qemuConnectMonitorQmp are also introduced.
> 
> qemuProcessLaunchQmp is just renamed from qemuProcessRun and
> encapsulates all of the original code.
> 
> qemuProcessInitQmp and qemuProcessMonitorQmp are introduced as empty
> wrappers into which subsequent patches will partition code from
> qemuProcessLaunchQmp.
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c |  4 +-
>  src/qemu/qemu_process.c      | 96 +++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_process.h      |  2 +-
>  3 files changed, 97 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index fbb4336201..7168c470f6 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4230,7 +4230,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>          goto cleanup;
>  
>  
> -    if (qemuProcessRun(proc) < 0)
> +    if (qemuProcessStartQmp(proc) < 0)
>          goto cleanup;
>  
>      if (!(mon = QEMU_PROCESS_MONITOR(proc))) {
> @@ -4249,7 +4249,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>          forceTCG = true;
>          proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
>  
> -        if (qemuProcessRun(proc_kvm) < 0)
> +        if (qemuProcessStartQmp(proc_kvm) < 0)
>              goto cleanup;
>  
>          if (!(mon_kvm = QEMU_PROCESS_MONITOR(proc_kvm))) {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2640ec2b32..b6aa3a9af3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8148,8 +8148,29 @@ qemuProcessNew(const char *binary,
>  }
>  
>  
> -int
> -qemuProcessRun(qemuProcessPtr proc){
> +/* Initialize configuration and paths prior to starting QEMU
> + */
> +static int
> +qemuProcessInitQmp(qemuProcessPtr proc)
> +{
> +    int ret = -1;
> +
> +    VIR_DEBUG("Beginning VM startup process"
> +              "emulator=%s",
> +              proc->binary);
> +
> +    ret = 0;
> +
> +    VIR_DEBUG("ret=%i", ret);
> +    return ret;
> +}
> +

I am sorry, but I'm failing to see the purpose of this function.

> +
> +/* Launch QEMU Process
> + */
> +static int
> +qemuProcessLaunchQmp(qemuProcessPtr proc)
> +{
>      virDomainXMLOptionPtr xmlopt = NULL;
>      const char *machine;
>      int status = 0;
> @@ -8226,6 +8247,77 @@ qemuProcessRun(qemuProcessPtr proc){
>  }
>  
>  
> +/* Connect Monitor to QEMU Process
> + */
> +static int
> +qemuConnectMonitorQmp(qemuProcessPtr proc)
> +{
> +    int ret = -1;
> +
> +    VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
> +              proc, NULLSTR(proc->binary), (long long) proc->pid);
> +
> +    ret = 0;
> +
> +    VIR_DEBUG("ret=%i", ret);
> +    return ret;
> +}

Or this function. Looking into next patches I can see that you're
extending them. Well, I still think it's not worth introducing empty
functions, just do the rename as you're doing in next patches.

> +
> +
> +/**
> + * qemuProcessStartQmp:
> + * @proc: Stores Process and Connection State
> + *
> + * Start and connect to QEMU binary so QMP queries can be made.
> + *
> + * Usage:
> + *   proc = qemuProcessNew(binary, forceTCG, libDir, runUid, runGid);
> + *   qemuProcessStartQmp(proc);
> + *   mon = QEMU_PROCESS_MONITOR(proc);
> + *   ** Send QMP Queries to QEMU using monitor **
> + *   qemuProcessStopQmp(proc);
> + *   qemuProcessFree(proc);
> + *
> + * Check monitor is not NULL before using.
> + *
> + * QEMU probing failure results in monitor being NULL but is not considered
> + * an error case and does not result in a negative return value.
> + *
> + * Both qemuProcessStopQmp and qemuProcessFree must be called to cleanup and
> + * free resources, even in activation failure cases.
> + *
> + * Process error output (proc->qmperr) remains available in qemuProcess struct
> + * until qemuProcessFree is called.
> + */
> +int
> +qemuProcessStartQmp(qemuProcessPtr proc)
> +{
> +    int ret = -1;
> +
> +    VIR_DEBUG("emulator=%s",
> +              proc->binary);
> +
> +    if (qemuProcessInitQmp(proc) < 0)
> +        goto cleanup;
> +
> +    if (qemuProcessLaunchQmp(proc) < 0)
> +        goto stop;
> +
> +    if (qemuConnectMonitorQmp(proc) < 0)
> +        goto stop;
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_DEBUG("ret=%i", ret);
> +    return ret;
> +
> + stop:
> +    qemuProcessStopQmp(proc);
> +    goto cleanup;
> +}
> +
> +
>  void
>  qemuProcessStopQmp(qemuProcessPtr proc)
>  {
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 37194e2501..c34ca52ef5 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -246,7 +246,7 @@ qemuProcessPtr qemuProcessNew(const char *binary,
>  
>  void qemuProcessFree(qemuProcessPtr proc);
>  
> -int qemuProcessRun(qemuProcessPtr proc);
> +int qemuProcessStartQmp(qemuProcessPtr proc);
>  
>  void qemuProcessStopQmp(qemuProcessPtr proc);
>  
> 

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Quoting Michal Privoznik (2018-11-14 09:45:07)
> On 11/11/2018 08:59 PM, Chris Venteicher wrote:
> > Move a step closer to the function structure used elsewhere in
> > qemu_process where qemuProcessStart and qemuProcessStop are the exposed
> > functions.
> > 
> > qemuProcessStartQmp mirrors qemuProcessStart in calling sub functions to
> > intialize, launch the process and connect the monitor to the QEMU
> > process.
> > 
> > static functions qemuProcessInitQmp, qemuProcessLaunchQmp and
> > qemuConnectMonitorQmp are also introduced.
> > 
> > qemuProcessLaunchQmp is just renamed from qemuProcessRun and
> > encapsulates all of the original code.
> > 
> > qemuProcessInitQmp and qemuProcessMonitorQmp are introduced as empty
> > wrappers into which subsequent patches will partition code from
> > qemuProcessLaunchQmp.
> > 
> > Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c |  4 +-
> >  src/qemu/qemu_process.c      | 96 +++++++++++++++++++++++++++++++++++-
> >  src/qemu/qemu_process.h      |  2 +-
> >  3 files changed, 97 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index fbb4336201..7168c470f6 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -4230,7 +4230,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> >          goto cleanup;
> >  
> >  
> > -    if (qemuProcessRun(proc) < 0)
> > +    if (qemuProcessStartQmp(proc) < 0)
> >          goto cleanup;
> >  
> >      if (!(mon = QEMU_PROCESS_MONITOR(proc))) {
> > @@ -4249,7 +4249,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> >          forceTCG = true;
> >          proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
> >  
> > -        if (qemuProcessRun(proc_kvm) < 0)
> > +        if (qemuProcessStartQmp(proc_kvm) < 0)
> >              goto cleanup;
> >  
> >          if (!(mon_kvm = QEMU_PROCESS_MONITOR(proc_kvm))) {
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 2640ec2b32..b6aa3a9af3 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -8148,8 +8148,29 @@ qemuProcessNew(const char *binary,
> >  }
> >  
> >  
> > -int
> > -qemuProcessRun(qemuProcessPtr proc){
> > +/* Initialize configuration and paths prior to starting QEMU
> > + */
> > +static int
> > +qemuProcessInitQmp(qemuProcessPtr proc)
> > +{
> > +    int ret = -1;
> > +
> > +    VIR_DEBUG("Beginning VM startup process"
> > +              "emulator=%s",
> > +              proc->binary);
> > +
> > +    ret = 0;
> > +
> > +    VIR_DEBUG("ret=%i", ret);
> > +    return ret;
> > +}
> > +
> 
> I am sorry, but I'm failing to see the purpose of this function.
> 
> > +
> > +/* Launch QEMU Process
> > + */
> > +static int
> > +qemuProcessLaunchQmp(qemuProcessPtr proc)
> > +{
> >      virDomainXMLOptionPtr xmlopt = NULL;
> >      const char *machine;
> >      int status = 0;
> > @@ -8226,6 +8247,77 @@ qemuProcessRun(qemuProcessPtr proc){
> >  }
> >  
> >  
> > +/* Connect Monitor to QEMU Process
> > + */
> > +static int
> > +qemuConnectMonitorQmp(qemuProcessPtr proc)
> > +{
> > +    int ret = -1;
> > +
> > +    VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
> > +              proc, NULLSTR(proc->binary), (long long) proc->pid);
> > +
> > +    ret = 0;
> > +
> > +    VIR_DEBUG("ret=%i", ret);
> > +    return ret;
> > +}
> 
> Or this function. Looking into next patches I can see that you're
> extending them. Well, I still think it's not worth introducing empty
> functions, just do the rename as you're doing in next patches.

Yep, I was trying to make it easier to review but didn't explain well
enough from the start. Sorry I wasn't clear.

Patch 10 (this patch) and 12 are about making it possible to do simple
cut/paste moves on semi-complicated blocks of original code moved
within patches 11 and 13.

The goal was to make patches 11 and 13 easy to review because
I don't actually change the code.  It's just moved.

If this seems good with the better explanation I can just try to make
that clear in the commit message for patch 10 and 12.

If it's more confusing this way I can start out with qemuProcesStartQmp
only calling qemuProcessLaunchQmp and the add qemuProcessInitQmp and
qemuConnectMonitorQmp as individual commits with full contents and just
explain that the guts are cut/paste moves with no changes in the commit
message.

Please let me know which approach you think is best.
>
> > +
> > +
> > +/**
> > + * qemuProcessStartQmp:
> > + * @proc: Stores Process and Connection State
> > + *
> > + * Start and connect to QEMU binary so QMP queries can be made.
> > + *
> > + * Usage:
> > + *   proc = qemuProcessNew(binary, forceTCG, libDir, runUid, runGid);
> > + *   qemuProcessStartQmp(proc);
> > + *   mon = QEMU_PROCESS_MONITOR(proc);
> > + *   ** Send QMP Queries to QEMU using monitor **
> > + *   qemuProcessStopQmp(proc);
> > + *   qemuProcessFree(proc);
> > + *
> > + * Check monitor is not NULL before using.
> > + *
> > + * QEMU probing failure results in monitor being NULL but is not considered
> > + * an error case and does not result in a negative return value.
> > + *
> > + * Both qemuProcessStopQmp and qemuProcessFree must be called to cleanup and
> > + * free resources, even in activation failure cases.
> > + *
> > + * Process error output (proc->qmperr) remains available in qemuProcess struct
> > + * until qemuProcessFree is called.
> > + */
> > +int
> > +qemuProcessStartQmp(qemuProcessPtr proc)
> > +{
> > +    int ret = -1;
> > +
> > +    VIR_DEBUG("emulator=%s",
> > +              proc->binary);
> > +
> > +    if (qemuProcessInitQmp(proc) < 0)
> > +        goto cleanup;
> > +
> > +    if (qemuProcessLaunchQmp(proc) < 0)
> > +        goto stop;
> > +
> > +    if (qemuConnectMonitorQmp(proc) < 0)
> > +        goto stop;
> > +
> > +    ret = 0;
> > +
> > + cleanup:
> > +    VIR_DEBUG("ret=%i", ret);
> > +    return ret;
> > +
> > + stop:
> > +    qemuProcessStopQmp(proc);
> > +    goto cleanup;
> > +}
> > +
> > +
> >  void
> >  qemuProcessStopQmp(qemuProcessPtr proc)
> >  {
> > diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> > index 37194e2501..c34ca52ef5 100644
> > --- a/src/qemu/qemu_process.h
> > +++ b/src/qemu/qemu_process.h
> > @@ -246,7 +246,7 @@ qemuProcessPtr qemuProcessNew(const char *binary,
> >  
> >  void qemuProcessFree(qemuProcessPtr proc);
> >  
> > -int qemuProcessRun(qemuProcessPtr proc);
> > +int qemuProcessStartQmp(qemuProcessPtr proc);
> >  
> >  void qemuProcessStopQmp(qemuProcessPtr proc);
> >  
> > 
> 
> Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Nov 14, 2018 at 10:47:14 -0600, Chris Venteicher wrote:
> Quoting Michal Privoznik (2018-11-14 09:45:07)
> > On 11/11/2018 08:59 PM, Chris Venteicher wrote:
> > > Move a step closer to the function structure used elsewhere in
> > > qemu_process where qemuProcessStart and qemuProcessStop are the exposed
> > > functions.
> > > 
> > > qemuProcessStartQmp mirrors qemuProcessStart in calling sub functions to
> > > intialize, launch the process and connect the monitor to the QEMU
> > > process.
> > > 
> > > static functions qemuProcessInitQmp, qemuProcessLaunchQmp and
> > > qemuConnectMonitorQmp are also introduced.
> > > 
> > > qemuProcessLaunchQmp is just renamed from qemuProcessRun and
> > > encapsulates all of the original code.
> > > 
> > > qemuProcessInitQmp and qemuProcessMonitorQmp are introduced as empty
> > > wrappers into which subsequent patches will partition code from
> > > qemuProcessLaunchQmp.
> > > 
...
> > > +/* Initialize configuration and paths prior to starting QEMU
> > > + */
> > > +static int
> > > +qemuProcessInitQmp(qemuProcessPtr proc)
> > > +{
> > > +    int ret = -1;
> > > +
> > > +    VIR_DEBUG("Beginning VM startup process"
> > > +              "emulator=%s",
> > > +              proc->binary);
> > > +
> > > +    ret = 0;
> > > +
> > > +    VIR_DEBUG("ret=%i", ret);
> > > +    return ret;
> > > +}
> > > +
> > 
> > I am sorry, but I'm failing to see the purpose of this function.
> > 
> > > +
> > > +/* Launch QEMU Process
> > > + */
> > > +static int
> > > +qemuProcessLaunchQmp(qemuProcessPtr proc)
> > > +{
> > >      virDomainXMLOptionPtr xmlopt = NULL;
> > >      const char *machine;
> > >      int status = 0;
> > > @@ -8226,6 +8247,77 @@ qemuProcessRun(qemuProcessPtr proc){
> > >  }
> > >  
> > >  
> > > +/* Connect Monitor to QEMU Process
> > > + */
> > > +static int
> > > +qemuConnectMonitorQmp(qemuProcessPtr proc)
> > > +{
> > > +    int ret = -1;
> > > +
> > > +    VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
> > > +              proc, NULLSTR(proc->binary), (long long) proc->pid);
> > > +
> > > +    ret = 0;
> > > +
> > > +    VIR_DEBUG("ret=%i", ret);
> > > +    return ret;
> > > +}
> > 
> > Or this function. Looking into next patches I can see that you're
> > extending them. Well, I still think it's not worth introducing empty
> > functions, just do the rename as you're doing in next patches.
> 
> Yep, I was trying to make it easier to review but didn't explain well
> enough from the start. Sorry I wasn't clear.
> 
> Patch 10 (this patch) and 12 are about making it possible to do simple
> cut/paste moves on semi-complicated blocks of original code moved
> within patches 11 and 13.
> 
> The goal was to make patches 11 and 13 easy to review because
> I don't actually change the code.  It's just moved.
> 
> If this seems good with the better explanation I can just try to make
> that clear in the commit message for patch 10 and 12.
> 
> If it's more confusing this way I can start out with qemuProcesStartQmp
> only calling qemuProcessLaunchQmp and the add qemuProcessInitQmp and
> qemuConnectMonitorQmp as individual commits with full contents and just
> explain that the guts are cut/paste moves with no changes in the commit
> message.

The approach would be fine, but I believe Michal just didn't see any
reason to split the function in three parts in the first place.

qemuProcessStart which you're trying to mirror was split into several
phase because there is a code which needs to call the individual phases
separately so that additional logic can be put between the phases.

But it doesn't look like you need to do anything like this in your
series. So unless you want to somehow merge parts of the code for
qemuProcessStart and qemuProcessStartQmp, there's no real reason for the
split. So either you need to explain why we need to split the function
or you can drop this and the follow up patches doing the split.

And since this and the changes I suggested for the patches 1-9 may
result non-trivial changes to the other patches in this series, I'm
going to stop my series now. The main reason is that it is not entirely
obvious which of the following patches are just advancing the function
split or fixing issues introduced by the split and which of them would
be needed anyway.

So please, digest the review and send an updated version for further
review. And as explained in my reply to the cover latter, please send
both this refactoring and the new CPU related code in one series.

I hope it won't take me so long to review. I apologize for the delay
this time.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list