[v3] QGA - Win fixes

[PATCH v3 1/3] qga: Installer: Wait for installation to finish

Posted by Basil Salman 5 years, 11 months ago

Installation might fail if we don't wait for the provider
unregisteration process to finish.

Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
---
 qga/installer/qemu-ga.wxs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 64bf90bd85..f6781752e6 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -81,7 +81,7 @@
               Arguments="-d --retry-path"
               >
             </ServiceInstall>
-            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
+            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" />
           </Component>
           <?ifdef var.InstallVss?>
           <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
-- 
2.17.2

Re: [PATCH v3 1/3] qga: Installer: Wait for installation to finish

Posted by Philippe Mathieu-Daudé 5 years, 10 months ago

Hi Basil,

On 3/11/20 6:04 PM, Basil Salman wrote:
> Installation might fail if we don't wait for the provider
> unregisteration process to finish.
> 
> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
> Signed-off-by: Basil Salman <basil@daynix.com>
> ---
>   qga/installer/qemu-ga.wxs | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 64bf90bd85..f6781752e6 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -81,7 +81,7 @@
>                 Arguments="-d --retry-path"
>                 >
>               </ServiceInstall>
> -            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
> +            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" />

As 'yes' is the default, can you simply remove 'Wait="no"'?

>             </Component>
>             <?ifdef var.InstallVss?>
>             <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
>

Re: [PATCH v3 1/3] qga: Installer: Wait for installation to finish

Posted by Marc-André Lureau 5 years, 10 months ago

Hi

On Tue, Mar 24, 2020 at 2:13 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Basil,
>
> On 3/11/20 6:04 PM, Basil Salman wrote:
> > Installation might fail if we don't wait for the provider
> > unregisteration process to finish.
> >
> > Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
> > Signed-off-by: Basil Salman <basil@daynix.com>
> > ---
> >   qga/installer/qemu-ga.wxs | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> > index 64bf90bd85..f6781752e6 100644
> > --- a/qga/installer/qemu-ga.wxs
> > +++ b/qga/installer/qemu-ga.wxs
> > @@ -81,7 +81,7 @@
> >                 Arguments="-d --retry-path"
> >                 >
> >               </ServiceInstall>
> > -            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
> > +            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" />
>
> As 'yes' is the default, can you simply remove 'Wait="no"'?

It looks like wixl doesn't follow the WiX default value though.

tools/wixl/msi.vala:
574                 (Wait != null && !rec.set_int (5, Wait ? 1 : 0)) ||

>
> >             </Component>
> >             <?ifdef var.InstallVss?>
> >             <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
> >
>
>


-- 
Marc-André Lureau

Re: [PATCH v3 1/3] qga: Installer: Wait for installation to finish

Posted by Philippe Mathieu-Daudé 5 years, 10 months ago

On 3/24/20 3:00 PM, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Mar 24, 2020 at 2:13 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Hi Basil,
>>
>> On 3/11/20 6:04 PM, Basil Salman wrote:
>>> Installation might fail if we don't wait for the provider
>>> unregisteration process to finish.
>>>
>>> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
>>> Signed-off-by: Basil Salman <basil@daynix.com>
>>> ---
>>>    qga/installer/qemu-ga.wxs | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
>>> index 64bf90bd85..f6781752e6 100644
>>> --- a/qga/installer/qemu-ga.wxs
>>> +++ b/qga/installer/qemu-ga.wxs
>>> @@ -81,7 +81,7 @@
>>>                  Arguments="-d --retry-path"
>>>                  >
>>>                </ServiceInstall>
>>> -            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
>>> +            <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" />
>>
>> As 'yes' is the default, can you simply remove 'Wait="no"'?
> 
> It looks like wixl doesn't follow the WiX default value though.
> 
> tools/wixl/msi.vala:
> 574                 (Wait != null && !rec.set_int (5, Wait ? 1 : 0)) ||

I trusted the documentation...
https://wixtoolset.org/documentation/manual/v3/xsd/wix/servicecontrol.html

Thanks for looking at the implementation.

So for this patch:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
>>
>>>              </Component>
>>>              <?ifdef var.InstallVss?>
>>>              <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
>>>
>>
>>
> 
>