Il 06/08/2012 10:24, ronniesahlb...@gmail.com ha scritto:
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 993a86d..243496b 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -896,23 +896,31 @@ static char *parse_initiator_name(const char *target)
> QemuOptsList *list;
> QemuOpts *opts;
> const char *name = NULL;
> + char *default_name;
> + const char *iscsi_name = qemu_get_vm_name();
> +
> + if (asprintf(&default_name, "iqn.2008-11.org.linux-kvm%s%s",
asprintf is not portable, g_strdup_printf is.
> + iscsi_name ? ":" : "",
> + iscsi_name ? iscsi_name : "") == -1) {
> + default_name = (char *)"iqn.2008-11.org.linux-kvm";
> + }
>
> list = qemu_find_opts("iscsi");
> if (!list) {
> - return g_strdup("iqn.2008-11.org.linux-kvm");
> + return g_strdup(default_name);
> }
>
> opts = qemu_opts_find(list, target);
> if (opts == NULL) {
> opts = QTAILQ_FIRST(&list->head);
> if (!opts) {
> - return g_strdup("iqn.2008-11.org.linux-kvm");
> + return g_strdup(default_name);
> }
> }
>
> name = qemu_opt_get(opts, "initiator-name");
> if (!name) {
> - return g_strdup("iqn.2008-11.org.linux-kvm");
> + return g_strdup(default_name);
> }
>
Each of these strdup calls is leaking the original default_name.
> return g_strdup(name);
iscsi_open is never going to free this, but neither is libiscsi.
Putting all this together, we need on top of your patch something like
this:
diff --git a/block/iscsi.c b/block/iscsi.c
index 243496b..69044b0 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -899,31 +899,27 @@ static char *parse_initiator_name(const char *target)
char *default_name;
const char *iscsi_name = qemu_get_vm_name();
- if (asprintf(&default_name, "iqn.2008-11.org.linux-kvm%s%s",
- iscsi_name ? ":" : "",
- iscsi_name ? iscsi_name : "") == -1) {
- default_name = (char *)"iqn.2008-11.org.linux-kvm";
- }
+ default_name = g_strdup_printf(&default_name,
"iqn.2008-11.org.linux-kvm%s%s",
+ iscsi_name ? ":" : "",
+ iscsi_name ? iscsi_name : "");
list = qemu_find_opts("iscsi");
- if (!list) {
- return g_strdup(default_name);
- }
-
- opts = qemu_opts_find(list, target);
- if (opts == NULL) {
- opts = QTAILQ_FIRST(&list->head);
+ if (list) {
+ opts = qemu_opts_find(list, target);
if (!opts) {
- return g_strdup(default_name);
+ opts = QTAILQ_FIRST(&list->head);
+ }
+ if (opts) {
+ name = qemu_opt_get(opts, "initiator-name");
}
}
- name = qemu_opt_get(opts, "initiator-name");
- if (!name) {
- return g_strdup(default_name);
+ if (name) {
+ g_free(default_name);
+ return g_strdup(name);
+ } else {
+ return default_name;
}
-
- return g_strdup(name);
}
/*
@@ -951,7 +947,7 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
error_report("Failed to parse URL : %s %s", filename,
iscsi_get_error(iscsi));
ret = -EINVAL;
- goto failed;
+ goto out;
}
memset(iscsilun, 0, sizeof(IscsiLun));
@@ -962,13 +958,13 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
if (iscsi == NULL) {
error_report("iSCSI: Failed to create iSCSI context.");
ret = -ENOMEM;
- goto failed;
+ goto out;
}
if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
error_report("iSCSI: Failed to set target name.");
ret = -EINVAL;
- goto failed;
+ goto out;
}
if (iscsi_url->user != NULL) {
@@ -977,7 +973,7 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
if (ret != 0) {
error_report("Failed to set initiator username and password");
ret = -EINVAL;
- goto failed;
+ goto out;
}
}
@@ -985,13 +981,13 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
if (parse_chap(iscsi, iscsi_url->target) != 0) {
error_report("iSCSI: Failed to set CHAP user/password");
ret = -EINVAL;
- goto failed;
+ goto out;
}
if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
error_report("iSCSI: Failed to set session type to normal.");
ret = -EINVAL;
- goto failed;
+ goto out;
}
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
@@ -1012,7 +1008,7 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
!= 0) {
error_report("iSCSI: Failed to start async connect.");
ret = -EINVAL;
- goto failed;
+ goto out;
}
while (!task.complete) {
@@ -1023,11 +1019,7 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
error_report("iSCSI: Failed to connect to LUN : %s",
iscsi_get_error(iscsi));
ret = -EINVAL;
- goto failed;
- }
-
- if (iscsi_url != NULL) {
- iscsi_destroy_url(iscsi_url);
+ goto out;
}
/* Medium changer or tape. We dont have any emulation for this so this must
@@ -1039,19 +1031,22 @@ static int iscsi_open(BlockDriverState *bs, const char
*filename, int flags)
bs->sg = 1;
}
- return 0;
+ ret = 0;
-failed:
+out:
if (initiator_name != NULL) {
g_free(initiator_name);
}
if (iscsi_url != NULL) {
iscsi_destroy_url(iscsi_url);
}
- if (iscsi != NULL) {
- iscsi_destroy_context(iscsi);
+
+ if (ret) {
+ if (iscsi != NULL) {
+ iscsi_destroy_context(iscsi);
+ }
+ memset(iscsilun, 0, sizeof(IscsiLun));
}
- memset(iscsilun, 0, sizeof(IscsiLun));
return ret;
}
Can you confirm that this is how the initiator name should be passed, so I can
split the above patch and commit it?
Paolo
