3:42 p.m.
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function xenParseXMGeneral(virConfPtr conf, ...);
This function parses the xm general options such as uuid and name
signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 65 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 25a042d..1953a85 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char
*uuid)
* 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)
+static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
+ virCapsPtr caps)
{
+
+ const char *defaultMachine;
const char *str;
int hvm = 0;
- int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
- virDomainHostdevDefPtr hostdev = NULL;
- size_t i;
- const char *defaultMachine;
- int vmlocaltime = 0;
- unsigned long count;
- char *script = NULL;
- char *listenAddr = 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;
+ return -1;
if (xenXMConfigGetUUID(conf, "uuid", def->uuid) < 0)
- goto cleanup;
-
+ return -1;
if ((xenXMConfigGetString(conf, "builder", &str, "linux") ==
0) &&
STREQ(str, "hvm"))
hvm = 1;
if (VIR_STRDUP(def->os.type, hvm ? "hvm" : "xen") < 0)
- goto cleanup;
+ return -1;
def->os.arch =
virCapabilitiesDefaultGuestArch(caps,
@@ -300,7 +281,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no supported architecture for os type
'%s'"),
def->os.type);
- goto cleanup;
+ return -1;
}
defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
@@ -309,9 +290,37 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainVirtTypeToString(def->virtType));
if (defaultMachine != NULL) {
if (VIR_STRDUP(def->os.machine, defaultMachine) < 0)
- goto cleanup;
+ return -1;
}
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ int val;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ virDomainHostdevDefPtr hostdev = NULL;
+ size_t i;
+ int vmlocaltime = 0;
+ unsigned long count;
+ 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;
+ hvm = (STREQ(def->os.type, "hvm"));
if (hvm) {
const char *boot;
if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
--
1.8.4.5
3:42 p.m.
New subject: [libvirt] [PREPOST 02/17] src/xenxs:Introduce function
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function xenParseXMMem(virConfPtr conf,....);
This function parses the xm memory options
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 1953a85..dc840e5 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -294,6 +294,21 @@ static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
}
return 0;
}
+static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
+{
+ if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
+ MIN_XEN_GUEST_SIZE * 2) < 0)
+ return -1;
+
+ if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
+ def->mem.cur_balloon) < 0)
+ return -1;
+
+ def->mem.cur_balloon *= 1024;
+ def->mem.max_balloon *= 1024;
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -372,18 +387,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
}
}
-
- if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
- MIN_XEN_GUEST_SIZE * 2) < 0)
- goto cleanup;
-
- if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
- def->mem.cur_balloon) < 0)
+ if (xenParseXMMem(conf, def) < 0)
goto cleanup;
- def->mem.cur_balloon *= 1024;
- def->mem.max_balloon *= 1024;
-
if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
MAX_VIRT_CPUS < count)
goto cleanup;
--
1.8.4.5
12:41 a.m.
New subject: [libvirt] [PREPOST 02/17] src/xenxs:Introduce function
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function xenParseXMMem(virConfPtr conf,....);
This function parses the xm memory options
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 1953a85..dc840e5 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -294,6 +294,21 @@ static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
}
return 0;
}
+static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
This file has a mixture of 1 and 2 blank lines between functions, but 2
are preferred in new code.
Regards,
Jim
+{
+ if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
+ MIN_XEN_GUEST_SIZE * 2) < 0)
+ return -1;
+
+ if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
+ def->mem.cur_balloon) < 0)
+ return -1;
+
+ def->mem.cur_balloon *= 1024;
+ def->mem.max_balloon *= 1024;
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -372,18 +387,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
}
}
-
- if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
- MIN_XEN_GUEST_SIZE * 2) < 0)
- goto cleanup;
-
- if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
- def->mem.cur_balloon) < 0)
+ if (xenParseXMMem(conf, def) < 0)
goto cleanup;
- def->mem.cur_balloon *= 1024;
- def->mem.max_balloon *= 1024;
-
if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
MAX_VIRT_CPUS < count)
goto cleanup;
3:42 p.m.
New subject: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
I introduced the function
xenFormatXMVif(virConfPtr conf, ......);
which parses network configuration
signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 298 ++++++++++++++++++++++++++++-------------------------
1 file changed, 155 insertions(+), 143 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
+{
+ char *script = NULL;
+ virDomainNetDefPtr net = NULL;
+ virConfValuePtr list = virConfGetValue(conf, "vif");
+ if (list && list->type == VIR_CONF_LIST) {
+ list = list->list;
+ while (list) {
+ char model[10];
+ char type[10];
+ char ip[16];
+ char mac[18];
+ char bridge[50];
+ char vifname[50];
+ char *key;
+
+ bridge[0] = '\0';
+ mac[0] = '\0';
+ ip[0] = '\0';
+ model[0] = '\0';
+ type[0] = '\0';
+ vifname[0] = '\0';
+
+ if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+ goto skipnic;
+
+ key = list->str;
+ while (key) {
+ char *data;
+ char *nextkey = strchr(key, ',');
+
+ if (!(data = strchr(key, '=')))
+ goto skipnic;
+ data++;
+
+ if (STRPREFIX(key, "mac=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+ if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("MAC address %s too big for
destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "bridge=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
+ if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Bridge %s too big for destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "script=")) {
+ int len = nextkey ? (nextkey - data) : strlen(data);
+ VIR_FREE(script);
+ if (VIR_STRNDUP(script, data, len) < 0)
+ goto cleanup;
+ } else if (STRPREFIX(key, "model=")) {
+ 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);
+ 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);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "vifname=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
+ if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Vifname %s too big for
destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "ip=")) {
+ 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);
+ goto skipnic;
+ }
+ }
+
+ while (nextkey && (nextkey[0] == ',' ||
+ nextkey[0] == ' ' ||
+ nextkey[0] == '\t'))
+ nextkey++;
+ key = nextkey;
+ }
+
+ if (VIR_ALLOC(net) < 0)
+ goto cleanup;
+
+ if (mac[0]) {
+ if (virMacAddrParse(mac, &net->mac) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("malformed mac address '%s'"),
mac);
+ goto cleanup;
+ }
+ }
+
+ if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
+ STREQ_NULLABLE(script, "vif-vnic")) {
+ net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ } else {
+ net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
+ }
+
+ if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge)
< 0)
+ goto cleanup;
+ if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip) < 0)
+ goto cleanup;
+ } else {
+ if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr, ip) <
0)
+ goto cleanup;
+ }
+
+ 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);
+ skipnic:
+ list = list->next;
+ virDomainNetDefFree(net);
+ }
+ }
+
+ return 0;
+cleanup:
+ VIR_FREE(script);
+ return -1;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -677,149 +830,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
}
}
-
- list = virConfGetValue(conf, "vif");
- if (list && list->type == VIR_CONF_LIST) {
- list = list->list;
- while (list) {
- char model[10];
- char type[10];
- char ip[16];
- char mac[18];
- char bridge[50];
- char vifname[50];
- char *key;
-
- bridge[0] = '\0';
- mac[0] = '\0';
- ip[0] = '\0';
- model[0] = '\0';
- type[0] = '\0';
- vifname[0] = '\0';
-
- if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
- goto skipnic;
-
- key = list->str;
- while (key) {
- char *data;
- char *nextkey = strchr(key, ',');
-
- if (!(data = strchr(key, '=')))
- goto skipnic;
- data++;
-
- if (STRPREFIX(key, "mac=")) {
- int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
- if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("MAC address %s too big for
destination"),
- data);
- goto skipnic;
- }
- } else if (STRPREFIX(key, "bridge=")) {
- int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
- if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Bridge %s too big for destination"),
- data);
- goto skipnic;
- }
- } else if (STRPREFIX(key, "script=")) {
- int len = nextkey ? (nextkey - data) : strlen(data);
- VIR_FREE(script);
- if (VIR_STRNDUP(script, data, len) < 0)
- goto cleanup;
- } else if (STRPREFIX(key, "model=")) {
- 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);
- 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);
- goto skipnic;
- }
- } else if (STRPREFIX(key, "vifname=")) {
- int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
- if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Vifname %s too big for
destination"),
- data);
- goto skipnic;
- }
- } else if (STRPREFIX(key, "ip=")) {
- 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);
- goto skipnic;
- }
- }
-
- while (nextkey && (nextkey[0] == ',' ||
- nextkey[0] == ' ' ||
- nextkey[0] == '\t'))
- nextkey++;
- key = nextkey;
- }
-
- if (VIR_ALLOC(net) < 0)
- goto cleanup;
-
- if (mac[0]) {
- if (virMacAddrParse(mac, &net->mac) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("malformed mac address '%s'"),
mac);
- goto cleanup;
- }
- }
-
- if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
- STREQ_NULLABLE(script, "vif-vnic")) {
- net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
- } else {
- net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
- }
-
- if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge)
< 0)
- goto cleanup;
- if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip) < 0)
- goto cleanup;
- } else {
- if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr, ip) <
0)
- goto cleanup;
- }
-
- 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;
-
- skipnic:
- list = list->next;
- virDomainNetDefFree(net);
- }
- }
+ if (xenParseXMVif(conf, def) < 0)
+ goto cleanup;
list = virConfGetValue(conf, "pci");
if (list && list->type == VIR_CONF_LIST) {
--
1.8.4.5
1:19 a.m.
New subject: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
I introduced the function
xenFormatXMVif(virConfPtr conf, ......);
which parses network configuration
signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 298 ++++++++++++++++++++++++++++-------------------------
1 file changed, 155 insertions(+), 143 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
Same comment here about blank lines between functions. I won't bore you
with repeating myself in all the patches. Also, remember Eric's comment
about function return type on one line and name and params on another.
E.g.
static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
+{
+ char *script = NULL;
+ virDomainNetDefPtr net = NULL;
+ virConfValuePtr list = virConfGetValue(conf, "vif");
Style is to have a blank line after local variable declarations.
I think you should also define a local variable ret, initialized to -1.
+ if (list && list->type == VIR_CONF_LIST) {
+ list = list->list;
+ while (list) {
+ char model[10];
+ char type[10];
+ char ip[16];
+ char mac[18];
+ char bridge[50];
+ char vifname[50];
+ char *key;
+
+ bridge[0] = '\0';
+ mac[0] = '\0';
+ ip[0] = '\0';
+ model[0] = '\0';
+ type[0] = '\0';
+ vifname[0] = '\0';
+
+ if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+ goto skipnic;
+
+ key = list->str;
+ while (key) {
+ char *data;
+ char *nextkey = strchr(key, ',');
+
+ if (!(data = strchr(key, '=')))
+ goto skipnic;
+ data++;
+
+ if (STRPREFIX(key, "mac=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+ if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("MAC address %s too big for
destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "bridge=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
+ if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Bridge %s too big for
destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "script=")) {
+ int len = nextkey ? (nextkey - data) : strlen(data);
+ VIR_FREE(script);
+ if (VIR_STRNDUP(script, data, len) < 0)
+ goto cleanup;
+ } else if (STRPREFIX(key, "model=")) {
+ 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);
+ 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);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "vifname=")) {
+ int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
+ if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Vifname %s too big for
destination"),
+ data);
+ goto skipnic;
+ }
+ } else if (STRPREFIX(key, "ip=")) {
+ 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);
+ goto skipnic;
+ }
+ }
+
+ while (nextkey && (nextkey[0] == ',' ||
+ nextkey[0] == ' ' ||
+ nextkey[0] == '\t'))
+ nextkey++;
+ key = nextkey;
+ }
+
+ if (VIR_ALLOC(net) < 0)
+ goto cleanup;
+
+ if (mac[0]) {
+ if (virMacAddrParse(mac, &net->mac) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("malformed mac address '%s'"),
mac);
+ goto cleanup;
+ }
+ }
+
+ if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
+ STREQ_NULLABLE(script, "vif-vnic")) {
+ net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+ } else {
+ net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
+ }
+
+ if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge)
< 0)
+ goto cleanup;
+ if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip) < 0)
+ goto cleanup;
+ } else {
+ if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr, ip) <
0)
+ goto cleanup;
+ }
+
+ 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);
Can be freed in cleanup.
+ skipnic:
+ list = list->next;
+ virDomainNetDefFree(net);
Set 'net = NULL' now that it has been freed.
+ }
+ }
+
+ return 0;
Set 'ret = 0'.
+cleanup:
+ VIR_FREE(script);
+ return -1;
Call virDomainNetDefFree(net) here and return ret. As written, net can
leak if you jump to the cleanup label after it is alloced.
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -677,149 +830,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
}
}
Since all of this vif parsing code is removed, you can also remove the
'net' and 'script' local variables. They are no longer used in
xenParseXM().
Regards,
Jim
11:19 p.m.
New subject: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
Jim Fehlig wrote:
David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>
> I introduced the function
> xenFormatXMVif(virConfPtr conf, ......);
> which parses network configuration
>
> signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
> ---
> src/xenxs/xen_xm.c | 298 ++++++++++++++++++++++++++++-------------------------
> 1 file changed, 155 insertions(+), 143 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index dc840e5..26ebd5a 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
>
> return 0;
> }
> +static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
>
>
Same comment here about blank lines between functions. I won't bore you
with repeating myself in all the patches. Also, remember Eric's comment
about function return type on one line and name and params on another.
E.g.
static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
> +{
> + char *script = NULL;
> + virDomainNetDefPtr net = NULL;
> + virConfValuePtr list = virConfGetValue(conf, "vif");
>
>
Style is to have a blank line after local variable declarations.
I think you should also define a local variable ret, initialized to -1.
> + if (list && list->type == VIR_CONF_LIST) {
> + list = list->list;
> + while (list) {
> + char model[10];
> + char type[10];
> + char ip[16];
> + char mac[18];
> + char bridge[50];
> + char vifname[50];
> + char *key;
> +
> + bridge[0] = '\0';
> + mac[0] = '\0';
> + ip[0] = '\0';
> + model[0] = '\0';
> + type[0] = '\0';
> + vifname[0] = '\0';
> +
> + if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
> + goto skipnic;
> +
> + key = list->str;
> + while (key) {
> + char *data;
> + char *nextkey = strchr(key, ',');
> +
> + if (!(data = strchr(key, '=')))
> + goto skipnic;
> + data++;
> +
> + if (STRPREFIX(key, "mac=")) {
> + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
> + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("MAC address %s too big for
destination"),
> + data);
> + goto skipnic;
> + }
> + } else if (STRPREFIX(key, "bridge=")) {
> + int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
> + if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Bridge %s too big for
destination"),
> + data);
> + goto skipnic;
> + }
> + } else if (STRPREFIX(key, "script=")) {
> + int len = nextkey ? (nextkey - data) : strlen(data);
> + VIR_FREE(script);
> + if (VIR_STRNDUP(script, data, len) < 0)
> + goto cleanup;
> + } else if (STRPREFIX(key, "model=")) {
> + 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);
> + 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);
> + goto skipnic;
> + }
> + } else if (STRPREFIX(key, "vifname=")) {
> + int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
> + if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Vifname %s too big for
destination"),
> + data);
> + goto skipnic;
> + }
> + } else if (STRPREFIX(key, "ip=")) {
> + 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);
> + goto skipnic;
> + }
> + }
> +
> + while (nextkey && (nextkey[0] == ',' ||
> + nextkey[0] == ' ' ||
> + nextkey[0] == '\t'))
> + nextkey++;
> + key = nextkey;
> + }
> +
> + if (VIR_ALLOC(net) < 0)
> + goto cleanup;
> +
> + if (mac[0]) {
> + if (virMacAddrParse(mac, &net->mac) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("malformed mac address
'%s'"), mac);
> + goto cleanup;
> + }
> + }
> +
> + if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
> + STREQ_NULLABLE(script, "vif-vnic")) {
> + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
> + } else {
> + net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> + }
> +
> + if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> + if (bridge[0] && VIR_STRDUP(net->data.bridge.brname,
bridge) < 0)
> + goto cleanup;
> + if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip) <
0)
> + goto cleanup;
> + } else {
> + if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr, ip)
< 0)
> + goto cleanup;
> + }
> +
> + 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);
>
>
Can be freed in cleanup.
> + skipnic:
> + list = list->next;
> + virDomainNetDefFree(net);
>
>
Set 'net = NULL' now that it has been freed.
Ignore this comment. It is similar to the comment I made in 7/17, which
Eric clarified
https://www.redhat.com/archives/libvir-list/2014-July/msg00628.html
Regards,
Jim
3:33 p.m.
New subject: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
I think you comments on PCI parsing code could also apply here
On Wed, Jul 16, 2014 at 12:19 AM, Jim Fehlig <jfehlig(a)suse.com> wrote:
Jim Fehlig wrote:
> David Kiarie wrote:
>
>> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>>
>> I introduced the function
>> xenFormatXMVif(virConfPtr conf, ......);
>> which parses network configuration
>>
>> signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
>> ---
>> src/xenxs/xen_xm.c | 298 ++++++++++++++++++++++++++++-------------------------
>> 1 file changed, 155 insertions(+), 143 deletions(-)
>>
>> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
>> index dc840e5..26ebd5a 100644
>> --- a/src/xenxs/xen_xm.c
>> +++ b/src/xenxs/xen_xm.c
>> @@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, virDomainDefPtr
def)
>>
>> return 0;
>> }
>> +static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
>>
>>
>
> Same comment here about blank lines between functions. I won't bore you
> with repeating myself in all the patches. Also, remember Eric's comment
> about function return type on one line and name and params on another.
> E.g.
>
> static int
> xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
> {
> ...
> }
>
>
>> +{
>> + char *script = NULL;
>> + virDomainNetDefPtr net = NULL;
>> + virConfValuePtr list = virConfGetValue(conf, "vif");
>>
>>
>
> Style is to have a blank line after local variable declarations.
>
> I think you should also define a local variable ret, initialized to -1.
>
>
>> + if (list && list->type == VIR_CONF_LIST) {
>> + list = list->list;
>> + while (list) {
>> + char model[10];
>> + char type[10];
>> + char ip[16];
>> + char mac[18];
>> + char bridge[50];
>> + char vifname[50];
>> + char *key;
>> +
>> + bridge[0] = '\0';
>> + mac[0] = '\0';
>> + ip[0] = '\0';
>> + model[0] = '\0';
>> + type[0] = '\0';
>> + vifname[0] = '\0';
>> +
>> + if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
>> + goto skipnic;
>> +
>> + key = list->str;
>> + while (key) {
>> + char *data;
>> + char *nextkey = strchr(key, ',');
>> +
>> + if (!(data = strchr(key, '=')))
>> + goto skipnic;
>> + data++;
>> +
>> + if (STRPREFIX(key, "mac=")) {
>> + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
>> + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("MAC address %s too big for
destination"),
>> + data);
>> + goto skipnic;
here
>> + }
>> + } else if (STRPREFIX(key, "bridge=")) {
>> + int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
>> + if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL)
{
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Bridge %s too big for
destination"),
>> + data);
>> + goto skipnic;
here
>> + }
>> + } else if (STRPREFIX(key, "script=")) {
>> + int len = nextkey ? (nextkey - data) : strlen(data);
>> + VIR_FREE(script);
>> + if (VIR_STRNDUP(script, data, len) < 0)
>> + goto cleanup;
>> + } else if (STRPREFIX(key, "model=")) {
>> + 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);
>> + goto skipnic;
here
>> + }
>> + } 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);
>> + goto skipnic;
here
>> + }
>> + } else if (STRPREFIX(key, "vifname=")) {
>> + int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
>> + if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL)
{
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Vifname %s too big for
destination"),
>> + data);
>> + goto skipnic;
here
>> + }
>> + } else if (STRPREFIX(key, "ip=")) {
>> + 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);
>> + goto skipnic;
here
>> + }
>> + }
>> +
>> + while (nextkey && (nextkey[0] == ',' ||
>> + nextkey[0] == ' ' ||
>> + nextkey[0] == '\t'))
>> + nextkey++;
>> + key = nextkey;
>> + }
>> +
>> + if (VIR_ALLOC(net) < 0)
>> + goto cleanup;
>> +
>> + if (mac[0]) {
>> + if (virMacAddrParse(mac, &net->mac) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("malformed mac address
'%s'"), mac);
>> + goto cleanup;
>> + }
>> + }
>> +
>> + if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge") ||
>> + STREQ_NULLABLE(script, "vif-vnic")) {
>> + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
>> + } else {
>> + net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
>> + }
>> +
>> + if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
>> + if (bridge[0] && VIR_STRDUP(net->data.bridge.brname,
bridge) < 0)
>> + goto cleanup;
>> + if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip)
< 0)
>> + goto cleanup;
>> + } else {
>> + if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr,
ip) < 0)
>> + goto cleanup;
>> + }
>> +
>> + 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);
>>
>>
>
> Can be freed in cleanup.
>
>
>> + skipnic:
>> + list = list->next;
>> + virDomainNetDefFree(net);
>>
>>
>
> Set 'net = NULL' now that it has been freed.
>
Ignore this comment. It is similar to the comment I made in 7/17, which
Eric clarified
https://www.redhat.com/archives/libvir-list/2014-July/msg00628.html
Regards,
Jim
Should I got ahead and make the changes?
3:36 p.m.
New subject: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
The use of virReportError,
On Wed, Jul 16, 2014 at 4:33 PM, David kiarie <davidkiarie4(a)gmail.com> wrote:
I think you comments on PCI parsing code could also apply here
On Wed, Jul 16, 2014 at 12:19 AM, Jim Fehlig <jfehlig(a)suse.com> wrote:
> Jim Fehlig wrote:
>> David Kiarie wrote:
>>
>>> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>>>
>>> I introduced the function
>>> xenFormatXMVif(virConfPtr conf, ......);
>>> which parses network configuration
>>>
>>> signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
>>> ---
>>> src/xenxs/xen_xm.c | 298
++++++++++++++++++++++++++++-------------------------
>>> 1 file changed, 155 insertions(+), 143 deletions(-)
>>>
>>> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
>>> index dc840e5..26ebd5a 100644
>>> --- a/src/xenxs/xen_xm.c
>>> +++ b/src/xenxs/xen_xm.c
>>> @@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf,
virDomainDefPtr def)
>>>
>>> return 0;
>>> }
>>> +static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
>>>
>>>
>>
>> Same comment here about blank lines between functions. I won't bore you
>> with repeating myself in all the patches. Also, remember Eric's comment
>> about function return type on one line and name and params on another.
>> E.g.
>>
>> static int
>> xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
>> {
>> ...
>> }
>>
>>
>>> +{
>>> + char *script = NULL;
>>> + virDomainNetDefPtr net = NULL;
>>> + virConfValuePtr list = virConfGetValue(conf, "vif");
>>>
>>>
>>
>> Style is to have a blank line after local variable declarations.
>>
>> I think you should also define a local variable ret, initialized to -1.
>>
>>
>>> + if (list && list->type == VIR_CONF_LIST) {
>>> + list = list->list;
>>> + while (list) {
>>> + char model[10];
>>> + char type[10];
>>> + char ip[16];
>>> + char mac[18];
>>> + char bridge[50];
>>> + char vifname[50];
>>> + char *key;
>>> +
>>> + bridge[0] = '\0';
>>> + mac[0] = '\0';
>>> + ip[0] = '\0';
>>> + model[0] = '\0';
>>> + type[0] = '\0';
>>> + vifname[0] = '\0';
>>> +
>>> + if ((list->type != VIR_CONF_STRING) || (list->str ==
NULL))
>>> + goto skipnic;
>>> +
>>> + key = list->str;
>>> + while (key) {
>>> + char *data;
>>> + char *nextkey = strchr(key, ',');
>>> +
>>> + if (!(data = strchr(key, '=')))
>>> + goto skipnic;
>>> + data++;
>>> +
>>> + if (STRPREFIX(key, "mac=")) {
>>> + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
>>> + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("MAC address %s too big for
destination"),
>>> + data);
>>> + goto skipnic;
here
>>> + }
>>> + } else if (STRPREFIX(key, "bridge=")) {
>>> + int len = nextkey ? (nextkey - data) : sizeof(bridge) -
1;
>>> + if (virStrncpy(bridge, data, len, sizeof(bridge)) ==
NULL) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("Bridge %s too big for
destination"),
>>> + data);
>>> + goto skipnic;
here
>>> + }
>>> + } else if (STRPREFIX(key, "script=")) {
>>> + int len = nextkey ? (nextkey - data) : strlen(data);
>>> + VIR_FREE(script);
>>> + if (VIR_STRNDUP(script, data, len) < 0)
>>> + goto cleanup;
>>> + } else if (STRPREFIX(key, "model=")) {
>>> + 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);
>>> + goto skipnic;
here
>>> + }
>>> + } 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);
>>> + goto skipnic;
here
>>> + }
>>> + } else if (STRPREFIX(key, "vifname=")) {
>>> + int len = nextkey ? (nextkey - data) : sizeof(vifname) -
1;
>>> + if (virStrncpy(vifname, data, len, sizeof(vifname)) ==
NULL) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("Vifname %s too big for
destination"),
>>> + data);
>>> + goto skipnic;
here
>>> + }
>>> + } else if (STRPREFIX(key, "ip=")) {
>>> + 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);
>>> + goto skipnic;
here
>>> + }
>>> + }
>>> +
>>> + while (nextkey && (nextkey[0] == ',' ||
>>> + nextkey[0] == ' ' ||
>>> + nextkey[0] == '\t'))
>>> + nextkey++;
>>> + key = nextkey;
>>> + }
>>> +
>>> + if (VIR_ALLOC(net) < 0)
>>> + goto cleanup;
>>> +
>>> + if (mac[0]) {
>>> + if (virMacAddrParse(mac, &net->mac) < 0) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("malformed mac address
'%s'"), mac);
>>> + goto cleanup;
>>> + }
>>> + }
>>> +
>>> + if (bridge[0] || STREQ_NULLABLE(script, "vif-bridge")
||
>>> + STREQ_NULLABLE(script, "vif-vnic")) {
>>> + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
>>> + } else {
>>> + net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
>>> + }
>>> +
>>> + if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
>>> + if (bridge[0] &&
VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
>>> + goto cleanup;
>>> + if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr,
ip) < 0)
>>> + goto cleanup;
>>> + } else {
>>> + if (ip[0] &&
VIR_STRDUP(net->data.ethernet.ipaddr, ip) < 0)
>>> + goto cleanup;
>>> + }
>>> +
>>> + 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);
>>>
>>>
>>
>> Can be freed in cleanup.
>>
>>
>>> + skipnic:
>>> + list = list->next;
>>> + virDomainNetDefFree(net);
>>>
>>>
>>
>> Set 'net = NULL' now that it has been freed.
>>
>
> Ignore this comment. It is similar to the comment I made in 7/17, which
> Eric clarified
>
> https://www.redhat.com/archives/libvir-list/2014-July/msg00628.html
>
> Regards,
> Jim
>
Should I got ahead and make the changes?
3:42 p.m.
New subject: [libvirt] [PREPOST 04/17] src/xenxs:Refactor code parsing time and event actions config
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the functions
xenParseXMTimeOffset(.......);
xenParseXMEventActions(......);
These parse the time config and Event actions configs
respectively
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 128 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 72 insertions(+), 56 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 26ebd5a..e30ab9c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -462,6 +462,73 @@ cleanup:
VIR_FREE(script);
return -1;
}
+static int xenParseXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ int vmlocaltime;
+
+ if (xenXMConfigGetBool(conf, "localtime", &vmlocaltime, 0) < 0)
+ return -1;
+
+ if (STREQ(def->os.type, "hvm")) {
+ /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
+ if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+ if (vmlocaltime)
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+ else
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+ def->clock.data.utc_reset = true;
+ } else {
+ unsigned long rtc_timeoffset;
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+ if (xenXMConfigGetULong(conf, "rtc_timeoffset",
&rtc_timeoffset, 0) < 0)
+ return -1;
+ def->clock.data.variable.adjustment = (int)rtc_timeoffset;
+ def->clock.data.variable.basis = vmlocaltime ?
+ VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
+ VIR_DOMAIN_CLOCK_BASIS_UTC;
+ }
+ } else {
+ /* PV domains do not have an emulated RTC and the offset is fixed. */
+ def->clock.offset = vmlocaltime ?
+ VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
+ VIR_DOMAIN_CLOCK_OFFSET_UTC;
+ def->clock.data.utc_reset = true;
+ } /* !hvm */
+
+ return 0;
+}
+static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+
+ if (xenXMConfigGetString(conf, "on_poweroff", &str,
"destroy") < 0)
+ return -1;
+ if ((def->onPoweroff = virDomainLifecycleTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_poweroff"), str);
+ return -1;
+ }
+
+ if (xenXMConfigGetString(conf, "on_reboot", &str, "restart")
< 0)
+ return -1;
+ if ((def->onReboot = virDomainLifecycleTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_reboot"), str);
+ return -1;
+ }
+
+ if (xenXMConfigGetString(conf, "on_crash", &str, "restart")
< 0)
+ return -1;
+ if ((def->onCrash = virDomainLifecycleCrashTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_crash"), str);
+ return -1;
+ }
+
+ return 0;
+
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -476,7 +543,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainGraphicsDefPtr graphics = NULL;
virDomainHostdevDefPtr hostdev = NULL;
size_t i;
- int vmlocaltime = 0;
unsigned long count;
char *script = NULL;
char *listenAddr = NULL;
@@ -556,32 +622,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
goto cleanup;
- if (xenXMConfigGetString(conf, "on_poweroff", &str,
"destroy") < 0)
- goto cleanup;
- if ((def->onPoweroff = virDomainLifecycleTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_poweroff"), str);
- goto cleanup;
- }
-
- if (xenXMConfigGetString(conf, "on_reboot", &str, "restart")
< 0)
- goto cleanup;
- if ((def->onReboot = virDomainLifecycleTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_reboot"), str);
- goto cleanup;
- }
-
- if (xenXMConfigGetString(conf, "on_crash", &str, "restart")
< 0)
- goto cleanup;
- if ((def->onCrash = virDomainLifecycleCrashTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_crash"), str);
- goto cleanup;
- }
-
-
-
if (hvm) {
if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
goto cleanup;
@@ -621,35 +661,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
def->clock.timers[0] = timer;
}
}
- if (xenXMConfigGetBool(conf, "localtime", &vmlocaltime, 0) < 0)
- goto cleanup;
-
- if (hvm) {
- /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
- if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
- if (vmlocaltime)
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
- else
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
- def->clock.data.utc_reset = true;
- } else {
- unsigned long rtc_timeoffset;
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
- if (xenXMConfigGetULong(conf, "rtc_timeoffset",
&rtc_timeoffset, 0) < 0)
- goto cleanup;
- def->clock.data.variable.adjustment = (int)rtc_timeoffset;
- def->clock.data.variable.basis = vmlocaltime ?
- VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
- VIR_DOMAIN_CLOCK_BASIS_UTC;
- }
- } else {
- /* PV domains do not have an emulated RTC and the offset is fixed. */
- def->clock.offset = vmlocaltime ?
- VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
- VIR_DOMAIN_CLOCK_OFFSET_UTC;
- def->clock.data.utc_reset = true;
- } /* !hvm */
-
if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
goto cleanup;
@@ -833,6 +844,11 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (xenParseXMVif(conf, def) < 0)
goto cleanup;
+ if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+ if (xenParseXMEventsActions(conf, def) < 0)
+ goto cleanup;
+
list = virConfGetValue(conf, "pci");
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
--
1.8.4.5
1:29 a.m.
New subject: [libvirt] [PREPOST 04/17] src/xenxs:Refactor code parsing time and event actions config
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the functions
xenParseXMTimeOffset(.......);
xenParseXMEventActions(......);
These aren't related and should be split into two patches IMO.
Regards,
Jim
These parse the time config and Event actions configs
respectively
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 128 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 72 insertions(+), 56 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 26ebd5a..e30ab9c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -462,6 +462,73 @@ cleanup:
VIR_FREE(script);
return -1;
}
+static int xenParseXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ int vmlocaltime;
+
+ if (xenXMConfigGetBool(conf, "localtime", &vmlocaltime, 0) < 0)
+ return -1;
+
+ if (STREQ(def->os.type, "hvm")) {
+ /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
+ if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+ if (vmlocaltime)
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+ else
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+ def->clock.data.utc_reset = true;
+ } else {
+ unsigned long rtc_timeoffset;
+ def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+ if (xenXMConfigGetULong(conf, "rtc_timeoffset",
&rtc_timeoffset, 0) < 0)
+ return -1;
+ def->clock.data.variable.adjustment = (int)rtc_timeoffset;
+ def->clock.data.variable.basis = vmlocaltime ?
+ VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
+ VIR_DOMAIN_CLOCK_BASIS_UTC;
+ }
+ } else {
+ /* PV domains do not have an emulated RTC and the offset is fixed. */
+ def->clock.offset = vmlocaltime ?
+ VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
+ VIR_DOMAIN_CLOCK_OFFSET_UTC;
+ def->clock.data.utc_reset = true;
+ } /* !hvm */
+
+ return 0;
+}
+static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+
+ if (xenXMConfigGetString(conf, "on_poweroff", &str,
"destroy") < 0)
+ return -1;
+ if ((def->onPoweroff = virDomainLifecycleTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_poweroff"), str);
+ return -1;
+ }
+
+ if (xenXMConfigGetString(conf, "on_reboot", &str, "restart")
< 0)
+ return -1;
+ if ((def->onReboot = virDomainLifecycleTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_reboot"), str);
+ return -1;
+ }
+
+ if (xenXMConfigGetString(conf, "on_crash", &str, "restart")
< 0)
+ return -1;
+ if ((def->onCrash = virDomainLifecycleCrashTypeFromString(str)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected value %s for on_crash"), str);
+ return -1;
+ }
+
+ return 0;
+
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -476,7 +543,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainGraphicsDefPtr graphics = NULL;
virDomainHostdevDefPtr hostdev = NULL;
size_t i;
- int vmlocaltime = 0;
unsigned long count;
char *script = NULL;
char *listenAddr = NULL;
@@ -556,32 +622,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
goto cleanup;
- if (xenXMConfigGetString(conf, "on_poweroff", &str,
"destroy") < 0)
- goto cleanup;
- if ((def->onPoweroff = virDomainLifecycleTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_poweroff"), str);
- goto cleanup;
- }
-
- if (xenXMConfigGetString(conf, "on_reboot", &str, "restart")
< 0)
- goto cleanup;
- if ((def->onReboot = virDomainLifecycleTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_reboot"), str);
- goto cleanup;
- }
-
- if (xenXMConfigGetString(conf, "on_crash", &str, "restart")
< 0)
- goto cleanup;
- if ((def->onCrash = virDomainLifecycleCrashTypeFromString(str)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected value %s for on_crash"), str);
- goto cleanup;
- }
-
-
-
if (hvm) {
if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
goto cleanup;
@@ -621,35 +661,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
def->clock.timers[0] = timer;
}
}
- if (xenXMConfigGetBool(conf, "localtime", &vmlocaltime, 0) < 0)
- goto cleanup;
-
- if (hvm) {
- /* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset */
- if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
- if (vmlocaltime)
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
- else
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
- def->clock.data.utc_reset = true;
- } else {
- unsigned long rtc_timeoffset;
- def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
- if (xenXMConfigGetULong(conf, "rtc_timeoffset",
&rtc_timeoffset, 0) < 0)
- goto cleanup;
- def->clock.data.variable.adjustment = (int)rtc_timeoffset;
- def->clock.data.variable.basis = vmlocaltime ?
- VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
- VIR_DOMAIN_CLOCK_BASIS_UTC;
- }
- } else {
- /* PV domains do not have an emulated RTC and the offset is fixed. */
- def->clock.offset = vmlocaltime ?
- VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
- VIR_DOMAIN_CLOCK_OFFSET_UTC;
- def->clock.data.utc_reset = true;
- } /* !hvm */
-
if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
goto cleanup;
@@ -833,6 +844,11 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (xenParseXMVif(conf, def) < 0)
goto cleanup;
+ if (xenParseXMTimeOffset(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+ if (xenParseXMEventsActions(conf, def) < 0)
+ goto cleanup;
+
list = virConfGetValue(conf, "pci");
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
3:42 p.m.
New subject: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenParseXMPCI(.........);
which parses PCI config
signed-off-by:David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 192 +++++++++++++++++++++++++++--------------------------
1 file changed, 99 insertions(+), 93 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index e30ab9c..7a5b47c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -529,6 +529,102 @@ static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr
def)
return 0;
}
+static int
+xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
+{
+ virConfValuePtr list = virConfGetValue(conf, "pci");
+ virDomainHostdevDefPtr hostdev = NULL;
+ if (list && list->type == VIR_CONF_LIST) {
+ list = list->list;
+ while (list) {
+ char domain[5];
+ char bus[3];
+ char slot[3];
+ char func[2];
+ char *key, *nextkey;
+ int domainID;
+ int busID;
+ int slotID;
+ int funcID;
+
+ domain[0] = bus[0] = slot[0] = func[0] = '\0';
+
+ if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+ goto skippci;
+
+ /* pci=['0000:00:1b.0','0000:00:13.0'] */
+ if (!(key = list->str))
+ goto skippci;
+ if (!(nextkey = strchr(key, ':')))
+ goto skippci;
+
+ if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Domain %s too big for destination"), key);
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (!(nextkey = strchr(key, ':')))
+ goto skippci;
+
+ if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Bus %s too big for destination"), key);
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (!(nextkey = strchr(key, '.')))
+ goto skippci;
+
+ if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Slot %s too big for destination"), key);
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (strlen(key) != 1)
+ goto skippci;
+
+ if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Function %s too big for destination"), key);
+ goto skippci;
+ }
+
+ if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
+ goto skippci;
+ if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
+ goto skippci;
+ if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
+ goto skippci;
+ if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
+ goto skippci;
+
+ if (!(hostdev = virDomainHostdevDefAlloc()))
+ return -1;
+
+ hostdev->managed = false;
+ hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
+ hostdev->source.subsys.u.pci.addr.domain = domainID;
+ hostdev->source.subsys.u.pci.addr.bus = busID;
+ hostdev->source.subsys.u.pci.addr.slot = slotID;
+ hostdev->source.subsys.u.pci.addr.function = funcID;
+
+ if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0)
{
+ virDomainHostdevDefFree(hostdev);
+ return -1;
+ }
+
+ skippci:
+ list = list->next;
+ }
+ }
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -541,7 +637,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainDiskDefPtr disk = NULL;
virDomainNetDefPtr net = NULL;
virDomainGraphicsDefPtr graphics = NULL;
- virDomainHostdevDefPtr hostdev = NULL;
size_t i;
unsigned long count;
char *script = NULL;
@@ -848,98 +943,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMEventsActions(conf, def) < 0)
goto cleanup;
-
- list = virConfGetValue(conf, "pci");
- if (list && list->type == VIR_CONF_LIST) {
- list = list->list;
- while (list) {
- char domain[5];
- char bus[3];
- char slot[3];
- char func[2];
- char *key, *nextkey;
- int domainID;
- int busID;
- int slotID;
- int funcID;
-
- domain[0] = bus[0] = slot[0] = func[0] = '\0';
-
- if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
- goto skippci;
-
- /* pci=['0000:00:1b.0','0000:00:13.0'] */
- if (!(key = list->str))
- goto skippci;
- if (!(nextkey = strchr(key, ':')))
- goto skippci;
-
- if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Domain %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (!(nextkey = strchr(key, ':')))
- goto skippci;
-
- if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Bus %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (!(nextkey = strchr(key, '.')))
- goto skippci;
-
- if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Slot %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (strlen(key) != 1)
- goto skippci;
-
- if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Function %s too big for destination"), key);
- goto skippci;
- }
-
- if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
- goto skippci;
- if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
- goto skippci;
- if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
- goto skippci;
- if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
- goto skippci;
-
- if (!(hostdev = virDomainHostdevDefAlloc()))
- goto cleanup;
-
- hostdev->managed = false;
- hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
- hostdev->source.subsys.u.pci.addr.domain = domainID;
- hostdev->source.subsys.u.pci.addr.bus = busID;
- hostdev->source.subsys.u.pci.addr.slot = slotID;
- hostdev->source.subsys.u.pci.addr.function = funcID;
-
- if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0)
{
- virDomainHostdevDefFree(hostdev);
- goto cleanup;
- }
-
- skippci:
- list = list->next;
- }
- }
-
- if (hvm) {
+ if (xenParseXMPCI(conf, def) < 0)
+ goto cleanup;
+ if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
if (str &&
--
1.8.4.5
7:09 p.m.
New subject: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenParseXMPCI(.........);
which parses PCI config
signed-off-by:David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 192 +++++++++++++++++++++++++++--------------------------
1 file changed, 99 insertions(+), 93 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index e30ab9c..7a5b47c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -529,6 +529,102 @@ static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr
def)
return 0;
}
+static int
+xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
+{
+ virConfValuePtr list = virConfGetValue(conf, "pci");
+ virDomainHostdevDefPtr hostdev = NULL;
Blank line after local variables.
+ if (list && list->type == VIR_CONF_LIST) {
+ list = list->list;
+ while (list) {
+ char domain[5];
+ char bus[3];
+ char slot[3];
+ char func[2];
+ char *key, *nextkey;
+ int domainID;
+ int busID;
+ int slotID;
+ int funcID;
+
+ domain[0] = bus[0] = slot[0] = func[0] = '\0';
+
+ if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+ goto skippci;
+
+ /* pci=['0000:00:1b.0','0000:00:13.0'] */
+ if (!(key = list->str))
+ goto skippci;
+ if (!(nextkey = strchr(key, ':')))
+ goto skippci;
+
+ if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Domain %s too big for destination"), key);
Pre-existing, but while we are touching the code, I wonder if the use of
virReportError here is correct? The device is skipped if there is a
problem parsing it. I think these errors should be logged via VIR_WARN,
but would like confirmation from another libvirt dev before asking you
to change them. At the very least, the error should be changed to
VIR_ERR_CONF_SYNTAX.
Regards,
Jim
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (!(nextkey = strchr(key, ':')))
+ goto skippci;
+
+ if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Bus %s too big for destination"), key);
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (!(nextkey = strchr(key, '.')))
+ goto skippci;
+
+ if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Slot %s too big for destination"), key);
+ goto skippci;
+ }
+
+ key = nextkey + 1;
+ if (strlen(key) != 1)
+ goto skippci;
+
+ if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Function %s too big for destination"),
key);
+ goto skippci;
+ }
+
+ if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
+ goto skippci;
+ if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
+ goto skippci;
+ if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
+ goto skippci;
+ if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
+ goto skippci;
+
+ if (!(hostdev = virDomainHostdevDefAlloc()))
+ return -1;
+
+ hostdev->managed = false;
+ hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
+ hostdev->source.subsys.u.pci.addr.domain = domainID;
+ hostdev->source.subsys.u.pci.addr.bus = busID;
+ hostdev->source.subsys.u.pci.addr.slot = slotID;
+ hostdev->source.subsys.u.pci.addr.function = funcID;
+
+ if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0)
{
+ virDomainHostdevDefFree(hostdev);
+ return -1;
+ }
+
+ skippci:
+ list = list->next;
+ }
+ }
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -541,7 +637,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainDiskDefPtr disk = NULL;
virDomainNetDefPtr net = NULL;
virDomainGraphicsDefPtr graphics = NULL;
- virDomainHostdevDefPtr hostdev = NULL;
size_t i;
unsigned long count;
char *script = NULL;
@@ -848,98 +943,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMEventsActions(conf, def) < 0)
goto cleanup;
-
- list = virConfGetValue(conf, "pci");
- if (list && list->type == VIR_CONF_LIST) {
- list = list->list;
- while (list) {
- char domain[5];
- char bus[3];
- char slot[3];
- char func[2];
- char *key, *nextkey;
- int domainID;
- int busID;
- int slotID;
- int funcID;
-
- domain[0] = bus[0] = slot[0] = func[0] = '\0';
-
- if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
- goto skippci;
-
- /* pci=['0000:00:1b.0','0000:00:13.0'] */
- if (!(key = list->str))
- goto skippci;
- if (!(nextkey = strchr(key, ':')))
- goto skippci;
-
- if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Domain %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (!(nextkey = strchr(key, ':')))
- goto skippci;
-
- if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Bus %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (!(nextkey = strchr(key, '.')))
- goto skippci;
-
- if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Slot %s too big for destination"), key);
- goto skippci;
- }
-
- key = nextkey + 1;
- if (strlen(key) != 1)
- goto skippci;
-
- if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Function %s too big for destination"),
key);
- goto skippci;
- }
-
- if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
- goto skippci;
- if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
- goto skippci;
- if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
- goto skippci;
- if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
- goto skippci;
-
- if (!(hostdev = virDomainHostdevDefAlloc()))
- goto cleanup;
-
- hostdev->managed = false;
- hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
- hostdev->source.subsys.u.pci.addr.domain = domainID;
- hostdev->source.subsys.u.pci.addr.bus = busID;
- hostdev->source.subsys.u.pci.addr.slot = slotID;
- hostdev->source.subsys.u.pci.addr.function = funcID;
-
- if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0)
{
- virDomainHostdevDefFree(hostdev);
- goto cleanup;
- }
-
- skippci:
- list = list->next;
- }
- }
-
- if (hvm) {
+ if (xenParseXMPCI(conf, def) < 0)
+ goto cleanup;
+ if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
if (str &&
11:26 p.m.
New subject: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
On 07/11/2014 11:09 AM, Jim Fehlig wrote:
David Kiarie wrote:
> +
> + /* pci=['0000:00:1b.0','0000:00:13.0'] */
> + if (!(key = list->str))
> + goto skippci;
> + if (!(nextkey = strchr(key, ':')))
> + goto skippci;
> +
> + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Domain %s too big for destination"),
key);
>
Pre-existing, but while we are touching the code, I wonder if the use of
virReportError here is correct? The device is skipped if there is a
problem parsing it. I think these errors should be logged via VIR_WARN,
but would like confirmation from another libvirt dev before asking you
to change them. At the very least, the error should be changed to
VIR_ERR_CONF_SYNTAX.
How easy is it to trigger this error path via user input? If the string
that we are splitting is normally provided from a sane source, using
VIR_ERR_INTERNAL_ERROR is okay; if the string we are splitting can come
from a user that can easily try to fuzz things to confuse us, then
VIR_ERR_CONF_SYNTAX is indeed nicer.
As for whether to skip a device we can't parse, or outright fail, I'm
not sure which is better. If you are just skipping the device, then
using VIR_WARN instead of virReportError will avoid the odd case of
returning success to the user while still having an error set.
Don't know if I helped or just made it more confusing.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:24 p.m.
New subject: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
Looking at both of your comments, I go with VIR_WARN
On Wed, Jul 16, 2014 at 12:26 AM, Eric Blake <eblake(a)redhat.com> wrote:
On 07/11/2014 11:09 AM, Jim Fehlig wrote:
> David Kiarie wrote:
>> +
>> + /* pci=['0000:00:1b.0','0000:00:13.0'] */
>> + if (!(key = list->str))
>> + goto skippci;
>> + if (!(nextkey = strchr(key, ':')))
>> + goto skippci;
>> +
>> + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) ==
NULL) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Domain %s too big for destination"),
key);
>>
>
> Pre-existing, but while we are touching the code, I wonder if the use of
> virReportError here is correct? The device is skipped if there is a
> problem parsing it. I think these errors should be logged via VIR_WARN,
> but would like confirmation from another libvirt dev before asking you
> to change them. At the very least, the error should be changed to
> VIR_ERR_CONF_SYNTAX.
How easy is it to trigger this error path via user input? If the string
that we are splitting is normally provided from a sane source, using
VIR_ERR_INTERNAL_ERROR is okay; if the string we are splitting can come
from a user that can easily try to fuzz things to confuse us, then
VIR_ERR_CONF_SYNTAX is indeed nicer.
As for whether to skip a device we can't parse, or outright fail, I'm
not sure which is better. If you are just skipping the device, then
using VIR_WARN instead of virReportError will avoid the odd case of
returning success to the user while still having an error set.
Don't know if I helped or just made it more confusing.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:43 p.m.
New subject: [libvirt] [PREPOST 06/17] src/xenxs:Refactor code parsing CPU config
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce xenParseXMCPUFeatures(.....);
This function parses config related to CPU and power
management
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 116 ++++++++++++++++++++++++++++-------------------------
1 file changed, 62 insertions(+), 54 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 7a5b47c..ebeeeb5 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -625,6 +625,66 @@ xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr def)
+{
+ unsigned long count;
+ const char *str;
+ int val;
+ if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
+ MAX_VIRT_CPUS < count)
+ return -1;
+ def->maxvcpus = count;
+ if (xenXMConfigGetULong(conf, "vcpu_avail", &count, -1) < 0)
+ return -1;
+ def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus);
+
+ if (xenXMConfigGetString(conf, "cpus", &str, NULL) < 0)
+ return -1;
+ if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
+ return -1;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON;
+
+ if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0)
+ return -1;
+ else if (val != -1) {
+ virDomainTimerDefPtr timer;
+
+ if (VIR_ALLOC_N(def->clock.timers, 1) < 0 ||
+ VIR_ALLOC(timer) < 0)
+ return -1;
+
+ timer->name = VIR_DOMAIN_TIMER_NAME_HPET;
+ timer->present = val;
+ timer->tickpolicy = -1;
+
+ def->clock.ntimers = 1;
+ def->clock.timers[0] = timer;
+ }
+ }
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -638,7 +698,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainNetDefPtr net = NULL;
virDomainGraphicsDefPtr graphics = NULL;
size_t i;
- unsigned long count;
char *script = NULL;
char *listenAddr = NULL;
@@ -703,59 +762,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
}
if (xenParseXMMem(conf, def) < 0)
goto cleanup;
-
- if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
- MAX_VIRT_CPUS < count)
- goto cleanup;
- def->maxvcpus = count;
- if (xenXMConfigGetULong(conf, "vcpu_avail", &count, -1) < 0)
- goto cleanup;
- def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus);
-
- if (xenXMConfigGetString(conf, "cpus", &str, NULL) < 0)
- goto cleanup;
- if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
- goto cleanup;
-
- if (hvm) {
- if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON;
-
- if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0)
- goto cleanup;
- else if (val != -1) {
- virDomainTimerDefPtr timer;
-
- if (VIR_ALLOC_N(def->clock.timers, 1) < 0 ||
- VIR_ALLOC(timer) < 0)
- goto cleanup;
-
- timer->name = VIR_DOMAIN_TIMER_NAME_HPET;
- timer->present = val;
- timer->tickpolicy = -1;
-
- def->clock.ntimers = 1;
- def->clock.timers[0] = timer;
- }
- }
if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
goto cleanup;
@@ -945,6 +951,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMPCI(conf, def) < 0)
goto cleanup;
+ if (xenParseXMCPUFeatures(conf, def) < 0)
+ goto cleanup;
if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
--
1.8.4.5
7:16 p.m.
New subject: [libvirt] [PREPOST 06/17] src/xenxs:Refactor code parsing CPU config
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce xenParseXMCPUFeatures(.....);
This function parses config related to CPU and power
management
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 116 ++++++++++++++++++++++++++++-------------------------
1 file changed, 62 insertions(+), 54 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 7a5b47c..ebeeeb5 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -625,6 +625,66 @@ xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr def)
+{
+ unsigned long count;
+ const char *str;
+ int val;
The usual whitespace comments, but otherwise looks good.
Regards,
Jim
+ if (xenXMConfigGetULong(conf, "vcpus", &count, 1)
< 0 ||
+ MAX_VIRT_CPUS < count)
+ return -1;
+ def->maxvcpus = count;
+ if (xenXMConfigGetULong(conf, "vcpu_avail", &count, -1) < 0)
+ return -1;
+ def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus);
+
+ if (xenXMConfigGetString(conf, "cpus", &str, NULL) < 0)
+ return -1;
+ if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
+ return -1;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
+ if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0)
+ return -1;
+ else if (val)
+ def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] =
VIR_DOMAIN_FEATURE_STATE_ON;
+
+ if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0)
+ return -1;
+ else if (val != -1) {
+ virDomainTimerDefPtr timer;
+
+ if (VIR_ALLOC_N(def->clock.timers, 1) < 0 ||
+ VIR_ALLOC(timer) < 0)
+ return -1;
+
+ timer->name = VIR_DOMAIN_TIMER_NAME_HPET;
+ timer->present = val;
+ timer->tickpolicy = -1;
+
+ def->clock.ntimers = 1;
+ def->clock.timers[0] = timer;
+ }
+ }
+
+ return 0;
+}
virDomainDefPtr
xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -638,7 +698,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainNetDefPtr net = NULL;
virDomainGraphicsDefPtr graphics = NULL;
size_t i;
- unsigned long count;
char *script = NULL;
char *listenAddr = NULL;
@@ -703,59 +762,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
}
if (xenParseXMMem(conf, def) < 0)
goto cleanup;
-
- if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
- MAX_VIRT_CPUS < count)
- goto cleanup;
- def->maxvcpus = count;
- if (xenXMConfigGetULong(conf, "vcpu_avail", &count, -1) < 0)
- goto cleanup;
- def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus);
-
- if (xenXMConfigGetString(conf, "cpus", &str, NULL) < 0)
- goto cleanup;
- if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0))
- goto cleanup;
-
- if (hvm) {
- if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
- if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0)
- goto cleanup;
- else if (val)
- def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] =
VIR_DOMAIN_FEATURE_STATE_ON;
-
- if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0)
- goto cleanup;
- else if (val != -1) {
- virDomainTimerDefPtr timer;
-
- if (VIR_ALLOC_N(def->clock.timers, 1) < 0 ||
- VIR_ALLOC(timer) < 0)
- goto cleanup;
-
- timer->name = VIR_DOMAIN_TIMER_NAME_HPET;
- timer->present = val;
- timer->tickpolicy = -1;
-
- def->clock.ntimers = 1;
- def->clock.timers[0] = timer;
- }
- }
if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
goto cleanup;
@@ -945,6 +951,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMPCI(conf, def) < 0)
goto cleanup;
+ if (xenParseXMCPUFeatures(conf, def) < 0)
+ goto cleanup;
if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
3:43 p.m.
New subject: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenParseXMDisk(..........);
Parses disk config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++-------------------------
1 file changed, 108 insertions(+), 96 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ebeeeb5..80a7941 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -685,87 +685,14 @@ static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr
def)
return 0;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
{
- const char *str;
- int hvm = 0;
- int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
+ const char *str = NULL;
virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
- size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
- const char *boot;
- if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
- goto cleanup;
-
- if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
- goto cleanup;
+ int hvm = STREQ(def->os.type, "hvm");
+ virConfValuePtr list = virConfGetValue(conf, "disk");
- for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
- switch (*boot) {
- case 'a':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
- break;
- case 'd':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
- break;
- case 'n':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
- break;
- case 'c':
- default:
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
- break;
- }
- def->os.nBootDevs++;
- }
- } else {
- const char *extra, *root;
-
- if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
- goto cleanup;
-
- if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
- goto cleanup;
-
- if (root) {
- if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra)
< 0)
- goto cleanup;
- } else {
- if (VIR_STRDUP(def->os.cmdline, extra) < 0)
- goto cleanup;
- }
- }
- if (xenParseXMMem(conf, def) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
- goto cleanup;
-
- list = virConfGetValue(conf, "disk");
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
while (list) {
@@ -779,7 +706,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
head = list->str;
if (!(disk = virDomainDiskDefNew()))
- goto cleanup;
+ return -1;
/*
* Disks have 3 components, SOURCE,DEST-DEVICE,MODE
@@ -798,10 +725,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
ignore_value(virDomainDiskSetSource(disk, NULL));
} else {
if (VIR_STRNDUP(tmp, head, offset - head) < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetSource(disk, tmp) < 0) {
VIR_FREE(tmp);
- goto cleanup;
+ return -1;
}
VIR_FREE(tmp);
}
@@ -815,12 +742,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (!(offset = strchr(head, ',')))
goto skipdisk;
if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0)
- goto cleanup;
+ return -1;
if (virStrncpy(disk->dst, head, offset - head,
(offset - head) + 1) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Dest file %s too big for destination"),
head);
- goto cleanup;
+ return -1;
}
head = offset + 1;
@@ -832,16 +759,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if ((tmp = strchr(src, ':')) != NULL) {
len = tmp - src;
if (VIR_STRNDUP(tmp, src, len) < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetDriver(disk, tmp) < 0) {
VIR_FREE(tmp);
- goto cleanup;
+ return -1;
}
VIR_FREE(tmp);
/* Strip the prefix we found off the source file name */
if (virDomainDiskSetSource(disk, src + len + 1) < 0)
- goto cleanup;
+ return -1;
src = virDomainDiskGetSource(disk);
}
@@ -854,7 +781,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
len = tmp - src;
if (VIR_STRNDUP(driverType, src, len) < 0)
- goto cleanup;
+ return -1;
if (STREQ(driverType, "aio"))
virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
else
@@ -865,12 +792,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown driver type %s"),
src);
- goto cleanup;
+ return -1;
}
/* Strip the prefix we found off the source file name */
if (virDomainDiskSetSource(disk, src + len + 1) < 0)
- goto cleanup;
+ return -1;
src = virDomainDiskGetSource(disk);
}
}
@@ -878,7 +805,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
/* No source, or driver name, so fix to phy: */
if (!virDomainDiskGetDriver(disk) &&
virDomainDiskSetDriver(disk, "phy") < 0)
- goto cleanup;
+ return -1;
/* phy: type indicates a block device */
@@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
/* Maintain list in sorted order according to target device name */
if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
- goto cleanup;
+ return -1;
skipdisk:
list = list->next;
@@ -922,26 +849,108 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (hvm && xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) {
if (xenXMConfigGetString(conf, "cdrom", &str, NULL) < 0)
- goto cleanup;
+ return -1;
if (str) {
if (!(disk = virDomainDiskDefNew()))
- goto cleanup;
+ return -1;
virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
if (virDomainDiskSetDriver(disk, "file") < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetSource(disk, str) < 0)
- goto cleanup;
+ return -1;
if (VIR_STRDUP(disk->dst, "hdc") < 0)
- goto cleanup;
+ return -1;
disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
disk->src->readonly = true;
if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+ return -1;
+ }
+ }
+
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ int val;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ size_t i;
+ 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;
+ hvm = (STREQ(def->os.type, "hvm"));
+ if (hvm) {
+ const char *boot;
+ if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
+ goto cleanup;
+
+ if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
+ goto cleanup;
+
+ for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
+ switch (*boot) {
+ case 'a':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
+ break;
+ case 'd':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
+ break;
+ case 'n':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
+ break;
+ case 'c':
+ default:
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
+ break;
+ }
+ def->os.nBootDevs++;
+ }
+ } else {
+ const char *extra, *root;
+
+ if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
+ goto cleanup;
+
+ if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
+ goto cleanup;
+
+ if (root) {
+ if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra)
< 0)
+ goto cleanup;
+ } else {
+ if (VIR_STRDUP(def->os.cmdline, extra) < 0)
goto cleanup;
}
}
+ if (xenParseXMMem(conf, def) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
+ goto cleanup;
if (xenParseXMVif(conf, def) < 0)
goto cleanup;
@@ -953,6 +962,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMCPUFeatures(conf, def) < 0)
goto cleanup;
+ if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+
if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
--
1.8.4.5
9:04 p.m.
New subject: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenParseXMDisk(..........);
Parses disk config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++-------------------------
1 file changed, 108 insertions(+), 96 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ebeeeb5..80a7941 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -685,87 +685,14 @@ static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr
def)
return 0;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
{
- const char *str;
- int hvm = 0;
- int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
+ const char *str = NULL;
virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
- size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
- const char *boot;
- if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
- goto cleanup;
-
- if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
- goto cleanup;
+ int hvm = STREQ(def->os.type, "hvm");
+ virConfValuePtr list = virConfGetValue(conf, "disk");
- for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
- switch (*boot) {
- case 'a':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
- break;
- case 'd':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
- break;
- case 'n':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
- break;
- case 'c':
- default:
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
- break;
- }
- def->os.nBootDevs++;
- }
- } else {
- const char *extra, *root;
-
- if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
- goto cleanup;
-
- if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
- goto cleanup;
-
- if (root) {
- if (virAsprintf(&def->os.cmdline, "root=%s %s", root,
extra) < 0)
- goto cleanup;
- } else {
- if (VIR_STRDUP(def->os.cmdline, extra) < 0)
- goto cleanup;
- }
- }
- if (xenParseXMMem(conf, def) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
- goto cleanup;
-
- list = virConfGetValue(conf, "disk");
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
while (list) {
@@ -779,7 +706,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
head = list->str;
if (!(disk = virDomainDiskDefNew()))
- goto cleanup;
+ return -1;
/*
* Disks have 3 components, SOURCE,DEST-DEVICE,MODE
@@ -798,10 +725,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
ignore_value(virDomainDiskSetSource(disk, NULL));
} else {
if (VIR_STRNDUP(tmp, head, offset - head) < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetSource(disk, tmp) < 0) {
VIR_FREE(tmp);
- goto cleanup;
+ return -1;
}
VIR_FREE(tmp);
}
@@ -815,12 +742,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if (!(offset = strchr(head, ',')))
goto skipdisk;
if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0)
- goto cleanup;
+ return -1;
if (virStrncpy(disk->dst, head, offset - head,
(offset - head) + 1) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Dest file %s too big for destination"),
head);
- goto cleanup;
+ return -1;
}
head = offset + 1;
@@ -832,16 +759,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
if ((tmp = strchr(src, ':')) != NULL) {
len = tmp - src;
if (VIR_STRNDUP(tmp, src, len) < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetDriver(disk, tmp) < 0) {
VIR_FREE(tmp);
- goto cleanup;
+ return -1;
}
VIR_FREE(tmp);
/* Strip the prefix we found off the source file name */
if (virDomainDiskSetSource(disk, src + len + 1) < 0)
- goto cleanup;
+ return -1;
src = virDomainDiskGetSource(disk);
}
@@ -854,7 +781,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
len = tmp - src;
if (VIR_STRNDUP(driverType, src, len) < 0)
- goto cleanup;
+ return -1;
if (STREQ(driverType, "aio"))
virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
else
@@ -865,12 +792,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown driver type %s"),
src);
- goto cleanup;
+ return -1;
}
/* Strip the prefix we found off the source file name */
if (virDomainDiskSetSource(disk, src + len + 1) < 0)
- goto cleanup;
+ return -1;
src = virDomainDiskGetSource(disk);
}
}
@@ -878,7 +805,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
/* No source, or driver name, so fix to phy: */
if (!virDomainDiskGetDriver(disk) &&
virDomainDiskSetDriver(disk, "phy") < 0)
- goto cleanup;
+ return -1;
The problem will all these 'return -1' is that you leak 'disk'. I think
it would be good to have cleanup label in this function. I'd suggested
a local variable 'ret' initialized to -1, which you set to 0 when the
function succeeds. You can then cleanup and return 'ret' in the cleanup
label.
/* phy: type indicates a block device */
@@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
/* Maintain list in sorted order according to target device name */
if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
- goto cleanup;
+ return -1;
skipdisk:
list = list->next;
The next line here is
virDomainDiskDefFree(disk);
which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
we don't free the disk we've just added to the disk list.
@@ -922,26 +849,108 @@ xenParseXM(virConfPtr conf, int
xendConfigVersion,
if (hvm && xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) {
if (xenXMConfigGetString(conf, "cdrom", &str, NULL) < 0)
- goto cleanup;
+ return -1;
if (str) {
if (!(disk = virDomainDiskDefNew()))
- goto cleanup;
+ return -1;
virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
if (virDomainDiskSetDriver(disk, "file") < 0)
- goto cleanup;
+ return -1;
if (virDomainDiskSetSource(disk, str) < 0)
- goto cleanup;
+ return -1;
if (VIR_STRDUP(disk->dst, "hdc") < 0)
- goto cleanup;
+ return -1;
disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
disk->src->readonly = true;
if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+ return -1;
+ }
+ }
+
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ int val;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
'disk' is no longer used in this function and can be removed.
Regards,
Jim
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ size_t i;
+ 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;
+ hvm = (STREQ(def->os.type, "hvm"));
+ if (hvm) {
+ const char *boot;
+ if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
+ goto cleanup;
+
+ if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
+ goto cleanup;
+
+ for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
+ switch (*boot) {
+ case 'a':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
+ break;
+ case 'd':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
+ break;
+ case 'n':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
+ break;
+ case 'c':
+ default:
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
+ break;
+ }
+ def->os.nBootDevs++;
+ }
+ } else {
+ const char *extra, *root;
+
+ if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
+ goto cleanup;
+
+ if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
+ goto cleanup;
+
+ if (root) {
+ if (virAsprintf(&def->os.cmdline, "root=%s %s", root,
extra) < 0)
+ goto cleanup;
+ } else {
+ if (VIR_STRDUP(def->os.cmdline, extra) < 0)
goto cleanup;
}
}
+ if (xenParseXMMem(conf, def) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "device_model", &def->emulator)
< 0)
+ goto cleanup;
if (xenParseXMVif(conf, def) < 0)
goto cleanup;
@@ -953,6 +962,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMCPUFeatures(conf, def) < 0)
goto cleanup;
+ if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+
if (hvm) {
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
goto cleanup;
9:44 p.m.
New subject: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
On 07/11/2014 01:04 PM, Jim Fehlig wrote:
David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>
> Introduce the function
> xenParseXMDisk(..........);
>
> Parses disk config
>
> signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
> ---
> src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++-------------------------
> 1 file changed, 108 insertions(+), 96 deletions(-)
>
> @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int
xendConfigVersion,
>
> /* Maintain list in sorted order according to target device name */
> if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
> - goto cleanup;
> + return -1;
>
> skipdisk:
> list = list->next;
>
The next line here is
virDomainDiskDefFree(disk);
which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
we don't free the disk we've just added to the disk list.
VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you
don't have to worry about double-freeing it (if you call the macro, you
are guaranteed either transfer semantics and success, or no change on
failure).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:28 a.m.
New subject: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
Eric Blake wrote:
On 07/11/2014 01:04 PM, Jim Fehlig wrote:
> David Kiarie wrote:
>
>> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>>
>> Introduce the function
>> xenParseXMDisk(..........);
>>
>> Parses disk config
>>
>> signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
>> ---
>> src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++-------------------------
>> 1 file changed, 108 insertions(+), 96 deletions(-)
>>
>>
>> @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
>>
>> /* Maintain list in sorted order according to target device name */
>> if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
>> - goto cleanup;
>> + return -1;
>>
>> skipdisk:
>> list = list->next;
>>
>>
> The next line here is
>
> virDomainDiskDefFree(disk);
>
> which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
> bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
> we don't free the disk we've just added to the disk list.
>
VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you
don't have to worry about double-freeing it (if you call the macro, you
are guaranteed either transfer semantics and success, or no change on
failure).
Ah, should have read the source :). Thanks Eric. That explains my
confusion as to why a crash was never reported in this code (or the vif
parsing code).
Regards,
Jim
3:43 p.m.
New subject: [libvirt] [PREPOST 08/17] src/xenxs:Refactor graphics config parsing code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
introduce function
xenParseXMVfb(.....);
which parses the graphics config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 247 ++++++++++++++++++++++++++++-------------------------
1 file changed, 131 insertions(+), 116 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 80a7941..89d5739 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -872,125 +872,14 @@ static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
return 0;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
{
- const char *str;
- int hvm = 0;
+ char *listenAddr;
int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
+ virConfValuePtr list = NULL;
virDomainGraphicsDefPtr graphics = NULL;
- size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
- const char *boot;
- if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
- goto cleanup;
-
- if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
- goto cleanup;
-
- for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
- switch (*boot) {
- case 'a':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
- break;
- case 'd':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
- break;
- case 'n':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
- break;
- case 'c':
- default:
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
- break;
- }
- def->os.nBootDevs++;
- }
- } else {
- const char *extra, *root;
-
- if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
- goto cleanup;
-
- if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
- goto cleanup;
-
- if (root) {
- if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra)
< 0)
- goto cleanup;
- } else {
- if (VIR_STRDUP(def->os.cmdline, extra) < 0)
- goto cleanup;
- }
- }
- 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 (hvm) {
- if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- goto cleanup;
- if (str &&
- (STREQ(str, "tablet") ||
- STREQ(str, "mouse") ||
- STREQ(str, "keyboard"))) {
- virDomainInputDefPtr input;
- if (VIR_ALLOC(input) < 0)
- goto cleanup;
- input->bus = VIR_DOMAIN_INPUT_BUS_USB;
- if (STREQ(str, "mouse"))
- input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
- else if (STREQ(str, "tablet"))
- input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
- else if (STREQ(str, "keyboard"))
- input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
- if (VIR_ALLOC_N(def->inputs, 1) < 0) {
- virDomainInputDefFree(input);
- goto cleanup;
- }
- def->inputs[0] = input;
- def->ninputs = 1;
- }
- }
-
+ int hvm = STREQ(def->os.type, "hvm");
/* HVM guests, or old PV guests use this config format */
if (hvm || xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
if (xenXMConfigGetBool(conf, "vnc", &val, 0) < 0)
@@ -1125,6 +1014,132 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
}
}
+ return 0;
+
+ cleanup:
+ virDomainGraphicsDefFree(graphics);
+ return -1;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ size_t i;
+ 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;
+ hvm = (STREQ(def->os.type, "hvm"));
+ if (hvm) {
+ const char *boot;
+ if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
+ goto cleanup;
+
+ if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
+ goto cleanup;
+
+ for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
+ switch (*boot) {
+ case 'a':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
+ break;
+ case 'd':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
+ break;
+ case 'n':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
+ break;
+ case 'c':
+ default:
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
+ break;
+ }
+ def->os.nBootDevs++;
+ }
+ } else {
+ const char *extra, *root;
+
+ if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
+ goto cleanup;
+
+ if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
+ goto cleanup;
+
+ if (root) {
+ if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra)
< 0)
+ goto cleanup;
+ } else {
+ if (VIR_STRDUP(def->os.cmdline, extra) < 0)
+ goto cleanup;
+ }
+ }
+ 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 (hvm) {
+ if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
+ goto cleanup;
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ goto cleanup;
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_ALLOC_N(def->inputs, 1) < 0) {
+ virDomainInputDefFree(input);
+ goto cleanup;
+ }
+ def->inputs[0] = input;
+ def->ninputs = 1;
+ }
+ }
+ if (xenParseXMVfb(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+
if (hvm) {
virDomainChrDefPtr chr = NULL;
--
1.8.4.5
11:43 p.m.
New subject: [libvirt] [PREPOST 08/17] src/xenxs:Refactor graphics config parsing code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
introduce function
xenParseXMVfb(.....);
which parses the graphics config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 247 ++++++++++++++++++++++++++++-------------------------
1 file changed, 131 insertions(+), 116 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 80a7941..89d5739 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -872,125 +872,14 @@ static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
return 0;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
{
- const char *str;
- int hvm = 0;
+ char *listenAddr;
int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
+ virConfValuePtr list = NULL;
virDomainGraphicsDefPtr graphics = NULL;
- size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
- const char *boot;
- if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
- goto cleanup;
-
- if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
- goto cleanup;
-
- for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
- switch (*boot) {
- case 'a':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
- break;
- case 'd':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
- break;
- case 'n':
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
- break;
- case 'c':
- default:
- def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
- break;
- }
- def->os.nBootDevs++;
- }
- } else {
- const char *extra, *root;
-
- if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
- goto cleanup;
-
- if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
- goto cleanup;
- if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
- goto cleanup;
- if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
- goto cleanup;
-
- if (root) {
- if (virAsprintf(&def->os.cmdline, "root=%s %s", root,
extra) < 0)
- goto cleanup;
- } else {
- if (VIR_STRDUP(def->os.cmdline, extra) < 0)
- goto cleanup;
- }
- }
- 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 (hvm) {
- if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- goto cleanup;
- if (str &&
- (STREQ(str, "tablet") ||
- STREQ(str, "mouse") ||
- STREQ(str, "keyboard"))) {
- virDomainInputDefPtr input;
- if (VIR_ALLOC(input) < 0)
- goto cleanup;
- input->bus = VIR_DOMAIN_INPUT_BUS_USB;
- if (STREQ(str, "mouse"))
- input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
- else if (STREQ(str, "tablet"))
- input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
- else if (STREQ(str, "keyboard"))
- input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
- if (VIR_ALLOC_N(def->inputs, 1) < 0) {
- virDomainInputDefFree(input);
- goto cleanup;
- }
- def->inputs[0] = input;
- def->ninputs = 1;
- }
- }
-
+ int hvm = STREQ(def->os.type, "hvm");
The usual whitespace comments: blank lines between functions,
indentation of the function params, and blank line after local variables.
/* HVM guests, or old PV guests use this config format */
if (hvm || xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
if (xenXMConfigGetBool(conf, "vnc", &val, 0) < 0)
@@ -1125,6 +1014,132 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
}
}
+ return 0;
+
+ cleanup:
+ virDomainGraphicsDefFree(graphics);
+ return -1;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ size_t i;
+ char *script = NULL;
+ char *listenAddr = NULL;
'graphics' and 'listenAddr' are no longer used in this function and can
be dropped.
Regards,
Jim
+
+ if (VIR_ALLOC(def) < 0)
+ return NULL;
+
+ def->virtType = VIR_DOMAIN_VIRT_XEN;
+ def->id = -1;
+ if (xenParseXMGeneral(conf, def, caps) < 0)
+ goto cleanup;
+ hvm = (STREQ(def->os.type, "hvm"));
+ if (hvm) {
+ const char *boot;
+ if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
+ goto cleanup;
+
+ if (xenXMConfigGetString(conf, "boot", &boot, "c") <
0)
+ goto cleanup;
+
+ for (i = 0; i < VIR_DOMAIN_BOOT_LAST && boot[i]; i++) {
+ switch (*boot) {
+ case 'a':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
+ break;
+ case 'd':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
+ break;
+ case 'n':
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
+ break;
+ case 'c':
+ default:
+ def->os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
+ break;
+ }
+ def->os.nBootDevs++;
+ }
+ } else {
+ const char *extra, *root;
+
+ if (xenXMConfigCopyStringOpt(conf, "bootloader",
&def->os.bootloader) < 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "bootargs",
&def->os.bootloaderArgs) < 0)
+ goto cleanup;
+
+ if (xenXMConfigCopyStringOpt(conf, "kernel", &def->os.kernel)
< 0)
+ goto cleanup;
+ if (xenXMConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd)
< 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "extra", &extra, NULL) < 0)
+ goto cleanup;
+ if (xenXMConfigGetString(conf, "root", &root, NULL) < 0)
+ goto cleanup;
+
+ if (root) {
+ if (virAsprintf(&def->os.cmdline, "root=%s %s", root,
extra) < 0)
+ goto cleanup;
+ } else {
+ if (VIR_STRDUP(def->os.cmdline, extra) < 0)
+ goto cleanup;
+ }
+ }
+ 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 (hvm) {
+ if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
+ goto cleanup;
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ goto cleanup;
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_ALLOC_N(def->inputs, 1) < 0) {
+ virDomainInputDefFree(input);
+ goto cleanup;
+ }
+ def->inputs[0] = input;
+ def->ninputs = 1;
+ }
+ }
+ if (xenParseXMVfb(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
+
if (hvm) {
virDomainChrDefPtr chr = NULL;
3:43 p.m.
New subject: [libvirt] [PREPOST 09/17] src/xenxs:Refactor OS and Miscellaneous deivices config
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce functions
xenParseXMOS(.....) and
xenParseXMMisc(.....) to parse the OS and
Miscellaneous devices configs
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 146 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 82 insertions(+), 64 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 89d5739..312d890 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1020,36 +1020,21 @@ static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
virDomainGraphicsDefFree(graphics);
return -1;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
{
- const char *str;
- int hvm = 0;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
+ if (STREQ(def->os.type, "hvm")) {
const char *boot;
+
+ if (xenXMConfigCopyStringOpt(conf, "device_model",
&def->emulator) < 0)
+ return -1;
+
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) {
@@ -1073,55 +1058,52 @@ 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;
}
}
- 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 (hvm) {
+
+ return 0;
+}
+static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenXMConfigGetString(conf, "soundhw", &str, NULL) < 0)
+ return -1;
+
+ if (str &&
+ xenParseSxprSound(def, str) < 0)
+ return -1;
+
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- goto cleanup;
+ return -1;
+
if (str &&
(STREQ(str, "tablet") ||
STREQ(str, "mouse") ||
STREQ(str, "keyboard"))) {
virDomainInputDefPtr input;
if (VIR_ALLOC(input) < 0)
- goto cleanup;
+ return -1;
input->bus = VIR_DOMAIN_INPUT_BUS_USB;
if (STREQ(str, "mouse"))
input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
@@ -1131,15 +1113,61 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
if (VIR_ALLOC_N(def->inputs, 1) < 0) {
virDomainInputDefFree(input);
- goto cleanup;
+ return -1;
}
def->inputs[0] = input;
def->ninputs = 1;
}
}
- if (xenParseXMVfb(conf, def, xendConfigVersion) < 0)
+
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ 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;
+
+ 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;
@@ -1221,16 +1249,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
def->consoles[0]->target.port = 0;
def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
}
-
- if (hvm) {
- if (xenXMConfigGetString(conf, "soundhw", &str, NULL) < 0)
- goto cleanup;
-
- if (str &&
- xenParseSxprSound(def, str) < 0)
- goto cleanup;
- }
-
VIR_FREE(script);
return def;
--
1.8.4.5
midnight
New subject: [libvirt] [PREPOST 09/17] src/xenxs:Refactor OS and Miscellaneous deivices config
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce functions
xenParseXMOS(.....) and
xenParseXMMisc(.....) to parse the OS and
Miscellaneous devices configs
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 146 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 82 insertions(+), 64 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 89d5739..312d890 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1020,36 +1020,21 @@ static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
virDomainGraphicsDefFree(graphics);
return -1;
}
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
- virCapsPtr caps)
+static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
{
- const char *str;
- int hvm = 0;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
size_t i;
- 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;
- hvm = (STREQ(def->os.type, "hvm"));
- if (hvm) {
+ if (STREQ(def->os.type, "hvm")) {
const char *boot;
+
+ if (xenXMConfigCopyStringOpt(conf, "device_model",
&def->emulator) < 0)
+ return -1;
+
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) {
@@ -1073,55 +1058,52 @@ 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;
}
}
- 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 (hvm) {
+
+ return 0;
+}
+static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def)
Hrm, not sure I'm fond of this name since it could become a dumping
ground for any "miscellaneous" stuff in the future. Perhaps
xenParseXMInputDevs?
Regards,
Jim
+{
+ const char *str;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenXMConfigGetString(conf, "soundhw", &str, NULL) < 0)
+ return -1;
+
+ if (str &&
+ xenParseSxprSound(def, str) < 0)
+ return -1;
+
if (xenXMConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- goto cleanup;
+ return -1;
+
if (str &&
(STREQ(str, "tablet") ||
STREQ(str, "mouse") ||
STREQ(str, "keyboard"))) {
virDomainInputDefPtr input;
if (VIR_ALLOC(input) < 0)
- goto cleanup;
+ return -1;
input->bus = VIR_DOMAIN_INPUT_BUS_USB;
if (STREQ(str, "mouse"))
input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
@@ -1131,15 +1113,61 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
if (VIR_ALLOC_N(def->inputs, 1) < 0) {
virDomainInputDefFree(input);
- goto cleanup;
+ return -1;
}
def->inputs[0] = input;
def->ninputs = 1;
}
}
- if (xenParseXMVfb(conf, def, xendConfigVersion) < 0)
+
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ 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;
+
+ 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;
@@ -1221,16 +1249,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
def->consoles[0]->target.port = 0;
def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
}
-
- if (hvm) {
- if (xenXMConfigGetString(conf, "soundhw", &str, NULL) < 0)
- goto cleanup;
-
- if (str &&
- xenParseSxprSound(def, str) < 0)
- goto cleanup;
- }
-
VIR_FREE(script);
return def;
3:43 p.m.
New subject: [libvirt] [PREPOST 10/17] src/xenxs:Refactor xenParseXM( )
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
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(a)gmail.com>
---
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:
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;
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;
+}
+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);
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);
virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def,
int xendConfigVersion);
--
1.8.4.5
12:17 a.m.
New subject: [libvirt] [PREPOST 10/17] src/xenxs:Refactor xenParseXM( )
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
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(a)gmail.com>
---
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);
3:43 p.m.
New subject: [libvirt] [PREPOST 11/17] src/xenxs:Refactor code formating general information
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
refactor code parsing uuid, memory and name options
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 657dd1c..a907252 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1609,6 +1609,31 @@ xenFormatXMPCI(virConfPtr conf,
either 32, or 64 on a platform where long is big enough. */
verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT);
+static int xenFormatXMGeneral(virConfPtr conf, virDomainDefPtr def)
+{
+ char uuid[VIR_UUID_STRING_BUFLEN];
+
+ if (xenXMConfigSetString(conf, "name", def->name) < 0)
+ return -1;
+
+ virUUIDFormat(def->uuid, uuid);
+ if (xenXMConfigSetString(conf, "uuid", uuid) < 0)
+ return -1;
+
+ return 0;
+}
+static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
+{
+ if (xenXMConfigSetInt(conf, "maxmem",
+ VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0)
+ return -1;
+
+ if (xenXMConfigSetInt(conf, "memory",
+ VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0)
+ return -1;
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1618,27 +1643,16 @@ virConfPtr xenFormatXM(virConnectPtr conn,
size_t i;
char *cpus = NULL;
const char *lifecycle;
- char uuid[VIR_UUID_STRING_BUFLEN];
virConfValuePtr diskVal = NULL;
virConfValuePtr netVal = NULL;
if (!(conf = virConfNew()))
goto cleanup;
-
- if (xenXMConfigSetString(conf, "name", def->name) < 0)
- goto cleanup;
-
- virUUIDFormat(def->uuid, uuid);
- if (xenXMConfigSetString(conf, "uuid", uuid) < 0)
+ if (xenFormatXMGeneral(conf, def) < 0)
goto cleanup;
- if (xenXMConfigSetInt(conf, "maxmem",
- VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0)
- goto cleanup;
-
- if (xenXMConfigSetInt(conf, "memory",
- VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0)
+ if (xenFormatXMMem(conf, def) < 0)
goto cleanup;
if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0)
--
1.8.4.5
12:24 a.m.
New subject: [libvirt] [PREPOST 11/17] src/xenxs:Refactor code formating general information
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
refactor code parsing uuid, memory and name options
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 657dd1c..a907252 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1609,6 +1609,31 @@ xenFormatXMPCI(virConfPtr conf,
either 32, or 64 on a platform where long is big enough. */
verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT);
+static int xenFormatXMGeneral(virConfPtr conf, virDomainDefPtr def)
Similar comment here as in 1/17 regarding the name xenFormatXMGeneral.
I'd suggest something like xenFormatXMGeneralMeta.
+{
+ char uuid[VIR_UUID_STRING_BUFLEN];
+
+ if (xenXMConfigSetString(conf, "name", def->name) < 0)
+ return -1;
+
+ virUUIDFormat(def->uuid, uuid);
+ if (xenXMConfigSetString(conf, "uuid", uuid) < 0)
+ return -1;
+
+ return 0;
+}
+static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
+{
+ if (xenXMConfigSetInt(conf, "maxmem",
+ VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0)
+ return -1;
+
+ if (xenXMConfigSetInt(conf, "memory",
+ VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0)
+ return -1;
+
+ return 0;
+}
You split the general and mem functions in separate patches on the parse
side. I think that is a good approach on the format side too.
Regards,
Jim
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1618,27 +1643,16 @@ virConfPtr xenFormatXM(virConnectPtr conn,
size_t i;
char *cpus = NULL;
const char *lifecycle;
- char uuid[VIR_UUID_STRING_BUFLEN];
virConfValuePtr diskVal = NULL;
virConfValuePtr netVal = NULL;
if (!(conf = virConfNew()))
goto cleanup;
-
- if (xenXMConfigSetString(conf, "name", def->name) < 0)
- goto cleanup;
-
- virUUIDFormat(def->uuid, uuid);
- if (xenXMConfigSetString(conf, "uuid", uuid) < 0)
+ if (xenFormatXMGeneral(conf, def) < 0)
goto cleanup;
- if (xenXMConfigSetInt(conf, "maxmem",
- VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0)
- goto cleanup;
-
- if (xenXMConfigSetInt(conf, "memory",
- VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0)
+ if (xenFormatXMMem(conf, def) < 0)
goto cleanup;
if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0)
3:43 p.m.
New subject: [libvirt] [PREPOST 12/17] src/xenxs:Refactor virtual time config options formating code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMTimeOffset(......)
which formats virtual time config
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 149 ++++++++++++++++++++++++++++-------------------------
1 file changed, 78 insertions(+), 71 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index a907252..c91fa73 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1634,12 +1634,88 @@ static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ int vmlocaltime;
+ if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+ /* <3.1: UTC and LOCALTIME */
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ vmlocaltime = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ vmlocaltime = 1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ } else {
+ if (STREQ(def->os.type, "hvm")) {
+ /* >=3.1 HV: VARIABLE */
+ int rtc_timeoffset;
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+ vmlocaltime = (int)def->clock.data.variable.basis;
+ rtc_timeoffset = def->clock.data.variable.adjustment;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ if (def->clock.data.utc_reset) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported clock
adjustment='reset'"));
+ return -1;
+ }
+ vmlocaltime = 0;
+ rtc_timeoffset = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ if (def->clock.data.utc_reset) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported clock
adjustment='reset'"));
+ return -1;
+ }
+ vmlocaltime = 1;
+ rtc_timeoffset = 0;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) <
0)
+ return -1;
+ } else {
+ /* >=3.1 PV: UTC and LOCALTIME */
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ vmlocaltime = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ vmlocaltime = 1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ } /* !hvm */
+ }
+ if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0)
+ return -1;
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
{
virConfPtr conf = NULL;
- int hvm = 0, vmlocaltime = 0;
+ int hvm = 0;
size_t i;
char *cpus = NULL;
const char *lifecycle;
@@ -1778,78 +1854,9 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
} /* !hvm */
-
- if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
- /* <3.1: UTC and LOCALTIME */
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- vmlocaltime = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- vmlocaltime = 1;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- } else {
- if (hvm) {
- /* >=3.1 HV: VARIABLE */
- int rtc_timeoffset;
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
- vmlocaltime = (int)def->clock.data.variable.basis;
- rtc_timeoffset = def->clock.data.variable.adjustment;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- if (def->clock.data.utc_reset) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported clock
adjustment='reset'"));
- goto cleanup;
- }
- vmlocaltime = 0;
- rtc_timeoffset = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- if (def->clock.data.utc_reset) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported clock
adjustment='reset'"));
- goto cleanup;
- }
- vmlocaltime = 1;
- rtc_timeoffset = 0;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) <
0)
- goto cleanup;
- } else {
- /* >=3.1 PV: UTC and LOCALTIME */
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- vmlocaltime = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- vmlocaltime = 1;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- } /* !hvm */
- }
- if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0)
+ if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) < 0)
goto cleanup;
-
if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected lifecycle action %d"),
def->onPoweroff);
--
1.8.4.5
12:28 a.m.
New subject: [libvirt] [PREPOST 12/17] src/xenxs:Refactor virtual time config options formating code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMTimeOffset(......)
which formats virtual time config
Ah, on the format side you split TimeOffset and EventActions as I
requested on the parse side. Looks good with the exception of the usual
whitespace comments.
Regards,
Jim
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 149 ++++++++++++++++++++++++++++-------------------------
1 file changed, 78 insertions(+), 71 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index a907252..c91fa73 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1634,12 +1634,88 @@ static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ int vmlocaltime;
+ if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
+ /* <3.1: UTC and LOCALTIME */
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ vmlocaltime = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ vmlocaltime = 1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ } else {
+ if (STREQ(def->os.type, "hvm")) {
+ /* >=3.1 HV: VARIABLE */
+ int rtc_timeoffset;
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+ vmlocaltime = (int)def->clock.data.variable.basis;
+ rtc_timeoffset = def->clock.data.variable.adjustment;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ if (def->clock.data.utc_reset) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported clock
adjustment='reset'"));
+ return -1;
+ }
+ vmlocaltime = 0;
+ rtc_timeoffset = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ if (def->clock.data.utc_reset) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported clock
adjustment='reset'"));
+ return -1;
+ }
+ vmlocaltime = 1;
+ rtc_timeoffset = 0;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) <
0)
+ return -1;
+ } else {
+ /* >=3.1 PV: UTC and LOCALTIME */
+ switch (def->clock.offset) {
+ case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+ vmlocaltime = 0;
+ break;
+ case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+ vmlocaltime = 1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported clock offset='%s'"),
+ virDomainClockOffsetTypeToString(def->clock.offset));
+ return -1;
+ }
+ } /* !hvm */
+ }
+ if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0)
+ return -1;
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
{
virConfPtr conf = NULL;
- int hvm = 0, vmlocaltime = 0;
+ int hvm = 0;
size_t i;
char *cpus = NULL;
const char *lifecycle;
@@ -1778,78 +1854,9 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
} /* !hvm */
-
- if (xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) {
- /* <3.1: UTC and LOCALTIME */
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- vmlocaltime = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- vmlocaltime = 1;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- } else {
- if (hvm) {
- /* >=3.1 HV: VARIABLE */
- int rtc_timeoffset;
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
- vmlocaltime = (int)def->clock.data.variable.basis;
- rtc_timeoffset = def->clock.data.variable.adjustment;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- if (def->clock.data.utc_reset) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported clock
adjustment='reset'"));
- goto cleanup;
- }
- vmlocaltime = 0;
- rtc_timeoffset = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- if (def->clock.data.utc_reset) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported clock
adjustment='reset'"));
- goto cleanup;
- }
- vmlocaltime = 1;
- rtc_timeoffset = 0;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) <
0)
- goto cleanup;
- } else {
- /* >=3.1 PV: UTC and LOCALTIME */
- switch (def->clock.offset) {
- case VIR_DOMAIN_CLOCK_OFFSET_UTC:
- vmlocaltime = 0;
- break;
- case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
- vmlocaltime = 1;
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported clock offset='%s'"),
- virDomainClockOffsetTypeToString(def->clock.offset));
- goto cleanup;
- }
- } /* !hvm */
- }
- if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0)
+ if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) < 0)
goto cleanup;
-
if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected lifecycle action %d"),
def->onPoweroff);
3:43 p.m.
New subject: [libvirt] [PREPOST 13/17] src/xenxs:Refactor code formating event actions config
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMEventActions(.....)
which formats actions following certain events
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 60 ++++++++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index c91fa73..367a8cd 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1710,6 +1710,38 @@ static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr
def,
return 0;
}
+static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *lifecycle = NULL;
+
+ if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"),
def->onPoweroff);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_poweroff", lifecycle) < 0)
+ return -1;
+
+
+ if (!(lifecycle = virDomainLifecycleTypeToString(def->onReboot))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"), def->onReboot);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_reboot", lifecycle) < 0)
+ return -1;
+
+
+ if (!(lifecycle = virDomainLifecycleCrashTypeToString(def->onCrash))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"), def->onCrash);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_crash", lifecycle) < 0)
+ return -1;
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1718,7 +1750,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
int hvm = 0;
size_t i;
char *cpus = NULL;
- const char *lifecycle;
virConfValuePtr diskVal = NULL;
virConfValuePtr netVal = NULL;
@@ -1857,33 +1888,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) < 0)
goto cleanup;
- if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"),
def->onPoweroff);
- goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_poweroff", lifecycle) < 0)
- goto cleanup;
-
-
- if (!(lifecycle = virDomainLifecycleTypeToString(def->onReboot))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"), def->onReboot);
- goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_reboot", lifecycle) < 0)
- goto cleanup;
-
-
- if (!(lifecycle = virDomainLifecycleCrashTypeToString(def->onCrash))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"), def->onCrash);
+ if (xenFormatXMEventActions(conf, def) < 0)
goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_crash", lifecycle) < 0)
- goto cleanup;
-
-
if (hvm) {
if (def->emulator &&
--
1.8.4.5
12:29 a.m.
New subject: [libvirt] [PREPOST 13/17] src/xenxs:Refactor code formating event actions config
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMEventActions(.....)
which formats actions following certain events
Looks good with exception of my tiring whitespace comments :-).
Regards,
Jim
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 60 ++++++++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index c91fa73..367a8cd 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1710,6 +1710,38 @@ static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr
def,
return 0;
}
+static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *lifecycle = NULL;
+
+ if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"),
def->onPoweroff);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_poweroff", lifecycle) < 0)
+ return -1;
+
+
+ if (!(lifecycle = virDomainLifecycleTypeToString(def->onReboot))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"),
def->onReboot);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_reboot", lifecycle) < 0)
+ return -1;
+
+
+ if (!(lifecycle = virDomainLifecycleCrashTypeToString(def->onCrash))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected lifecycle action %d"), def->onCrash);
+ return -1;
+ }
+ if (xenXMConfigSetString(conf, "on_crash", lifecycle) < 0)
+ return -1;
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1718,7 +1750,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
int hvm = 0;
size_t i;
char *cpus = NULL;
- const char *lifecycle;
virConfValuePtr diskVal = NULL;
virConfValuePtr netVal = NULL;
@@ -1857,33 +1888,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) < 0)
goto cleanup;
- if (!(lifecycle = virDomainLifecycleTypeToString(def->onPoweroff))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"),
def->onPoweroff);
- goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_poweroff", lifecycle) < 0)
- goto cleanup;
-
-
- if (!(lifecycle = virDomainLifecycleTypeToString(def->onReboot))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"),
def->onReboot);
- goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_reboot", lifecycle) < 0)
- goto cleanup;
-
-
- if (!(lifecycle = virDomainLifecycleCrashTypeToString(def->onCrash))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected lifecycle action %d"), def->onCrash);
+ if (xenFormatXMEventActions(conf, def) < 0)
goto cleanup;
- }
- if (xenXMConfigSetString(conf, "on_crash", lifecycle) < 0)
- goto cleanup;
-
-
if (hvm) {
if (def->emulator &&
3:43 p.m.
New subject: [libvirt] [PREPOST 14/17] src/xenxs:Refactor code formating char devices config
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMCharDev(....);
which formats char devices config
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 155 ++++++++++++++++++++++++++++-------------------------
1 file changed, 82 insertions(+), 73 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 367a8cd..ee5dc19 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1742,6 +1742,85 @@ static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr
def)
return 0;
}
+static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ if (STREQ(def->os.type, "hvm")) {
+ if (def->nparallels) {
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *str;
+ int ret;
+
+ ret = xenFormatSxprChr(def->parallels[0], &buf);
+ str = virBufferContentAndReset(&buf);
+ if (ret == 0)
+ ret = xenXMConfigSetString(conf, "parallel", str);
+ VIR_FREE(str);
+ if (ret < 0)
+ goto cleanup;
+ } else {
+ if (xenXMConfigSetString(conf, "parallel", "none") <
0)
+ goto cleanup;
+ }
+
+ if (def->nserials) {
+ if ((def->nserials == 1) && (def->serials[0]->target.port ==
0)) {
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *str;
+ int ret;
+
+ ret = xenFormatSxprChr(def->serials[0], &buf);
+ str = virBufferContentAndReset(&buf);
+ if (ret == 0)
+ ret = xenXMConfigSetString(conf, "serial", str);
+ VIR_FREE(str);
+ if (ret < 0)
+ goto cleanup;
+ } else {
+ size_t j = 0;
+ int maxport = -1, port;
+ virConfValuePtr serialVal = NULL;
+
+ if (VIR_ALLOC(serialVal) < 0)
+ goto cleanup;
+ serialVal->type = VIR_CONF_LIST;
+ serialVal->list = NULL;
+
+ for (i = 0; i < def->nserials; i++)
+ if (def->serials[i]->target.port > maxport)
+ maxport = def->serials[i]->target.port;
+
+ for (port = 0; port <= maxport; port++) {
+ virDomainChrDefPtr chr = NULL;
+ for (j = 0; j < def->nserials; j++) {
+ if (def->serials[j]->target.port == port) {
+ chr = def->serials[j];
+ break;
+ }
+ }
+ if (xenFormatXMSerial(serialVal, chr) < 0) {
+ virConfFreeValue(serialVal);
+ goto cleanup;
+ }
+ }
+
+ if (serialVal->list != NULL) {
+ int ret = virConfSetValue(conf, "serial", serialVal);
+ serialVal = NULL;
+ if (ret < 0)
+ goto cleanup;
+ }
+ VIR_FREE(serialVal);
+ }
+ } else {
+ if (xenXMConfigSetString(conf, "serial", "none") < 0)
+ goto cleanup;
+ }
+ }
+ return 0;
+cleanup:
+ return -1;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2067,79 +2146,10 @@ virConfPtr xenFormatXM(virConnectPtr conn,
if (xenFormatXMPCI(conf, def) < 0)
goto cleanup;
- if (hvm) {
- if (def->nparallels) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *str;
- int ret;
-
- ret = xenFormatSxprChr(def->parallels[0], &buf);
- str = virBufferContentAndReset(&buf);
- if (ret == 0)
- ret = xenXMConfigSetString(conf, "parallel", str);
- VIR_FREE(str);
- if (ret < 0)
- goto cleanup;
- } else {
- if (xenXMConfigSetString(conf, "parallel", "none") <
0)
- goto cleanup;
- }
-
- if (def->nserials) {
- if ((def->nserials == 1) && (def->serials[0]->target.port ==
0)) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *str;
- int ret;
-
- ret = xenFormatSxprChr(def->serials[0], &buf);
- str = virBufferContentAndReset(&buf);
- if (ret == 0)
- ret = xenXMConfigSetString(conf, "serial", str);
- VIR_FREE(str);
- if (ret < 0)
- goto cleanup;
- } else {
- size_t j = 0;
- int maxport = -1, port;
- virConfValuePtr serialVal = NULL;
-
- if (VIR_ALLOC(serialVal) < 0)
- goto cleanup;
- serialVal->type = VIR_CONF_LIST;
- serialVal->list = NULL;
-
- for (i = 0; i < def->nserials; i++)
- if (def->serials[i]->target.port > maxport)
- maxport = def->serials[i]->target.port;
-
- for (port = 0; port <= maxport; port++) {
- virDomainChrDefPtr chr = NULL;
- for (j = 0; j < def->nserials; j++) {
- if (def->serials[j]->target.port == port) {
- chr = def->serials[j];
- break;
- }
- }
- if (xenFormatXMSerial(serialVal, chr) < 0) {
- virConfFreeValue(serialVal);
- goto cleanup;
- }
- }
-
- if (serialVal->list != NULL) {
- int ret = virConfSetValue(conf, "serial", serialVal);
- serialVal = NULL;
- if (ret < 0)
- goto cleanup;
- }
- VIR_FREE(serialVal);
- }
- } else {
- if (xenXMConfigSetString(conf, "serial", "none") < 0)
- goto cleanup;
- }
-
+ if (xenFormatXMCharDev(conf, def) < 0)
+ goto cleanup;
+ if (hvm) {
if (def->sounds) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *str = NULL;
@@ -2153,7 +2163,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
}
}
-
return conf;
cleanup:
--
1.8.4.5
12:35 a.m.
New subject: [libvirt] [PREPOST 14/17] src/xenxs:Refactor code formating char devices config
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMCharDev(....);
which formats char devices config
signed-off-by:David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 155 ++++++++++++++++++++++++++++-------------------------
1 file changed, 82 insertions(+), 73 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 367a8cd..ee5dc19 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1742,6 +1742,85 @@ static int xenFormatXMEventActions(virConfPtr conf,
virDomainDefPtr def)
return 0;
}
+static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ if (STREQ(def->os.type, "hvm")) {
+ if (def->nparallels) {
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *str;
+ int ret;
+
+ ret = xenFormatSxprChr(def->parallels[0], &buf);
+ str = virBufferContentAndReset(&buf);
+ if (ret == 0)
+ ret = xenXMConfigSetString(conf, "parallel", str);
+ VIR_FREE(str);
+ if (ret < 0)
+ goto cleanup;
+ } else {
+ if (xenXMConfigSetString(conf, "parallel", "none") <
0)
+ goto cleanup;
+ }
+
+ if (def->nserials) {
+ if ((def->nserials == 1) && (def->serials[0]->target.port
== 0)) {
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *str;
+ int ret;
+
+ ret = xenFormatSxprChr(def->serials[0], &buf);
+ str = virBufferContentAndReset(&buf);
+ if (ret == 0)
+ ret = xenXMConfigSetString(conf, "serial", str);
+ VIR_FREE(str);
+ if (ret < 0)
+ goto cleanup;
+ } else {
+ size_t j = 0;
+ int maxport = -1, port;
+ virConfValuePtr serialVal = NULL;
+
+ if (VIR_ALLOC(serialVal) < 0)
+ goto cleanup;
+ serialVal->type = VIR_CONF_LIST;
+ serialVal->list = NULL;
+
+ for (i = 0; i < def->nserials; i++)
+ if (def->serials[i]->target.port > maxport)
+ maxport = def->serials[i]->target.port;
+
+ for (port = 0; port <= maxport; port++) {
+ virDomainChrDefPtr chr = NULL;
+ for (j = 0; j < def->nserials; j++) {
+ if (def->serials[j]->target.port == port) {
+ chr = def->serials[j];
+ break;
+ }
+ }
+ if (xenFormatXMSerial(serialVal, chr) < 0) {
+ virConfFreeValue(serialVal);
+ goto cleanup;
+ }
+ }
+
+ if (serialVal->list != NULL) {
+ int ret = virConfSetValue(conf, "serial", serialVal);
+ serialVal = NULL;
+ if (ret < 0)
+ goto cleanup;
+ }
+ VIR_FREE(serialVal);
+ }
+ } else {
+ if (xenXMConfigSetString(conf, "serial", "none") <
0)
+ goto cleanup;
+ }
+ }
+ return 0;
+cleanup:
+ return -1;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2067,79 +2146,10 @@ virConfPtr xenFormatXM(virConnectPtr conn,
if (xenFormatXMPCI(conf, def) < 0)
goto cleanup;
- if (hvm) {
- if (def->nparallels) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *str;
- int ret;
-
- ret = xenFormatSxprChr(def->parallels[0], &buf);
- str = virBufferContentAndReset(&buf);
- if (ret == 0)
- ret = xenXMConfigSetString(conf, "parallel", str);
- VIR_FREE(str);
- if (ret < 0)
- goto cleanup;
- } else {
- if (xenXMConfigSetString(conf, "parallel", "none") <
0)
- goto cleanup;
- }
-
- if (def->nserials) {
- if ((def->nserials == 1) && (def->serials[0]->target.port
== 0)) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *str;
- int ret;
-
- ret = xenFormatSxprChr(def->serials[0], &buf);
- str = virBufferContentAndReset(&buf);
- if (ret == 0)
- ret = xenXMConfigSetString(conf, "serial", str);
- VIR_FREE(str);
- if (ret < 0)
- goto cleanup;
- } else {
- size_t j = 0;
- int maxport = -1, port;
- virConfValuePtr serialVal = NULL;
-
- if (VIR_ALLOC(serialVal) < 0)
- goto cleanup;
- serialVal->type = VIR_CONF_LIST;
- serialVal->list = NULL;
-
- for (i = 0; i < def->nserials; i++)
- if (def->serials[i]->target.port > maxport)
- maxport = def->serials[i]->target.port;
-
- for (port = 0; port <= maxport; port++) {
- virDomainChrDefPtr chr = NULL;
- for (j = 0; j < def->nserials; j++) {
- if (def->serials[j]->target.port == port) {
- chr = def->serials[j];
- break;
- }
- }
- if (xenFormatXMSerial(serialVal, chr) < 0) {
- virConfFreeValue(serialVal);
- goto cleanup;
- }
- }
-
- if (serialVal->list != NULL) {
- int ret = virConfSetValue(conf, "serial", serialVal);
- serialVal = NULL;
- if (ret < 0)
- goto cleanup;
- }
- VIR_FREE(serialVal);
- }
- } else {
- if (xenXMConfigSetString(conf, "serial", "none") <
0)
- goto cleanup;
- }
-
+ if (xenFormatXMCharDev(conf, def) < 0)
+ goto cleanup;
+ if (hvm) {
if (def->sounds) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *str = NULL;
@@ -2153,7 +2163,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
}
}
-
Spurious whitespace change.
Regards,
Jim
return conf;
cleanup:
3:43 p.m.
New subject: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenFormatXMDomainNet(......)
I think this could be done in a cleaner way
signed-of-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 49 +++++++++++++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ee5dc19..8dd2823 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr
def)
cleanup:
return -1;
}
+static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
+ virDomainDefPtr def, int xendConfigVersion)
+{
+ virConfValuePtr netVal = NULL;
+ size_t i;
+
+ int hvm = STREQ(def->os.type, "hvm");
+
+ if (VIR_ALLOC(netVal) < 0)
+ goto cleanup;
+ netVal->type = VIR_CONF_LIST;
+ netVal->list = NULL;
+
+ for (i = 0; i < def->nnets; i++) {
+ if (xenFormatXMNet(conn, netVal, def->nets[i],
+ hvm, xendConfigVersion) < 0)
+ return -1;
+ }
+ if (netVal->list != NULL) {
+ int ret = virConfSetValue(conf, "vif", netVal);
+ netVal = NULL;
+ if (ret < 0)
+ return -1;
+ }
+ VIR_FREE(netVal);
+ return 0;
+ cleanup:
+ VIR_FREE(netVal);
+ return -1;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
}
VIR_FREE(diskVal);
-
- if (VIR_ALLOC(netVal) < 0)
+ if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) < 0)
goto cleanup;
- netVal->type = VIR_CONF_LIST;
- netVal->list = NULL;
-
- for (i = 0; i < def->nnets; i++) {
- if (xenFormatXMNet(conn, netVal, def->nets[i],
- hvm, xendConfigVersion) < 0)
- goto cleanup;
- }
- if (netVal->list != NULL) {
- int ret = virConfSetValue(conf, "vif", netVal);
- netVal = NULL;
- if (ret < 0)
- goto cleanup;
- }
- VIR_FREE(netVal);
-
if (xenFormatXMPCI(conf, def) < 0)
goto cleanup;
--
1.8.4.5
12:49 a.m.
New subject: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce the function
xenFormatXMDomainNet(......)
On the parsing side, you called this xenParseXMVif. To be consistent,
this should be xenFormatXMVif.
I think this could be done in a cleaner way
signed-of-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 49 +++++++++++++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ee5dc19..8dd2823 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr
def)
cleanup:
return -1;
}
+static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
+ virDomainDefPtr def, int xendConfigVersion)
+{
+ virConfValuePtr netVal = NULL;
+ size_t i;
+
+ int hvm = STREQ(def->os.type, "hvm");
+
+ if (VIR_ALLOC(netVal) < 0)
+ goto cleanup;
+ netVal->type = VIR_CONF_LIST;
+ netVal->list = NULL;
+
+ for (i = 0; i < def->nnets; i++) {
+ if (xenFormatXMNet(conn, netVal, def->nets[i],
+ hvm, xendConfigVersion) < 0)
Ah, I see xenFormatXMNet already exists. So maybe xenFormatXMVifs for
the list of VIFs and xenFormatXMVif for each individual VIF is clearer.
Regards,
Jim
+ return -1;
+ }
+ if (netVal->list != NULL) {
+ int ret = virConfSetValue(conf, "vif", netVal);
+ netVal = NULL;
+ if (ret < 0)
+ return -1;
+ }
+ VIR_FREE(netVal);
+ return 0;
+ cleanup:
+ VIR_FREE(netVal);
+ return -1;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
}
VIR_FREE(diskVal);
-
- if (VIR_ALLOC(netVal) < 0)
+ if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) < 0)
goto cleanup;
- netVal->type = VIR_CONF_LIST;
- netVal->list = NULL;
-
- for (i = 0; i < def->nnets; i++) {
- if (xenFormatXMNet(conn, netVal, def->nets[i],
- hvm, xendConfigVersion) < 0)
- goto cleanup;
- }
- if (netVal->list != NULL) {
- int ret = virConfSetValue(conf, "vif", netVal);
- netVal = NULL;
- if (ret < 0)
- goto cleanup;
- }
- VIR_FREE(netVal);
-
if (xenFormatXMPCI(conf, def) < 0)
goto cleanup;
4:41 p.m.
New subject: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
Thanks for the review, am applying changes to address your comments.
On Tue, Jul 15, 2014 at 1:49 AM, Jim Fehlig <jfehlig(a)suse.com> wrote:
David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>
> Introduce the function
> xenFormatXMDomainNet(......)
>
On the parsing side, you called this xenParseXMVif. To be consistent,
this should be xenFormatXMVif.
> I think this could be done in a cleaner way
>
> signed-of-by: David Kiarie<davidkiarie4(a)gmail.com>
> ---
> src/xenxs/xen_xm.c | 49 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index ee5dc19..8dd2823 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr
def)
> cleanup:
> return -1;
> }
> +static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
> + virDomainDefPtr def, int xendConfigVersion)
> +{
> + virConfValuePtr netVal = NULL;
> + size_t i;
> +
> + int hvm = STREQ(def->os.type, "hvm");
> +
> + if (VIR_ALLOC(netVal) < 0)
> + goto cleanup;
> + netVal->type = VIR_CONF_LIST;
> + netVal->list = NULL;
> +
> + for (i = 0; i < def->nnets; i++) {
> + if (xenFormatXMNet(conn, netVal, def->nets[i],
> + hvm, xendConfigVersion) < 0)
>
Ah, I see xenFormatXMNet already exists. So maybe xenFormatXMVifs for
the list of VIFs and xenFormatXMVif for each individual VIF is clearer.
Regards,
Jim
> + return -1;
> + }
> + if (netVal->list != NULL) {
> + int ret = virConfSetValue(conf, "vif", netVal);
> + netVal = NULL;
> + if (ret < 0)
> + return -1;
> + }
> + VIR_FREE(netVal);
> + return 0;
> + cleanup:
> + VIR_FREE(netVal);
> + return -1;
> +}
> virConfPtr xenFormatXM(virConnectPtr conn,
> virDomainDefPtr def,
> int xendConfigVersion)
> @@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
> goto cleanup;
> }
> VIR_FREE(diskVal);
> -
> - if (VIR_ALLOC(netVal) < 0)
> + if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) < 0)
> goto cleanup;
> - netVal->type = VIR_CONF_LIST;
> - netVal->list = NULL;
> -
> - for (i = 0; i < def->nnets; i++) {
> - if (xenFormatXMNet(conn, netVal, def->nets[i],
> - hvm, xendConfigVersion) < 0)
> - goto cleanup;
> - }
> - if (netVal->list != NULL) {
> - int ret = virConfSetValue(conf, "vif", netVal);
> - netVal = NULL;
> - if (ret < 0)
> - goto cleanup;
> - }
> - VIR_FREE(netVal);
> -
> if (xenFormatXMPCI(conf, def) < 0)
> goto cleanup;
>
>
11:23 p.m.
New subject: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
David kiarie wrote:
Thanks for the review, am applying changes to address your comments.
Thanks David, looking forward to a V2.
BTW, 16/17 and 17/17 look good with exception of the usual whitespace
comments.
Regards,
Jim
3:43 p.m.
New subject: [libvirt] [PREPOST 16/17] src/xenxs:Refactor Vfb config parsing code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMVfb(.......);
which formats Vfb config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 207 +++++++++++++++++++++++++++--------------------------
1 file changed, 107 insertions(+), 100 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 8dd2823..613730f 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1851,6 +1851,111 @@ static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr
conn,
VIR_FREE(netVal);
return -1;
}
+static int xenFormatXMVfb(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ int hvm = STREQ(def->os.type, "hvm");
+ if (def->ngraphics == 1) {
+ if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) {
+ if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
+ if (xenXMConfigSetInt(conf, "sdl", 1) < 0)
+ return -1;
+ if (xenXMConfigSetInt(conf, "vnc", 0) < 0)
+ return -1;
+ if (def->graphics[0]->data.sdl.display &&
+ xenXMConfigSetString(conf, "display",
+ def->graphics[0]->data.sdl.display) < 0)
+ return -1;
+ if (def->graphics[0]->data.sdl.xauth &&
+ xenXMConfigSetString(conf, "xauthority",
+ def->graphics[0]->data.sdl.xauth) < 0)
+ return -1;
+ } else {
+ const char *listenAddr;
+
+ if (xenXMConfigSetInt(conf, "sdl", 0) < 0)
+ return -1;
+ if (xenXMConfigSetInt(conf, "vnc", 1) < 0)
+ return -1;
+ if (xenXMConfigSetInt(conf, "vncunused",
+ def->graphics[0]->data.vnc.autoport ? 1 : 0) < 0)
+ return -1;
+ if (!def->graphics[0]->data.vnc.autoport &&
+ xenXMConfigSetInt(conf, "vncdisplay",
+ def->graphics[0]->data.vnc.port - 5900) < 0)
+ return -1;
+ listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
+ if (listenAddr &&
+ xenXMConfigSetString(conf, "vnclisten", listenAddr) <
0)
+ return -1;
+ if (def->graphics[0]->data.vnc.auth.passwd &&
+ xenXMConfigSetString(conf, "vncpasswd",
+ def->graphics[0]->data.vnc.auth.passwd)
< 0)
+ return -1;
+ if (def->graphics[0]->data.vnc.keymap &&
+ xenXMConfigSetString(conf, "keymap",
+ def->graphics[0]->data.vnc.keymap) < 0)
+ return -1;
+ }
+ } else {
+ virConfValuePtr vfb, disp;
+ char *vfbstr = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
+ virBufferAddLit(&buf, "type=sdl");
+ if (def->graphics[0]->data.sdl.display)
+ virBufferAsprintf(&buf, ",display=%s",
+ def->graphics[0]->data.sdl.display);
+ if (def->graphics[0]->data.sdl.xauth)
+ virBufferAsprintf(&buf, ",xauthority=%s",
+ def->graphics[0]->data.sdl.xauth);
+ } else {
+ const char *listenAddr
+ = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
+
+ virBufferAddLit(&buf, "type=vnc");
+ virBufferAsprintf(&buf, ",vncunused=%d",
+ def->graphics[0]->data.vnc.autoport ? 1 : 0);
+ if (!def->graphics[0]->data.vnc.autoport)
+ virBufferAsprintf(&buf, ",vncdisplay=%d",
+ def->graphics[0]->data.vnc.port - 5900);
+ if (listenAddr)
+ virBufferAsprintf(&buf, ",vnclisten=%s", listenAddr);
+ if (def->graphics[0]->data.vnc.auth.passwd)
+ virBufferAsprintf(&buf, ",vncpasswd=%s",
+ def->graphics[0]->data.vnc.auth.passwd);
+ if (def->graphics[0]->data.vnc.keymap)
+ virBufferAsprintf(&buf, ",keymap=%s",
+ def->graphics[0]->data.vnc.keymap);
+ }
+ if (virBufferCheckError(&buf) < 0)
+ return -1;
+
+ vfbstr = virBufferContentAndReset(&buf);
+
+ if (VIR_ALLOC(vfb) < 0) {
+ VIR_FREE(vfbstr);
+ return -1;
+ }
+
+ if (VIR_ALLOC(disp) < 0) {
+ VIR_FREE(vfb);
+ VIR_FREE(vfbstr);
+ return -1;
+ }
+
+ vfb->type = VIR_CONF_LIST;
+ vfb->list = disp;
+ disp->type = VIR_CONF_STRING;
+ disp->str = vfbstr;
+
+ if (virConfSetValue(conf, "vfb", vfb) < 0)
+ return -1;
+ }
+ }
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2027,106 +2132,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
}
}
}
-
- if (def->ngraphics == 1) {
- if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) {
- if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
- if (xenXMConfigSetInt(conf, "sdl", 1) < 0)
- goto cleanup;
- if (xenXMConfigSetInt(conf, "vnc", 0) < 0)
- goto cleanup;
- if (def->graphics[0]->data.sdl.display &&
- xenXMConfigSetString(conf, "display",
- def->graphics[0]->data.sdl.display) < 0)
- goto cleanup;
- if (def->graphics[0]->data.sdl.xauth &&
- xenXMConfigSetString(conf, "xauthority",
- def->graphics[0]->data.sdl.xauth) < 0)
- goto cleanup;
- } else {
- const char *listenAddr;
-
- if (xenXMConfigSetInt(conf, "sdl", 0) < 0)
- goto cleanup;
- if (xenXMConfigSetInt(conf, "vnc", 1) < 0)
- goto cleanup;
- if (xenXMConfigSetInt(conf, "vncunused",
- def->graphics[0]->data.vnc.autoport ? 1 : 0) < 0)
- goto cleanup;
- if (!def->graphics[0]->data.vnc.autoport &&
- xenXMConfigSetInt(conf, "vncdisplay",
- def->graphics[0]->data.vnc.port - 5900) < 0)
- goto cleanup;
- listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
- if (listenAddr &&
- xenXMConfigSetString(conf, "vnclisten", listenAddr) <
0)
- goto cleanup;
- if (def->graphics[0]->data.vnc.auth.passwd &&
- xenXMConfigSetString(conf, "vncpasswd",
- def->graphics[0]->data.vnc.auth.passwd)
< 0)
- goto cleanup;
- if (def->graphics[0]->data.vnc.keymap &&
- xenXMConfigSetString(conf, "keymap",
- def->graphics[0]->data.vnc.keymap) < 0)
- goto cleanup;
- }
- } else {
- virConfValuePtr vfb, disp;
- char *vfbstr = NULL;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
- virBufferAddLit(&buf, "type=sdl");
- if (def->graphics[0]->data.sdl.display)
- virBufferAsprintf(&buf, ",display=%s",
- def->graphics[0]->data.sdl.display);
- if (def->graphics[0]->data.sdl.xauth)
- virBufferAsprintf(&buf, ",xauthority=%s",
- def->graphics[0]->data.sdl.xauth);
- } else {
- const char *listenAddr
- = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
-
- virBufferAddLit(&buf, "type=vnc");
- virBufferAsprintf(&buf, ",vncunused=%d",
- def->graphics[0]->data.vnc.autoport ? 1 : 0);
- if (!def->graphics[0]->data.vnc.autoport)
- virBufferAsprintf(&buf, ",vncdisplay=%d",
- def->graphics[0]->data.vnc.port - 5900);
- if (listenAddr)
- virBufferAsprintf(&buf, ",vnclisten=%s", listenAddr);
- if (def->graphics[0]->data.vnc.auth.passwd)
- virBufferAsprintf(&buf, ",vncpasswd=%s",
- def->graphics[0]->data.vnc.auth.passwd);
- if (def->graphics[0]->data.vnc.keymap)
- virBufferAsprintf(&buf, ",keymap=%s",
- def->graphics[0]->data.vnc.keymap);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
-
- vfbstr = virBufferContentAndReset(&buf);
-
- if (VIR_ALLOC(vfb) < 0) {
- VIR_FREE(vfbstr);
- goto cleanup;
- }
-
- if (VIR_ALLOC(disp) < 0) {
- VIR_FREE(vfb);
- VIR_FREE(vfbstr);
- goto cleanup;
- }
-
- vfb->type = VIR_CONF_LIST;
- vfb->list = disp;
- disp->type = VIR_CONF_STRING;
- disp->str = vfbstr;
-
- if (virConfSetValue(conf, "vfb", vfb) < 0)
- goto cleanup;
- }
- }
-
+ if (xenFormatXMVfb(conf, def, xendConfigVersion) < 0)
+ goto cleanup;
/* analyze of the devices */
if (VIR_ALLOC(diskVal) < 0)
goto cleanup;
--
1.8.4.5
3:43 p.m.
New subject: [libvirt] [PREPOST 17/17] src/xenxs:Refactor disk config formating code
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function
xenFormatXMDomainDisks(virConfPtr conf, .....);
which formats disk config
signed-off-by: David Kiarie<davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 62 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 36 insertions(+), 26 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 613730f..10f3d05 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1956,6 +1956,41 @@ static int xenFormatXMVfb(virConfPtr conf, virDomainDefPtr def,
return 0;
}
+static int xenFormatXMDomainDisks(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+ virConfValuePtr diskVal = NULL;
+ size_t i = 0;
+ int hvm = STREQ(def->os.type, "hvm");
+ if (VIR_ALLOC(diskVal) < 0)
+ return -1;
+ diskVal->type = VIR_CONF_LIST;
+ diskVal->list = NULL;
+
+ for (i = 0; i < def->ndisks; i++) {
+ if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2 &&
+ def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
+ def->disks[i]->dst &&
+ STREQ(def->disks[i]->dst, "hdc")) {
+ continue;
+ }
+ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ continue;
+
+ if (xenFormatXMDisk(diskVal, def->disks[i],
+ hvm, xendConfigVersion) < 0)
+ return -1;
+ }
+ if (diskVal->list != NULL) {
+ int ret = virConfSetValue(conf, "disk", diskVal);
+ diskVal = NULL;
+ if (ret < 0)
+ return -1;
+ }
+ VIR_FREE(diskVal);
+
+ return 0;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2134,33 +2169,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
}
if (xenFormatXMVfb(conf, def, xendConfigVersion) < 0)
goto cleanup;
- /* analyze of the devices */
- if (VIR_ALLOC(diskVal) < 0)
+ if (xenFormatXMDomainDisks(conf, def, xendConfigVersion) < 0)
goto cleanup;
- diskVal->type = VIR_CONF_LIST;
- diskVal->list = NULL;
-
- for (i = 0; i < def->ndisks; i++) {
- if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2 &&
- def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
- def->disks[i]->dst &&
- STREQ(def->disks[i]->dst, "hdc")) {
- continue;
- }
- if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
- continue;
-
- if (xenFormatXMDisk(diskVal, def->disks[i],
- hvm, xendConfigVersion) < 0)
- goto cleanup;
- }
- if (diskVal->list != NULL) {
- int ret = virConfSetValue(conf, "disk", diskVal);
- diskVal = NULL;
- if (ret < 0)
- goto cleanup;
- }
- VIR_FREE(diskVal);
if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) < 0)
goto cleanup;
if (xenFormatXMPCI(conf, def) < 0)
--
1.8.4.5
6:35 p.m.
On 07/10/2014 07:42 AM, David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function xenParseXMGeneral(virConfPtr conf, ...);
This function parses the xm general options such as uuid and name
[meta-comment]:
When posting a series, it's useful to post a cover letter (0/17) to
which all patches reply to, rather than having 2-17 reply to 1/17 with
no lead-in on the series as a whole.
signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 65 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 25a042d..1953a85 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char
*uuid)
* 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)
+static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
+ virCapsPtr caps)
Indentation is off; per our style, this should be:
static int
xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
virCapsPtr caps)
+}
+virDomainDefPtr
2 blank lines between functions.
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
Indentation is off; the second line should line up to the character
after the ( of the first line.
Otherwise, this looks like it is just a refactoring of splitting out a
helper function for later reuse.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:08 a.m.
Eric Blake wrote:
On 07/10/2014 07:42 AM, David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
>
> Introduce function xenParseXMGeneral(virConfPtr conf, ...);
> This function parses the xm general options such as uuid and name
>
[meta-comment]:
When posting a series, it's useful to post a cover letter (0/17) to
which all patches reply to, rather than having 2-17 reply to 1/17 with
no lead-in on the series as a whole.
David, you can add '--cover-letter --annotate' to your git send-email
command to have git generate the cover letter and let you edit it before
sending.
> signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
> ---
> src/xenxs/xen_xm.c | 65 +++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index 25a042d..1953a85 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned
char *uuid)
> * 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)
> +static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
> + virCapsPtr caps)
>
Indentation is off; per our style, this should be:
static int
xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
virCapsPtr caps)
> +}
> +virDomainDefPtr
>
2 blank lines between functions.
> +xenParseXM(virConfPtr conf, int xendConfigVersion,
> + virCapsPtr caps)
>
Indentation is off; the second line should line up to the character
after the ( of the first line.
Otherwise, this looks like it is just a refactoring of splitting out a
helper function for later reuse.
David is looking into supporting Xen's xl config format in
domxml-{from,to}-native. For the most part, xm is a subset of xl, so a
lot of the xm parsing code can be used by the xl parser.
David, this type of info would be useful in a cover letter, so reviewers
know what all the code motion is about :-).
Regards,
Jim
11:58 p.m.
David Kiarie wrote:
From: Kiarie Kahurani <davidkiarie4(a)gmail.com>
Introduce function xenParseXMGeneral(virConfPtr conf, ...);
This function parses the xm general options such as uuid and name
signed-off-by: David Kiarie <davidkiarie4(a)gmail.com>
---
src/xenxs/xen_xm.c | 65 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 25a042d..1953a85 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char
*uuid)
* 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)
+static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
+ virCapsPtr caps)
Eric already mentioned the indentation, but I'll mention the name bugs
me a bit :). Other names that come to mind are xenParseXMBasic,
xenParseXMMetadata, and xenParseXMGeneralMeta. I tend to prefer the
last one.
Regards,
Jim
{
+
+ const char *defaultMachine;
const char *str;
int hvm = 0;
- int val;
- virConfValuePtr list;
- virDomainDefPtr def = NULL;
- virDomainDiskDefPtr disk = NULL;
- virDomainNetDefPtr net = NULL;
- virDomainGraphicsDefPtr graphics = NULL;
- virDomainHostdevDefPtr hostdev = NULL;
- size_t i;
- const char *defaultMachine;
- int vmlocaltime = 0;
- unsigned long count;
- char *script = NULL;
- char *listenAddr = 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;
+ return -1;
if (xenXMConfigGetUUID(conf, "uuid", def->uuid) < 0)
- goto cleanup;
-
+ return -1;
if ((xenXMConfigGetString(conf, "builder", &str, "linux") ==
0) &&
STREQ(str, "hvm"))
hvm = 1;
if (VIR_STRDUP(def->os.type, hvm ? "hvm" : "xen") < 0)
- goto cleanup;
+ return -1;
def->os.arch =
virCapabilitiesDefaultGuestArch(caps,
@@ -300,7 +281,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no supported architecture for os type
'%s'"),
def->os.type);
- goto cleanup;
+ return -1;
}
defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
@@ -309,9 +290,37 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
virDomainVirtTypeToString(def->virtType));
if (defaultMachine != NULL) {
if (VIR_STRDUP(def->os.machine, defaultMachine) < 0)
- goto cleanup;
+ return -1;
}
+ return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+ virCapsPtr caps)
+{
+ const char *str;
+ int hvm = 0;
+ int val;
+ virConfValuePtr list;
+ virDomainDefPtr def = NULL;
+ virDomainDiskDefPtr disk = NULL;
+ virDomainNetDefPtr net = NULL;
+ virDomainGraphicsDefPtr graphics = NULL;
+ virDomainHostdevDefPtr hostdev = NULL;
+ size_t i;
+ int vmlocaltime = 0;
+ unsigned long count;
+ 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;
+ hvm = (STREQ(def->os.type, "hvm"));
if (hvm) {
const char *boot;
if (xenXMConfigCopyString(conf, "kernel", &def->os.loader) <
0)
3715
days inactive
3721
days old
42 comments
4 participants
participants (4)
-
David Kiarie -
David kiarie
-
Eric Blake -
Jim Fehlig