[libvirt] [PATCH v2 10/25] src/xenxs:Refactor code parsing Vif
Jim Fehlig
jfehlig at suse.com
Mon Aug 4 20:18:27 UTC 2014
David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4 at gmail.com>
>
> introduce function
> xenParseXMVif(virConfPtr conf, .......)
> which parses char Vif config instead
>
> signed-off-by: David Kiarie<davidkiarie4 at gmail.com>
> ---
> 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) {
>
More information about the libvir-list
mailing list