diff mbox

Sort the fw_cfg file list

Message ID 1457972631-1376-1-git-send-email-minyard@acm.org (mailing list archive)
State New, archived
Headers show

Commit Message

Corey Minyard March 14, 2016, 4:23 p.m. UTC 
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 new machine type for compatibility.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---

I don't really like the double-negative created by dont_sort_fw_cfgs,
but it seems more consistent with the other fields (no_floppy, etc.)
where the default is 0 and the compat is 1.

 hw/i386/pc_piix.c   | 16 +++++++++++++---
 hw/i386/pc_q35.c    | 13 +++++++++++--
 hw/nvram/fw_cfg.c   | 32 ++++++++++++++++++++++++++------
 include/hw/boards.h |  3 ++-
 4 files changed, 52 insertions(+), 12 deletions(-)

Comments

Paolo Bonzini March 14, 2016, 4:33 p.m. UTC | #1 
On 14/03/2016 17:23, 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 new machine type for compatibility.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> 
> I don't really like the double-negative created by dont_sort_fw_cfgs,
> but it seems more consistent with the other fields (no_floppy, etc.)
> where the default is 0 and the compat is 1.

You can use the 2.6 machine type too.

Paolo
Corey Minyard March 14, 2016, 4:39 p.m. UTC | #2 
On 03/14/2016 11:33 PM, Paolo Bonzini wrote:
>
> On 14/03/2016 17:23, 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 new machine type for compatibility.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>>
>> I don't really like the double-negative created by dont_sort_fw_cfgs,
>> but it seems more consistent with the other fields (no_floppy, etc.)
>> where the default is 0 and the compat is 1.
> You can use the 2.6 machine type too.

Oh, duh.  Yeah, I should use that.

Thanks,

-corey
Michael S. Tsirkin March 15, 2016, 6:56 a.m. UTC | #3 
On Mon, Mar 14, 2016 at 11:23:51PM +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 new machine type for compatibility.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

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).




> ---
> 
> I don't really like the double-negative created by dont_sort_fw_cfgs,
> but it seems more consistent with the other fields (no_floppy, etc.)
> where the default is 0 and the compat is 1.
> 
>  hw/i386/pc_piix.c   | 16 +++++++++++++---
>  hw/i386/pc_q35.c    | 13 +++++++++++--
>  hw/nvram/fw_cfg.c   | 32 ++++++++++++++++++++++++++------
>  include/hw/boards.h |  3 ++-
>  4 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 6f8c2cd..302ab63 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -411,13 +411,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
>      m->default_display = "std";
>  }
>  
> -static void pc_i440fx_2_6_machine_options(MachineClass *m)
> +static void pc_i440fx_2_7_machine_options(MachineClass *m)
>  {
>      pc_i440fx_machine_options(m);
>      m->alias = "pc";
>      m->is_default = 1;
>  }
>  
> +DEFINE_I440FX_MACHINE(v2_7, "pc-i440fx-2.7", NULL,
> +                      pc_i440fx_2_7_machine_options);
> +
> +
> +static void pc_i440fx_2_6_machine_options(MachineClass *m)
> +{
> +    pc_i440fx_2_7_machine_options(m);
> +    m->alias = NULL;
> +    m->is_default = 0;
> +    m->dont_sort_fw_cfgs = 1;
> +}
> +
>  DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
>                        pc_i440fx_2_6_machine_options);
>  
> @@ -426,8 +438,6 @@ static void pc_i440fx_2_5_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_i440fx_2_6_machine_options(m);
> -    m->alias = NULL;
> -    m->is_default = 0;
>      pcmc->save_tsc_khz = false;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 208a224..793f054 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -339,12 +339,22 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->no_tco = 0;
>  }
>  
> -static void pc_q35_2_6_machine_options(MachineClass *m)
> +static void pc_q35_2_7_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
>      m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v2_7, "pc-q35-2.7", NULL,
> +                   pc_q35_2_7_machine_options);
> +
> +static void pc_q35_2_6_machine_options(MachineClass *m)
> +{
> +    pc_q35_2_7_machine_options(m);
> +    m->alias = NULL;
> +    m->dont_sort_fw_cfgs = 1;
> +}
> +
>  DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
>                     pc_q35_2_6_machine_options);
>  
> @@ -352,7 +362,6 @@ static void pc_q35_2_5_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_q35_2_6_machine_options(m);
> -    m->alias = NULL;
>      pcmc->save_tsc_khz = false;
>      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
diff mbox

Patch

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6f8c2cd..302ab63 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -411,13 +411,25 @@  static void pc_i440fx_machine_options(MachineClass *m)
     m->default_display = "std";
 }
 
-static void pc_i440fx_2_6_machine_options(MachineClass *m)
+static void pc_i440fx_2_7_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v2_7, "pc-i440fx-2.7", NULL,
+                      pc_i440fx_2_7_machine_options);
+
+
+static void pc_i440fx_2_6_machine_options(MachineClass *m)
+{
+    pc_i440fx_2_7_machine_options(m);
+    m->alias = NULL;
+    m->is_default = 0;
+    m->dont_sort_fw_cfgs = 1;
+}
+
 DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
                       pc_i440fx_2_6_machine_options);
 
@@ -426,8 +438,6 @@  static void pc_i440fx_2_5_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_2_6_machine_options(m);
-    m->alias = NULL;
-    m->is_default = 0;
     pcmc->save_tsc_khz = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 208a224..793f054 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -339,12 +339,22 @@  static void pc_q35_machine_options(MachineClass *m)
     m->no_tco = 0;
 }
 
-static void pc_q35_2_6_machine_options(MachineClass *m)
+static void pc_q35_2_7_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v2_7, "pc-q35-2.7", NULL,
+                   pc_q35_2_7_machine_options);
+
+static void pc_q35_2_6_machine_options(MachineClass *m)
+{
+    pc_q35_2_7_machine_options(m);
+    m->alias = NULL;
+    m->dont_sort_fw_cfgs = 1;
+}
+
 DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
                    pc_q35_2_6_machine_options);
 
@@ -352,7 +362,6 @@  static void pc_q35_2_5_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_2_6_machine_options(m);
-    m->alias = NULL;
     pcmc->save_tsc_khz = false;
     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;