Hi, Paolo > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Subject: Re: [PATCH v5 2/2] ACPI: Add -acpitable fadt= to allow FADT > revision changes > > > > On 11/08/2016 11:36, Lv Zheng wrote: > > > > - error_setg(errp, "'-acpitable' requires one of 'data' or 'file'"); > > + val = qemu_opt_get((QemuOpts *)opts, "fadt"); > > + if (val) { > > + unsigned long rev; > > Don't use qemu_opt_get. Add the field to AcpiTableOptions in > qapi-schema.json, and then use hdrs->has_fadt, hdrs->fadt.
1. If I do so, users may be confused when only -acpitable fadt=3 is specified (no user tables are provided), qemu may exit because of "data or file option missing". 2. If we doesn't want qemu exit in the above case, code in acpi_table_add() will be too complicated. 3. If I put fadt into AcpiTableOptions in the schema, hdrs->has_fadt/hdrs->fadt will become 2 more useless options (just like hders->file/hdrs->data/hdrs->has_file/hdrs->has_data, see comments of acpi_table_install()) passed to acpi_table_install(). That's why I enhanced -acpitable to convert it into an option with 3 mandatory sub-options: -acpitable data/file .... -acpitable fadt .... Thanks Lv