[v1] BaselineHypervisorCPU using QEMU QMP exchanges

[libvirt] [PATCHv2 04/11] qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents

Posted by Chris Venteicher 7 years, 7 months ago

These forms modify contents of a qemuMonitorCPUModelInfo structure but
do not allocate or free the actual structure.

Init - Initialize model name and empty properties within existing structure
FreeContents - Free model name and properties within existing structure
---
 src/qemu/qemu_monitor.c | 35 ++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_monitor.h |  4 ++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 371aaa15da..2d9297c3a7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3636,8 +3636,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model)
+{
+    int ret = -1;
+
+    if (!model)
+        goto cleanup;
+
+    model->name = NULL;
+    model->nprops = 0;
+    model->props = NULL;
+    model->props_migratable_valid = false;
+
+    if (VIR_STRDUP(model->name, name) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
 void
-qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
+qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info)
 {
     size_t i;
 
@@ -3652,6 +3675,16 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
 
     VIR_FREE(model_info->props);
     VIR_FREE(model_info->name);
+
+    model_info->nprops = 0;
+    model_info->props_migratable_valid = false;
+}
+
+
+void
+qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
+{
+    qemuMonitorCPUModelInfoFreeContents(model_info);
     VIR_FREE(model_info);
 }
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 208a7f5d21..0b84a91fbc 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1021,6 +1021,10 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
                                     qemuMonitorCPUModelInfoPtr *model_info);
 
 void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info);
+
+int qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 qemuMonitorCPUModelInfoPtr
 qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
-- 
2.17.1

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

Re: [libvirt] [PATCHv2 04/11] qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents

Posted by Jiri Denemark 7 years, 6 months ago

On Mon, Jul 09, 2018 at 22:56:48 -0500, Chris Venteicher wrote:
> These forms modify contents of a qemuMonitorCPUModelInfo structure but
> do not allocate or free the actual structure.
> 
> Init - Initialize model name and empty properties within existing structure
> FreeContents - Free model name and properties within existing structure

We call such function with "Clear" suffix, i.e.,
qemuMonitorCPUModelInfoClear.

But it is usually used when we have a structure stored somewhere
directly rather than having a pointer to it. And this was not the case
so far and I don't think there's any reason to introduce a code which
would need any of these functions.

NACK to this patch.

Jirka

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

Re: [libvirt] [PATCHv2 04/11] qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents

Posted by Chris Venteicher 7 years, 6 months ago

Quoting Jiri Denemark (2018-07-12 08:13:07)
> On Mon, Jul 09, 2018 at 22:56:48 -0500, Chris Venteicher wrote:
> > These forms modify contents of a qemuMonitorCPUModelInfo structure but
> > do not allocate or free the actual structure.
> > 
> > Init - Initialize model name and empty properties within existing structure
> > FreeContents - Free model name and properties within existing structure
> 
> We call such function with "Clear" suffix, i.e.,
> qemuMonitorCPUModelInfoClear.
> 
> But it is usually used when we have a structure stored somewhere
> directly rather than having a pointer to it. And this was not the case
> so far and I don't think there's any reason to introduce a code which
> would need any of these functions.
> 
> NACK to this patch.
>
Hi Jirka.  I see what you mean about combining dependent patches... It would be 
helpful if this patch was coupled with the qemuMonitorGetCPUModelExpansion 
patch.

Could I get you're thoughts on the qemuMonitorGetCPUModelExpansion patch to know 
what to do with this one?  

Specifically, I seem to need to send a full CPUModelInfo to QEMU (w/ model name 
+ properties) and get a full CPUModelInfo back from QEMU (again w/ model name + 
expanded properties).

I implemented this by rewriting the contents (property list) of the CPUModelInfo 
structure that is passed in to qemuMonitorGetCPUModelExpansion.  

I do a similar thing in qemuMonitorCPUModelInfoRemovePropByBoolValue... I 
rewrite the property list rather than allocating and returning a completely new 
CPUModelInfo for output.

Is this consistent with other functions or would I be better off allocating and 
returning a completely new CPUModelInfo for the output?

Or something else.

Thanks for feedback. Chris

> 
> Jirka

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

Re: [libvirt] [PATCHv2 04/11] qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents

Posted by Jiri Denemark 7 years, 6 months ago

On Thu, Jul 12, 2018 at 11:35:23 -0500, Chris Venteicher wrote:
> Quoting Jiri Denemark (2018-07-12 08:13:07)
> > On Mon, Jul 09, 2018 at 22:56:48 -0500, Chris Venteicher wrote:
> > > These forms modify contents of a qemuMonitorCPUModelInfo structure but
> > > do not allocate or free the actual structure.
> > > 
> > > Init - Initialize model name and empty properties within existing structure
> > > FreeContents - Free model name and properties within existing structure
> > 
> > We call such function with "Clear" suffix, i.e.,
> > qemuMonitorCPUModelInfoClear.
> > 
> > But it is usually used when we have a structure stored somewhere
> > directly rather than having a pointer to it. And this was not the case
> > so far and I don't think there's any reason to introduce a code which
> > would need any of these functions.
> > 
> > NACK to this patch.
> >
> Hi Jirka.  I see what you mean about combining dependent patches... It would be 
> helpful if this patch was coupled with the qemuMonitorGetCPUModelExpansion 
> patch.
> 
> Could I get you're thoughts on the qemuMonitorGetCPUModelExpansion patch to know 
> what to do with this one?  
> 
> Specifically, I seem to need to send a full CPUModelInfo to QEMU (w/ model name 
> + properties) and get a full CPUModelInfo back from QEMU (again w/ model name + 
> expanded properties).
> 
> I implemented this by rewriting the contents (property list) of the CPUModelInfo 
> structure that is passed in to qemuMonitorGetCPUModelExpansion.  
> 
> I do a similar thing in qemuMonitorCPUModelInfoRemovePropByBoolValue... I 
> rewrite the property list rather than allocating and returning a completely new 
> CPUModelInfo for output.
> 
> Is this consistent with other functions or would I be better off allocating and 
> returning a completely new CPUModelInfo for the output?

Yeah, that's the solution I was thinking about. With it you don't need
these fragile FreeContents/Init functions and the function won't touch
the input data unless it's finished at which point it will just free the
input and replace it with the new structure.

Jirka

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