* [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
@ 2015-09-11 6:00 Fam Zheng
2015-09-11 15:20 ` Eric Blake
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2015-09-11 6:00 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Peter Lieven, qemu-block,
Ronnie Sahlberg
Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
specify iscsi connection parameters, unfortunately it doesn't work with
qemu-img.
This patch adds per drive options to iscsi driver so that at least
qemu-img can use the "json:{...}" filename magic.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 64 insertions(+), 19 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 5002916..9efb9ec 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1011,7 +1011,9 @@ retry:
return 0;
}
-static void parse_chap(struct iscsi_context *iscsi, const char *target,
+static void parse_chap(struct iscsi_context *iscsi,
+ QemuOpts *img_opts,
+ const char *target,
Error **errp)
{
QemuOptsList *list;
@@ -1025,19 +1027,22 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target,
}
opts = qemu_opts_find(list, target);
- if (opts == NULL) {
+ if (!opts) {
opts = QTAILQ_FIRST(&list->head);
- if (!opts) {
- return;
- }
}
- user = qemu_opt_get(opts, "user");
+ user = qemu_opt_get(img_opts, "user");
+ if (!user && opts) {
+ user = qemu_opt_get(opts, "user");
+ }
if (!user) {
return;
}
- password = qemu_opt_get(opts, "password");
+ password = qemu_opt_get(img_opts, "password");
+ if (!password && opts) {
+ password = qemu_opt_get(opts, "password");
+ }
if (!password) {
error_setg(errp, "CHAP username specified but no password was given");
return;
@@ -1048,13 +1053,20 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target,
}
}
-static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
+static void parse_header_digest(struct iscsi_context *iscsi,
+ QemuOpts *img_opts,
+ const char *target,
Error **errp)
{
QemuOptsList *list;
QemuOpts *opts;
const char *digest = NULL;
+ digest = qemu_opt_get(img_opts, "header-digest");
+ if (digest) {
+ goto found;
+ }
+
list = qemu_find_opts("iscsi");
if (!list) {
return;
@@ -1073,6 +1085,7 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
return;
}
+found:
if (!strcmp(digest, "CRC32C")) {
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
} else if (!strcmp(digest, "NONE")) {
@@ -1086,7 +1099,7 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
}
}
-static char *parse_initiator_name(const char *target)
+static char *parse_initiator_name(QemuOpts *img_opts, const char *target)
{
QemuOptsList *list;
QemuOpts *opts;
@@ -1094,6 +1107,11 @@ static char *parse_initiator_name(const char *target)
char *iscsi_name;
UuidInfo *uuid_info;
+ name = qemu_opt_get(img_opts, "initiator-name");
+ if (name) {
+ return g_strdup(name);
+ }
+
list = qemu_find_opts("iscsi");
if (list) {
opts = qemu_opts_find(list, target);
@@ -1120,12 +1138,17 @@ static char *parse_initiator_name(const char *target)
return iscsi_name;
}
-static int parse_timeout(const char *target)
+static int parse_timeout(QemuOpts *img_opts, const char *target)
{
QemuOptsList *list;
QemuOpts *opts;
const char *timeout;
+ timeout = qemu_opt_get(img_opts, "iscsi");
+ if (timeout) {
+ goto out;
+ }
+
list = qemu_find_opts("iscsi");
if (list) {
opts = qemu_opts_find(list, target);
@@ -1134,13 +1157,14 @@ static int parse_timeout(const char *target)
}
if (opts) {
timeout = qemu_opt_get(opts, "timeout");
- if (timeout) {
- return atoi(timeout);
- }
}
}
-
- return 0;
+out:
+ if (timeout) {
+ return atoi(timeout);
+ } else {
+ return 0;
+ }
}
static void iscsi_nop_timed_event(void *opaque)
@@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
.name = "filename",
.type = QEMU_OPT_STRING,
.help = "URL to the iscsi image",
+ },{
+ .name = "user",
+ .type = QEMU_OPT_STRING,
+ .help = "username for CHAP authentication to target",
+ },{
+ .name = "password",
+ .type = QEMU_OPT_STRING,
+ .help = "password for CHAP authentication to target",
+ },{
+ .name = "header-digest",
+ .type = QEMU_OPT_STRING,
+ .help = "HeaderDigest setting. "
+ "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
+ },{
+ .name = "initiator-name",
+ .type = QEMU_OPT_STRING,
+ .help = "Initiator iqn name to use when connecting",
+ },{
+ .name = "timeout",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Request timeout in seconds (default 0 = no timeout)",
},
{ /* end of list */ }
},
@@ -1390,7 +1435,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
memset(iscsilun, 0, sizeof(IscsiLun));
- initiator_name = parse_initiator_name(iscsi_url->target);
+ initiator_name = parse_initiator_name(opts, iscsi_url->target);
iscsi = iscsi_create_context(initiator_name);
if (iscsi == NULL) {
@@ -1416,7 +1461,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* check if we got CHAP username/password via the options */
- parse_chap(iscsi, iscsi_url->target, &local_err);
+ parse_chap(iscsi, opts, iscsi_url->target, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
@@ -1432,7 +1477,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
/* check if we got HEADER_DIGEST via the options */
- parse_header_digest(iscsi, iscsi_url->target, &local_err);
+ parse_header_digest(iscsi, opts, iscsi_url->target, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
@@ -1440,7 +1485,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* timeout handling is broken in libiscsi before 1.15.0 */
- timeout = parse_timeout(iscsi_url->target);
+ timeout = parse_timeout(opts, iscsi_url->target);
#if defined(LIBISCSI_API_VERSION) && LIBISCSI_API_VERSION >= 20150621
iscsi_set_timeout(iscsi, timeout);
#else
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
2015-09-11 6:00 [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options Fam Zheng
@ 2015-09-11 15:20 ` Eric Blake
2015-09-11 15:27 ` ronnie sahlberg
0 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2015-09-11 15:20 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Peter Lieven, Ronnie Sahlberg,
qemu-block
[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]
On 09/11/2015 12:00 AM, Fam Zheng wrote:
> Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
> specify iscsi connection parameters, unfortunately it doesn't work with
> qemu-img.
>
> This patch adds per drive options to iscsi driver so that at least
> qemu-img can use the "json:{...}" filename magic.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 64 insertions(+), 19 deletions(-)
It would be nice to also add a matching BlockdevOptionsIscsi to
qapi/block-core.json, to allow setting these structured options from
QMP. Separate patch is fine, but we need to do the work for ALL of the
remaining block devices eventually, and now that you are structuring the
command line is a good time to think about it.
> static void iscsi_nop_timed_event(void *opaque)
> @@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
> .name = "filename",
> .type = QEMU_OPT_STRING,
> .help = "URL to the iscsi image",
> + },{
> + .name = "user",
> + .type = QEMU_OPT_STRING,
> + .help = "username for CHAP authentication to target",
> + },{
> + .name = "password",
> + .type = QEMU_OPT_STRING,
> + .help = "password for CHAP authentication to target",
> + },{
Also, this requires passing the password in the command line. We
_really_ need to solve the problem of allowing the password to be passed
via a fd or other QMP command, rather than on the command line.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
2015-09-11 15:20 ` Eric Blake
@ 2015-09-11 15:27 ` ronnie sahlberg
2015-09-14 6:38 ` Fam Zheng
0 siblings, 1 reply; 6+ messages in thread
From: ronnie sahlberg @ 2015-09-11 15:27 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Fam Zheng, qemu-block, Peter Lieven, qemu-devel,
Paolo Bonzini
On Fri, Sep 11, 2015 at 8:20 AM, Eric Blake <eblake@redhat.com> wrote:
> On 09/11/2015 12:00 AM, Fam Zheng wrote:
>> Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
>> specify iscsi connection parameters, unfortunately it doesn't work with
>> qemu-img.
>>
>> This patch adds per drive options to iscsi driver so that at least
>> qemu-img can use the "json:{...}" filename magic.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>> block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 64 insertions(+), 19 deletions(-)
>
> It would be nice to also add a matching BlockdevOptionsIscsi to
> qapi/block-core.json, to allow setting these structured options from
> QMP. Separate patch is fine, but we need to do the work for ALL of the
> remaining block devices eventually, and now that you are structuring the
> command line is a good time to think about it.
>
>
>> static void iscsi_nop_timed_event(void *opaque)
>> @@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
>> .name = "filename",
>> .type = QEMU_OPT_STRING,
>> .help = "URL to the iscsi image",
>> + },{
>> + .name = "user",
>> + .type = QEMU_OPT_STRING,
>> + .help = "username for CHAP authentication to target",
>> + },{
>> + .name = "password",
>> + .type = QEMU_OPT_STRING,
>> + .help = "password for CHAP authentication to target",
>> + },{
>
> Also, this requires passing the password in the command line. We
> _really_ need to solve the problem of allowing the password to be passed
> via a fd or other QMP command, rather than on the command line.
Passing via command line is evil. It should still be possible to pass
all this via a config file to qemu :
"""
...
Howto use a configuration file to set iSCSI configuration options:
@example
cat >iscsi.conf <<EOF
[iscsi "iqn.target.name"]
user = "me"
password = "my password"
initiator-name = "iqn.qemu.test:my-initiator"
header-digest = "CRC32C"
EOF
qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
-readconfig iscsi.conf
@end example
...
"""
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
2015-09-11 15:27 ` ronnie sahlberg
@ 2015-09-14 6:38 ` Fam Zheng
2015-09-14 6:50 ` Peter Lieven
0 siblings, 1 reply; 6+ messages in thread
From: Fam Zheng @ 2015-09-14 6:38 UTC (permalink / raw)
To: Eric Blake, ronnie sahlberg
Cc: Kevin Wolf, Paolo Bonzini, Peter Lieven, qemu-devel, qemu-block
On Fri, 09/11 08:27, ronnie sahlberg wrote:
> On Fri, Sep 11, 2015 at 8:20 AM, Eric Blake <eblake@redhat.com> wrote:
> > On 09/11/2015 12:00 AM, Fam Zheng wrote:
> >> Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
> >> specify iscsi connection parameters, unfortunately it doesn't work with
> >> qemu-img.
> >>
> >> This patch adds per drive options to iscsi driver so that at least
> >> qemu-img can use the "json:{...}" filename magic.
> >>
> >> Signed-off-by: Fam Zheng <famz@redhat.com>
> >> ---
> >> block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
> >> 1 file changed, 64 insertions(+), 19 deletions(-)
> >
> > It would be nice to also add a matching BlockdevOptionsIscsi to
> > qapi/block-core.json, to allow setting these structured options from
> > QMP. Separate patch is fine, but we need to do the work for ALL of the
> > remaining block devices eventually, and now that you are structuring the
> > command line is a good time to think about it.
> >
> >
> >> static void iscsi_nop_timed_event(void *opaque)
> >> @@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
> >> .name = "filename",
> >> .type = QEMU_OPT_STRING,
> >> .help = "URL to the iscsi image",
> >> + },{
> >> + .name = "user",
> >> + .type = QEMU_OPT_STRING,
> >> + .help = "username for CHAP authentication to target",
> >> + },{
> >> + .name = "password",
> >> + .type = QEMU_OPT_STRING,
> >> + .help = "password for CHAP authentication to target",
> >> + },{
> >
> > Also, this requires passing the password in the command line. We
> > _really_ need to solve the problem of allowing the password to be passed
> > via a fd or other QMP command, rather than on the command line.
>
>
> Passing via command line is evil. It should still be possible to pass
> all this via a config file to qemu :
>
> """
> ...
> Howto use a configuration file to set iSCSI configuration options:
> @example
> cat >iscsi.conf <<EOF
> [iscsi "iqn.target.name"]
> user = "me"
> password = "my password"
> initiator-name = "iqn.qemu.test:my-initiator"
> header-digest = "CRC32C"
> EOF
>
> qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
> -readconfig iscsi.conf
> @end example
> ...
> """
I agree passing password with clear text command line is bad, but -readconfig
doesn't work for qemu-img and qemu-io. Any idea how to make that work?
Fam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
2015-09-14 6:38 ` Fam Zheng
@ 2015-09-14 6:50 ` Peter Lieven
2015-09-14 15:04 ` Eric Blake
0 siblings, 1 reply; 6+ messages in thread
From: Peter Lieven @ 2015-09-14 6:50 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, qemu-block@nongnu.org, qemu-devel, ronnie sahlberg,
Paolo Bonzini
> Am 14.09.2015 um 08:38 schrieb Fam Zheng <famz@redhat.com>:
>
>> On Fri, 09/11 08:27, ronnie sahlberg wrote:
>>> On Fri, Sep 11, 2015 at 8:20 AM, Eric Blake <eblake@redhat.com> wrote:
>>>> On 09/11/2015 12:00 AM, Fam Zheng wrote:
>>>> Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
>>>> specify iscsi connection parameters, unfortunately it doesn't work with
>>>> qemu-img.
>>>>
>>>> This patch adds per drive options to iscsi driver so that at least
>>>> qemu-img can use the "json:{...}" filename magic.
>>>>
>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>> ---
>>>> block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
>>>> 1 file changed, 64 insertions(+), 19 deletions(-)
>>>
>>> It would be nice to also add a matching BlockdevOptionsIscsi to
>>> qapi/block-core.json, to allow setting these structured options from
>>> QMP. Separate patch is fine, but we need to do the work for ALL of the
>>> remaining block devices eventually, and now that you are structuring the
>>> command line is a good time to think about it.
>>>
>>>
>>>> static void iscsi_nop_timed_event(void *opaque)
>>>> @@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
>>>> .name = "filename",
>>>> .type = QEMU_OPT_STRING,
>>>> .help = "URL to the iscsi image",
>>>> + },{
>>>> + .name = "user",
>>>> + .type = QEMU_OPT_STRING,
>>>> + .help = "username for CHAP authentication to target",
>>>> + },{
>>>> + .name = "password",
>>>> + .type = QEMU_OPT_STRING,
>>>> + .help = "password for CHAP authentication to target",
>>>> + },{
>>>
>>> Also, this requires passing the password in the command line. We
>>> _really_ need to solve the problem of allowing the password to be passed
>>> via a fd or other QMP command, rather than on the command line.
>>
>>
>> Passing via command line is evil. It should still be possible to pass
>> all this via a config file to qemu :
>>
>> """
>> ...
>> Howto use a configuration file to set iSCSI configuration options:
>> @example
>> cat >iscsi.conf <<EOF
>> [iscsi "iqn.target.name"]
>> user = "me"
>> password = "my password"
>> initiator-name = "iqn.qemu.test:my-initiator"
>> header-digest = "CRC32C"
>> EOF
>>
>> qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
>> -readconfig iscsi.conf
>> @end example
>> ...
>> """
>
> I agree passing password with clear text command line is bad, but -readconfig
> doesn't work for qemu-img and qemu-io. Any idea how to make that work?
you can pass the secrets via environment variables (see libiscsi readme).
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options
2015-09-14 6:50 ` Peter Lieven
@ 2015-09-14 15:04 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2015-09-14 15:04 UTC (permalink / raw)
To: Peter Lieven, Fam Zheng
Cc: Kevin Wolf, Paolo Bonzini, qemu-block@nongnu.org, qemu-devel,
ronnie sahlberg
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
On 09/14/2015 12:50 AM, Peter Lieven wrote:
>>>> It would be nice to also add a matching BlockdevOptionsIscsi to
>>>> qapi/block-core.json, to allow setting these structured options from
>>>> QMP. Separate patch is fine, but we need to do the work for ALL of the
>>>> remaining block devices eventually, and now that you are structuring the
>>>> command line is a good time to think about it.
>>>>
>>>>
>>> Passing via command line is evil. It should still be possible to pass
>>> all this via a config file to qemu :
>>>
>>
>> I agree passing password with clear text command line is bad, but -readconfig
>> doesn't work for qemu-img and qemu-io. Any idea how to make that work?
>
> you can pass the secrets via environment variables (see libiscsi readme).
Environment variables are no more secure than command line parameters -
both are visible via ps to other processes, and hence relatively
insecure. We need a way to pass secrets over a file descriptor, whether
that file descriptor be a config file, or whether it be a pipe.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-14 15:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-11 6:00 [Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options Fam Zheng
2015-09-11 15:20 ` Eric Blake
2015-09-11 15:27 ` ronnie sahlberg
2015-09-14 6:38 ` Fam Zheng
2015-09-14 6:50 ` Peter Lieven
2015-09-14 15:04 ` Eric Blake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).