RE: [PATCH] Prevent XenVbd from starting if emulated disk present

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Owen Smith
> Sent: 17 June 2020 14:58
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [PATCH] Prevent XenVbd from starting if emulated disk present
> 
> If any emulated disks are present (i.e. have not been unplugged), then
> XenVbd will fail to start. This failure to start should indicate that a
> reboot is required to complete the installation, and trigger the
> DeviceInstall service to require a reboot.
> Only the first 3 disks are checked, with the assumption that the 4th IDE
> disk is an emulated DVD-ROM.

I don't think we need make that assumption: __AdapterHiddenTarget() will return 
TRUE for anything that is not a disk, so no target
will be created for a CDROM regardless of which slot it is in.
I think it would be much cleaner if we could create all targets but prevent 
them from transitioning to D0 if an emulated device
exists (and request the reboot at that point).

  Paul

> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/xenvbd/adapter.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/src/xenvbd/adapter.c b/src/xenvbd/adapter.c
> index 5e20944..15c4f5a 100644
> --- a/src/xenvbd/adapter.c
> +++ b/src/xenvbd/adapter.c
> @@ -947,6 +947,14 @@ AdapterD3ToD0(
>      if (!NT_SUCCESS(status))
>          goto fail4;
> 
> +    for (TargetId = 0; TargetId < 3; ++TargetId) {
> +        // Check the IDE (1st 4 targets) or NVME (1st target) devices are 
> not emulated
> +        // skip TargetId 3 as its used for the emulated DVD-ROM
> +        status = STATUS_UNSUCCESSFUL;
> +        if (AdapterIsTargetEmulated(Adapter, TargetId))
> +            goto fail5;
> +    }
> +
>      for (TargetId = 0; TargetId < XENVBD_MAX_TARGETS; ++TargetId) {
>          PXENVBD_TARGET Target = AdapterGetTarget(Adapter, TargetId);
>          if (Target == NULL)
> @@ -955,7 +963,7 @@ AdapterD3ToD0(
>          status = TargetD3ToD0(Target);
> 
>          if (!NT_SUCCESS(status))
> -            goto fail5;
> +            goto fail6;
>      }
> 
>      status = XENBUS_SUSPEND(Register,
> @@ -965,14 +973,14 @@ AdapterD3ToD0(
>                              Adapter,
>                              &Adapter->SuspendCallback);
>      if (!NT_SUCCESS(status))
> -        goto fail6;
> +        goto fail7;
> 
>      return STATUS_SUCCESS;
> 
> +fail7:
> +    Error("fail7\n");
>  fail6:
>      Error("fail6\n");
> -fail5:
> -    Error("fail5\n");
> 
>      for (TargetId = 0; TargetId < XENVBD_MAX_TARGETS; ++TargetId) {
>          PXENVBD_TARGET Target = AdapterGetTarget(Adapter, TargetId);
> @@ -981,6 +989,9 @@ fail5:
>          TargetD0ToD3(Target);
>      }
> 
> +fail5:
> +    Error("fail5\n");
> +
>      __AdapterD0ToD3(Adapter);
> 
>  fail4:
> @@ -2178,7 +2189,8 @@ AdapterHwPassiveInitialize(
>  fail3:
>      Error("fail2\n");
> 
> -    AdapterUnplugRequest(Adapter, FALSE);
> +    if (status != STATUS_UNSUCCESSFUL)
> +        AdapterUnplugRequest(Adapter, FALSE);
>      AdapterTeardown(Adapter);
> 
>  fail2:
> --
> 2.16.2.windows.1
>