[cc: Eric]
Amos Kong <ak...@redhat.com> writes:
> This patch makes all names in option table to match with actual
> command-line spelling, it will be helpful when we search in option
> tables.
As discussed in [*], the QemuOptsList member name values are ABI:
changing them can break existing -readconfig configuration files. If we
decide breaking ABI is okay here (big if!), we need to document it
prominently in the commit message.
> Signed-off-by: Amos Kong <ak...@redhat.com>
> ---
> V2: fix name in acpi option table
> ---
> hw/acpi/core.c | 8 ++++----
> hw/nvram/fw_cfg.c | 4 ++--
> include/qemu/option.h | 2 +-
> vl.c | 14 +++++++-------
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 79414b4..12e9ae8 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -54,16 +54,16 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE -
> ACPI_TABLE_PFX_SIZE] =
> char unsigned *acpi_tables;
> size_t acpi_tables_len;
>
> -static QemuOptsList qemu_acpi_opts = {
> - .name = "acpi",
> +static QemuOptsList qemu_acpitable_opts = {
> + .name = "acpitable",
> .implied_opt_name = "data",
> - .head = QTAILQ_HEAD_INITIALIZER(qemu_acpi_opts.head),
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_acpitable_opts.head),
> .desc = { { 0 } } /* validated with OptsVisitor */
> };
>
> static void acpi_register_config(void)
> {
> - qemu_add_opts(&qemu_acpi_opts);
> + qemu_add_opts(&qemu_acpitable_opts);
> }
>
> machine_init(acpi_register_config);
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index cb36dc2..1c75507 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -125,7 +125,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
> const char *temp;
>
> /* get user configuration */
> - QemuOptsList *plist = qemu_find_opts("boot-opts");
> + QemuOptsList *plist = qemu_find_opts("boot");
> QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> if (opts != NULL) {
> temp = qemu_opt_get(opts, "splash");
> @@ -191,7 +191,7 @@ static void fw_cfg_reboot(FWCfgState *s)
> const char *temp;
>
> /* get user configuration */
> - QemuOptsList *plist = qemu_find_opts("boot-opts");
> + QemuOptsList *plist = qemu_find_opts("boot");
> QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> if (opts != NULL) {
> temp = qemu_opt_get(opts, "reboot-timeout");
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index 8c0ac34..96b7c29 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -102,7 +102,7 @@ typedef struct QemuOptDesc {
> } QemuOptDesc;
>
> struct QemuOptsList {
> - const char *name;
> + const char *name; /* option name */
> const char *implied_opt_name;
> bool merge_lists; /* Merge multiple uses of option into a single list?
> */
> QTAILQ_HEAD(, QemuOpts) head;
Your patch makes the code adhere to an convention "QemuOptsList name
must match the name of the (non-sugared) command line option using it".
Apart from the comment you add here, it's an unspoken convention.
Making such a convention stick usually takes a tests that fail when it's
violated.
> diff --git a/vl.c b/vl.c
> index 685a7a9..2bcf5fe 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -380,7 +380,7 @@ static QemuOptsList qemu_machine_opts = {
> };
>
> static QemuOptsList qemu_boot_opts = {
> - .name = "boot-opts",
> + .name = "boot",
> .implied_opt_name = "order",
> .merge_lists = true,
> .head = QTAILQ_HEAD_INITIALIZER(qemu_boot_opts.head),
> @@ -1304,7 +1304,7 @@ static void numa_add(const char *optarg)
> }
>
> static QemuOptsList qemu_smp_opts = {
> - .name = "smp-opts",
> + .name = "smp",
> .implied_opt_name = "cpus",
> .merge_lists = true,
> .head = QTAILQ_HEAD_INITIALIZER(qemu_smp_opts.head),
> @@ -3124,7 +3124,7 @@ int main(int argc, char **argv, char **envp)
> drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
> break;
> case QEMU_OPTION_boot:
> - opts = qemu_opts_parse(qemu_find_opts("boot-opts"), optarg,
> 1);
> + opts = qemu_opts_parse(qemu_find_opts("boot"), optarg, 1);
> if (!opts) {
> exit(1);
> }
> @@ -3493,7 +3493,7 @@ int main(int argc, char **argv, char **envp)
> break;
> }
> case QEMU_OPTION_acpitable:
> - opts = qemu_opts_parse(qemu_find_opts("acpi"), optarg, 1);
> + opts = qemu_opts_parse(qemu_find_opts("acpitable"), optarg,
> 1);
> if (!opts) {
> exit(1);
> }
> @@ -3560,7 +3560,7 @@ int main(int argc, char **argv, char **envp)
> }
> break;
> case QEMU_OPTION_smp:
> - if (!qemu_opts_parse(qemu_find_opts("smp-opts"), optarg, 1))
> {
> + if (!qemu_opts_parse(qemu_find_opts("smp"), optarg, 1)) {
> exit(1);
> }
> break;
> @@ -3896,7 +3896,7 @@ int main(int argc, char **argv, char **envp)
> data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
> }
>
> - smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
> + smp_parse(qemu_opts_find(qemu_find_opts("smp"), NULL));
>
> machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
> if (smp_cpus > machine->max_cpus) {
> @@ -4072,7 +4072,7 @@ int main(int argc, char **argv, char **envp)
> bios_name = qemu_opt_get(machine_opts, "firmware");
>
> boot_order = machine->default_boot_order;
> - opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
> + opts = qemu_opts_find(qemu_find_opts("boot"), NULL);
> if (opts) {
> char *normal_boot_order;
> const char *order, *once;
[*] https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg01373.html
