A future patch wants to create disk definitions with non-zero default contents; to avoid crashes, all callers that allocate a disk definition should go through a common point. I found allocation points by looking for any code that increments ndisks, as well as any matches for ALLOC.*disk. Most places that modified ndisks were covered by the parse from XML to domain/device definition by initial domain creation or device hotplug; I also hand-checked all drivers that generate a device struct on the fly during getXMLDesc. * src/conf/domain_conf.h (virDomainDiskDefNew): New prototype. * src/conf/domain_conf.c (virDomainDiskDefNew): New function. (virDomainDiskDefParseXML): Use it. * src/parallels/parallels_driver.c (parallelsAddHddInfo): Likewise. * src/qemu/qemu_command.c (qemuParseCommandLine): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise. * src/vmx/vmx.c (virVMXParseDisk): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr): Likewise. * src/xenxs/xen_xm.c (xenParseXM): Likewise. * src/libvirt_private.syms (domain_conf.h): Export it. Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> --- src/conf/domain_conf.c | 12 +++++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/parallels/parallels_driver.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/vbox/vbox_tmpl.c | 6 +++--- src/vmx/vmx.c | 2 +- src/xenxs/xen_sxpr.c | 8 ++++---- src/xenxs/xen_xm.c | 4 ++-- 9 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 40c385e..97968b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1181,6 +1181,16 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) } +virDomainDiskDefPtr +virDomainDiskDefNew(void) +{ + virDomainDiskDefPtr ret; + + ignore_value(VIR_ALLOC(ret)); + return ret; +} + + void virDomainDiskDefFree(virDomainDiskDefPtr def) { @@ -5211,7 +5221,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, int expected_secret_usage = -1; int auth_secret_usage = -1; - if (VIR_ALLOC(def) < 0) + if (!(def = virDomainDiskDefNew())) return NULL; def->geometry.cylinders = 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bde303c..4f4ce02 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2127,6 +2127,7 @@ void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); +virDomainDiskDefPtr virDomainDiskDefNew(void); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); int virDomainDiskGetType(virDomainDiskDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3332c9..009d286 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -207,6 +207,7 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDefGetSecurityLabelDef; +virDomainDiskDefNew; virDomainDiskDeviceTypeToString; virDomainDiskDiscardTypeToString; virDomainDiskErrorPolicyTypeFromString; diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ab59599..a2665dd 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -381,7 +381,7 @@ parallelsAddHddInfo(virDomainDefPtr def, const char *key, virJSONValuePtr value) { virDomainDiskDefPtr disk = NULL; - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (parallelsGetHddInfo(def, disk, key, value)) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 193959f..f5b81c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10969,7 +10969,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, STRPREFIX(arg, "-fd") || STREQ(arg, "-cdrom")) { WANT_VALUE(); - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (STRPREFIX(val, "/dev/")) @@ -11268,7 +11268,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; } } else if (STRPREFIX(val, "disk:")) { - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (VIR_STRDUP(disk->src.path, val + strlen("disk:")) < 0) goto error; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index e124e69..6fef074 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2768,7 +2768,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if ((def->ndisks > 0) && (VIR_ALLOC_N(def->disks, def->ndisks) >= 0)) { for (i = 0; i < def->ndisks; i++) { - if (VIR_ALLOC(def->disks[i]) >= 0) { + if ((def->disks[i] = virDomainDiskDefNew())) { def->disks[i]->device = VIR_DOMAIN_DISK_DEVICE_DISK; def->disks[i]->bus = VIR_DOMAIN_DISK_BUS_IDE; virDomainDiskSetType(def->disks[i], @@ -3247,7 +3247,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { def->ndisks++; if (VIR_REALLOC_N(def->disks, def->ndisks) >= 0) { - if (VIR_ALLOC(def->disks[def->ndisks - 1]) >= 0) { + if ((def->disks[def->ndisks - 1] = virDomainDiskDefNew())) { def->disks[def->ndisks - 1]->device = VIR_DOMAIN_DISK_DEVICE_CDROM; def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_IDE; virDomainDiskSetType(def->disks[def->ndisks - 1], @@ -3294,7 +3294,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { def->ndisks++; if (VIR_REALLOC_N(def->disks, def->ndisks) >= 0) { - if (VIR_ALLOC(def->disks[def->ndisks - 1]) >= 0) { + if ((def->disks[def->ndisks - 1] = virDomainDiskDefNew())) { def->disks[def->ndisks - 1]->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; def->disks[def->ndisks - 1]->bus = VIR_DOMAIN_DISK_BUS_FDC; virDomainDiskSetType(def->disks[def->ndisks - 1], diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 169440c..9b576f7 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1998,7 +1998,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con return -1; } - if (VIR_ALLOC(*def) < 0) + if (!(*def = virDomainDiskDefNew())) return -1; (*def)->device = device; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 29316a4..aacf74c 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -370,7 +370,7 @@ xenParseSxprDisks(virDomainDefPtr def, bootable = sexpr_node(node, "device/tap/bootable"); } - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (dst == NULL) { @@ -1304,7 +1304,7 @@ xenParseSxpr(const struct sexpr *root, tmp = sexpr_node(root, "domain/image/hvm/cdrom"); if ((tmp != NULL) && (tmp[0] != 0)) { virDomainDiskDefPtr disk; - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (virDomainDiskSetSource(disk, tmp) < 0) { virDomainDiskDefFree(disk); @@ -1339,10 +1339,10 @@ xenParseSxpr(const struct sexpr *root, tmp = sexpr_fmt_node(root, "domain/image/hvm/%s", fds[i]); if ((tmp != NULL) && (tmp[0] != 0)) { virDomainDiskDefPtr disk; - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto error; if (virDomainDiskSetSource(disk, tmp) < 0) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto error; } virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index c0422cf..b2db97d 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -486,7 +486,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto skipdisk; head = list->str; - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto cleanup; /* @@ -632,7 +632,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (xenXMConfigGetString(conf, "cdrom", &str, NULL) < 0) goto cleanup; if (str) { - if (VIR_ALLOC(disk) < 0) + if (!(disk = virDomainDiskDefNew())) goto cleanup; virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); -- 1.9.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list