Hi,
> From: Zheng, Lv
> Subject: [PATCH v4 1/2] ACPI: Cleanup -acpitable option code
>
> In -acpitable options, at least/most one data/file sub-option is mandatory,
> this patch cleans up the code to reflect this in a managed manner so that
> the follow-up mandatory sub-options can be added to -acpitable.
>
> Signed-off-by: Lv Zheng <lv.zh...@intel.com>
> ---
> hw/acpi/core.c | 32 +++++++++++++++++++++++---------
> qapi-schema.json | 27 ++++++++-------------------
> qemu-options.hx | 7 +++++--
> 3 files changed, 36 insertions(+), 30 deletions(-)
>
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index e890a5d..cd1f9e4 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -89,8 +89,6 @@ static int acpi_checksum(const uint8_t *data, int len)
> * It is valid to call this function with
> * (@blob == NULL && bloblen == 0 && !has_header).
> *
> - * @hdrs->file and @hdrs->data are ignored.
> - *
> * SIZE_MAX is considered "infinity" in this function.
> *
> * The number of tables that can be installed is not limited, but the 16-bit
> @@ -229,7 +227,8 @@ static void acpi_table_install(const char unsigned
> *blob, size_t bloblen,
> ACPI_TABLE_PFX_SIZE,
> acpi_payload_size);
> }
>
> -void acpi_table_add(const QemuOpts *opts, Error **errp)
> +static void acpi_table_from_file(bool has_header, const char *file,
> + AcpiTableOptions *hdrs, Error **errp)
[Lv Zheng]
It looks this patch still has problem here.
Please ignore the v3/v4 submissions.
Sorry for the noise.
Thanks
Lv
> {
> AcpiTableOptions *hdrs = NULL;
> Error *err = NULL;
> @@ -249,12 +248,8 @@ void acpi_table_add(const QemuOpts *opts,
> Error **errp)
> if (err) {
> goto out;
> }
> - if (hdrs->has_file == hdrs->has_data) {
> - error_setg(&err, "'-acpitable' requires one of 'data' or 'file'");
> - goto out;
> - }
>
> - pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
> + pathnames = g_strsplit(file, ":", 0);
> if (pathnames == NULL || pathnames[0] == NULL) {
> error_setg(&err, "'-acpitable' requires at least one pathname");
> goto out;
> @@ -291,7 +286,7 @@ void acpi_table_add(const QemuOpts *opts, Error
> **errp)
> close(fd);
> }
>
> - acpi_table_install(blob, bloblen, hdrs->has_file, hdrs, &err);
> + acpi_table_install(blob, bloblen, has_header, hdrs, &err);
>
> out:
> g_free(blob);
> @@ -301,6 +296,25 @@ out:
> error_propagate(errp, err);
> }
>
> +void acpi_table_add(const QemuOpts *opts, Error **errp)
> +{
> + const char *val;
> +
> + val = qemu_opt_get((QemuOpts *)opts, "file");
> + if (val) {
> + acpi_table_from_file(true, val, hdrs, errp);
> + return;
> + }
> +
> + val = qemu_opt_get((QemuOpts *)opts, "data");
> + if (val) {
> + acpi_table_from_file(false, val, hdrs, errp);
> + return;
> + }
> +
> + error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
> +}
> +
> static bool acpi_table_builtin = false;
>
> void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5658723..a5e219f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3597,17 +3597,18 @@
> ##
> # @AcpiTableOptions
> #
> -# Specify an ACPI table on the command line to load.
> +# Specify ACPI table options for the table loaded on the command line.
> #
> -# At most one of @file and @data can be specified. The list of files
> specified
> -# by any one of them is loaded and concatenated in order. If both are
> omitted,
> -# @data is implied.
> +# ACPI table can be loaded via 'file' and 'data' options. At most one of
> +# 'file' and 'data' can be specified. The list of files specified by any one
> +# of them is loaded and concatenated in order. If both # are omitted,
> 'data'
> +# is implied.
> #
> # Other fields / optargs can be used to override fields of the generic ACPI
> # table header; refer to the ACPI specification 5.0, section 5.2.6 System
> # Description Table Header. If a header field is not overridden, then the
> -# corresponding value from the concatenated blob is used (in case of
> @file), or
> -# it is filled in with a hard-coded value (in case of @data).
> +# corresponding value from the concatenated blob is used (in case of
> 'file'),
> +# or it is filled in with a hard-coded value (in case of 'data').
> #
> # String fields are copied into the matching ACPI member from lowest
> address
> # upwards, and silently truncated / NUL-padded to length.
> @@ -3628,16 +3629,6 @@
> # @asl_compiler_rev: #optional revision number of the utility that
> created the
> # table (4 bytes)
> #
> -# @file: #optional colon (:) separated list of pathnames to load and
> -# concatenate as table data. The resultant binary blob is expected to
> -# have an ACPI table header. At least one file is required. This field
> -# excludes @data.
> -#
> -# @data: #optional colon (:) separated list of pathnames to load and
> -# concatenate as table data. The resultant binary blob must not have
> an
> -# ACPI table header. At least one file is required. This field
> excludes
> -# @file.
> -#
> # Since 1.5
> ##
> { 'struct': 'AcpiTableOptions',
> @@ -3648,9 +3639,7 @@
> '*oem_table_id': 'str',
> '*oem_rev': 'uint32',
> '*asl_compiler_id': 'str',
> - '*asl_compiler_rev': 'uint32',
> - '*file': 'str',
> - '*data': 'str' }}
> + '*asl_compiler_rev': 'uint32' }}
>
> ##
> # @CommandLineParameterType:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a71aaf8..5fe7f87 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1493,10 +1493,13 @@ Disable HPET support.
> ETEXI
>
> DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
> - "-acpitable
> [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compi
> ler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
> + "-acpitable {data|file}=file1[:file2]...\n"
> + " [,sig=str][,rev=n]\n"
> + " [,oem_id=str][,oem_table_id=str][,oem_rev=n]\n"
> + " [,asl_compiler_id=str][,asl_compiler_rev=n]\n"
> " ACPI table description\n", QEMU_ARCH_I386)
> STEXI
> -@item -acpitable
> [sig=@var{str}][,rev=@var{n}][,oem_id=@var{str}][,oem_table_id=@var{st
> r}][,oem_rev=@var{n}]
> [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}][,data=@var{file1
> }[:@var{file2}]...]
> +@item -acpitable
> data=@var{file1}[:@var{file2}]...[,sig=@var{str}][,rev=@var{n}][,oem_id=@
> var{str}][,oem_table_id=@var{str}][,oem_rev=@var{n}]
> [,asl_compiler_id=@var{str}][,asl_compiler_rev=@var{n}]
> @findex -acpitable
> Add ACPI table with specified header fields and context from specified files.
> For file=, take whole ACPI table from the specified files, including all
> --
> 1.7.10
