[Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[Michael Roth]

The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:

  s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)

are available in the git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2015-09-01-tag

for you to fetch changes up to 9680171bcd317927c0cbaaa291dd9072e79fb32c:

  Makefile: qemu-ga: fix msi target error message (2015-09-01 11:40:35 -0500)

----------------------------------------------------------------
qemu-ga patch queue

* add config file dump/load support for qemu-ga
* various w32 build fixes, particularly WRT to msi package creation
* fixes for msi installer
* w32 support for guest-set-user-password

----------------------------------------------------------------
Leonid Bloch (7):
      qemu-ga: Add .msi files to .gitignore
      qemu-ga: Two MSI related cosmetic changes
      qemu-ga: Fixed GUID capitalization
      qemu-ga: Minor cosmetic changes to the WXS file
      qemu-ga: Created a separate component for each installed file in the MSI
      qemu-ga: Prevent QEMU-GA VSS provider from being unregistered on MSI reinstall
      qemu-ga: Fixed paths issue with MSI build

Marc-André Lureau (14):
      qga: misc spelling
      qga: use exit() when parsing options
      qga: move string split in separate function
      qga: make split_list() return allocated strings
      qga: rename 'path' to 'channel_path'
      qga: copy argument strings
      qga: move option parsing to separate function
      qga: fill default options in main()
      qga: move agent run in a separate function
      qga: free a bit more
      qga: add an optional qemu-ga.conf system configuration
      qga: add --dump-conf option
      qga: start a man page
      qemu-ga: implement win32 guest-set-user-password

Michael Roth (5):
      configure: qemu-ga: report MSI install support in summary
      configure: qemu-ga: move MSI installer probe after qga probe
      configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
      build: qemu-ga: fix VSS dependencies
      Makefile: qemu-ga: fix msi target error message

 .gitignore                |   1 +
 Makefile                  |  33 +++--
 configure                 | 122 ++++++++-------
 qemu-doc.texi             |   6 +
 qemu-ga.texi              | 137 +++++++++++++++++
 qga/commands-posix.c      |   6 +-
 qga/commands-win32.c      |  81 +++++++++-
 qga/installer/qemu-ga.wxs |  78 ++++++----
 qga/main.c                | 466 +++++++++++++++++++++++++++++++++++++++++-----------------
 qga/qapi-schema.json      |   2 +-
 10 files changed, 689 insertions(+), 243 deletions(-)
 create mode 100644 qemu-ga.texi

[Qemu-devel] [PATCH 01/26] qemu-ga: Add .msi files to .gitignore

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 61bc492..cb4b8ec 100644
--- a/.gitignore
+++ b/.gitignore
@@ -58,6 +58,7 @@
 *.cp
 *.dvi
 *.exe
+*.msi
 *.dll
 *.so
 *.mo
-- 
1.9.1

[Qemu-devel] [PATCH 02/26] qemu-ga: Two MSI related cosmetic changes

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile                  | 2 +-
 qga/installer/qemu-ga.wxs | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 340d9c8..9a4c3c6 100644
--- a/Makefile
+++ b/Makefile
@@ -295,7 +295,7 @@ qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
 
-msi: ${QEMU_GA_MSI}
+msi: $(QEMU_GA_MSI)
 
 $(QEMU_GA_MSI): qemu-ga.exe
 
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 2c43f1b..4e917a9 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -126,7 +126,8 @@
               Property="cmd"
               Impersonate="no"
               Return="check"
-              ></CustomAction>
+              >
+    </CustomAction>
     <?endif?>
 
     <Feature Id="QEMUFeature" Title="QEMU Guest Agent" Level="1">
-- 
1.9.1

[Qemu-devel] [PATCH 03/26] qemu-ga: Fixed GUID capitalization

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

For compatibility, all the letters in GUID should be capital.

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/installer/qemu-ga.wxs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 4e917a9..dcd3e36 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -42,7 +42,7 @@
   <Product
     Name="QEMU guest agent"
     Id="*"
-    UpgradeCode="{EB6B8302-C06E-4bec-ADAC-932C68A3A98D}"
+    UpgradeCode="{EB6B8302-C06E-4BEC-ADAC-932C68A3A98D}"
     Manufacturer="$(env.QEMU_GA_MANUFACTURER)"
     Version="$(env.QEMU_GA_VERSION)"
     Language="1033">
@@ -69,7 +69,7 @@
     <Directory Id="TARGETDIR" Name="SourceDir">
       <Directory Id="$(var.GaProgramFilesFolder)" Name="QEMU Guest Agent">
         <Directory Id="qemu_ga_directory" Name="Qemu-ga">
-          <Component Id="qemu_ga" Guid="{908B7199-DE2A-4dc6-A8D0-27A5AE444FEA}">
+          <Component Id="qemu_ga" Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
             <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="../../qemu-ga.exe" KeyPath="yes" DiskId="1"/>
             <?ifdef var.InstallVss ?>
             <File Id="qga_vss.dll" Name="qga-vss.dll" Source="../vss-win32/qga-vss.dll" KeyPath="no" DiskId="1"/>
@@ -98,7 +98,7 @@
             <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
           </Component>
 
-          <Component Id="registry_entries" Guid="d075d109-51ca-11e3-9f8b-000c29858960">
+          <Component Id="registry_entries" Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
             <RegistryKey Root="HKLM"
                          Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
               <RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
-- 
1.9.1

[Qemu-devel] [PATCH 04/26] qemu-ga: Minor cosmetic changes to the WXS file

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/installer/qemu-ga.wxs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index dcd3e36..a3396d7 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -111,9 +111,9 @@
 
     <Property Id="cmd" Value="cmd.exe"/>
 
-    <?ifdef var.InstallVss ?>
+    <?ifdef var.InstallVss?>
     <CustomAction Id="RegisterCom"
-             ExeCommand='/c "[qemu_ga_directory]qemu-ga.exe" -s vss-install'
+              ExeCommand='/c "[qemu_ga_directory]qemu-ga.exe" -s vss-install'
               Execute="deferred"
               Property="cmd"
               Impersonate="no"
@@ -137,7 +137,7 @@
 
     <InstallExecuteSequence>
       <RemoveExistingProducts Before="InstallInitialize" />
-      <?ifdef var.InstallVss ?>
+      <?ifdef var.InstallVss?>
       <Custom Action="RegisterCom" After="InstallServices">NOT Installed</Custom>
       <Custom Action="UnRegisterCom" After="StopServices">Installed</Custom>
       <?endif?>
-- 
1.9.1

[Qemu-devel] [PATCH 05/26] qemu-ga: Created a separate component for each installed file in the MSI

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

This is done to follow the recommendations given here: https://msdn.microsoft.com/en-us/library/aa368269%28VS.85%29.aspx

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/installer/qemu-ga.wxs | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index a3396d7..b32064e 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -71,16 +71,6 @@
         <Directory Id="qemu_ga_directory" Name="Qemu-ga">
           <Component Id="qemu_ga" Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
             <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="../../qemu-ga.exe" KeyPath="yes" DiskId="1"/>
-            <?ifdef var.InstallVss ?>
-            <File Id="qga_vss.dll" Name="qga-vss.dll" Source="../vss-win32/qga-vss.dll" KeyPath="no" DiskId="1"/>
-            <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="../vss-win32/qga-vss.tlb" KeyPath="no" DiskId="1"/>
-            <?endif?>
-            <File Id="iconv.dll" Name="iconv.dll" Source="$(var.Mingw_bin)/iconv.dll" KeyPath="no" DiskId="1"/>
-            <File Id="libgcc_arch_lib" Name="$(var.ArchLib)" Source="$(var.Mingw_bin)/$(var.ArchLib)" KeyPath="no" DiskId="1"/>
-            <File Id="libglib_2.0_0.dll" Name="libglib-2.0-0.dll" Source="$(var.Mingw_bin)/libglib-2.0-0.dll" KeyPath="no" DiskId="1"/>
-            <File Id="libintl_8.dll" Name="libintl-8.dll" Source="$(var.Mingw_bin)/libintl-8.dll" KeyPath="no" DiskId="1"/>
-            <File Id="libssp_0.dll" Name="libssp-0.dll" Source="$(var.Mingw_bin)/libssp-0.dll" KeyPath="no" DiskId="1"/>
-            <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll" Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="no" DiskId="1"/>
             <ServiceInstall
               Id="ServiceInstaller"
               Type="ownProcess"
@@ -97,7 +87,32 @@
             </ServiceInstall>
             <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
           </Component>
-
+          <?ifdef var.InstallVss?>
+          <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
+            <File Id="qga_vss.dll" Name="qga-vss.dll" Source="../vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="qga_vss_tlb" Guid="{D8D584B1-59C2-4FB7-A91F-636FF7BFA66E}">
+            <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="../vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <?endif?>
+          <Component Id="iconv" Guid="{35EE3558-D34B-4F0A-B8BD-430FF0775246}">
+            <File Id="iconv.dll" Name="iconv.dll" Source="$(var.Mingw_bin)/iconv.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="libgcc_arch_lib" Guid="{ADD4D07D-4515-4AB6-AF3E-C904961B4BB0}">
+            <File Id="libgcc_arch_lib" Name="$(var.ArchLib)" Source="$(var.Mingw_bin)/$(var.ArchLib)" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="libglib" Guid="{D31BFD83-2773-4B65-B45A-E0D2ADA58679}">
+            <File Id="libglib_2.0_0.dll" Name="libglib-2.0-0.dll" Source="$(var.Mingw_bin)/libglib-2.0-0.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="libintl" Guid="{A641BC2D-A907-4A94-9149-F30ED430878F}">
+            <File Id="libintl_8.dll" Name="libintl-8.dll" Source="$(var.Mingw_bin)/libintl-8.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="libssp" Guid="{7880087B-02B4-4EF6-A5D3-D18F8E3D90E1}">
+            <File Id="libssp_0.dll" Name="libssp-0.dll" Source="$(var.Mingw_bin)/libssp-0.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
+          <Component Id="libwinpthread" Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
+            <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll" Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
           <Component Id="registry_entries" Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
             <RegistryKey Root="HKLM"
                          Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
@@ -132,6 +147,16 @@
 
     <Feature Id="QEMUFeature" Title="QEMU Guest Agent" Level="1">
       <ComponentRef Id="qemu_ga" />
