On Sat, May 26, 2018 at 11:00:25PM +0200, Fabiano Fidêncio wrote:
From: Fabiano Fidêncio <fidencio@xxxxxxxxxx> Signed-off-by: Fabiano Fidêncio <fabiano@xxxxxxxxxxxx>
The S-o-B address should match the one of the author.
---
src/xenconfig/xen_xm.c | 268 ++++++++++++++++++++++++-------------------------
1 file changed, 132 insertions(+), 136 deletions(-)
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 4becb40b4c..fc88ac8238 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -112,172 +112,168 @@ xenParseXMDisk(virConfPtr conf, virDomainDefPtr def)
{
virDomainDiskDefPtr disk = NULL;
int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
- virConfValuePtr list = virConfGetValue(conf, "disk");
+ char **disks = NULL, **entries;
- if (list && list->type == VIR_CONF_LIST) {
- list = list->list;
Adding
else
list = NULL;
Would let you move the while below outside of the if body and separate
the whitespace changes from the virConfGetValue -> StringList change.
Or, even better, the whole per-disk logic can be moved to a separate
function.
- while (list) {
- char *head;
- char *offset;
- char *tmp;
- const char *src;
+ if (virConfGetValueStringList(conf, "disk", false, &disks) < 0)
+ goto cleanup;
- if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
- goto skipdisk;
+ for (entries = disks; *entries; entries++) {
+ char *head = *entries;
+ char *offset;
+ char *tmp;
+ const char *src;
- head = list->str;
- if (!(disk = virDomainDiskDefNew(NULL)))
- return -1;
+ if (!(disk = virDomainDiskDefNew(NULL)))
+ return -1;
+
+ /*
+ * Disks have 3 components, SOURCE,DEST-DEVICE,MODE
+ * eg, phy:/dev/HostVG/XenGuest1,xvda,w
+ * The SOURCE is usually prefixed with a driver type,
+ * and optionally driver sub-type
+ * The DEST-DEVICE is optionally post-fixed with disk type
+ */
+
+ /* Extract the source file path*/
+ if (!(offset = strchr(head, ',')))
+ goto skipdisk;
Using a separate function would also get rid of this unusual label.
+
+ if (offset == head) {
+ /* No source file given, eg CDROM with no media */
+ ignore_value(virDomainDiskSetSource(disk, NULL));
+ } else {
+ if (VIR_STRNDUP(tmp, head, offset - head) < 0)
+ goto cleanup;
+
+ if (virDomainDiskSetSource(disk, tmp) < 0) {
+ VIR_FREE(tmp);
+ goto cleanup;
+ }
+ VIR_FREE(tmp);
+ }
[...]
- skipdisk:
- list = list->next;
- virDomainDiskDefFree(disk);
- disk = NULL;
- }
+ skipdisk:
+ virDomainDiskDefFree(disk);
+ disk = NULL;
}
return 0;
cleanup:
+ virStringListFree(disks);
The list needs to be freed even in the success path. (NB: this usage of 'cleanup' label is not customary - for code paths returning an error, we use 'error'. 'cleanup' is meant for code shared between the success and error paths) Jano
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
