Re: [PATCH] Invalidate FDOs when no devices are present

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 09/06/2025 15:24, Tu Dinh wrote:
> Hi Paul,
>
> Sorry for the late reply.
>
> On 02/12/2024 13:06, Paul Durrant wrote:
> > From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> >
> > In cases where the entire xenstore device key is removed, FdoScan()
> > currently skips reenumeration entirely. This causes it to not pass the
> > removal event to xennet. Fix the issue by reenumerating devices even if
> > xenstore key does not exist.
> >
> > Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> >
> > Ported from XENVIF.
> >
> > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> > ---
> >    src/xenvkbd/fdo.c | 86 ++++++++++++++++++++++++++---------------------
> >    1 file changed, 48 insertions(+), 38 deletions(-)
> >
> > diff --git a/src/xenvkbd/fdo.c b/src/xenvkbd/fdo.c
> > index 7a87ead0801a..98297fc1a1a1 100644
> > --- a/src/xenvkbd/fdo.c
> > +++ b/src/xenvkbd/fdo.c
> > @@ -756,36 +756,38 @@ __FdoEnumerate(
> >                Name = PdoGetName(Pdo);
> >                Missing = TRUE;
> >    
> > -            // If the PDO already exists and its name is in the device list
> > -            // then we don't want to remove it.
> > -            for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> > -                PANSI_STRING Device = &Devices[Index];
> > -
> > -                if (Device->Length == 0)
> > -                    continue;
> > -
> > -                if (strcmp(Name, Device->Buffer) == 0) {
> > -                    Missing = FALSE;
> > -                    Device->Length = 0;  // avoid duplication
> > -                    break;
> > +            if (Devices != NULL) {
> > +                // If the PDO already exists and its name is in the device list
> > +                // then we don't want to remove it.
> > +                for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> > +                    PANSI_STRING Device = &Devices[Index];
> > +
> > +                    if (Device->Length == 0)
> > +                        continue;
> > +
> > +                    if (strcmp(Name, Device->Buffer) == 0) {
> > +                        Missing = FALSE;
> > +                        Device->Length = 0;  // avoid duplication
> > +                        break;
> > +                    }
> >                    }
> > -            }
> >    
> > -            if (!PdoIsMissing(Pdo)) {
> > -                if (PdoIsEjectRequested(Pdo)) {
> > -                    IoRequestDeviceEject(PdoGetDeviceObject(Pdo));
> > -                } else if (Missing) {
> > -                    PdoSetMissing(Pdo, "device disappeared");
> > -
> > -                    // If the PDO has not yet been enumerated then we can
> > -                    // go ahead and mark it as deleted, otherwise we need
> > -                    // to notify PnP manager and wait for the REMOVE_DEVICE
> > -                    // IRP.
> > -                    if (PdoGetDevicePnpState(Pdo) == Present) {
> > -                        PdoSetDevicePnpState(Pdo, Deleted);
> > -                        PdoDestroy(Pdo);
> > -                    } else {
> > -                        NeedInvalidate = TRUE;
> > +                if (!PdoIsMissing(Pdo)) {
> > +                    if (PdoIsEjectRequested(Pdo)) {
> > +                        IoRequestDeviceEject(PdoGetDeviceObject(Pdo));
> > +                    } else if (Missing) {
> > +                        PdoSetMissing(Pdo, "device disappeared");
> > +
> > +                        // If the PDO has not yet been enumerated then we can
> > +                        // go ahead and mark it as deleted, otherwise we need
> > +                        // to notify PnP manager and wait for the REMOVE_DEVICE
> > +                        // IRP.
> > +                        if (PdoGetDevicePnpState(Pdo) == Present) {
> > +                            PdoSetDevicePnpState(Pdo, Deleted);
> > +                            PdoDestroy(Pdo);
> > +                        } else {
> > +                            NeedInvalidate = TRUE;
> > +                        }
> >                        }
> >                    }
> >                }
> With the ported patch, the `if (!PdoIsMissing(Pdo))` check becomes
> subordinate to the `if (Devices != NULL)` check:
>
>       if (Devices != NULL) {
>           for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
>               // ...
>           }
>
>           if (!PdoIsMissing(Pdo)) {
>               // ...
>           }
>       }
>
> Shouldn't it be outside that check (like in Xenvif):
>
>       if (Devices != NULL) {
>           for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
>               // ...
>           }
>       }
>
>       if (!PdoIsMissing(Pdo)) {
>           // ...
>       }
Probably. XENVIF is general definitive as it has more widely used and 
has been around for longer; hence has had more debug time.
   Paul