+      <?ifdef var.InstallVss?>
+      <ComponentRef Id="qga_vss_dll" />
+      <ComponentRef Id="qga_vss_tlb" />
+      <?endif?>
+      <ComponentRef Id="iconv" />
+      <ComponentRef Id="libgcc_arch_lib" />
+      <ComponentRef Id="libglib" />
+      <ComponentRef Id="libintl" />
+      <ComponentRef Id="libssp" />
+      <ComponentRef Id="libwinpthread" />
       <ComponentRef Id="registry_entries" />
     </Feature>
 
-- 
1.9.1

[Qemu-devel] [PATCH 06/26] qemu-ga: Prevent QEMU-GA VSS provider from being unregistered on MSI reinstall

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

Previously, running the .msi would unregister the QEMU GA VSS service if QEMU GA was already installed on the machine, and then register it only if QEMU GA was NOT previously installed. This behavior caused the service to be registered only after the INITIAL installation, and any subsequent run of the .msi (to redo, repair, or upgrade the installation) ended in the service being unregistered.

Now, the VSS service is still unregistered if QEMU GA is already installed (so that a fix or an update could be performed) but then it is registered again (if the GA is not being uninstalled) thus finishing the repair/upgrade correctly. Additionally, downgrading is now prevented. If a user would like to downgrade a version, he/she must uninstall the newer version first.

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/installer/qemu-ga.wxs | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index b32064e..c195b4b 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -58,13 +58,9 @@
       />
     <Media Id="1" Cabinet="qemu_ga.$(env.QEMU_GA_VERSION).cab" EmbedCab="yes" />
     <Property Id="WHSLogo">1</Property>
-    <Property Id="PREVIOUSVERSIONSINSTALLED" />
-    <Upgrade Id="{EB6B8302-C06E-4bec-ADAC-932C68A3A98D}">
-      <UpgradeVersion
-        Minimum="1.0.0.0" Maximum="$(env.QEMU_GA_VERSION)"
-        Property="PREVIOUSVERSIONSINSTALLED"
-        IncludeMinimum="yes" IncludeMaximum="no" />
-    </Upgrade>
+    <MajorUpgrade
+      DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
+      />
 
     <Directory Id="TARGETDIR" Name="SourceDir">
       <Directory Id="$(var.GaProgramFilesFolder)" Name="QEMU Guest Agent">
@@ -125,6 +121,7 @@
     </Directory>
 
     <Property Id="cmd" Value="cmd.exe"/>
+    <Property Id="REINSTALLMODE" Value="amus"/>
 
     <?ifdef var.InstallVss?>
     <CustomAction Id="RegisterCom"
@@ -161,10 +158,9 @@
     </Feature>
 
     <InstallExecuteSequence>
-      <RemoveExistingProducts Before="InstallInitialize" />
       <?ifdef var.InstallVss?>
-      <Custom Action="RegisterCom" After="InstallServices">NOT Installed</Custom>
       <Custom Action="UnRegisterCom" After="StopServices">Installed</Custom>
+      <Custom Action="RegisterCom" After="InstallServices">NOT REMOVE</Custom>
       <?endif?>
     </InstallExecuteSequence>
   </Product>
-- 
1.9.1

[Qemu-devel] [PATCH 07/26] qemu-ga: Fixed paths issue with MSI build

[Michael Roth]

From: Leonid Bloch <leonid@daynix.com>

Previously, if building out-of-tree, the MSI build would fail since
it wasn't able to find the needed files.

Signed-off-by: Leonid Bloch <leonid@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
* fixed up commit msg formating
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile                  | 4 ++--
 qga/installer/qemu-ga.wxs | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9a4c3c6..f98a554 100644
--- a/Makefile
+++ b/Makefile
@@ -305,8 +305,8 @@ endif
 
 $(QEMU_GA_MSI): config-host.mak
 
-$(QEMU_GA_MSI):  qga/installer/qemu-ga.wxs
-	$(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" \
+$(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
+	$(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" BUILD_DIR="$(BUILD_DIR)" \
 	wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) $(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
 else
 msi:
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index c195b4b..6804f02 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -66,7 +66,7 @@
       <Directory Id="$(var.GaProgramFilesFolder)" Name="QEMU Guest Agent">
         <Directory Id="qemu_ga_directory" Name="Qemu-ga">
           <Component Id="qemu_ga" Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
-            <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="../../qemu-ga.exe" KeyPath="yes" DiskId="1"/>
+            <File Id="qemu_ga.exe" Name="qemu-ga.exe" Source="$(env.BUILD_DIR)/qemu-ga.exe" KeyPath="yes" DiskId="1"/>
             <ServiceInstall
               Id="ServiceInstaller"
               Type="ownProcess"
@@ -85,10 +85,10 @@
           </Component>
           <?ifdef var.InstallVss?>
           <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
-            <File Id="qga_vss.dll" Name="qga-vss.dll" Source="../vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
+            <File Id="qga_vss.dll" Name="qga-vss.dll" Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
           </Component>
           <Component Id="qga_vss_tlb" Guid="{D8D584B1-59C2-4FB7-A91F-636FF7BFA66E}">
-            <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="../vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
+            <File Id="qga_vss.tlb" Name="qga-vss.tlb" Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
           </Component>
           <?endif?>
           <Component Id="iconv" Guid="{35EE3558-D34B-4F0A-B8BD-430FF0775246}">
-- 
1.9.1

[Qemu-devel] [PATCH 08/26] configure: qemu-ga: report MSI install support in summary

[Michael Roth]

Currently we need to examine config-host.mak to determine whether
options/probes for MSI package generation had desired result. Report
this more prominently in ./configure summary as we do with other
guest agent configure options.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 9d24d59..86a38fe 100755
--- a/configure
+++ b/configure
@@ -4577,6 +4577,7 @@ echo "libnfs support    $libnfs"
 echo "build guest agent $guest_agent"
 echo "QGA VSS support   $guest_agent_with_vss"
 echo "QGA w32 disk info $guest_agent_ntddscsi"
+echo "QGA MSI support   $guest_agent_msi"
 echo "seccomp support   $seccomp"
 echo "coroutine backend $coroutine"
 echo "coroutine pool    $coroutine_pool"
-- 
1.9.1

[Qemu-devel] [PATCH 09/26] qga: misc spelling

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/qapi-schema.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 18e3cc3..6b0bd16 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -793,7 +793,7 @@
 # scheme. Refer to the documentation of the guest operating system
 # in question to determine what is supported.
 #
-# Note all guest operating systems will support use of the
+# Not all guest operating systems will support use of the
 # @crypted flag, as they may require the clear-text password
 #
 # The @password parameter must always be base64 encoded before
-- 
1.9.1

[Qemu-devel] [PATCH 10/26] qga: use exit() when parsing options

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The option parsing is going to be moved to a separate function,
use exit() consistently.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 791982e..10bb2f7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -992,14 +992,14 @@ int main(int argc, char **argv)
             break;
         case 'V':
             printf("QEMU Guest Agent %s\n", QEMU_VERSION);
-            return 0;
+            exit(EXIT_SUCCESS);
         case 'd':
             daemonize = 1;
             break;
         case 'b': {
             if (is_help_option(optarg)) {
                 qmp_for_each_command(ga_print_cmd, NULL);
-                return 0;
+                exit(EXIT_SUCCESS);
             }
             for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
                 if (optarg[i] == ',') {
@@ -1027,36 +1027,36 @@ int main(int argc, char **argv)
                                   NULL :
                                   state_dir;
                 if (ga_install_vss_provider()) {
-                    return EXIT_FAILURE;
+                    exit(EXIT_FAILURE);
                 }
                 if (ga_install_service(path, log_filepath, fixed_state_dir)) {
-                    return EXIT_FAILURE;
+                    exit(EXIT_FAILURE);
                 }
-                return 0;
+                exit(EXIT_SUCCESS);
             } else if (strcmp(service, "uninstall") == 0) {
                 ga_uninstall_vss_provider();
-                return ga_uninstall_service();
+                exit(ga_uninstall_service());
             } else if (strcmp(service, "vss-install") == 0) {
                 if (ga_install_vss_provider()) {
-                    return EXIT_FAILURE;
+                    exit(EXIT_FAILURE);
                 }
-                return EXIT_SUCCESS;
+                exit(EXIT_SUCCESS);
             } else if (strcmp(service, "vss-uninstall") == 0) {
                 ga_uninstall_vss_provider();
-                return EXIT_SUCCESS;
+                exit(EXIT_SUCCESS);
             } else {
                 printf("Unknown service command.\n");
-                return EXIT_FAILURE;
+                exit(EXIT_FAILURE);
             }
             break;
 #endif
         case 'h':
             usage(argv[0]);
-            return 0;
+            exit(EXIT_SUCCESS);
         case '?':
             g_print("Unknown option, try '%s --help' for more information.\n",
                     argv[0]);
-            return EXIT_FAILURE;
+            exit(EXIT_FAILURE);
         }
     }
 
-- 
1.9.1

[Qemu-devel] [PATCH 11/26] qga: move string split in separate function

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The function is going to be reused in a later patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 10bb2f7..e75022c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -921,6 +921,26 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
     printf("%s\n", qmp_command_name(cmd));
 }
 
+static GList *split_list(gchar *str, const gchar separator)
+{
+    GList *list = NULL;
+    int i, j, len;
+
+    for (j = 0, i = 0, len = strlen(str); i < len; i++) {
+        if (str[i] == separator) {
+            str[i] = 0;
+            list = g_list_append(list, &str[j]);
+            j = i + 1;
+        }
+    }
+
+    if (j < i) {
+        list = g_list_append(list, &str[j]);
+    }
+
+    return list;
+}
+
 int main(int argc, char **argv)
 {
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
@@ -953,7 +973,7 @@ int main(int argc, char **argv)
         { "statedir", 1, NULL, 't' },
         { NULL, 0, NULL, 0 }
     };
-    int opt_ind = 0, ch, daemonize = 0, i, j, len;
+    int opt_ind = 0, ch, daemonize = 0;
     GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
     GList *blacklist = NULL;
     GAState *s;
@@ -1001,16 +1021,7 @@ int main(int argc, char **argv)
                 qmp_for_each_command(ga_print_cmd, NULL);
                 exit(EXIT_SUCCESS);
             }
-            for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
-                if (optarg[i] == ',') {
-                    optarg[i] = 0;
-                    blacklist = g_list_append(blacklist, &optarg[j]);
-                    j = i + 1;
-                }
-            }
-            if (j < i) {
-                blacklist = g_list_append(blacklist, &optarg[j]);
-            }
+            blacklist = g_list_concat(blacklist, split_list(optarg, ','));
             break;
         }
 #ifdef _WIN32
