|
From: | Paul Durrant |
Subject: | Re: [PATCH 06/12] hw/xen: add get_frontend_path() method to XenDeviceClass |
Date: | Tue, 24 Oct 2023 13:59:07 +0100 |
User-agent: | Mozilla Thunderbird |
On 24/10/2023 13:56, David Woodhouse wrote:
On Tue, 2023-10-24 at 13:42 +0100, Paul Durrant wrote:--- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -711,8 +711,16 @@ static void xen_device_frontend_create(XenDevice *xendev, Error **errp) { ERRP_GUARD(); XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev))); + XenDeviceClass *xendev_class = XEN_DEVICE_GET_CLASS(xendev);- xendev->frontend_path = xen_device_get_frontend_path(xendev);+ if (xendev_class->get_frontend_path) { + xendev->frontend_path = xendev_class->get_frontend_path(xendev, errp); + if (!xendev->frontend_path) { + return;I think you need to update errp here to note that you are failing to create the frontend.If xendev_class->get_frontend_path returned NULL it will have filled in errp.
Ok, but a prepend to say that a lack of path there means we skip frontend creation seems reasonable?
As a general rule (I'll be doing a bombing run on xen-bus once I get my patch queue down into single digits) we should never check 'if (*errp)' to check if a function had an error. It should *also* return a success or failure indication, and we should cope with errp being NULL.
I'm pretty sure someone told me the exact opposite a few years back. Paul
[Prev in Thread] | Current Thread | [Next in Thread] |