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

Kiarie Kahurani wrote: > introduce function > xenParseXMOS(virConfPtr conf,...........); > which parses the OS config instead > > signed-off-by: Kiarie Kahurani <davidkiarie4(a)gmail.com&gt; > Signed-off-by: Kiarie Kahurani <davidkiarie4(a)gmail.com&gt; > --- > 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; > >