[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 1/6] blockdev: fix drive_init() opts and bs_opts lea
From: |
Stefan Hajnoczi |
Subject: |
[Qemu-devel] [PATCH 1/6] blockdev: fix drive_init() opts and bs_opts leaks |
Date: |
Tue, 10 Sep 2013 18:21:07 +0200 |
These memory leaks also make drive_add if=none,id=drive0 without a file=
option leak the options list. This keeps ID "drive0" around forever.
Signed-off-by: Stefan Hajnoczi <address@hidden>
---
blockdev.c | 51 +++++++++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e70e16e..c053a58 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -360,7 +360,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (error_is_set(&error)) {
qerror_report_err(error);
error_free(error);
- return NULL;
+ goto early_err;
}
if (id) {
@@ -390,7 +390,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
;
if (type == IF_COUNT) {
error_report("unsupported bus type '%s'", buf);
- return NULL;
+ goto early_err;
}
} else {
type = block_default_type;
@@ -401,15 +401,15 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (cyls || heads || secs) {
if (cyls < 1) {
error_report("invalid physical cyls number");
- return NULL;
+ goto early_err;
}
if (heads < 1) {
error_report("invalid physical heads number");
- return NULL;
+ goto early_err;
}
if (secs < 1) {
error_report("invalid physical secs number");
- return NULL;
+ goto early_err;
}
}
@@ -417,7 +417,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (!cyls) {
error_report("'%s' trans must be used with cyls, heads and secs",
buf);
- return NULL;
+ goto early_err;
}
if (!strcmp(buf, "none"))
translation = BIOS_ATA_TRANSLATION_NONE;
@@ -427,7 +427,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
translation = BIOS_ATA_TRANSLATION_AUTO;
else {
error_report("'%s' invalid translation type", buf);
- return NULL;
+ goto early_err;
}
}
@@ -437,19 +437,19 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
} else if (!strcmp(buf, "cdrom")) {
if (cyls || secs || heads) {
error_report("CHS can't be set with media=%s", buf);
- return NULL;
+ goto early_err;
}
media = MEDIA_CDROM;
} else {
error_report("'%s' invalid media", buf);
- return NULL;
+ goto early_err;
}
}
if ((buf = qemu_opt_get(opts, "discard")) != NULL) {
if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) {
error_report("invalid discard option");
- return NULL;
+ goto early_err;
}
}
@@ -471,7 +471,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
/* this is the default */
} else {
error_report("invalid aio option");
- return NULL;
+ goto early_err;
}
}
#endif
@@ -481,7 +481,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
error_printf("Supported formats:");
bdrv_iterate_format(bdrv_format_print, NULL);
error_printf("\n");
- return NULL;
+ goto early_err;
}
drv = bdrv_find_whitelisted_format(buf, ro);
@@ -491,7 +491,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
} else {
error_report("'%s' invalid format", buf);
}
- return NULL;
+ goto early_err;
}
}
@@ -512,7 +512,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (!do_check_io_limits(&io_limits, &error)) {
error_report("%s", error_get_pretty(error));
error_free(error);
- return NULL;
+ goto early_err;
}
if (qemu_opt_get(opts, "boot") != NULL) {
@@ -525,12 +525,12 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type !=
IF_NONE) {
error_report("werror is not supported by this bus type");
- return NULL;
+ goto early_err;
}
on_write_error = parse_block_error_action(buf, 0);
if (on_write_error < 0) {
- return NULL;
+ goto early_err;
}
}
@@ -538,19 +538,19 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type !=
IF_NONE) {
error_report("rerror is not supported by this bus type");
- return NULL;
+ goto early_err;
}
on_read_error = parse_block_error_action(buf, 1);
if (on_read_error < 0) {
- return NULL;
+ goto early_err;
}
}
if ((devaddr = qemu_opt_get(opts, "addr")) != NULL) {
if (type != IF_VIRTIO) {
error_report("addr is not supported by this bus type");
- return NULL;
+ goto early_err;
}
}
@@ -559,7 +559,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (index != -1) {
if (bus_id != 0 || unit_id != -1) {
error_report("index cannot be used with bus and unit");
- return NULL;
+ goto early_err;
}
bus_id = drive_index_to_bus_id(type, index);
unit_id = drive_index_to_unit_id(type, index);
@@ -585,7 +585,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (max_devs && unit_id >= max_devs) {
error_report("unit %d too big (max is %d)",
unit_id, max_devs - 1);
- return NULL;
+ goto early_err;
}
/*
@@ -595,7 +595,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (drive_get(type, bus_id, unit_id) != NULL) {
error_report("drive with bus=%d, unit=%d (index=%d) exists",
bus_id, unit_id, index);
- return NULL;
+ goto early_err;
}
/* init */
@@ -672,6 +672,8 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
if (has_driver_specific_opts) {
file = NULL;
} else {
+ QDECREF(bs_opts);
+ qemu_opts_del(opts);
return dinfo;
}
}
@@ -730,12 +732,13 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
return dinfo;
err:
- qemu_opts_del(opts);
- QDECREF(bs_opts);
bdrv_delete(dinfo->bdrv);
g_free(dinfo->id);
QTAILQ_REMOVE(&drives, dinfo, next);
g_free(dinfo);
+early_err:
+ QDECREF(bs_opts);
+ qemu_opts_del(opts);
return NULL;
}
--
1.8.3.1
- [Qemu-devel] [PATCH 0/6] qdev and blockdev refcount leak fixes, Stefan Hajnoczi, 2013/09/10
- [Qemu-devel] [PATCH 3/6] libqtest: rename qmp() to qmp_discard_response(), Stefan Hajnoczi, 2013/09/10
- [Qemu-devel] [PATCH 4/6] libqtest: add qmp(fmt, ...) -> QDict* function, Stefan Hajnoczi, 2013/09/10
- [Qemu-devel] [PATCH 5/6] blockdev-test: add test case for drive_add duplicate IDs, Stefan Hajnoczi, 2013/09/10
- [Qemu-devel] [PATCH 6/6] qdev-monitor-test: add device_add leak test cases, Stefan Hajnoczi, 2013/09/10