Re: [PATCH 2/4] XenAgent: Advertise all features in 1 location

[Owen Smith <owen.smith@xxxxxxxxxx>]

This is primarily to clean up the code - I will resubmit with a better comment

Owen

On Thu, Feb 1, 2024 at 5:49 PM Paul Durrant <xadimgnik@xxxxxxxxx> wrote:

On 31/01/2024 08:12, Owen Smith wrote:> Signed-off-by: Owen Smith
<owen.smith@xxxxxxxxx>

Can we have *some* text to justify the change?

   Paul

> ---
>   src/xenagent/xenifacedevice.cpp | 32 +++++++++++++++++++++-----------
>   src/xenagent/xenifacedevice.h   |  1 +
>   2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/src/xenagent/xenifacedevice.cpp b/src/xenagent/xenifacedevice.cpp
> index 69a584b..9ed1aa7 100644
> --- a/src/xenagent/xenifacedevice.cpp
> +++ b/src/xenagent/xenifacedevice.cpp
> @@ -213,6 +213,8 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) : CDeviceList(GUID_IN
>       if (m_agent->ConvDevicePresent())
>           StartSlateModeWatch(device);
>   
> +    AdvertiseFeatures(device, true);
> +
>       SetXenTime(device);
>   }
>   
> @@ -228,6 +230,8 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) : CDeviceList(GUID_IN
>           device->SuspendDeregister(m_ctxt_suspend);
>       m_ctxt_suspend = NULL;
>   
> +    AdvertiseFeatures(device, false);
> +
>       if (m_agent->ConvDevicePresent())
>           StopSlateModeWatch(device);
>   
> @@ -242,6 +246,8 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) : CDeviceList(GUID_IN
>       if (dev != GetFirstDevice())
>           return;
>   
> +    AdvertiseFeatures(device, false);
> +
>       if (m_agent->ConvDevicePresent())
>           StopSlateModeWatch(device);
>   
> @@ -260,6 +266,8 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) : CDeviceList(GUID_IN
>   
>       if (m_agent->ConvDevicePresent())
>           StartSlateModeWatch(device);
> +
> +    AdvertiseFeatures(device, true);
>   }
>   
>   void CXenIfaceDeviceList::Log(const char* message)
> @@ -381,6 +389,8 @@ void CXenIfaceDeviceList::CheckSuspend()
>       if (m_agent->ConvDevicePresent())
>           StartSlateModeWatch(device);
>   
> +    AdvertiseFeatures(device, true);
> +
>       m_count = count;
>   }
>   
> @@ -425,11 +435,6 @@ void CXenIfaceDeviceList::StartShutdownWatch(CXenIfaceDevice* device)
>           return;
>   
>       device->StoreAddWatch("control/shutdown", m_evt_shutdown, &m_ctxt_shutdown);
> -
> -    device->StoreWrite("control/feature-poweroff", "1");
> -    device->StoreWrite("control/feature-reboot", "1");
> -    device->StoreWrite("control/feature-s3", "1");
> -    device->StoreWrite("control/feature-s4", "1");
>   }
>   
>   void CXenIfaceDeviceList::StopShutdownWatch(CXenIfaceDevice* device)
> @@ -437,11 +442,6 @@ void CXenIfaceDeviceList::StopShutdownWatch(CXenIfaceDevice* device)
>       if (!m_ctxt_shutdown)
>           return;
>   
> -    device->StoreWrite("control/feature-poweroff", "");
> -    device->StoreWrite("control/feature-reboot", "");
> -    device->StoreWrite("control/feature-s3", "");
> -    device->StoreWrite("control/feature-s4", "");
> -
>       device->StoreRemoveWatch(m_ctxt_shutdown);
>       m_ctxt_shutdown = NULL;
>   }
> @@ -452,7 +452,6 @@ void CXenIfaceDeviceList::StartSlateModeWatch(CXenIfaceDevice* device)
>           return;
>   
>       device->StoreAddWatch("control/laptop-slate-mode", m_evt_slate_mode, &m_ctxt_slate_mode);
> -    device->StoreWrite("control/feature-laptop-slate-mode", "1");
>   }
>   
>   void CXenIfaceDeviceList::StopSlateModeWatch(CXenIfaceDevice* device)
> @@ -466,6 +465,17 @@ void CXenIfaceDeviceList::StopSlateModeWatch(CXenIfaceDevice* device)
>       m_ctxt_slate_mode = NULL;
>   }
>   
> +void CXenIfaceDeviceList::AdvertiseFeatures(CXenIfaceDevice* device, bool add)
> +{
> +    const char* value = add ? "1" : "";
> +
> +    device->StoreWrite("control/feature-poweroff", value);
> +    device->StoreWrite("control/feature-reboot", value);
> +    device->StoreWrite("control/feature-s3", value);
> +    device->StoreWrite("control/feature-s4", value);
> +    device->StoreWrite("control/feature-laptop-slate-mode", value);
> +}
> +
>   void CXenIfaceDeviceList::AcquireShutdownPrivilege()
>   {
>       HANDLE          token;
> diff --git a/src/xenagent/xenifacedevice.h b/src/xenagent/xenifacedevice.h
> index d85e469..29ac100 100644
> --- a/src/xenagent/xenifacedevice.h
> +++ b/src/xenagent/xenifacedevice.h
> @@ -94,6 +94,7 @@ private:
>       void StopShutdownWatch(CXenIfaceDevice* device);
>       void StartSlateModeWatch(CXenIfaceDevice* device);
>       void StopSlateModeWatch(CXenIfaceDevice* device);
> +    void AdvertiseFeatures(CXenIfaceDevice* device, bool add);
>       void AcquireShutdownPrivilege();
>       void SetXenTime(CXenIfaceDevice* device);
>