<arei.gong...@huawei.com> writes: > From: Gonglei <arei.gong...@huawei.com> > > The reset logic can be done by both machine reset and > boot handler. So we shouldn't return error when the boot > handler callback don't be set. > > Signed-off-by: Gonglei <arei.gong...@huawei.com> > Reviewed-by: Alexander Graf <ag...@suse.de> > --- > bootdevice.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/bootdevice.c b/bootdevice.c > index 5914417..52d3f9e 100644 > --- a/bootdevice.c > +++ b/bootdevice.c > @@ -51,19 +51,15 @@ void qemu_boot_set(const char *boot_order, Error **errp) > { > Error *local_err = NULL; > > - if (!boot_set_handler) { > - error_setg(errp, "no function defined to set boot device list for" > - " this architecture"); > - return; > - } > - > validate_bootdevices(boot_order, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > } > > - boot_set_handler(boot_set_opaque, boot_order, errp); > + if (boot_set_handler) { > + boot_set_handler(boot_set_opaque, boot_order, errp); > + } > } > > void validate_bootdevices(const char *devices, Error **errp)
You didn't address my review of v2 (appended for your convenience). You replied to it, pointing to previous conversation, but I'm afraid don't understand how that conversation applies to changing behavior of HMP command boot_set. If changing boot_set to silently do nothing instead of failing loudly when the target doesn't support changing the boot order is what you want, then you have to document it *prominently* in the commit message. My advice is not to change boot_set's behavior that way, because when the user's command makes no sense, ignoring it silently instead of telling him about the problem is not nice. My review comment describes one way to do that. There are others. Review of v2: Two callers: * HMP command boot_set Before your patch: command fails when the target doesn't support changing the boot order. After your patch: command silently does nothing. I'm afraid that's a regression. Aside: looks like there's no QMP command. * restore_boot_order() No change yet, because restore_boot_order() ignores errors. But PATCH 3 will make it abort on error. I guess that's why you make the change here. To avoid the regression, you could drop PATCH 1, and change PATCH 3 to something like - qemu_boot_set(normal_boot_order, NULL); + if (boot_set_handler) { + qemu_boot_set(normal_boot_order, &error_abort); + } There are other ways, but this looks like the simplest one.