-- 
1.9.1

[Qemu-devel] [PATCH 12/26] qga: make split_list() return allocated strings

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

In order to avoid any confusion, let's allocate new strings when
splitting.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c |  6 +++---
 qga/commands-win32.c |  4 ++--
 qga/main.c           | 22 +++++++++-------------
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 675f4b4..fc4fc72 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2454,7 +2454,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
         char **p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, *p++);
+            blacklist = g_list_append(blacklist, g_strdup(*p++));
         }
     }
 #endif
@@ -2468,13 +2468,13 @@ GList *ga_command_blacklist_init(GList *blacklist)
         char **p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, *p++);
+            blacklist = g_list_append(blacklist, g_strdup(*p++));
         }
     }
 #endif
 
 #if !defined(CONFIG_FSTRIM)
-    blacklist = g_list_append(blacklist, (char *)"guest-fstrim");
+    blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
 #endif
 
     return blacklist;
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index a7822d5..1152c46 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1233,7 +1233,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
     char **p = (char **)list_unsupported;
 
     while (*p) {
-        blacklist = g_list_append(blacklist, *p++);
+        blacklist = g_list_append(blacklist, g_strdup(*p++));
     }
 
     if (!vss_init(true)) {
@@ -1244,7 +1244,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
         p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, *p++);
+            blacklist = g_list_append(blacklist, g_strdup(*p++));
         }
     }
 
diff --git a/qga/main.c b/qga/main.c
index e75022c..a7df6c8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -921,22 +921,17 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
     printf("%s\n", qmp_command_name(cmd));
 }
 
-static GList *split_list(gchar *str, const gchar separator)
+static GList *split_list(const gchar *str, const gchar *delim)
 {
     GList *list = NULL;
-    int i, j, len;
+    int i;
+    gchar **strv;
 
-    for (j = 0, i = 0, len = strlen(str); i < len; i++) {
-        if (str[i] == separator) {
-            str[i] = 0;
-            list = g_list_append(list, &str[j]);
-            j = i + 1;
-        }
-    }
-
-    if (j < i) {
-        list = g_list_append(list, &str[j]);
+    strv = g_strsplit(str, delim, -1);
+    for (i = 0; strv[i]; i++) {
+        list = g_list_prepend(list, strv[i]);
     }
+    g_free(strv);
 
     return list;
 }
@@ -1021,7 +1016,7 @@ int main(int argc, char **argv)
                 qmp_for_each_command(ga_print_cmd, NULL);
                 exit(EXIT_SUCCESS);
             }
-            blacklist = g_list_concat(blacklist, split_list(optarg, ','));
+            blacklist = g_list_concat(blacklist, split_list(optarg, ","));
             break;
         }
 #ifdef _WIN32
@@ -1201,6 +1196,7 @@ int main(int argc, char **argv)
     }
 #endif
 
+    g_list_free_full(ga_state->blacklist, g_free);
     ga_command_state_cleanup_all(ga_state->command_state);
     ga_channel_free(ga_state->channel);
 
-- 
1.9.1

[Qemu-devel] [PATCH 13/26] qga: rename 'path' to 'channel_path'

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

