Re: [Qemu-devel] [PATCH v8 09/12] vl: Switch qemu_uuid to QemuUUID

[Jeff Cody]

On Sun, Sep 18, 2016 at 12:25:29PM +0800, Fam Zheng wrote:
> Update all qemu_uuid users as well, especially get rid of the duplicated
> low level g_strdup_printf, sscanf and snprintf calls with QEMU UUID API.
> 
> Since qemu_uuid_parse is quite tangled with qemu_uuid, its switching to
> QemuUUID is done here too to keep everything in sync and avoid code
> churn.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  hw/ipmi/ipmi_bmc_sim.c     |  2 +-
>  hw/nvram/fw_cfg.c          |  2 +-
>  hw/ppc/spapr.c             |  7 +------
>  hw/ppc/spapr_rtas.c        |  3 ++-
>  hw/smbios/smbios.c         | 12 ++++++------
>  hw/xenpv/xen_domainbuild.c |  6 +-----
>  include/qemu/uuid.h        |  2 +-
>  include/sysemu/sysemu.h    |  3 ++-
>  qmp.c                      | 10 ++--------
>  ui/spice-core.c            |  2 +-
>  util/uuid.c                | 11 ++++++-----
>  vl.c                       |  6 +++---
>  12 files changed, 27 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index dc9c14c..17c7c0e 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -1773,7 +1773,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error 
> **errp)
>      ibs->acpi_power_state[1] = 0;
>  
>      if (qemu_uuid_set) {
> -        memcpy(&ibs->uuid, qemu_uuid, 16);
> +        memcpy(&ibs->uuid, &qemu_uuid, 16);
>      } else {
>          memset(&ibs->uuid, 0, 16);
>      }
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 1776b1b..92aa563 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -883,7 +883,7 @@ static void fw_cfg_init1(DeviceState *dev)
>      qdev_init_nofail(dev);
>  
>      fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> -    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> +    fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
>      fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics);
>      fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>      fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ca77bb0..bdb689c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -332,12 +332,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>          g_free(buf);
>      }
>  
> -    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> -                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
> -                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> -                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
> -                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> -                          qemu_uuid[14], qemu_uuid[15]);
> +    buf = qemu_uuid_unparse_strdup(&qemu_uuid);
>  
>      _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
>      if (qemu_uuid_set) {
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 27b5ad4..02ce273 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -303,7 +303,8 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>          break;
>      }
>      case RTAS_SYSPARM_UUID:
> -        ret = sysparm_st(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 
> 0));
> +        ret = sysparm_st(buffer, length, (unsigned char *)&qemu_uuid,
> +                         (qemu_uuid_set ? 16 : 0));
>          break;
>      default:
>          ret = RTAS_OUT_NOT_SUPPORTED;
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 0705eb1..9a6552a 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -80,7 +80,7 @@ static struct {
>  
>  static struct {
>      const char *manufacturer, *product, *version, *serial, *sku, *family;
> -    /* uuid is in qemu_uuid[] */
> +    /* uuid is in qemu_uuid */
>  } type1;
>  
>  static struct {
> @@ -409,7 +409,7 @@ static void smbios_build_type_1_fields(void)
>           * BIOS.
>           */
>          smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
> -                         qemu_uuid, 16);
> +                         &qemu_uuid, 16);
>      }
>  }
>  
> @@ -484,9 +484,9 @@ static void smbios_build_type_0_table(void)
>  /* Encode UUID from the big endian encoding described on RFC4122 to the wire
>   * format specified by SMBIOS version 2.6.
>   */
> -static void smbios_encode_uuid(struct smbios_uuid *uuid, const uint8_t *buf)
> +static void smbios_encode_uuid(struct smbios_uuid *uuid, QemuUUID *in)
>  {
> -    memcpy(uuid, buf, 16);
> +    memcpy(uuid, &in, 16);
>      if (smbios_uuid_encoded) {
>          uuid->time_low = bswap32(uuid->time_low);
>          uuid->time_mid = bswap16(uuid->time_mid);
> @@ -503,7 +503,7 @@ static void smbios_build_type_1_table(void)
>      SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
>      SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
>      if (qemu_uuid_set) {
> -        smbios_encode_uuid(&t->uuid, qemu_uuid);
> +        smbios_encode_uuid(&t->uuid, &qemu_uuid);
>      } else {
>          memset(&t->uuid, 0, 16);
>      }
> @@ -1002,7 +1002,7 @@ void smbios_entry_add(QemuOpts *opts)
>  
>              val = qemu_opt_get(opts, "uuid");
>              if (val) {
> -                if (qemu_uuid_parse(val, qemu_uuid) != 0) {
> +                if (qemu_uuid_parse(val, &qemu_uuid) != 0) {
>                      error_report("Invalid UUID");
>                      exit(1);
>                  }
> diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
> index 5a9f5ac..b439b0e 100644
> --- a/hw/xenpv/xen_domainbuild.c
> +++ b/hw/xenpv/xen_domainbuild.c
> @@ -53,11 +53,7 @@ int xenstore_domain_init1(const char *kernel, const char 
> *ramdisk,
>      char *dom, uuid_string[42], vm[256], path[256];
>      int i;
>  
> -    snprintf(uuid_string, sizeof(uuid_string), UUID_FMT,
> -             qemu_uuid[0], qemu_uuid[1], qemu_uuid[2], qemu_uuid[3],
> -             qemu_uuid[4], qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> -             qemu_uuid[8], qemu_uuid[9], qemu_uuid[10], qemu_uuid[11],
> -             qemu_uuid[12], qemu_uuid[13], qemu_uuid[14], qemu_uuid[15]);
> +    qemu_uuid_unparse(&qemu_uuid, uuid_string);
>      dom = xs_get_domain_path(xenstore, xen_domid);
>      snprintf(vm,  sizeof(vm),  "/vm/%s", uuid_string);
>  
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> index bc0601e..afe4840 100644
> --- a/include/qemu/uuid.h
> +++ b/include/qemu/uuid.h
> @@ -52,7 +52,7 @@ void qemu_uuid_unparse(const QemuUUID *uuid, char *out);
>  
>  char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +int qemu_uuid_parse(const char *str, QemuUUID *uuid);
>  
>  void qemu_uuid_bswap(QemuUUID *uuid);
>  
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6111950..ef2c50b 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/uuid.h"
>  #include "qom/object.h"
>  
>  /* vl.c */
> @@ -16,7 +17,7 @@
>  extern const char *bios_name;
>  
>  extern const char *qemu_name;
> -extern uint8_t qemu_uuid[];
> +extern QemuUUID qemu_uuid;
>  extern bool qemu_uuid_set;
>  
>  bool runstate_check(RunState state);
> diff --git a/qmp.c b/qmp.c
> index c47c3a3..660339f 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -36,6 +36,7 @@
>  #include "qom/object_interfaces.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> +#include "qemu/uuid.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -85,15 +86,8 @@ KvmInfo *qmp_query_kvm(Error **errp)
>  UuidInfo *qmp_query_uuid(Error **errp)
>  {
>      UuidInfo *info = g_malloc0(sizeof(*info));
> -    char uuid[64];
>  
> -    snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> -                   qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
> -                   qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
> -                   qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], 
> qemu_uuid[13],
> -                   qemu_uuid[14], qemu_uuid[15]);
> -
> -    info->UUID = g_strdup(uuid);
> +    info->UUID = qemu_uuid_unparse_strdup(&qemu_uuid);
>      return info;
>  }
>  
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index da05054..1452e77 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -796,7 +796,7 @@ void qemu_spice_init(void)
>      qemu_opt_foreach(opts, add_channel, &tls_port, NULL);
>  
>      spice_server_set_name(spice_server, qemu_name);
> -    spice_server_set_uuid(spice_server, qemu_uuid);
> +    spice_server_set_uuid(spice_server, (unsigned char *)&qemu_uuid);
>  
>      seamless_migration = qemu_opt_get_bool(opts, "seamless-migration", 0);
>      spice_server_set_seamless_migration(spice_server, seamless_migration);
> diff --git a/util/uuid.c b/util/uuid.c
> index f0c1eeb..4701903 100644
> --- a/util/uuid.c
> +++ b/util/uuid.c
> @@ -61,18 +61,19 @@ char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
>                             uu[13], uu[14], uu[15]);
>  }
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> +int qemu_uuid_parse(const char *str, QemuUUID *uuid)
>  {
> +    unsigned char *uu = &uuid->data[0];
>      int ret;
>  
>      if (strlen(str) != 36) {
>          return -1;
>      }
>  
> -    ret = sscanf(str, UUID_FMT, &uuid[0], &uuid[1], &uuid[2], &uuid[3],
> -                 &uuid[4], &uuid[5], &uuid[6], &uuid[7], &uuid[8], &uuid[9],
> -                 &uuid[10], &uuid[11], &uuid[12], &uuid[13], &uuid[14],
> -                 &uuid[15]);
> +    ret = sscanf(str, UUID_FMT, &uu[0], &uu[1], &uu[2], &uu[3],
> +                 &uu[4], &uu[5], &uu[6], &uu[7], &uu[8], &uu[9],
> +                 &uu[10], &uu[11], &uu[12], &uu[13], &uu[14],
> +                 &uu[15]);
>  
>      if (ret != 16) {
>          return -1;
> diff --git a/vl.c b/vl.c
> index 1e787a0..a12170f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -183,10 +183,10 @@ uint8_t qemu_extra_params_fw[2];
>  
>  int icount_align_option;
>  
> -/* The bytes in qemu_uuid[] are in the order specified by RFC4122, _not_ in 
> the
> +/* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
>   * little-endian "wire format" described in the SMBIOS 2.6 specification.
>   */
> -uint8_t qemu_uuid[16];
> +QemuUUID qemu_uuid;
>  bool qemu_uuid_set;
>  
>  static NotifierList exit_notifiers =
> @@ -3739,7 +3739,7 @@ int main(int argc, char **argv, char **envp)
>                  cursor_hide = 0;
>                  break;
>              case QEMU_OPTION_uuid:
> -                if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
> +                if (qemu_uuid_parse(optarg, &qemu_uuid) < 0) {
>                      error_report("failed to parse UUID string: wrong 
> format");
>                      exit(1);
>                  }
> -- 
> 2.7.4
> 
> 
Reviewed-by: Jeff Cody <address@hidden>