David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4@xxxxxxxxx>
>
> A couple of miscellaneous fixed and wrap code common code into
> xenParseConfigCommon(...).I will drop some of the functions later
> though
>
> signed-off-by: David Kiarie<davidkiarie4@xxxxxxxxx>
> ---
> src/xenxs/xen_xm.c | 134 ++++++++++++++++++++++++++++-------------------------
> src/xenxs/xen_xm.h | 3 +-
> 2 files changed, 73 insertions(+), 64 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index 312d890..657dd1c 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -458,7 +458,7 @@ static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
> }
>
> return 0;
> -cleanup:
> + cleanup:
>
This should be in 3/17 to fix 'make syntax-check'.
> VIR_FREE(script);
> return -1;
> }
> @@ -1026,7 +1026,8 @@ static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
>
> if (STREQ(def->os.type, "hvm")) {
> const char *boot;
> -
> + if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> + return -1;
>
This replicates the next lines of code. Not needed IMO.
> if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> return -1;
>
> @@ -1122,54 +1123,13 @@ static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def)
>
> return 0;
> }
> -virDomainDefPtr
> -xenParseXM(virConfPtr conf, int xendConfigVersion,
> - virCapsPtr caps)
> +static int xenParseXMCharDev(virConfPtr conf, virDomainDefPtr def)
> {
> const char *str;
> - int hvm = 0;
> - virConfValuePtr list;
> - virDomainDefPtr def = NULL;
> - virDomainDiskDefPtr disk = NULL;
> - virDomainNetDefPtr net = NULL;
> - virDomainGraphicsDefPtr graphics = NULL;
> - char *script = NULL;
> - char *listenAddr = NULL;
> -
> - if (VIR_ALLOC(def) < 0)
> - return NULL;
> -
> - def->virtType = VIR_DOMAIN_VIRT_XEN;
> - def->id = -1;
> - if (xenParseXMGeneral(conf, def, caps) < 0)
> - goto cleanup;
> -
> - if (xenParseXMOS(conf, def) < 0)
> - goto cleanup;
> + virConfValuePtr value = NULL;
> + virDomainChrDefPtr chr = NULL;
>
> - hvm = (STREQ(def->os.type, "hvm"));
> - if (xenParseXMMem(conf, def) < 0)
> - goto cleanup;
> - if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0)
> - goto cleanup;
> - if (xenParseXMVif(conf, def) < 0)
> - goto cleanup;
> - if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0)
> - goto cleanup;
> - if (xenParseXMEventsActions(conf, def) < 0)
> - goto cleanup;
> - if (xenParseXMPCI(conf, def) < 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 (xenParseXMMisc(conf, def) < 0)
> - goto cleanup;
> - if (hvm) {
> - virDomainChrDefPtr chr = NULL;
> + if (STREQ(def->os.type, "hvm")) {
>
> if (xenXMConfigGetString(conf, "parallel", &str, NULL) < 0)
> goto cleanup;
> @@ -1179,7 +1139,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
>
> if (chr) {
> if (VIR_ALLOC_N(def->parallels, 1) < 0) {
> - virDomainChrDefFree(chr);
> goto cleanup;
> }
> chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL;
> @@ -1190,21 +1149,21 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> }
>
> /* Try to get the list of values to support multiple serial ports */
> - list = virConfGetValue(conf, "serial");
> - if (list && list->type == VIR_CONF_LIST) {
> + value = virConfGetValue(conf, "serial");
> + if (value && value->type == VIR_CONF_LIST) {
> int portnum = -1;
>
> - list = list->list;
> - while (list) {
> + value = value->list;
> + while (value) {
> char *port = NULL;
>
> - if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
> + if ((value->type != VIR_CONF_STRING) || (value->str == NULL))
> goto cleanup;
>
> - port = list->str;
> + port = value->str;
> portnum++;
> if (STREQ(port, "none")) {
> - list = list->next;
> + value = value->next;
> continue;
> }
>
> @@ -1215,11 +1174,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> chr->target.port = portnum;
>
> if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) {
> - virDomainChrDefFree(chr);
> goto cleanup;
> }
>
> - list = list->next;
> + value = value->next;
> }
> } else {
> /* If domain is not using multiple serial ports we parse data old way */
> @@ -1249,16 +1207,66 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
> def->consoles[0]->target.port = 0;
> def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
> }
> - VIR_FREE(script);
> +
> + return 0;
> +
> + cleanup:
> + virDomainChrDefFree(chr);
> + return -1;
> +}
>
I think introducing xenParseXMCharDev and xenParseConfigCommon should be
done in separate patches.
> +int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
> + virCapsPtr caps, int xendConfigVersion )
> +{
> +
> + if (xenParseXMGeneral(conf, def, caps) < 0)
> + return -1;
> +
> + if (xenParseXMOS(conf, def) < 0)
> + return -1;
> +
> + if (xenParseXMMem(conf, def) < 0)
> + return -1;
> + if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0)
> + return -1;
> + if (xenParseXMEventsActions(conf, def) < 0)
> + return -1;
> + if (xenParseXMPCI(conf, def) < 0)
> + return -1;
> + if (xenParseXMCPUFeatures(conf, def) < 0)
> + return -1;
> +
> + if (xenParseXMMisc(conf, def) < 0)
> + return -1;
> + if (xenParseXMCharDev(conf, def) < 0)
> + return -1;
> +
> + if (xenParseXMVfb(conf, def, xendConfigVersion) < 0)
> + return -1;
> +
> + return 0;
> +}
> +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 (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0)
> + goto cleanup;
> + if (xenParseXMVif(conf, def) < 0)
> + goto cleanup;
> + if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
> + goto cleanup;
> +
> return def;
>
> cleanup:
> - virDomainGraphicsDefFree(graphics);
> - virDomainNetDefFree(net);
> - virDomainDiskDefFree(disk);
> virDomainDefFree(def);
> - VIR_FREE(script);
> - VIR_FREE(listenAddr);
>
All of these *Free calls should have been removed in the respective
patches that removed the code from xenParseXM.
> return NULL;
> }
>
> diff --git a/src/xenxs/xen_xm.h b/src/xenxs/xen_xm.h
> index 629a4b3..621cbe1 100644
> --- a/src/xenxs/xen_xm.h
> +++ b/src/xenxs/xen_xm.h
> @@ -29,7 +29,8 @@
> # include "internal.h"
> # include "virconf.h"
> # include "domain_conf.h"
> -
> +int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def,
> + virCapsPtr caps, int xendConfigVersion);
>
Blank line between these function declarations.
Regards,
Jim
> virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def,
> int xendConfigVersion);
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list