[libvirt] [PATCH 12/24] src/xenxs: Refactor code parsing OS config

[David Kiarie]

On Fri, Aug 08, 2014 at 03:50:44PM -0600, Jim Fehlig wrote:
> Kiarie Kahurani wrote:
> > introduce function
> >   xenParseXMOS(virConfPtr conf,...........);
> > which parses the OS config instead
> >
> > signed-off-by: Kiarie Kahurani <davidkiarie4 at gmail.com>
> > Signed-off-by: Kiarie Kahurani <davidkiarie4 at gmail.com>
> > ---
> >  src/xenxs/xen_xm.c | 80 ++++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 50 insertions(+), 30 deletions(-)
> >
> > diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> > index 32c6d8c..b21d794 100644
> > --- a/src/xenxs/xen_xm.c
> > +++ b/src/xenxs/xen_xm.c
> > @@ -1176,33 +1176,26 @@ xenParseXMGeneralMeta(virConfPtr conf, virDomainDefPtr def,
> >  
> >      return 0;
> >  }
> > -/*
> > - * Turn a config record into a lump of XML describing the
> > - * domain, suitable for later feeding for virDomainCreateXML
> > - */
> > -virDomainDefPtr
> > -xenParseXM(virConfPtr conf, int xendConfigVersion,
> > -           virCapsPtr caps)
> > -{
> > -    virDomainDefPtr def = NULL;
> > -    size_t i;
> >  
> > -    if (VIR_ALLOC(def) < 0)
> > -        return NULL;
> > -
> > -    def->virtType = VIR_DOMAIN_VIRT_XEN;
> > -    def->id = -1;
> >  
> > -    if (xenParseXMGeneralMeta(conf, def, caps) < 0)
> > -        goto cleanup;
> > +static int
> > +xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
> > +{
> > +    size_t i;
> >  
> >      if (STREQ(def->os.type, "hvm")) {
> >          const char *boot;
> > +        if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> > +            return -1;
> > +
> > +        if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> > +            return -1;
> >   
> 
> Opps, copy and paste one time too many.  This should also be outside the
> 'if (hvm)', as it was originally.
> 
> ACK otherwise.  I've fixed this up and pushed 1-12.  The rest will have
> to wait until next week.  Thanks!
> 
> Regards,
> Jim
> 
> > +
> >          if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) < 0)
> > -            goto cleanup;
> > +            return -1;
> >  
> >          if (xenXMConfigGetString(conf, "boot", &boot, "c") < 0)
> > -            goto cleanup;
> > +            return -1;
> >  
> >          for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
> >              switch (*boot) {
> > @@ -1226,28 +1219,58 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> >          const char *extra, *root;
> >  
> >          if (xenXMConfigCopyStringOpt(conf, "bootloader", &def->os.bootloader) < 0)
> > -            goto cleanup;
> > +            return -1;
> > +
> >          if (xenXMConfigCopyStringOpt(conf, "bootargs", &def->os.bootloaderArgs) < 0)
> > -            goto cleanup;
> > +            return -1;
> >  
> >          if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel) < 0)
> > -            goto cleanup;
> > +            return -1;
> > +
> >          if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd) < 0)
> > -            goto cleanup;
> > +            return -1;
> > +
> >          if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
> > -            goto cleanup;
> > +            return -1;
> > +
> >          if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
> > -            goto cleanup;
> > +            return -1;
> >  
> >          if (root) {
> >              if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) < 0)
> > -                goto cleanup;
> > +                return -1;
> > +
> >          } else {
> >              if (VIR_STRDUP(def->os.cmdline, extra) < 0)
> > -                goto cleanup;
> > +                return -1;
> > +
> >          }
> >      }
> >  
> > +    return 0;
> > +}
> > +/*
> > + * Turn a config record into a lump of XML describing the
> > + * domain, suitable for later feeding for virDomainCreateXML
> > + */
> > +virDomainDefPtr
> > +xenParseXM(virConfPtr conf, int xendConfigVersion,
> > +           virCapsPtr caps)
> > +{
> > +    virDomainDefPtr def = NULL;
> > +
> > +    if (VIR_ALLOC(def) < 0)
> > +        return NULL;
> > +
> > +    def->virtType = VIR_DOMAIN_VIRT_XEN;
> > +    def->id = -1;
> > +
> > +    if (xenParseXMGeneralMeta(conf, def, caps) < 0)
> > +        goto cleanup;
> > +
> > +    if (xenParseXMOS(conf, def) < 0)
> > +        goto cleanup;
> > +
> >      if (xenParseXMMem(conf, def) < 0)
> >          goto cleanup;
> >  
> > @@ -1263,9 +1286,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> >      if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
> >          goto cleanup;
> >  
> > -    if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> > -        goto cleanup;
> > -
> >      if (xenParseXMVif(conf, def) < 0)
> >          goto cleanup;
> >  
> >  

Thanks for fixing *sooo* much crap.

What can I say, I thought "git commit" and not "git send-email" signs of
the patches and hence would not resign my patches :( and again didn't
check the patches after sending 

David.