[6/6] iscsi: Move iqn generation code to util

Function qmp_query_uuid, even with a version in libqemustub.a, is not
present in qemu-img, unless we move it to something that is linked with
block-obj-y. Since it's a helper function, move it to util makes sense.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c         | 15 +--------------
 include/qemu-common.h |  3 +++
 util/Makefile.objs    |  1 +
 util/iqn.c            | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 util/iqn.c

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Function qmp_query_uuid, even with a version in libqemustub.a, is not
> present in qemu-img, unless we move it to something that is linked with
> block-obj-y. Since it's a helper function, move it to util makes sense.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/iscsi.c         | 15 +--------------
>  include/qemu-common.h |  3 +++
>  util/Makefile.objs    |  1 +
>  util/iqn.c            | 38 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 util/iqn.c
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 2c9cfc1..b09f539 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -37,8 +37,6 @@
>  #include "trace.h"
>  #include "block/scsi.h"
>  #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> -#include "qmp-commands.h"
>  
>  #include <iscsi/iscsi.h>
>  #include <iscsi/scsi-lowlevel.h>
> @@ -1034,8 +1032,6 @@ static char *parse_initiator_name(const char *target)
>      QemuOptsList *list;
>      QemuOpts *opts;
>      const char *name;
> -    char *iscsi_name;
> -    UuidInfo *uuid_info;
>  
>      list = qemu_find_opts("iscsi");
>      if (list) {
> @@ -1051,16 +1047,7 @@ static char *parse_initiator_name(const char *target)
>          }
>      }
>  
> -    uuid_info = qmp_query_uuid(NULL);
> -    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> -        name = qemu_get_vm_name();
> -    } else {
> -        name = uuid_info->UUID;
> -    }
> -    iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
> -                                 name ? ":" : "", name ? name : "");
> -    qapi_free_UuidInfo(uuid_info);
> -    return iscsi_name;
> +    return iqn_generate("iqn.2008-11.org.linux-kvm");
>  }
>  
>  static void iscsi_nop_timed_event(void *opaque)
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index bcf7a6a..ba7700f 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -420,6 +420,9 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
>  /* unicode.c */
>  int mod_utf8_codepoint(const char *s, size_t n, char **end);
>  
> +/* iqn.c */
> +char *iqn_generate(const char *prefix);
> +
>  /*
>   * Hexdump a buffer to a file. An optional string prefix is added to every line
>   */
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 65a36f6..5c043e9 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -14,3 +14,4 @@ util-obj-y += throttle.o
>  util-obj-y += getauxval.o
>  util-obj-y += readline.o
>  util-obj-y += rfifolock.o
> +util-obj-y += iqn.o
> diff --git a/util/iqn.c b/util/iqn.c
> new file mode 100644
> index 0000000..c8d1eda
> --- /dev/null
> +++ b/util/iqn.c
> @@ -0,0 +1,38 @@
> +/*
> + * iqn generat function
> + *
> + * Copyright Red Hat, Inc., 2014
> + *
> + * Author: Paolo Bonzini <pbonzini@redhat.com>

Why me? :)

> + *         Fam Zheng <famz@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include <glib.h>
> +#include "qemu/error-report.h"
> +#include "qemu-common.h"
> +#include "sysemu/sysemu.h"
> +#include "qmp-commands.h"
> +
> +char *iqn_generate(const char *prefix)
> +{
> +    const char *name;
> +    char *iqn;
> +    UuidInfo *uuid_info;
> +
> +    uuid_info = qmp_query_uuid(NULL);
> +    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> +        name = qemu_get_vm_name();
> +    } else {
> +        name = uuid_info->UUID;
> +    }
> +    iqn = g_strdup_printf("%s%s%s",
> +                                 prefix,
> +                                 name ? ":" : "",
> +                                 name ? : "");
> +    qapi_free_UuidInfo(uuid_info);
> +
> +    return iqn;
> +}
> 

Ouch, this is ugly... I cannot think of any other way to do it, but
perhaps Benoit's QMP-in-tools series could help here?

In any case, if we need it I think there's no need to keep the argument
to iqn_generate.

Paolo

On Wed, 08/20 15:32, Paolo Bonzini wrote:
> Il 20/08/2014 12:01, Fam Zheng ha scritto:
> > diff --git a/util/iqn.c b/util/iqn.c
> > new file mode 100644
> > index 0000000..c8d1eda
> > --- /dev/null
> > +++ b/util/iqn.c
> > @@ -0,0 +1,38 @@
> > +/*
> > + * iqn generat function
> > + *
> > + * Copyright Red Hat, Inc., 2014
> > + *
> > + * Author: Paolo Bonzini <pbonzini@redhat.com>
> 
> Why me? :)

For the copied uuid part :)

> 
> > + *         Fam Zheng <famz@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * later.  See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include <glib.h>
> > +#include "qemu/error-report.h"
> > +#include "qemu-common.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qmp-commands.h"
> > +
> > +char *iqn_generate(const char *prefix)
> > +{
> > +    const char *name;
> > +    char *iqn;
> > +    UuidInfo *uuid_info;
> > +
> > +    uuid_info = qmp_query_uuid(NULL);
> > +    if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> > +        name = qemu_get_vm_name();
> > +    } else {
> > +        name = uuid_info->UUID;
> > +    }
> > +    iqn = g_strdup_printf("%s%s%s",
> > +                                 prefix,
> > +                                 name ? ":" : "",
> > +                                 name ? : "");
> > +    qapi_free_UuidInfo(uuid_info);
> > +
> > +    return iqn;
> > +}
> > 
> 
> Ouch, this is ugly... I cannot think of any other way to do it, but
> perhaps Benoit's QMP-in-tools series could help here?

I'll have a look to see if it helps.

> 
> In any case, if we need it I think there's no need to keep the argument
> to iqn_generate.

Fam

On Wed, 08/20 15:32, Paolo Bonzini wrote:
> 
> In any case, if we need it I think there's no need to keep the argument
> to iqn_generate.

How about multiple initiators? Should they use the same iqn?

Fam

Il 21/08/2014 05:54, Fam Zheng ha scritto:
>> > In any case, if we need it I think there's no need to keep the argument
>> > to iqn_generate.
> How about multiple initiators? Should they use the same iqn?

block/iscsi.c uses -iscsi for the options so that you don't have to
place them in cleartext in the command line (instead you can use a
-readconfig file and delete them).  user and password can be included in
the URL as well, though, and we could add an initiator-name option to
the iscsi driver to be used with qemu-img/qemu-io.

The default would remain to always use the same iqn.

Paolo

On Thu, 08/21 11:02, Paolo Bonzini wrote:
> Il 21/08/2014 05:54, Fam Zheng ha scritto:
> >> > In any case, if we need it I think there's no need to keep the argument
> >> > to iqn_generate.
> > How about multiple initiators? Should they use the same iqn?
> 
> block/iscsi.c uses -iscsi for the options so that you don't have to
> place them in cleartext in the command line (instead you can use a
> -readconfig file and delete them).  user and password can be included in
> the URL as well, though, and we could add an initiator-name option to
> the iscsi driver to be used with qemu-img/qemu-io.
> 
> The default would remain to always use the same iqn.
> 

Makes sense to me. Thanks,

Fam