Re: [Qemu-devel] [PATCH v2] Sort the fw_cfg file list

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Mar 14, 2016 at 11:55:31PM +0700, minyard@acm.org wrote:
> From: Gerd Hoffmann <kraxel@redhat.com>
> 
> Entries are inserted at the correct place instead of being
> appended to the end in case sorting is enabled.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Added a machine type handling for compatibility.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> 
> Don't add a new machine type in this version, just use the 2.6 one.

Unfortunately this patch won't help any, as your next patch reorders fw
cfg files which will affect the old machine types.

What is needed is not dont_sort_fw_cfgs, instead we need to create a
list of existing fw cfg files in the order they appear currently, and
sort them according to this predefined order (at least for old machine
types).


>  hw/i386/pc_piix.c   |  1 +
>  hw/i386/pc_q35.c    |  1 +
>  hw/nvram/fw_cfg.c   | 32 ++++++++++++++++++++++++++------
>  include/hw/boards.h |  3 ++-
>  4 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 6f8c2cd..6c68e63 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -429,6 +429,7 @@ static void pc_i440fx_2_5_machine_options(MachineClass *m)
>      m->alias = NULL;
>      m->is_default = 0;
>      pcmc->save_tsc_khz = false;
> +    m->dont_sort_fw_cfgs = 1;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 208a224..417fb57 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -354,6 +354,7 @@ static void pc_q35_2_5_machine_options(MachineClass *m)
>      pc_q35_2_6_machine_options(m);
>      m->alias = NULL;
>      pcmc->save_tsc_khz = false;
> +    m->dont_sort_fw_cfgs = 1;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
>  }
>  
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 79c5742..10dab77 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -28,6 +28,7 @@
>  #include "hw/isa/isa.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/sysbus.h"
> +#include "hw/boards.h"
>  #include "trace.h"
>  #include "qemu/error-report.h"
>  #include "qemu/config-file.h"
> @@ -669,8 +670,9 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
>                                FWCfgReadCallback callback, void *callback_opaque,
>                                void *data, size_t len)
>  {
> -    int i, index;
> +    int i, index, count;
>      size_t dsize;
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
>  
>      if (!s->files) {
>          dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS;
> @@ -678,13 +680,31 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
>          fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize);
>      }
>  
> -    index = be32_to_cpu(s->files->count);
> -    assert(index < FW_CFG_FILE_SLOTS);
> +    count = be32_to_cpu(s->files->count);
> +    assert(count < FW_CFG_FILE_SLOTS);
> +
> +    index = count;
> +    if (!mc->dont_sort_fw_cfgs) {
> +        while (index > 0 && strcmp(filename, s->files->f[index-1].name) < 0) {
> +            s->files->f[index] =
> +                s->files->f[index - 1];
> +            s->files->f[index].select =
> +                cpu_to_be16(FW_CFG_FILE_FIRST + index);
> +            s->entries[0][FW_CFG_FILE_FIRST + index] =
> +                s->entries[0][FW_CFG_FILE_FIRST + index - 1];
> +            index--;
> +        }
> +        memset(&s->files->f[index],
> +               0, sizeof(FWCfgFile));
> +        memset(&s->entries[0][FW_CFG_FILE_FIRST + index],
> +               0, sizeof(FWCfgEntry));
> +    }
>  
>      pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
>              filename);
> -    for (i = 0; i < index; i++) {
> -        if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
> +    for (i = 0; i <= count; i++) {
> +        if (i != index &&
> +            strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
>              error_report("duplicate fw_cfg file name: %s",
>                           s->files->f[index].name);
>              exit(1);
> @@ -698,7 +718,7 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
>      s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
>      trace_fw_cfg_add_file(s, index, s->files->f[index].name, len);
>  
> -    s->files->count = cpu_to_be32(index+1);
> +    s->files->count = cpu_to_be32(count+1);
>  }
>  
>  void fw_cfg_add_file(FWCfgState *s,  const char *filename,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 0f30959..f8d99d2 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -85,7 +85,8 @@ struct MachineClass {
>          no_sdcard:1,
>          has_dynamic_sysbus:1,
>          no_tco:1,
> -        pci_allow_0_address:1;
> +        pci_allow_0_address:1,
> +        dont_sort_fw_cfgs:1;
>      int is_default;
>      const char *default_machine_opts;
>      const char *default_boot_order;
> -- 
> 2.5.0