"Gonglei (Arei)" <arei.gong...@huawei.com> writes:
> Hi,
>
>>
>> > On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com wrote:
>> > [...]
>> > > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>> > > + const char *suffix)
>> > > +{
>> > > + FWBootEntry *i, *old_entry = NULL;
>> > > +
>> > > + assert(dev != NULL || suffix != NULL);
>> > > +
>> > > + if (bootindex >= 0) {
>> > > + QTAILQ_FOREACH(i, &fw_boot_order, link) {
>> > > + if (i->bootindex == bootindex) {
>> > > + qerror_report(ERROR_CLASS_GENERIC_ERROR,
>> > > + "The bootindex %d has already been
>> used",
>> > > + bootindex);
>> >
>> > Isn't an Error** parameter preferable here, instead of using
>> > qerror_report()?
>>
>> Good catch. I'm not following this series, but using qerror_report() is
>> almost always wrong these days.
Yes. http://wiki.qemu.org/CodeTransitions#Error_reporting explains:
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere. Use
Error objects instead with error_propagate() and error_setg()
instead.
> Would you give me some advice? Thanks.
> Using error_report() instead of qerror_report()?
Depends on how the function is used.
If you know this can only run during QEMU startup (e.g. command line
processing) or in a *human* monitor, error_report() is fine.
If the error is propagated up the call chain to some place that reports
it via its Error ** parameter to its caller, then you should consider
passing an Error object created with error_setg() here up the call
chain.
Not the case right now, as your modify_boot_device_path() cannot fail.
Whether that's appropriate I can't tell without examining more of your
patch.
