Re: [PATCH v3 06/20] Make qemuCapsProbeMachineTypes & qemuCapsProbeCPUModels static

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

 



On Wed, Sep 26, 2012 at 11:42:09AM +0200, Jiri Denemark wrote:
> On Tue, Sep 25, 2012 at 18:59:59 +0100, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> > 
> > The qemuCapsProbeMachineTypes & qemuCapsProbeCPUModels methods
> > do not need to be invoked directly anymore. Make them static
> > and refactor them to directly populate the qemuCapsPtr object
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> >  src/qemu/qemu_capabilities.c | 238 ++++++++++++++++---------------------------
> >  src/qemu/qemu_capabilities.h |  13 +--
> >  2 files changed, 90 insertions(+), 161 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 888c56c..7e4ea50 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -305,17 +305,16 @@ qemuCapsProbeCommand(const char *qemu,
> >   */
> >  static int
> >  qemuCapsParseMachineTypesStr(const char *output,
> > -                             virCapsGuestMachinePtr **machines,
> > -                             size_t *nmachines)
> > +                             qemuCapsPtr caps)
> >  {
> >      const char *p = output;
> >      const char *next;
> > -    virCapsGuestMachinePtr *list = NULL;
> > -    int nitems = 0;
> > +    size_t defIdx = 0;
> >  
> >      do {
> >          const char *t;
> > -        virCapsGuestMachinePtr machine;
> > +        char *name;
> > +        char *canonical = NULL;
> >  
> >          if ((next = strchr(p, '\n')))
> >              ++next;
> > @@ -326,56 +325,61 @@ qemuCapsParseMachineTypesStr(const char *output,
> >          if (!(t = strchr(p, ' ')) || (next && t >= next))
> >              continue;
> >  
> > -        if (VIR_ALLOC(machine) < 0)
> > +        if (!(name = strndup(p, t - p)))
> >              goto no_memory;
> >  
> > -        if (!(machine->name = strndup(p, t - p))) {
> > -            VIR_FREE(machine);
> > -            goto no_memory;
> > -        }
> > -
> > -        if (VIR_REALLOC_N(list, nitems + 1) < 0) {
> > -            VIR_FREE(machine->name);
> > -            VIR_FREE(machine);
> > -            goto no_memory;
> > -        }
> > -
> > -        p = t;
> > -        if (!(t = strstr(p, "(default)")) || (next && t >= next)) {
> > -            list[nitems++] = machine;
> > -        } else {
> > -            /* put the default first in the list */
> > -            memmove(list + 1, list, sizeof(*list) * nitems);
> > -            list[0] = machine;
> > -            nitems++;
> > -        }
> > +        if (strstr(p, "(default)"))
> > +            defIdx = caps->nmachineTypes;
> 
> While this will work because it sets defIdx to 1, 2, 4, ... until it stops at
> the one which is really default. Can we preserve the condition that checks if
> (default) was found at the current line rather than just somewhere in the rest
> of the qemu output to make this less magic? And I would even preserve the
> p = t assignment above.

Ok, so IIUC the folowing change:

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fed7a06..b519db7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -328,7 +328,8 @@ qemuCapsParseMachineTypesStr(const char *output,
         if (!(name = strndup(p, t - p)))
             goto no_memory;
 
-        if (strstr(p, "(default)"))
+        p = t;
+        if (!(t = strstr(p, "(default)")) && (!next || t < next)) {
             defIdx = caps->nmachineTypes;
 
         if ((t = strstr(p, "(alias of ")) && (!next || t < next)) {

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]