Re: [Qemu-block] [Qemu-devel] [PATCH] iscsi: allow "initiator-name" as block option

[Max Reitz]

On 12.04.2016 16:57, Pino Toscano wrote:
> Allow the "initiator-name" for both the -iscsi and the block options:
> this way it is possible to set it directly as option in the -drive
> specification.
> The current way to specify the initiator name for a certain iSCSI
> target is:
>   -iscsi id=TARGET,initiator-name=IQN
> which cannot be actually done when TARGET has the optional part, as
> colon is not accepted as id for QemuOpts [1].
> 
> Hence, allow the "initiator-name" also in block options: this way
> it is possible to set it directly as option in -drive, e.g.:
>   -drive file=URI,driver=iscsi,initiator-name=IQN
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg06501.html
> 
> Signed-off-by: Pino Toscano <address@hidden>
> ---
>  block/iscsi.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 302baf8..4a1c300 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1161,7 +1161,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(QDict *options, const char *target)
>  {
>      QemuOptsList *list;
>      QemuOpts *opts;
> @@ -1169,6 +1169,11 @@ static char *parse_initiator_name(const char *target)
>      char *iscsi_name;
>      UuidInfo *uuid_info;
>  
> +    name = qdict_get_try_str(options, "initiator-name");
> +    if (name != NULL) {
> +        return g_strdup(name);
> +    }

You should not be using the "options" QDict here but the already parsed
QemuOpts ("opts" in the caller). That is, the caller should either pass
said @opts or use qemu_opt_get(opts, "initiator-name") to get the name
and pass that then.

> +
>      list = qemu_find_opts("iscsi");
>      if (list) {
>          opts = qemu_opts_find(list, target);
> @@ -1304,11 +1309,19 @@ static void iscsi_readcapacity_sync(IscsiLun 
> *iscsilun, Error **errp)
>      }
>  }
>  
> +#define COMMON_ISCSI_OPTS \
> +    { \
> +        .name = "initiator-name", \
> +        .type = QEMU_OPT_STRING, \
> +        .help = "Initiator iqn name to use when connecting", \
> +    }
> +
>  /* TODO Convert to fine grained options */
>  static QemuOptsList runtime_opts = {
>      .name = "iscsi",
>      .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
>      .desc = {
> +        COMMON_ISCSI_OPTS,
>          {
>              .name = "filename",
>              .type = QEMU_OPT_STRING,
> @@ -1473,7 +1486,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(bs->options, iscsi_url->target);
>  
>      iscsi = iscsi_create_context(initiator_name);
>      if (iscsi == NULL) {
> @@ -1864,6 +1877,7 @@ static QemuOptsList qemu_iscsi_opts = {
>      .name = "iscsi",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
>      .desc = {
> +        COMMON_ISCSI_OPTS,
>          {
>              .name = "user",
>              .type = QEMU_OPT_STRING,
> @@ -1883,10 +1897,6 @@ static QemuOptsList qemu_iscsi_opts = {
>              .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)",
> 

This needs to be rebased on block-next (which will be merged to master
once the 2.8 window is open), because these options are now in vl.c.

Max

signature.asc
Description: OpenPGP digital signature