David Kiarie wrote: > From: Kiarie Kahurani <davidkiarie4@xxxxxxxxx> > > introduce function > xenParseXMVif(virConfPtr conf, .......) > which parses char Vif config instead > > signed-off-by: David Kiarie<davidkiarie4@xxxxxxxxx> > --- > src/xenxs/xen_xm.c | 161 ++++++++++++++++++++++++++++------------------------- > 1 file changed, 86 insertions(+), 75 deletions(-) > > diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c > index 48b68fb..e6a1b7d 100644 > --- a/src/xenxs/xen_xm.c > +++ b/src/xenxs/xen_xm.c > @@ -1011,73 +1011,13 @@ int xenParseXMCharDev(virConfPtr conf, virDomainDefPtr def) > } > > > -virDomainDefPtr > -xenParseXM(virConfPtr conf, int xendConfigVersion, > - virCapsPtr caps) > +static > +int xenParseXMVif(virConfPtr conf, virDomainDefPtr def) > I'll stop mentioning return type and function name of separate lines :-). > { > - const char *str; > - int hvm = 0; > - virConfValuePtr list; > - virDomainDefPtr def = NULL; > - virDomainDiskDefPtr disk = NULL; > - virDomainNetDefPtr net = NULL; > - const char *defaultMachine; > char *script = NULL; > + virDomainNetDefPtr net = NULL; > + virConfValuePtr list = virConfGetValue(conf, "vif"); > > - if (VIR_ALLOC(def) < 0) > - return NULL; > - > - def->virtType = VIR_DOMAIN_VIRT_XEN; > - def->id = -1; > - > - if (xenXMConfigCopyString(conf, "name", &def->name) < 0) > - goto cleanup; > - if (xenXMConfigGetUUID(conf, "uuid", def->uuid) < 0) > - goto cleanup; > - > - > - if ((xenXMConfigGetString(conf, "builder", &str, "linux") == 0) && > - STREQ(str, "hvm")) > - hvm = 1; > - > - if (VIR_STRDUP(def->os.type, hvm ? "hvm" : "xen") < 0) > - goto cleanup; > - > - def->os.arch = > - virCapabilitiesDefaultGuestArch(caps, > - def->os.type, > - virDomainVirtTypeToString(def->virtType)); > - if (!def->os.arch) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("no supported architecture for os type '%s'"), > - def->os.type); > - goto cleanup; > - } > - > - defaultMachine = virCapabilitiesDefaultGuestMachine(caps, > - def->os.type, > - def->os.arch, > - virDomainVirtTypeToString(def->virtType)); > - if (defaultMachine != NULL) { > - if (VIR_STRDUP(def->os.machine, defaultMachine) < 0) > - goto cleanup; > - } > - > - if (xenParseXMOS(conf, def) < 0) > - goto cleanup; > - if (xenParseXMMem(conf, def) < 0) > - goto cleanup; > - if (xenParseXMEventsActions(conf, def) < 0) > - goto cleanup; > - if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0) > - goto cleanup; > - if (xenParseXMCPUFeatures(conf, def) < 0) > - goto cleanup; > - if (xenParseXMDisk(conf, def, xendConfigVersion) < 0) > - goto cleanup; > - if (xenParseXMVfb(conf, def, xendConfigVersion) < 0) > - goto cleanup; > - list = virConfGetValue(conf, "vif"); > if (list && list->type == VIR_CONF_LIST) { > list = list->list; > while (list) { > @@ -1098,7 +1038,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > > if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) > goto skipnic; > - > More spurious whitespace changes. > key = list->str; > while (key) { > char *data; > @@ -1107,7 +1046,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > if (!(data = strchr(key, '='))) > goto skipnic; > data++; > - > Here too. > if (STRPREFIX(key, "mac=")) { > int len = nextkey ? (nextkey - data) : sizeof(mac) - 1; > if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { > @@ -1133,14 +1071,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > int len = nextkey ? (nextkey - data) : sizeof(model) - 1; > if (virStrncpy(model, data, len, sizeof(model)) == NULL) { > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Model %s too big for destination"), data); > + _("Model %s too big for destination"), > + data); > goto skipnic; > } > } else if (STRPREFIX(key, "type=")) { > int len = nextkey ? (nextkey - data) : sizeof(type) - 1; > if (virStrncpy(type, data, len, sizeof(type)) == NULL) { > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Type %s too big for destination"), data); > + _("Type %s too big for destination"), > + data); > goto skipnic; > } > } else if (STRPREFIX(key, "vifname=")) { > @@ -1155,7 +1095,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > int len = nextkey ? (nextkey - data) : sizeof(ip) - 1; > if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("IP %s too big for destination"), data); > + _("IP %s too big for destination"), > + data); > The original actually fit within 80 columns. > goto skipnic; > } > } > @@ -1169,7 +1110,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > > if (VIR_ALLOC(net) < 0) > goto cleanup; > - > if (mac[0]) { > if (virMacAddrParse(mac, &net->mac) < 0) { > virReportError(VIR_ERR_INTERNAL_ERROR, > @@ -1198,28 +1138,99 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, > if (script && script[0] && > VIR_STRDUP(net->script, script) < 0) > goto cleanup; > - > if (model[0] && > VIR_STRDUP(net->model, model) < 0) > goto cleanup; > - > if (!model[0] && type[0] && STREQ(type, "netfront") && > VIR_STRDUP(net->model, "netfront") < 0) > goto cleanup; > - > if (vifname[0] && > VIR_STRDUP(net->ifname, vifname) < 0) > goto cleanup; > - > if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) > goto cleanup; > - > + VIR_FREE(script); > The existing whitespace in these two hunks is fine IMO and makes the function more readable. Regards, Jim > skipnic: > list = list->next; > virDomainNetDefFree(net); > } > } > > + return 0; > + > + cleanup: > + VIR_FREE(script); > + return -1; > +} > + > + > +virDomainDefPtr > +xenParseXM(virConfPtr conf, int xendConfigVersion, > + virCapsPtr caps) > +{ > + const char *str; > + int hvm = 0; > + virDomainDefPtr def = NULL; > + virDomainDiskDefPtr disk = NULL; > + virDomainNetDefPtr net = NULL; > + const char *defaultMachine; > + char *script = NULL; > + > + if (VIR_ALLOC(def) < 0) > + return NULL; > + > + def->virtType = VIR_DOMAIN_VIRT_XEN; > + def->id = -1; > + > + if (xenXMConfigCopyString(conf, "name", &def->name) < 0) > + goto cleanup; > + if (xenXMConfigGetUUID(conf, "uuid", def->uuid) < 0) > + goto cleanup; > + > + > + if ((xenXMConfigGetString(conf, "builder", &str, "linux") == 0) && > + STREQ(str, "hvm")) > + hvm = 1; > + > + if (VIR_STRDUP(def->os.type, hvm ? "hvm" : "xen") < 0) > + goto cleanup; > + > + def->os.arch = > + virCapabilitiesDefaultGuestArch(caps, > + def->os.type, > + virDomainVirtTypeToString(def->virtType)); > + if (!def->os.arch) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("no supported architecture for os type '%s'"), > + def->os.type); > + goto cleanup; > + } > + > + defaultMachine = virCapabilitiesDefaultGuestMachine(caps, > + def->os.type, > + def->os.arch, > + virDomainVirtTypeToString(def->virtType)); > + if (defaultMachine != NULL) { > + if (VIR_STRDUP(def->os.machine, defaultMachine) < 0) > + goto cleanup; > + } > + > + if (xenParseXMOS(conf, def) < 0) > + goto cleanup; > + if (xenParseXMMem(conf, def) < 0) > + goto cleanup; > + if (xenParseXMEventsActions(conf, def) < 0) > + goto cleanup; > + if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0) > + goto cleanup; > + if (xenParseXMCPUFeatures(conf, def) < 0) > + goto cleanup; > + if (xenParseXMDisk(conf, def, xendConfigVersion) < 0) > + goto cleanup; > + if (xenParseXMVfb(conf, def, xendConfigVersion) < 0) > + goto cleanup; > + if (xenParseXMVif(conf, def) < 0) > + goto cleanup; > if (xenParseXMPCI(conf, def) < 0) > goto cleanup; > if (hvm) { > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list