[Qemu-devel] [PATCH 0/4] QGA - Win fixes

[Qemu-devel] [PATCH 0/4] QGA - Win fixes

[Basil Salman]

This patch series addresses serveral issues with qga-win please review
them and share your thoughts.

Basil Salman (3):
  qga-win: prevent crash when executing guest-file-read with large count
  qga: fix send_response error handling
  qga: Installer: Wait for installation to finish

Sameeh Jubran (1):
  qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error

 qga/commands-win32.c      |  8 +++++++-
 qga/installer/qemu-ga.wxs |  2 +-
 qga/main.c                | 13 +++++++++++++
 qga/vss-win32/install.cpp | 10 ++++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH 1/4] qga-win: prevent crash when executing guest-file-read with large count

[Basil Salman]

BZ: #1594054
guest-file-read command is currently implelmented to read from a
file handle count number of bytes. when executed with a very large count number
qemu-ga crashes.
after some digging turns out that qemu-ga crashes after trying to allocate
a buffer large enough to save the data read in it, the buffer was allocated using
g_malloc0 which is not fail safe, and results a crash in case of failure.
g_malloc0 was replaced with g_try_malloc0() which returns NULL on failure,
A check was added for that case in order to prevent qemu-ga from crashing
and to send a response to the qemu-ga client accordingly.

Signed-off-by: Basil Salman <basil@daynix.com>
---
 qga/commands-win32.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 62e1b51dfe..4260faa573 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -345,7 +345,13 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
     }
 
     fh = gfh->fh;
-    buf = g_malloc0(count+1);
+    buf = g_try_malloc0(count + 1);
+    if (!buf) {
+        error_setg(errp,
+                   "failed to allocate sufficient memory"
+                   "to complete the requested service");
+        return read_data;
+    }
     is_ok = ReadFile(fh, buf, count, &read_count, NULL);
     if (!is_ok) {
         error_setg_win32(errp, GetLastError(), "failed to read file");
-- 
2.17.2

[Qemu-devel] [PATCH 2/4] qga: fix send_response error handling

[Basil Salman]

Sometimes qemu-ga fails to send a response to client due to memory allocation
issues due to a large response message, this can be experienced while trying
to read large number of bytes using QMP command guest-file-read.

Added a check to send an error response to qemu-ga client in such cases.

Signed-off-by: Basil Salman <basil@daynix.com>
---
 qga/main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/qga/main.c b/qga/main.c
index 87a0711c14..964275c40c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -561,6 +561,8 @@ static void process_command(GAState *s, QDict *req)
 {
     QDict *rsp;
     int ret;
+    QDict *ersp;
+    Error *err = NULL;
 
     g_assert(req);
     g_debug("processing command");
@@ -569,9 +571,20 @@ static void process_command(GAState *s, QDict *req)
         ret = send_response(s, rsp);
         if (ret < 0) {
             g_warning("error sending response: %s", strerror(-ret));
+            goto err;
         }
         qobject_unref(rsp);
     }
+    return;
+err:
+    error_setg(&err, "Insufficient system resources exist to "
+                      "complete the requested service");
+    ersp = qmp_error_response(err);
+    ret = send_response(s, ersp);
+    if (ret < 0) {
+        g_warning("error sending error response: %s", strerror(-ret));
+    }
+    qobject_unref(ersp);
 }
 
 /* handle requests/control events coming in over the channel */
-- 
2.17.2

[Qemu-devel] [PATCH 3/4] qga: Installer: Wait for installation to finish

[Basil Salman]

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

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

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

[Qemu-devel] [PATCH 4/4] qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error

[Basil Salman]

From: Sameeh Jubran <sjubran@redhat.com>

This patch handles the case where VSS Provider is already registered,
where in such case qga uninstalls the provider and registers it again.

Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
---
 qga/vss-win32/install.cpp | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index 6713e58670..28deeb9f0e 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -443,6 +443,16 @@ STDAPI DllRegisterServer(void)
                                      VSS_PROV_SOFTWARE,
                                      const_cast<WCHAR*>(QGA_PROVIDER_VERSION),
                                      g_gProviderVersion);
+    if (hr == (long int) VSS_E_PROVIDER_ALREADY_REGISTERED)
+    {
+        DllUnregisterServer();
+        hr = pVssAdmin->RegisterProvider(g_gProviderId, CLSID_QGAVSSProvider,
+                                         const_cast<WCHAR*>(QGA_PROVIDER_LNAME),
+                                         VSS_PROV_SOFTWARE,
+                                         const_cast<WCHAR*>(QGA_PROVIDER_VERSION),
+                                         g_gProviderVersion);
+    }
+
     if (FAILED(hr)) {
         errmsg_dialog(hr, "RegisterProvider failed");
     }
-- 
2.17.2

Re: [Qemu-devel] [PATCH 0/4] QGA - Win fixes

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190103122323.1273034-1-basil@daynix.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190103122323.1273034-1-basil@daynix.com
Subject: [Qemu-devel] [PATCH 0/4] QGA - Win fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e24fda7 qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error
62ac12a qga: Installer: Wait for installation to finish
0cb3e6e qga: fix send_response error handling
589af7c qga-win: prevent crash when executing guest-file-read with large count

=== OUTPUT BEGIN ===
Checking PATCH 1/4: qga-win: prevent crash when executing guest-file-read with large count...
Checking PATCH 2/4: qga: fix send_response error handling...
Checking PATCH 3/4: qga: Installer: Wait for installation to finish...
Checking PATCH 4/4: qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error...
ERROR: that open brace { should be on the previous line
#21: FILE: qga/vss-win32/install.cpp:446:
+    if (hr == (long int) VSS_E_PROVIDER_ALREADY_REGISTERED)
+    {

ERROR: spaces required around that '*' (ctx:VxO)
#25: FILE: qga/vss-win32/install.cpp:450:
+                                         const_cast<WCHAR*>(QGA_PROVIDER_LNAME),
                                                          ^

WARNING: line over 80 characters
#27: FILE: qga/vss-win32/install.cpp:452:
+                                         const_cast<WCHAR*>(QGA_PROVIDER_VERSION),

ERROR: spaces required around that '*' (ctx:VxO)
#27: FILE: qga/vss-win32/install.cpp:452:
+                                         const_cast<WCHAR*>(QGA_PROVIDER_VERSION),
                                                          ^

total: 3 errors, 1 warnings, 16 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190103122323.1273034-1-basil@daynix.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com