'path' is already a global function, rename the variable since it's
going to be in global scope in a later patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index a7df6c8..7fa6dcc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -939,7 +939,7 @@ static GList *split_list(const gchar *str, const gchar *delim)
 int main(int argc, char **argv)
 {
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-    const char *method = NULL, *path = NULL;
+    const char *method = NULL, *channel_path = NULL;
     const char *log_filepath = NULL;
     const char *pid_filepath;
 #ifdef CONFIG_FSFREEZE
@@ -985,7 +985,7 @@ int main(int argc, char **argv)
             method = optarg;
             break;
         case 'p':
-            path = optarg;
+            channel_path = optarg;
             break;
         case 'l':
             log_filepath = optarg;
@@ -1035,7 +1035,8 @@ int main(int argc, char **argv)
                 if (ga_install_vss_provider()) {
                     exit(EXIT_FAILURE);
                 }
-                if (ga_install_service(path, log_filepath, fixed_state_dir)) {
+                if (ga_install_service(channel_path, log_filepath,
+                                       fixed_state_dir)) {
                     exit(EXIT_FAILURE);
                 }
                 exit(EXIT_SUCCESS);
@@ -1180,7 +1181,7 @@ int main(int argc, char **argv)
 #endif
 
     s->main_loop = g_main_loop_new(NULL, false);
-    if (!channel_init(ga_state, method, path)) {
+    if (!channel_init(ga_state, method, channel_path)) {
         g_critical("failed to initialize guest agent channel");
         goto out_bad;
     }
-- 
1.9.1

[Qemu-devel] [PATCH 14/26] qga: copy argument strings

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Following patch will return allocated strings, so we must correctly
initialize alloc & free them. The nice side effect is that we no longer
have to check for "fixed_state_dir" to call ga_install_service() with a
NULL state dir. The default values are set after parsing the command
line options.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 57 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 7fa6dcc..766cb93 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -939,13 +939,13 @@ static GList *split_list(const gchar *str, const gchar *delim)
 int main(int argc, char **argv)
 {
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-    const char *method = NULL, *channel_path = NULL;
-    const char *log_filepath = NULL;
-    const char *pid_filepath;
+    char *method = NULL, *channel_path = NULL;
+    char *log_filepath = NULL;
+    char *pid_filepath = NULL;
 #ifdef CONFIG_FSFREEZE
-    const char *fsfreeze_hook = NULL;
+    char *fsfreeze_hook = NULL;
 #endif
-    const char *state_dir;
+    char *state_dir = NULL;
 #ifdef _WIN32
     const char *service = NULL;
 #endif
@@ -976,31 +976,28 @@ int main(int argc, char **argv)
     module_call_init(MODULE_INIT_QAPI);
 
     init_dfl_pathnames();
-    pid_filepath = dfl_pathnames.pidfile;
-    state_dir = dfl_pathnames.state_dir;
-
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
-            method = optarg;
+            method = g_strdup(optarg);
             break;
         case 'p':
-            channel_path = optarg;
+            channel_path = g_strdup(optarg);
             break;
         case 'l':
-            log_filepath = optarg;
+            log_filepath = g_strdup(optarg);
             break;
         case 'f':
-            pid_filepath = optarg;
+            pid_filepath = g_strdup(optarg);
             break;
 #ifdef CONFIG_FSFREEZE
         case 'F':
-            fsfreeze_hook = optarg ? optarg : QGA_FSFREEZE_HOOK_DEFAULT;
+            fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
             break;
 #endif
         case 't':
-             state_dir = optarg;
-             break;
+            state_dir = g_strdup(optarg);
+            break;
         case 'v':
             /* enable all log levels */
             log_level = G_LOG_LEVEL_MASK;
@@ -1023,20 +1020,10 @@ int main(int argc, char **argv)
         case 's':
             service = optarg;
             if (strcmp(service, "install") == 0) {
-                const char *fixed_state_dir;
-
-                /* If the user passed the "-t" option, we save that state dir
-                 * in the service. Otherwise we let the service fetch the state
-                 * dir from the environment when it starts.
-                 */
-                fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
-                                  NULL :
-                                  state_dir;
                 if (ga_install_vss_provider()) {
                     exit(EXIT_FAILURE);
                 }
-                if (ga_install_service(channel_path, log_filepath,
-                                       fixed_state_dir)) {
+                if (ga_install_service(channel_path, log_filepath, state_dir)) {
                     exit(EXIT_FAILURE);
                 }
                 exit(EXIT_SUCCESS);
@@ -1067,6 +1054,14 @@ int main(int argc, char **argv)
         }
     }
 
+    if (pid_filepath == NULL) {
+        pid_filepath = g_strdup(dfl_pathnames.pidfile);
+    }
+
+    if (state_dir == NULL) {
+        state_dir = g_strdup(dfl_pathnames.state_dir);
+    }
+
 #ifdef _WIN32
     /* On win32 the state directory is application specific (be it the default
      * or a user override). We got past the command line parsing; let's create
@@ -1210,5 +1205,15 @@ out_bad:
     if (daemonize) {
         unlink(pid_filepath);
     }
+
+    g_free(method);
+    g_free(log_filepath);
+    g_free(pid_filepath);
+    g_free(state_dir);
+    g_free(channel_path);
+#ifdef CONFIG_FSFREEZE
+    g_free(fsfreeze_hook);
+#endif
+
     return EXIT_FAILURE;
 }
-- 
1.9.1

[Qemu-devel] [PATCH 15/26] qga: move option parsing to separate function

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Move option parsing out of giant main().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 165 +++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 96 insertions(+), 69 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 766cb93..900b68c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -936,19 +936,28 @@ static GList *split_list(const gchar *str, const gchar *delim)
     return list;
 }
 
-int main(int argc, char **argv)
-{
-    const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-    char *method = NULL, *channel_path = NULL;
-    char *log_filepath = NULL;
-    char *pid_filepath = NULL;
+typedef struct GAConfig {
+    char *channel_path;
+    char *method;
+    char *log_filepath;
+    char *pid_filepath;
 #ifdef CONFIG_FSFREEZE
-    char *fsfreeze_hook = NULL;
+    char *fsfreeze_hook;
 #endif
-    char *state_dir = NULL;
+    char *state_dir;
 #ifdef _WIN32
-    const char *service = NULL;
+    const char *service;
 #endif
+    GList *blacklist;
+    int daemonize;
+    GLogLevelFlags log_level;
+} GAConfig;
+
+static GAConfig *config_parse(int argc, char **argv)
+{
+    GAConfig *config = g_new0(GAConfig, 1);
+    const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
+    int opt_ind = 0, ch;
     const struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -968,74 +977,71 @@ int main(int argc, char **argv)
         { "statedir", 1, NULL, 't' },
         { NULL, 0, NULL, 0 }
     };
-    int opt_ind = 0, ch, daemonize = 0;
-    GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
-    GList *blacklist = NULL;
-    GAState *s;
 
-    module_call_init(MODULE_INIT_QAPI);
+    config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
 
-    init_dfl_pathnames();
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
-            method = g_strdup(optarg);
+            config->method = g_strdup(optarg);
             break;
         case 'p':
-            channel_path = g_strdup(optarg);
+            config->channel_path = g_strdup(optarg);
             break;
         case 'l':
-            log_filepath = g_strdup(optarg);
+            config->log_filepath = g_strdup(optarg);
             break;
         case 'f':
-            pid_filepath = g_strdup(optarg);
+            config->pid_filepath = g_strdup(optarg);
             break;
 #ifdef CONFIG_FSFREEZE
         case 'F':
-            fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
+            config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
             break;
 #endif
         case 't':
-            state_dir = g_strdup(optarg);
+            config->state_dir = g_strdup(optarg);
             break;
         case 'v':
             /* enable all log levels */
-            log_level = G_LOG_LEVEL_MASK;
+            config->log_level = G_LOG_LEVEL_MASK;
             break;
         case 'V':
             printf("QEMU Guest Agent %s\n", QEMU_VERSION);
             exit(EXIT_SUCCESS);
         case 'd':
-            daemonize = 1;
+            config->daemonize = 1;
             break;
         case 'b': {
             if (is_help_option(optarg)) {
                 qmp_for_each_command(ga_print_cmd, NULL);
                 exit(EXIT_SUCCESS);
             }
-            blacklist = g_list_concat(blacklist, split_list(optarg, ","));
+            config->blacklist = g_list_concat(config->blacklist,
+                                             split_list(optarg, ","));
             break;
         }
 #ifdef _WIN32
         case 's':
-            service = optarg;
-            if (strcmp(service, "install") == 0) {
+            config->service = optarg;
+            if (strcmp(config->service, "install") == 0) {
                 if (ga_install_vss_provider()) {
                     exit(EXIT_FAILURE);
                 }
-                if (ga_install_service(channel_path, log_filepath, state_dir)) {
+                if (ga_install_service(config->channel_path,
+                                       config->log_filepath, config->state_dir)) {
                     exit(EXIT_FAILURE);
                 }
                 exit(EXIT_SUCCESS);
-            } else if (strcmp(service, "uninstall") == 0) {
+            } else if (strcmp(config->service, "uninstall") == 0) {
                 ga_uninstall_vss_provider();
                 exit(ga_uninstall_service());
-            } else if (strcmp(service, "vss-install") == 0) {
+            } else if (strcmp(config->service, "vss-install") == 0) {
                 if (ga_install_vss_provider()) {
                     exit(EXIT_FAILURE);
                 }
                 exit(EXIT_SUCCESS);
-            } else if (strcmp(service, "vss-uninstall") == 0) {
+            } else if (strcmp(config->service, "vss-uninstall") == 0) {
                 ga_uninstall_vss_provider();
                 exit(EXIT_SUCCESS);
             } else {
@@ -1054,12 +1060,39 @@ int main(int argc, char **argv)
         }
     }
 
-    if (pid_filepath == NULL) {
-        pid_filepath = g_strdup(dfl_pathnames.pidfile);
+    return config;
+}
+
+static void config_free(GAConfig *config)
+{
+    g_free(config->method);
+    g_free(config->log_filepath);
+    g_free(config->pid_filepath);
+    g_free(config->state_dir);
+    g_free(config->channel_path);
+#ifdef CONFIG_FSFREEZE
+    g_free(config->fsfreeze_hook);
+#endif
+    g_free(config);
+}
+
+int main(int argc, char **argv)
+{
+    GAState *s;
+    GAConfig *config;
+
+    module_call_init(MODULE_INIT_QAPI);
+
+    init_dfl_pathnames();
+
+    config = config_parse(argc, argv);
+
+    if (config->pid_filepath == NULL) {
+        config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
     }
 
-    if (state_dir == NULL) {
-        state_dir = g_strdup(dfl_pathnames.state_dir);
+    if (config->state_dir == NULL) {
+        config->state_dir = g_strdup(dfl_pathnames.state_dir);
     }
 
 #ifdef _WIN32
@@ -1069,25 +1102,25 @@ int main(int argc, char **argv)
      * error later on, we won't try to clean up the directory, it is considered
      * persistent.
      */
-    if (g_mkdir_with_parents(state_dir, S_IRWXU) == -1) {
+    if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
         g_critical("unable to create (an ancestor of) the state directory"
-                   " '%s': %s", state_dir, strerror(errno));
+                   " '%s': %s", config->state_dir, strerror(errno));
         return EXIT_FAILURE;
     }
 #endif
 
     s = g_malloc0(sizeof(GAState));
-    s->log_level = log_level;
+    s->log_level = config->log_level;
     s->log_file = stderr;
 #ifdef CONFIG_FSFREEZE
-    s->fsfreeze_hook = fsfreeze_hook;
+    s->fsfreeze_hook = config->fsfreeze_hook;
 #endif
     g_log_set_default_handler(ga_log, s);
     g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
     ga_enable_logging(s);
     s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
-                                                 state_dir);
-    s->pstate_filepath = g_strdup_printf("%s/qga.state", state_dir);
+                                                 config->state_dir);
+    s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
     s->frozen = false;
 
 #ifndef _WIN32
@@ -1120,23 +1153,23 @@ int main(int argc, char **argv)
 #endif
 
     if (ga_is_frozen(s)) {
-        if (daemonize) {
+        if (config->daemonize) {
             /* delay opening/locking of pidfile till filesystems are unfrozen */
-            s->deferred_options.pid_filepath = pid_filepath;
+            s->deferred_options.pid_filepath = config->pid_filepath;
             become_daemon(NULL);
         }
-        if (log_filepath) {
+        if (config->log_filepath) {
             /* delay opening the log file till filesystems are unfrozen */
-            s->deferred_options.log_filepath = log_filepath;
+            s->deferred_options.log_filepath = config->log_filepath;
         }
         ga_disable_logging(s);
         qmp_for_each_command(ga_disable_non_whitelisted, NULL);
     } else {
-        if (daemonize) {
-            become_daemon(pid_filepath);
+        if (config->daemonize) {
+            become_daemon(config->pid_filepath);
         }
-        if (log_filepath) {
-            FILE *log_file = ga_open_logfile(log_filepath);
+        if (config->log_filepath) {
+            FILE *log_file = ga_open_logfile(config->log_filepath);
             if (!log_file) {
                 g_critical("unable to open specified log file: %s",
                            strerror(errno));
@@ -1154,14 +1187,15 @@ int main(int argc, char **argv)
         goto out_bad;
     }
 
-    blacklist = ga_command_blacklist_init(blacklist);
-    if (blacklist) {
-        s->blacklist = blacklist;
+    config->blacklist = ga_command_blacklist_init(config->blacklist);
+    if (config->blacklist) {
+        GList *l = config->blacklist;
+        s->blacklist = config->blacklist;
         do {
-            g_debug("disabling command: %s", (char *)blacklist->data);
-            qmp_disable_command(blacklist->data);
-            blacklist = g_list_next(blacklist);
-        } while (blacklist);
+            g_debug("disabling command: %s", (char *)l->data);
+            qmp_disable_command(l->data);
+            l = g_list_next(l);
+        } while (l);
     }
     s->command_state = ga_command_state_new();
     ga_command_state_init(s, s->command_state);
@@ -1176,14 +1210,14 @@ int main(int argc, char **argv)
 #endif
 
     s->main_loop = g_main_loop_new(NULL, false);
-    if (!channel_init(ga_state, method, channel_path)) {
+    if (!channel_init(ga_state, config->method, config->channel_path)) {
         g_critical("failed to initialize guest agent channel");
         goto out_bad;
     }
 #ifndef _WIN32
     g_main_loop_run(ga_state->main_loop);
 #else
-    if (daemonize) {
+    if (config->daemonize) {
         SERVICE_TABLE_ENTRY service_table[] = {
             { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
         StartServiceCtrlDispatcher(service_table);
@@ -1196,24 +1230,17 @@ int main(int argc, char **argv)
     ga_command_state_cleanup_all(ga_state->command_state);
     ga_channel_free(ga_state->channel);
 
-    if (daemonize) {
-        unlink(pid_filepath);
+    if (config->daemonize) {
+        unlink(config->pid_filepath);
     }
     return 0;
 
 out_bad:
-    if (daemonize) {
-        unlink(pid_filepath);
+    if (config->daemonize) {
+        unlink(config->pid_filepath);
     }
 
-    g_free(method);
-    g_free(log_filepath);
-    g_free(pid_filepath);
-    g_free(state_dir);
-    g_free(channel_path);
-#ifdef CONFIG_FSFREEZE
-    g_free(fsfreeze_hook);
-#endif
+    config_free(config);
 
     return EXIT_FAILURE;
 }
-- 
1.9.1

[Qemu-devel] [PATCH 16/26] qga: fill default options in main()

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Fill all default options during main(). This is a preparation patch
to allow to dump the configuration.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 900b68c..38ee196 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -658,23 +658,6 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
 {
     GAChannelMethod channel_method;
 
-    if (method == NULL) {
-        method = "virtio-serial";
-    }
-
-    if (path == NULL) {
-        if (strcmp(method, "virtio-serial") == 0 ) {
-            /* try the default path for the virtio-serial port */
-            path = QGA_VIRTIO_PATH_DEFAULT;
-        } else if (strcmp(method, "isa-serial") == 0){
-            /* try the default path for the serial port - COM1 */
-            path = QGA_SERIAL_PATH_DEFAULT;
-        } else {
-            g_critical("must specify a path for this channel");
-            return false;
-        }
-    }
-
     if (strcmp(method, "virtio-serial") == 0) {
         s->virtio = true; /* virtio requires special handling in some cases */
         channel_method = GA_CHANNEL_VIRTIO_SERIAL;
@@ -1095,6 +1078,23 @@ int main(int argc, char **argv)
         config->state_dir = g_strdup(dfl_pathnames.state_dir);
     }
 
+    if (config->method == NULL) {
+        config->method = g_strdup("virtio-serial");
+    }
+
+    if (config->channel_path == NULL) {
+        if (strcmp(config->method, "virtio-serial") == 0) {
+            /* try the default path for the virtio-serial port */
+            config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
+        } else if (strcmp(config->method, "isa-serial") == 0) {
+            /* try the default path for the serial port - COM1 */
+            config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
+        } else {
+            g_critical("must specify a path for this channel");
+            goto out_bad;
+        }
+    }
+
 #ifdef _WIN32
     /* On win32 the state directory is application specific (be it the default
      * or a user override). We got past the command line parsing; let's create
-- 
1.9.1

[Qemu-devel] [PATCH 17/26] qga: move agent run in a separate function

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Once the options are populated, move the running state to
a run_agent() function.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
*fixed up an s/ga_state/s/ artifact causing segfault
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 166 +++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 90 insertions(+), 76 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 38ee196..0fef3be 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1059,70 +1059,8 @@ static void config_free(GAConfig *config)
     g_free(config);
 }
 
-int main(int argc, char **argv)
+static bool check_is_frozen(GAState *s)
 {
-    GAState *s;
-    GAConfig *config;
-
-    module_call_init(MODULE_INIT_QAPI);
-
-    init_dfl_pathnames();
-
-    config = config_parse(argc, argv);
-
-    if (config->pid_filepath == NULL) {
-        config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
-    }
-
-    if (config->state_dir == NULL) {
-        config->state_dir = g_strdup(dfl_pathnames.state_dir);
-    }
-
-    if (config->method == NULL) {
-        config->method = g_strdup("virtio-serial");
-    }
-
-    if (config->channel_path == NULL) {
-        if (strcmp(config->method, "virtio-serial") == 0) {
-            /* try the default path for the virtio-serial port */
-            config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
-        } else if (strcmp(config->method, "isa-serial") == 0) {
-            /* try the default path for the serial port - COM1 */
-            config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
-        } else {
-            g_critical("must specify a path for this channel");
-            goto out_bad;
-        }
-    }
-
-#ifdef _WIN32
-    /* On win32 the state directory is application specific (be it the default
-     * or a user override). We got past the command line parsing; let's create
-     * the directory (with any intermediate directories). If we run into an
-     * error later on, we won't try to clean up the directory, it is considered
-     * persistent.
-     */
-    if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
-        g_critical("unable to create (an ancestor of) the state directory"
-                   " '%s': %s", config->state_dir, strerror(errno));
-        return EXIT_FAILURE;
-    }
-#endif
-
-    s = g_malloc0(sizeof(GAState));
-    s->log_level = config->log_level;
-    s->log_file = stderr;
-#ifdef CONFIG_FSFREEZE
-    s->fsfreeze_hook = config->fsfreeze_hook;
-#endif
-    g_log_set_default_handler(ga_log, s);
-    g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
-    ga_enable_logging(s);
-    s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
-                                                 config->state_dir);
-    s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
-    s->frozen = false;
-
 #ifndef _WIN32
     /* check if a previous instance of qemu-ga exited with filesystems' state
      * marked as frozen. this could be a stale value (a non-qemu-ga process
@@ -1148,7 +1086,31 @@ int main(int argc, char **argv)
                   " guest-fsfreeze-thaw is issued, or filesystems are"
                   " manually unfrozen and the file %s is removed",
                   s->state_filepath_isfrozen);
-        s->frozen = true;
+        return true;
+    }
+#endif
+    return false;
+}
+
+static int run_agent(GAState *s, GAConfig *config)
+{
+    ga_state = s;
+
+    g_log_set_default_handler(ga_log, s);
+    g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
+    ga_enable_logging(s);
+
+#ifdef _WIN32
+    /* On win32 the state directory is application specific (be it the default
+     * or a user override). We got past the command line parsing; let's create
+     * the directory (with any intermediate directories). If we run into an
+     * error later on, we won't try to clean up the directory, it is considered
+     * persistent.
+     */
+    if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
+        g_critical("unable to create (an ancestor of) the state directory"
+                   " '%s': %s", config->state_dir, strerror(errno));
+        return EXIT_FAILURE;
     }
 #endif
 
@@ -1173,7 +1135,7 @@ int main(int argc, char **argv)
             if (!log_file) {
                 g_critical("unable to open specified log file: %s",
                            strerror(errno));
-                goto out_bad;
+                return EXIT_FAILURE;
             }
             s->log_file = log_file;
         }
@@ -1184,7 +1146,7 @@ int main(int argc, char **argv)
                                s->pstate_filepath,
                                ga_is_frozen(s))) {
         g_critical("failed to load persistent state");
-        goto out_bad;
+        return EXIT_FAILURE;
     }
 
     config->blacklist = ga_command_blacklist_init(config->blacklist);
@@ -1205,14 +1167,14 @@ int main(int argc, char **argv)
 #ifndef _WIN32
     if (!register_signal_handlers()) {
         g_critical("failed to register signal handlers");
-        goto out_bad;
+        return EXIT_FAILURE;
     }
 #endif
 
     s->main_loop = g_main_loop_new(NULL, false);
     if (!channel_init(ga_state, config->method, config->channel_path)) {
         g_critical("failed to initialize guest agent channel");
-        goto out_bad;
+        return EXIT_FAILURE;
     }
 #ifndef _WIN32
     g_main_loop_run(ga_state->main_loop);
@@ -1226,21 +1188,73 @@ int main(int argc, char **argv)
     }
 #endif
 
-    g_list_free_full(ga_state->blacklist, g_free);
-    ga_command_state_cleanup_all(ga_state->command_state);
-    ga_channel_free(ga_state->channel);
+    return EXIT_SUCCESS;
+}
 
-    if (config->daemonize) {
-        unlink(config->pid_filepath);
+int main(int argc, char **argv)
+{
+    int ret = EXIT_SUCCESS;
+    GAState *s = g_new0(GAState, 1);
+    GAConfig *config;
+
+    module_call_init(MODULE_INIT_QAPI);
+
+    init_dfl_pathnames();
+
+    config = config_parse(argc, argv);
+
+    if (config->pid_filepath == NULL) {
+        config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
     }
-    return 0;
 
-out_bad:
+    if (config->state_dir == NULL) {
+        config->state_dir = g_strdup(dfl_pathnames.state_dir);
+    }
+
+    if (config->method == NULL) {
+        config->method = g_strdup("virtio-serial");
+    }
+
+    if (config->channel_path == NULL) {
+        if (strcmp(config->method, "virtio-serial") == 0) {
+            /* try the default path for the virtio-serial port */
+            config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
+        } else if (strcmp(config->method, "isa-serial") == 0) {
+            /* try the default path for the serial port - COM1 */
+            config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
+        } else {
+            g_critical("must specify a path for this channel");
+            ret = EXIT_FAILURE;
+            goto end;
+        }
+    }
+
+    s->log_level = config->log_level;
+    s->log_file = stderr;
+#ifdef CONFIG_FSFREEZE
+    s->fsfreeze_hook = config->fsfreeze_hook;
+#endif
+    s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
+    s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
+                                                 config->state_dir);
+    s->frozen = check_is_frozen(s);
+
+    ret = run_agent(s, config);
+
+end:
+    if (s->command_state) {
+        ga_command_state_cleanup_all(s->command_state);
+    }
+    if (s->channel) {
+        ga_channel_free(s->channel);
+    }
+    g_list_free_full(s->blacklist, g_free);
+
     if (config->daemonize) {
         unlink(config->pid_filepath);
     }
 
     config_free(config);
 
-    return EXIT_FAILURE;
+    return ret;
 }
-- 
1.9.1

[Qemu-devel] [PATCH 18/26] qga: free a bit more

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Now that main() has a single exit point, we can free a few
more allocations.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 0fef3be..720a36d 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -82,7 +82,7 @@ struct GAState {
     bool delimit_response;
     bool frozen;
     GList *blacklist;
-    const char *state_filepath_isfrozen;
+    char *state_filepath_isfrozen;
     struct {
         const char *log_filepath;
         const char *pid_filepath;
@@ -90,7 +90,7 @@ struct GAState {
 #ifdef CONFIG_FSFREEZE
     const char *fsfreeze_hook;
 #endif
-    const gchar *pstate_filepath;
+    gchar *pstate_filepath;
     GAPersistentState pstate;
 };
 
@@ -1249,6 +1249,8 @@ end:
         ga_channel_free(s->channel);
     }
     g_list_free_full(s->blacklist, g_free);
+    g_free(s->pstate_filepath);
+    g_free(s->state_filepath_isfrozen);
 
     if (config->daemonize) {
         unlink(config->pid_filepath);
-- 
1.9.1

[Qemu-devel] [PATCH 19/26] qga: add an optional qemu-ga.conf system configuration

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Learn to configure the agent with a system configuration.

This may simplify command-line handling, especially when the blacklist
is long.

Among the other benefits, this may standardize the configuration of an
init service (instead of distro-specific init keys/files)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
* removed unecessary keyfile != NULL prior to free
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 720a36d..322f81d 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -56,6 +56,7 @@
 #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
 #endif
 #define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
 
 static struct {
     const char *state_dir;
@@ -931,14 +932,78 @@ typedef struct GAConfig {
 #ifdef _WIN32
     const char *service;
 #endif
+    gchar *bliststr; /* blacklist may point to this string */
     GList *blacklist;
     int daemonize;
     GLogLevelFlags log_level;
 } GAConfig;
 
-static GAConfig *config_parse(int argc, char **argv)
+static void config_load(GAConfig *config)
+{
+    GError *gerr = NULL;
+    GKeyFile *keyfile;
+
+    /* read system config */
+    keyfile = g_key_file_new();
+    if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
+        goto end;
+    }
+    if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
+        config->daemonize =
+            g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
+        config->method =
+            g_key_file_get_string(keyfile, "general", "method", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
+        config->channel_path =
+            g_key_file_get_string(keyfile, "general", "path", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
+        config->log_filepath =
+            g_key_file_get_string(keyfile, "general", "logfile", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
+        config->pid_filepath =
+            g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
+    }
+#ifdef CONFIG_FSFREEZE
+    if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
+        config->fsfreeze_hook =
+            g_key_file_get_string(keyfile,
+                                  "general", "fsfreeze-hook", &gerr);
+    }
+#endif
+    if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
+        config->state_dir =
+            g_key_file_get_string(keyfile, "general", "statedir", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
+        g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
+        /* enable all log levels */
+        config->log_level = G_LOG_LEVEL_MASK;
+    }
+    if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
+        config->bliststr =
+            g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
+        config->blacklist = g_list_concat(config->blacklist,
+                                          split_list(config->bliststr, ","));
+    }
+
+end:
+    g_key_file_free(keyfile);
+    if (gerr &&
+        !(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
+        g_critical("error loading configuration from path: %s, %s",
+                   QGA_CONF_DEFAULT, gerr->message);
+        exit(EXIT_FAILURE);
+    }
+    g_clear_error(&gerr);
+}
+
+static void config_parse(GAConfig *config, int argc, char **argv)
 {
-    GAConfig *config = g_new0(GAConfig, 1);
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
     int opt_ind = 0, ch;
     const struct option lopt[] = {
@@ -966,23 +1031,29 @@ static GAConfig *config_parse(int argc, char **argv)
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
+            g_free(config->method);
             config->method = g_strdup(optarg);
             break;
         case 'p':
+            g_free(config->channel_path);
             config->channel_path = g_strdup(optarg);
             break;
         case 'l':
+            g_free(config->log_filepath);
             config->log_filepath = g_strdup(optarg);
             break;
         case 'f':
+            g_free(config->pid_filepath);
             config->pid_filepath = g_strdup(optarg);
             break;
 #ifdef CONFIG_FSFREEZE
         case 'F':
+            g_free(config->fsfreeze_hook);
             config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
             break;
 #endif
         case 't':
+            g_free(config->state_dir);
             config->state_dir = g_strdup(optarg);
             break;
         case 'v':
@@ -1042,8 +1113,6 @@ static GAConfig *config_parse(int argc, char **argv)
             exit(EXIT_FAILURE);
         }
     }
-
-    return config;
 }
 
 static void config_free(GAConfig *config)
@@ -1053,6 +1122,7 @@ static void config_free(GAConfig *config)
     g_free(config->pid_filepath);
     g_free(config->state_dir);
     g_free(config->channel_path);
+    g_free(config->bliststr);
 #ifdef CONFIG_FSFREEZE
     g_free(config->fsfreeze_hook);
 #endif
@@ -1195,13 +1265,13 @@ int main(int argc, char **argv)
 {
     int ret = EXIT_SUCCESS;
     GAState *s = g_new0(GAState, 1);
-    GAConfig *config;
+    GAConfig *config = g_new0(GAConfig, 1);
 
     module_call_init(MODULE_INIT_QAPI);
 
     init_dfl_pathnames();
-
-    config = config_parse(argc, argv);
+    config_load(config);
+    config_parse(config, argc, argv);
 
     if (config->pid_filepath == NULL) {
         config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
-- 
1.9.1

[Qemu-devel] [PATCH 20/26] qga: add --dump-conf option

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This new option allows to review the agent configuration,
and ease the task of writing a configuration file.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
* removed unecessary keyfile != NULL prior to free
* documented --dump-conf is qemu-ga --help output
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/qga/main.c b/qga/main.c
index 322f81d..f75ab4b 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -216,6 +216,8 @@ static void usage(const char *cmd)
 #endif
 "  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, \"?\"\n"
 "                    to list available RPCs)\n"
+"  -D, --dump-conf   dump a qemu-ga config file based on current config\n"
+"                    options / command-line parameters to stdout\n"
 "  -h, --help        display this help and exit\n"
 "\n"
 "Report bugs to <mdroth@linux.vnet.ibm.com>\n"
@@ -936,6 +938,7 @@ typedef struct GAConfig {
     GList *blacklist;
     int daemonize;
     GLogLevelFlags log_level;
+    int dumpconf;
 } GAConfig;
 
 static void config_load(GAConfig *config)
@@ -1002,6 +1005,58 @@ end:
     g_clear_error(&gerr);
 }
 
+static gchar *list_join(GList *list, const gchar separator)
+{
+    GString *str = g_string_new("");
+
+    while (list) {
+        str = g_string_append(str, (gchar *)list->data);
+        list = g_list_next(list);
+        if (list) {
+            str = g_string_append_c(str, separator);
+        }
+    }
+
+    return g_string_free(str, FALSE);
+}
+
+static void config_dump(GAConfig *config)
+{
+    GError *error = NULL;
+    GKeyFile *keyfile;
+    gchar *tmp;
+
+    keyfile = g_key_file_new();
+    g_assert(keyfile);
+
+    g_key_file_set_boolean(keyfile, "general", "daemon", config->daemonize);
+    g_key_file_set_string(keyfile, "general", "method", config->method);
+    g_key_file_set_string(keyfile, "general", "path", config->channel_path);
+    if (config->log_filepath) {
+        g_key_file_set_string(keyfile, "general", "logfile",
+                              config->log_filepath);
+    }
+    g_key_file_set_string(keyfile, "general", "pidfile", config->pid_filepath);
+#ifdef CONFIG_FSFREEZE
+    if (config->fsfreeze_hook) {
+        g_key_file_set_string(keyfile, "general", "fsfreeze-hook",
+                              config->fsfreeze_hook);
+    }
+#endif
+    g_key_file_set_string(keyfile, "general", "statedir", config->state_dir);
+    g_key_file_set_boolean(keyfile, "general", "verbose",
+                           config->log_level == G_LOG_LEVEL_MASK);
+    tmp = list_join(config->blacklist, ',');
+    g_key_file_set_string(keyfile, "general", "blacklist", tmp);
+    g_free(tmp);
+
+    tmp = g_key_file_to_data(keyfile, NULL, &error);
+    printf("%s", tmp);
+
+    g_free(tmp);
+    g_key_file_free(keyfile);
+}
+
 static void config_parse(GAConfig *config, int argc, char **argv)
 {
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
@@ -1009,6 +1064,7 @@ static void config_parse(GAConfig *config, int argc, char **argv)
     const struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
+        { "dump-conf", 0, NULL, 'D' },
         { "logfile", 1, NULL, 'l' },
         { "pidfile", 1, NULL, 'f' },
 #ifdef CONFIG_FSFREEZE
@@ -1066,6 +1122,9 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         case 'd':
             config->daemonize = 1;
             break;
+        case 'D':
+            config->dumpconf = 1;
+            break;
         case 'b': {
             if (is_help_option(optarg)) {
                 qmp_for_each_command(ga_print_cmd, NULL);
@@ -1309,6 +1368,11 @@ int main(int argc, char **argv)
                                                  config->state_dir);
     s->frozen = check_is_frozen(s);
 
+    if (config->dumpconf) {
+        config_dump(config);
+        goto end;
+    }
+
     ret = run_agent(s, config);
 
 end:
-- 
1.9.1

[Qemu-devel] [PATCH 21/26] qga: start a man page

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add a simple man page for the qemu agent.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
*squashed in review comments from Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile      |  14 +++++-
 qemu-doc.texi |   6 +++
 qemu-ga.texi  | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 2 deletions(-)
 create mode 100644 qemu-ga.texi

diff --git a/Makefile b/Makefile
index f98a554..c13a83d 100644
--- a/Makefile
+++ b/Makefile
@@ -88,7 +88,8 @@ LIBS+=-lz $(LIBS_TOOLS)
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
-DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qmp-commands.txt
+DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
+DOCS+=qmp-commands.txt
 ifdef CONFIG_LINUX
 DOCS+=kvm_stat.1
 endif
@@ -400,6 +401,9 @@ ifneq ($(TOOLS),)
 	$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
 	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
 endif
+ifneq (,$(findstring qemu-ga,$(TOOLS)))
+	$(INSTALL_DATA) qemu-ga.8 "$(DESTDIR)$(mandir)/man8"
+endif
 endif
 ifdef CONFIG_VIRTFS
 	$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
@@ -538,6 +542,12 @@ qemu-nbd.8: qemu-nbd.texi
 	  $(POD2MAN) --section=8 --center=" " --release=" " qemu-nbd.pod > $@, \
 	  "  GEN   $@")
 
+qemu-ga.8: qemu-ga.texi
+	$(call quiet-command, \
+	  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-ga.pod && \
+	  $(POD2MAN) --section=8 --center=" " --release=" " qemu-ga.pod > $@, \
+	  "  GEN   $@")
+
 kvm_stat.1: scripts/kvm/kvm_stat.texi
 	$(call quiet-command, \
 	  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< kvm_stat.pod && \
@@ -551,7 +561,7 @@ pdf: qemu-doc.pdf qemu-tech.pdf
 
 qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
 	qemu-img.texi qemu-nbd.texi qemu-options.texi \
-	qemu-monitor.texi qemu-img-cmds.texi
+	qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi
 
 ifdef CONFIG_WIN32
 
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f1c38b6..ea9b3fb 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -412,6 +412,7 @@ snapshots.
 * vm_snapshots::              VM snapshots
 * qemu_img_invocation::       qemu-img Invocation
 * qemu_nbd_invocation::       qemu-nbd Invocation
+* qemu_ga_invocation::        qemu-ga Invocation
 * disk_images_formats::       Disk image file formats
 * host_drives::               Using host drives
 * disk_images_fat_images::    Virtual FAT disk images
@@ -505,6 +506,11 @@ state is not saved or restored properly (in particular USB).
 
 @include qemu-nbd.texi
 
+@node qemu_ga_invocation
+@subsection @code{qemu-ga} Invocation
+
+@include qemu-ga.texi
+
 @node disk_images_formats
 @subsection Disk image file formats
 
diff --git a/qemu-ga.texi b/qemu-ga.texi
new file mode 100644
index 0000000..536a9b5
--- /dev/null
+++ b/qemu-ga.texi
@@ -0,0 +1,137 @@
+@example
+@c man begin SYNOPSIS
+usage: qemu-ga [OPTIONS]
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+
+The QEMU Guest Agent is a daemon intended to be run within virtual
+machines. It allows the hypervisor host to perform various operations
+in the guest, such as:
+
+@itemize
+@item
+get information from the guest
+@item
+set the guest's system time
+@item
+read/write a file
+@item
+sync and freeze the filesystems
+@item
+suspend the guest
+@item
+reconfigure guest local processors
+@item
+set user's password
+@item
+...
+@end itemize
+
+qemu-ga will read a system configuration file on startup (located at
+q@file{/etc/qemu/qemu-ga.conf} by default), then parse remaining
+configuration options on the command line. For the same key, the last
+option wins, but the lists accumulate (see below for configuration
+file format).
+
+@c man end
+
+@c man begin OPTIONS
+@table @option
+@item -m, --method=@var{method}
+  Transport method: one of @samp{unix-listen}, @samp{virtio-serial}, or
+  @samp{isa-serial} (@samp{virtio-serial} is the default).
+
+@item -p, --path=@var{path}
+  Device/socket path (the default for virtio-serial is
+  @samp{/dev/virtio-ports/org.qemu.guest_agent.0},
+  the default for isa-serial is @samp{/dev/ttyS0})
+
+@item -l, --logfile=@var{path}
+  Set log file path (default is stderr).
+
+@item -f, --pidfile=@var{path}
+  Specify pid file (default is @samp{/var/run/qemu-ga.pid}).
+
+@item -F, --fsfreeze-hook=@var{path}
+  Enable fsfreeze hook. Accepts an optional argument that specifies
+  script to run on freeze/thaw. Script will be called with
+  'freeze'/'thaw' arguments accordingly (default is
+  @samp{/etc/qemu/fsfreeze-hook}). If using -F with an argument, do
+  not follow -F with a space (for example:
+  @samp{-F/var/run/fsfreezehook.sh}).
+
+@item -t, --statedir=@var{path}
+  Specify the directory to store state information (absolute paths only,
+  default is @samp{/var/run}).
+
+@item -v, --verbose
+  Log extra debugging information.
+
+@item -V, --version
+  Print version information and exit.
+
+@item -d, --daemon
+  Daemonize after startup (detach from terminal).
+
+@item -b, --blacklist=@var{list}
+  Comma-separated list of RPCs to disable (no spaces, @samp{?} to list
+  available RPCs).
+
+@item -D, --dump-conf
+  Dump the configuration in a format compatible with @file{qemu-ga.conf}
+  and exit.
+
+@item -h, --help
+  Display this help and exit.
+@end table
+
+@c man end
+
+@c man begin FILES
+
+The syntax of the @file{qemu-ga.conf} configuration file follows the
+Desktop Entry Specification, here is a quick summary: it consists of
+groups of key-value pairs, interspersed with comments.
+
+@example
+# qemu-ga configuration sample
+[general]
+daemonize = 0
+pidfile = /var/run/qemu-ga.pid
+verbose = 0
+method = virtio-serial
+path = /dev/virtio-ports/org.qemu.guest_agent.0
+statedir = /var/run
+@end example
+
+The list of keys follows the command line options:
+@table @option
+@item daemon= boolean
+@item method= string
+@item path= string
+@item logfile= string
+@item pidfile= string
+@item fsfreeze-hook= string
+@item statedir= string
+@item verbose= boolean
+@item blacklist= string list
+@end table
+
+@c man end
+
+@ignore
+
+@setfilename qemu-ga
+@settitle QEMU Guest Agent
+
+@c man begin AUTHOR
+Michael Roth <mdroth@linux.vnet.ibm.com>
+@c man end
+
+@c man begin SEEALSO
+qemu(1)
+@c man end
+
+@end ignore
-- 
1.9.1

[Qemu-devel] [PATCH 22/26] qemu-ga: implement win32 guest-set-user-password

[Michael Roth]

From: Marc-André Lureau <marcandre.lureau@gmail.com>

Use NetUserSetInfo() to set the user password.

This function is notoriously known to be problematic for users with EFS
encrypted files. But the alternative, NetUserChangePassword() requires
the old password. Nevertheless, The EFS file should be recovered by
changing back to the old password.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure            |  2 +-
 qga/commands-win32.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 86a38fe..1f033e9 100755
--- a/configure
+++ b/configure
@@ -732,7 +732,7 @@ if test "$mingw32" = "yes" ; then
   sysconfdir="\${prefix}"
   local_statedir=
   confsuffix=""
-  libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga"
+  libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi -lnetapi32 $libs_qga"
 fi
 
 werror=""
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1152c46..cbee186 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -26,6 +26,8 @@
 #include <setupapi.h>
 #include <initguid.h>
 #endif
+#include <lm.h>
+
 #include "qga/guest-agent-core.h"
 #include "qga/vss-win32.h"
 #include "qga-qmp-commands.h"
@@ -1192,12 +1194,84 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return -1;
 }
 
+static gchar *
+get_net_error_message(gint error)
+{
+    HMODULE module = NULL;
+    gchar *retval = NULL;
+    wchar_t *msg = NULL;
+    int flags, nchars;
+
+    flags = FORMAT_MESSAGE_ALLOCATE_BUFFER
+        |FORMAT_MESSAGE_IGNORE_INSERTS
+        |FORMAT_MESSAGE_FROM_SYSTEM;
+
+    if (error >= NERR_BASE && error <= MAX_NERR) {
+        module = LoadLibraryExW(L"netmsg.dll", NULL, LOAD_LIBRARY_AS_DATAFILE);
+
+        if (module != NULL) {
+            flags |= FORMAT_MESSAGE_FROM_HMODULE;
+        }
+    }
+
+    FormatMessageW(flags, module, error, 0, (LPWSTR)&msg, 0, NULL);
+
+    if (msg != NULL) {
+        nchars = wcslen(msg);
+
+        if (nchars > 2 && msg[nchars-1] == '\n' && msg[nchars-2] == '\r') {
+            msg[nchars-2] = '\0';
+        }
+
+        retval = g_utf16_to_utf8(msg, -1, NULL, NULL, NULL);
+
+        LocalFree(msg);
+    }
+
+    if (module != NULL) {
+        FreeLibrary(module);
+    }
+
+    return retval;
+}
+
 void qmp_guest_set_user_password(const char *username,
                                  const char *password,
                                  bool crypted,
                                  Error **errp)
 {
-    error_setg(errp, QERR_UNSUPPORTED);
+    NET_API_STATUS nas;
+    char *rawpasswddata = NULL;
+    size_t rawpasswdlen;
+    wchar_t *user, *wpass;
+    USER_INFO_1003 pi1003 = { 0, };
+
+    if (crypted) {
+        error_setg(errp, QERR_UNSUPPORTED);
+        return;
+    }
+
+    rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
+    rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
+    rawpasswddata[rawpasswdlen] = '\0';
+
+    user = g_utf8_to_utf16(username, -1, NULL, NULL, NULL);
+    wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, NULL);
+
+    pi1003.usri1003_password = wpass;
+    nas = NetUserSetInfo(NULL, user,
+                         1003, (LPBYTE)&pi1003,
+                         NULL);
+
+    if (nas != NERR_Success) {
+        gchar *msg = get_net_error_message(nas);
+        error_setg(errp, "failed to set password: %s", msg);
+        g_free(msg);
+    }
+
+    g_free(user);
+    g_free(wpass);
+    g_free(rawpasswddata);
 }
 
 GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
@@ -1225,7 +1299,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
     const char *list_unsupported[] = {
         "guest-suspend-hybrid",
         "guest-get-vcpus", "guest-set-vcpus",
-        "guest-set-user-password",
         "guest-get-memory-blocks", "guest-set-memory-blocks",
         "guest-get-memory-block-size",
         "guest-fsfreeze-freeze-list",
-- 
1.9.1

[Qemu-devel] [PATCH 23/26] configure: qemu-ga: move MSI installer probe after qga probe

[Michael Roth]

MSI probe assumes that qemu-ga support has been probed already, but in
cases where --enable-guest-agent/--disable-guest-agent have not been
passed to configure, qemu-ga support may end up getting enabled later,
as is the case with w32 builds. This leads to MSI probe prematurely
reporting error due to lack of qemu-ga support.

Fix this by moving MSI installer probe after the final qga probes.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure | 106 ++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

diff --git a/configure b/configure
index 1f033e9..c31c652 100755
--- a/configure
+++ b/configure
@@ -3905,58 +3905,6 @@ EOF
 fi
 
 ##########################################
-# Guest agent Window MSI  package
-
-if test "$guest_agent" != yes; then
-  if test "$guest_agent_msi" = yes; then
-    error_exit "MSI guest agent package requires guest agent enabled"
-  fi
-  guest_agent_msi=no
-elif test "$mingw32" != "yes"; then
-  if test "$guest_agent_msi" = "yes"; then
-    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
-  fi
-  guest_agent_msi=no
-elif ! has wixl; then
-  if test "$guest_agent_msi" = "yes"; then
-    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
-  fi
-  guest_agent_msi=no
-fi
-
-if test "$guest_agent_msi" != "no"; then
-  if test "$guest_agent_with_vss" = "yes"; then
-    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
-  fi
-
-  if test "$QEMU_GA_MANUFACTURER" = ""; then
-    QEMU_GA_MANUFACTURER=QEMU
-  fi
-
-  if test "$QEMU_GA_DISTRO" = ""; then
-    QEMU_GA_DISTRO=Linux
-  fi
-
-  if test "$QEMU_GA_VERSION" = ""; then
-      QEMU_GA_VERSION=`cat $source_path/VERSION`
-  fi
-
-  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
-  
-  case "$cpu" in
-  x86_64)
-    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
-    ;;
-  i386)
-    QEMU_GA_MSI_ARCH="-D Arch=32"
-    ;;
-  *)
-    error_exit "CPU $cpu not supported for building installation package"
-    ;;
-  esac
-fi
-
-##########################################
 # check if we have fdatasync
 
 fdatasync=no
@@ -4396,6 +4344,9 @@ if test "$softmmu" = yes ; then
     fi
   fi
 fi
+
+# Probe for guest agent support/options
+
 if [ "$guest_agent" != "no" ]; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
       tools="qemu-ga\$(EXESUF) $tools"
@@ -4410,6 +4361,57 @@ if [ "$guest_agent" != "no" ]; then
   fi
 fi
 
+# Guest agent Window MSI  package
+
+if test "$guest_agent" != yes; then
+  if test "$guest_agent_msi" = yes; then
+    error_exit "MSI guest agent package requires guest agent enabled"
+  fi
+  guest_agent_msi=no
+elif test "$mingw32" != "yes"; then
+  if test "$guest_agent_msi" = "yes"; then
+    error_exit "MSI guest agent package is available only for MinGW Windows cross-compilation"
+  fi
+  guest_agent_msi=no
+elif ! has wixl; then
+  if test "$guest_agent_msi" = "yes"; then
+    error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
+  fi
+  guest_agent_msi=no
+fi
+
+if test "$guest_agent_msi" != "no"; then
+  if test "$guest_agent_with_vss" = "yes"; then
+    QEMU_GA_MSI_WITH_VSS="-D InstallVss"
+  fi
+
+  if test "$QEMU_GA_MANUFACTURER" = ""; then
+    QEMU_GA_MANUFACTURER=QEMU
+  fi
+
+  if test "$QEMU_GA_DISTRO" = ""; then
+    QEMU_GA_DISTRO=Linux
+  fi
+
+  if test "$QEMU_GA_VERSION" = ""; then
+      QEMU_GA_VERSION=`cat $source_path/VERSION`
+  fi
+
+  QEMU_GA_MSI_MINGW_DLL_PATH="-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin"
+
+  case "$cpu" in
+  x86_64)
+    QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
+    ;;
+  i386)
+    QEMU_GA_MSI_ARCH="-D Arch=32"
+    ;;
+  *)
+    error_exit "CPU $cpu not supported for building installation package"
+    ;;
+  esac
+fi
+
 # Mac OS X ships with a broken assembler
 roms=
 if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \
-- 
1.9.1

[Qemu-devel] [PATCH 24/26] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed

[Michael Roth]

Currently, if we don't explicitly disable support for MSI installer
via --disable-guest-agent-msi, the configure variable that tracks
the flag, 'guest_agent_msi', never gets set unless one of the probes
fails. Subsequent code then treats this unset value the same as if it
were a "yes" value (via != "no" style checks).

Instead, set the default "yes" value explicitly after the probes, then
make subsequent code expect the values to be set.

This makes it easier to report on whether or not MSI support was
enabled via probe by looking at the ./configure summary.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 configure | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index c31c652..a52ef50 100755
--- a/configure
+++ b/configure
@@ -4378,9 +4378,15 @@ elif ! has wixl; then
     error_exit "MSI guest agent package requires wixl tool installed ( usually from msitools package )"
   fi
   guest_agent_msi=no
+else
+  # we support qemu-ga, mingw32, and wixl: default to MSI enabled if it wasn't
+  # disabled explicitly
+  if test "$guest_agent_msi" != "no"; then
+    guest_agent_msi=yes
+  fi
 fi
 
-if test "$guest_agent_msi" != "no"; then
+if test "$guest_agent_msi" = "yes"; then
   if test "$guest_agent_with_vss" = "yes"; then
     QEMU_GA_MSI_WITH_VSS="-D InstallVss"
   fi
@@ -4659,7 +4665,7 @@ if test "$mingw32" = "yes" ; then
   if test "$guest_agent_ntddscsi" = "yes" ; then
     echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak
   fi
-  if test "$guest_agent_msi" != "no"; then
+  if test "$guest_agent_msi" = "yes"; then
     echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak  
     echo "QEMU_GA_MSI_MINGW_DLL_PATH=${QEMU_GA_MSI_MINGW_DLL_PATH}" >> $config_host_mak
     echo "QEMU_GA_MSI_WITH_VSS=${QEMU_GA_MSI_WITH_VSS}" >> $config_host_mak
-- 
1.9.1

[Qemu-devel] [PATCH 25/26] build: qemu-ga: fix VSS dependencies

[Michael Roth]

Currently VSS dll/tlb files for use in w32 builds are only built as a
result of having been added to the general 'tools' target alongside
qemu-ga. This is fine for default make target, but if we build
qemu-ga directly via `make qemu-ga.exe`, the VSS files are not
created.

Fix this by moving the VSS dependencies to qemu-ga.exe directly.
With this move we can move the VSS files back out of 'tools',
and drop the extra handling from MSI target in Makefile.

Now we can build qemu-ga MSI package with:
  ./configure ...
  make qemu-ga.exe
  make msi

or simply:
  ./configure ...
  make msi

and no longer need to do a full build beforehand.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile  | 11 +++++------
 configure |  5 ++---
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index c13a83d..dbdeb47 100644
--- a/Makefile
+++ b/Makefile
@@ -290,8 +290,11 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
-	$(call LINK, $^)
+# we require QGA_VSS_PROVIDER files to be built alongside qemu-ga
+# executable since they are shipped together, but we don't want to actually
+# link against them
+qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER)
+	$(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^))
 
 ifdef QEMU_GA_MSI_ENABLED
 QEMU_GA_MSI=qemu-ga-$(ARCH).msi
@@ -300,10 +303,6 @@ msi: $(QEMU_GA_MSI)
 
 $(QEMU_GA_MSI): qemu-ga.exe
 
-ifdef QEMU_GA_MSI_WITH_VSS
-$(QEMU_GA_MSI): qga/vss-win32/qga-vss.dll
-endif
-
 $(QEMU_GA_MSI): config-host.mak
 
 $(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
diff --git a/configure b/configure
index a52ef50..21c4089 100755
--- a/configure
+++ b/configure
@@ -3851,6 +3851,7 @@ EOF
     guest_agent_with_vss="yes"
     QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
     libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ -Wl,--enable-stdcall-fixup $libs_qga"
+    qga_vss_provider="qga/vss-win32/qga-vss.dll qga/vss-win32/qga-vss.tlb"
   else
     if test "$vss_win32_sdk" != "" ; then
       echo "ERROR: Please download and install Microsoft VSS SDK:"
@@ -4350,9 +4351,6 @@ fi
 if [ "$guest_agent" != "no" ]; then
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
       tools="qemu-ga\$(EXESUF) $tools"
-      if [ "$mingw32" = "yes" -a "$guest_agent_with_vss" = "yes" ]; then
-        tools="qga/vss-win32/qga-vss.dll qga/vss-win32/qga-vss.tlb $tools"
-      fi
       guest_agent=yes
   elif [ "$guest_agent" != yes ]; then
       guest_agent=no
@@ -4660,6 +4658,7 @@ if test "$mingw32" = "yes" ; then
   echo "CONFIG_PRODUCTVERSION=$version_major,$version_minor,$version_subminor,$version_micro" >> $config_host_mak
   if test "$guest_agent_with_vss" = "yes" ; then
     echo "CONFIG_QGA_VSS=y" >> $config_host_mak
+    echo "QGA_VSS_PROVIDER=$qga_vss_provider" >> $config_host_mak
     echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak
   fi
   if test "$guest_agent_ntddscsi" = "yes" ; then
-- 
1.9.1

[Qemu-devel] [PATCH 26/26] Makefile: qemu-ga: fix msi target error message

[Michael Roth]

'msi' target reports error if we attempt to use it when QEMU hasn't
been ./configure'd to enable it. The parenthesis cause an interpreter
error if we don't enclose the error in quotes.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dbdeb47..9ce3972 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ $(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
 	wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) $(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
 else
 msi:
-	@echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)
+	@echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)"
 endif
 
 clean:
-- 
1.9.1

Re: [Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[Peter Maydell]

On 1 September 2015 at 18:00, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:
>
>   s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)
>
> are available in the git repository at:
>
>   git://github.com/mdroth/qemu.git tags/qga-pull-2015-09-01-tag
>
> for you to fetch changes up to 9680171bcd317927c0cbaaa291dd9072e79fb32c:
>
>   Makefile: qemu-ga: fix msi target error message (2015-09-01 11:40:35 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue
>
> * add config file dump/load support for qemu-ga
> * various w32 build fixes, particularly WRT to msi package creation
> * fixes for msi installer
> * w32 support for guest-set-user-password

Hi. I'm afraid this fails to compile with our minimum glib version:

/Users/pm215/src/qemu/qga/main.c:1385:5: warning: implicit declaration of
 function 'g_list_free_full' is invalid in C99 [-Wimplicit-function-declaration]
  g_list_free_full(s->blacklist, g_free);

That function wasn't added to glib until 2.28, and our minimum is
2.22.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[Michael Roth]

Quoting Peter Maydell (2015-09-01 12:12:39)
> On 1 September 2015 at 18:00, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:
> >
> >   s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/mdroth/qemu.git tags/qga-pull-2015-09-01-tag
> >
> > for you to fetch changes up to 9680171bcd317927c0cbaaa291dd9072e79fb32c:
> >
> >   Makefile: qemu-ga: fix msi target error message (2015-09-01 11:40:35 -0500)
> >
> > ----------------------------------------------------------------
> > qemu-ga patch queue
> >
> > * add config file dump/load support for qemu-ga
> > * various w32 build fixes, particularly WRT to msi package creation
> > * fixes for msi installer
> > * w32 support for guest-set-user-password
> 
> Hi. I'm afraid this fails to compile with our minimum glib version:
> 
> /Users/pm215/src/qemu/qga/main.c:1385:5: warning: implicit declaration of
>  function 'g_list_free_full' is invalid in C99 [-Wimplicit-function-declaration]
>   g_list_free_full(s->blacklist, g_free);
> 
> That function wasn't added to glib until 2.28, and our minimum is
> 2.22.

Sorry for missing this, will send a v2 shortly.

Do you happen to know a way to check/enforce max version levels for glib
functions? Apparently there's a GLIB_VERSION_MAX_ALLOWED that serves
this purpose, but it didn't generate any warnings in this particular
case:

https://developer.gnome.org/glib/stable/glib-Version-Information.html#GLIB-VERSION-MAX-ALLOWED:CAPS

If we could get that working in tree it might help with these glib compat
issues (which seem to happen fairly frequently)

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[Peter Maydell]

On 1 September 2015 at 19:33, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Quoting Peter Maydell (2015-09-01 12:12:39)
>> That function wasn't added to glib until 2.28, and our minimum is
>> 2.22.
>
> Sorry for missing this, will send a v2 shortly.
>
> Do you happen to know a way to check/enforce max version levels for glib
> functions?

The only way I know that works is "do a test compile against 2.22".

> Apparently there's a GLIB_VERSION_MAX_ALLOWED that serves
> this purpose, but it didn't generate any warnings in this particular
> case:
>
> https://developer.gnome.org/glib/stable/glib-Version-Information.html#GLIB-VERSION-MAX-ALLOWED:CAPS
>
> If we could get that working in tree it might help with these glib compat
> issues (which seem to happen fairly frequently)

David Gilbert had a look at that a little while back, but it's
trickier than it first looks.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/26] qemu-ga patch queue for 2015-09-01

[John Snow]

On 09/01/2015 02:37 PM, Peter Maydell wrote:
> On 1 September 2015 at 19:33, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
>> Quoting Peter Maydell (2015-09-01 12:12:39)
>>> That function wasn't added to glib until 2.28, and our minimum is
>>> 2.22.
>>
>> Sorry for missing this, will send a v2 shortly.
>>
>> Do you happen to know a way to check/enforce max version levels for glib
>> functions?
> 
> The only way I know that works is "do a test compile against 2.22".
> 
>> Apparently there's a GLIB_VERSION_MAX_ALLOWED that serves
>> this purpose, but it didn't generate any warnings in this particular
>> case:
>>
>> https://developer.gnome.org/glib/stable/glib-Version-Information.html#GLIB-VERSION-MAX-ALLOWED:CAPS
>>
>> If we could get that working in tree it might help with these glib compat
>> issues (which seem to happen fairly frequently)
> 
> David Gilbert had a look at that a little while back, but it's
> trickier than it first looks.
> 
> thanks
> -- PMM
> 

I've looked at it before, too. The problem is that we have backported
some glib functionality from beyond 2.22 and these macros will fuss at
our usage of those functions too, IIRC.

We could solve this by *always* using our own version of a glib-provided
function if it's from beyond 2.22 and then torpedoing the glib-compat file.

--js