[Qemu-devel] [PULL 0/7] Qio next patches

[Qemu-devel] [PULL 0/7] Qio next patches

[Daniel P. Berrangé]

The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:

  Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' into staging (2018-02-15 13:00:44 +0000)

are available in the Git repository at:

  ssh://git@github.com/berrange/qemu tags/qio-next-pull-request

for you to fetch changes up to 6809df1df036840d41a0cc9ca77cc6a0214fb1b5:

  allow to build with older sed (2018-02-15 16:54:57 +0000)

----------------------------------------------------------------

----------------------------------------------------------------

Edgar Kaziakhmedov (1):
  io/channel-websock: handle continuous reads without any data

Jan Beulich (1):
  allow to build with older sed

Paolo Bonzini (1):
  io: fix QIONetListener memory leak

Ross Lagerwall (3):
  io: Fix QIOChannelFile when creating and opening read-write
  io: Don't call close multiple times in QIOChannelFile
  io: Add /dev/fdset/ support to QIOChannelFile

Thomas Huth (1):
  io/channel-command: Do not kill the child process after closing the
    pipe

 Makefile                     |  3 +--
 include/io/channel-file.h    |  2 +-
 io/channel-command.c         | 12 +++++++++++-
 io/channel-file.c            | 11 ++++-------
 io/channel-websock.c         |  7 +++++--
 io/net-listener.c            |  1 +
 tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
 7 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.14.3

[Qemu-devel] [PULL 1/7] io: fix QIONetListener memory leak

[Daniel P. Berrangé]

From: Paolo Bonzini <pbonzini@redhat.com>

The sources array does not escape out of qio_net_listener_wait_client, so
we have to free it.

Reported by Coverity.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 io/net-listener.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io/net-listener.c b/io/net-listener.c
index 77a4e2831c..de38dfae99 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -234,6 +234,7 @@ QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
     for (i = 0; i < listener->nsioc; i++) {
         g_source_unref(sources[i]);
     }
+    g_free(sources);
     g_main_loop_unref(loop);
     g_main_context_unref(ctxt);
 
-- 
2.14.3

[Qemu-devel] [PULL 2/7] io/channel-websock: handle continuous reads without any data

[Daniel P. Berrangé]

From: Edgar Kaziakhmedov <edgar.kaziakhmedov@virtuozzo.com>

According to the current implementation of websocket protocol in QEMU,
qio_channel_websock_handshake_io tries to read handshake from the
channel to start communication over socket. But this approach
doesn't cover scenario when socket was closed while handshaking.
Therefore, if G_IO_IN is caught and qio_channel_read returns zero,
error has to be set and connection has to be done.

Such behaviour causes 100% CPU load in main QEMU loop, because main loop
poll continues to receive and handle G_IO_IN events from websocket.

Step to reproduce 100% CPU load:
1) start qemu with the simplest configuration
$ qemu -vnc [::1]:1,websocket=7500
2) open any vnc listener (which doesn't follow websocket
protocol)
$ vncviewer :7500
3) kill listener
4) qemu main thread eats 100% CPU

Signed-off-by: Edgar Kaziakhmedov <edgar.kaziakhmedov@virtuozzo.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 io/channel-websock.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index 7fd6bb68ba..ec48a305f0 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -499,9 +499,12 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
             error_setg(errp,
                        "End of headers not found in first 4096 bytes");
             return 1;
-        } else {
-            return 0;
+        } else if (ret == 0) {
+            error_setg(errp,
+                       "End of headers not found before connection closed");
+            return -1;
         }
+        return 0;
     }
     *handshake_end = '\0';
 
-- 
2.14.3

[Qemu-devel] [PULL 3/7] io: Fix QIOChannelFile when creating and opening read-write

[Daniel P. Berrangé]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

The code wrongly passes the mode to open() only if O_WRONLY is set.
Instead, the mode should be passed when O_CREAT is set (or O_TMPFILE on
Linux). Fix this by always passing the mode since open() will correctly
ignore the mode if it is not needed. Add a testcase which exercises this
bug and also change the existing testcase to check that the mode of the
created file is correct.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/io/channel-file.h    |  2 +-
 io/channel-file.c            |  6 +-----
 tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 79245f1183..ebfe54ec70 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -73,7 +73,7 @@ qio_channel_file_new_fd(int fd);
  * qio_channel_file_new_path:
  * @path: the file path
  * @flags: the open flags (O_RDONLY|O_WRONLY|O_RDWR, etc)
- * @mode: the file creation mode if O_WRONLY is set in @flags
+ * @mode: the file creation mode if O_CREAT is set in @flags
  * @errp: pointer to initialized error object
  *
  * Create a new IO channel object for a file represented
diff --git a/io/channel-file.c b/io/channel-file.c
index b383273201..16bf7ed270 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,11 +50,7 @@ qio_channel_file_new_path(const char *path,
 
     ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
 
-    if (flags & O_WRONLY) {
-        ioc->fd = open(path, flags, mode);
-    } else {
-        ioc->fd = open(path, flags);
-    }
+    ioc->fd = open(path, flags, mode);
     if (ioc->fd < 0) {
         object_unref(OBJECT(ioc));
         error_setg_errno(errp, errno,
diff --git a/tests/test-io-channel-file.c b/tests/test-io-channel-file.c
index 6bfede6bb7..2e94f638f2 100644
--- a/tests/test-io-channel-file.c
+++ b/tests/test-io-channel-file.c
@@ -24,16 +24,21 @@
 #include "io-channel-helpers.h"
 #include "qapi/error.h"
 
-static void test_io_channel_file(void)
+#define TEST_FILE "tests/test-io-channel-file.txt"
+#define TEST_MASK 0600
+
+static void test_io_channel_file_helper(int flags)
 {
     QIOChannel *src, *dst;
     QIOChannelTest *test;
+    struct stat st;
+    mode_t mask;
+    int ret;
 
-#define TEST_FILE "tests/test-io-channel-file.txt"
     unlink(TEST_FILE);
     src = QIO_CHANNEL(qio_channel_file_new_path(
                           TEST_FILE,
-                          O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600,
+                          flags, TEST_MASK,
                           &error_abort));
     dst = QIO_CHANNEL(qio_channel_file_new_path(
                           TEST_FILE,
@@ -45,18 +50,33 @@ static void test_io_channel_file(void)
     qio_channel_test_run_reader(test, dst);
     qio_channel_test_validate(test);
 
+    /* Check that the requested mode took effect. */
+    mask = umask(0);
+    umask(mask);
+    ret = stat(TEST_FILE, &st);
+    g_assert_cmpint(ret, >, -1);
+    g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0777);
+
     unlink(TEST_FILE);
     object_unref(OBJECT(src));
     object_unref(OBJECT(dst));
 }
 
+static void test_io_channel_file(void)
+{
+    test_io_channel_file_helper(O_WRONLY | O_CREAT | O_TRUNC | O_BINARY);
+}
+
+static void test_io_channel_file_rdwr(void)
+{
+    test_io_channel_file_helper(O_RDWR | O_CREAT | O_TRUNC | O_BINARY);
+}
 
 static void test_io_channel_fd(void)
 {
     QIOChannel *ioc;
     int fd = -1;
 
-#define TEST_FILE "tests/test-io-channel-file.txt"
     fd = open(TEST_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
     g_assert_cmpint(fd, >, -1);
 
@@ -114,6 +134,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     g_test_add_func("/io/channel/file", test_io_channel_file);
+    g_test_add_func("/io/channel/file/rdwr", test_io_channel_file_rdwr);
     g_test_add_func("/io/channel/file/fd", test_io_channel_fd);
 #ifndef _WIN32
     g_test_add_func("/io/channel/pipe/sync", test_io_channel_pipe_sync);
-- 
2.14.3

[Qemu-devel] [PULL 4/7] io: Don't call close multiple times in QIOChannelFile

[Daniel P. Berrangé]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

If the file descriptor underlying QIOChannelFile is closed in the
io_close() method, don't close it again in the finalize() method since
the file descriptor number may have been reused in the meantime.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 io/channel-file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io/channel-file.c b/io/channel-file.c
index 16bf7ed270..1f2f710bf9 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -178,6 +178,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
                          "Unable to close file");
         return -1;
     }
+    fioc->fd = -1;
     return 0;
 }
 
-- 
2.14.3

[Qemu-devel] [PULL 5/7] io: Add /dev/fdset/ support to QIOChannelFile

[Daniel P. Berrangé]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
of open() and qemu_close() instead of close(). There is a subtle
semantic change since qemu_open() automatically sets O_CLOEXEC, but this
doesn't affect any of the users of the function.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 io/channel-file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/io/channel-file.c b/io/channel-file.c
index 1f2f710bf9..db948abc3e 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,7 +50,7 @@ qio_channel_file_new_path(const char *path,
 
     ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
 
-    ioc->fd = open(path, flags, mode);
+    ioc->fd = qemu_open(path, flags, mode);
     if (ioc->fd < 0) {
         object_unref(OBJECT(ioc));
         error_setg_errno(errp, errno,
@@ -74,7 +74,7 @@ static void qio_channel_file_finalize(Object *obj)
 {
     QIOChannelFile *ioc = QIO_CHANNEL_FILE(obj);
     if (ioc->fd != -1) {
-        close(ioc->fd);
+        qemu_close(ioc->fd);
         ioc->fd = -1;
     }
 }
@@ -173,7 +173,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
 {
     QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
 
-    if (close(fioc->fd) < 0) {
+    if (qemu_close(fioc->fd) < 0) {
         error_setg_errno(errp, errno,
                          "Unable to close file");
         return -1;
-- 
2.14.3

[Qemu-devel] [PULL 6/7] io/channel-command: Do not kill the child process after closing the pipe

[Daniel P. Berrangé]

From: Thomas Huth <thuth@redhat.com>

We are currently facing some migration failure on s390x when running
certain avocado-vt tests, e.g. when running the test
type_specific.io-github-autotest-qemu.migrate.with_reboot.exec.gzip_exec.
This test is using 'migrate -d "exec:nc localhost 5200"' for the migration.
The problem is detected at the receiving side, where the migration stream
apparently ends too early. However, the cause for the problem is at the
sending side: After writing the migration stream into the pipe to netcat,
the source QEMU calls qio_channel_command_close() which closes the pipe
and immediately (!) kills the child process afterwards (via the function
qio_channel_command_abort()). So if the  sending netcat did not read the
final bytes from the pipe yet, or  if it did not manage to send out all
its buffers yet, it is killed before the whole migration stream is passed
to the destination side.

QEMU can not know how much time is required by the child process to send
over all migration data, so we should not kill it, neither directly nor
after a delay. Let's simply wait for the child process to exit gracefully
instead (this was also the behaviour of pclose() that was used in "exec:"
migration before the QIOChannel rework).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 io/channel-command.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/io/channel-command.c b/io/channel-command.c
index 319c5ed50c..3e7eb17eff 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -301,6 +301,9 @@ static int qio_channel_command_close(QIOChannel *ioc,
 {
     QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc);
     int rv = 0;
+#ifndef WIN32
+    pid_t wp;
+#endif
 
     /* We close FDs before killing, because that
      * gives a better chance of clean shutdown
@@ -315,11 +318,18 @@ static int qio_channel_command_close(QIOChannel *ioc,
         rv = -1;
     }
     cioc->writefd = cioc->readfd = -1;
+
 #ifndef WIN32
-    if (qio_channel_command_abort(cioc, errp) < 0) {
+    do {
+        wp = waitpid(cioc->pid, NULL, 0);
+    } while (wp == (pid_t)-1 && errno == EINTR);
+    if (wp == (pid_t)-1) {
+        error_setg_errno(errp, errno, "Failed to wait for pid %llu",
+                         (unsigned long long)cioc->pid);
         return -1;
     }
 #endif
+
     if (rv < 0) {
         error_setg_errno(errp, errno, "%s",
                          "Unable to close command");
-- 
2.14.3

[Qemu-devel] [PULL 7/7] allow to build with older sed

[Daniel P. Berrangé]

From: Jan Beulich <JBeulich@suse.com>

sed's -E option may not be supported by older distros. As there's no
point using sed here at all, use just shell mechanisms to establish the
variable values, starting from the stem instead of the full target.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b5a6d602b2..90e05ac409 100644
--- a/Makefile
+++ b/Makefile
@@ -256,8 +256,7 @@ GENERATED_FILES += $(KEYCODEMAP_FILES)
 
 ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) $(SRC_PATH)/ui/Makefile.objs
 	$(call quiet-command,\
-	    src=$$(echo $@ | sed -E -e "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\1,") && \
-	    dst=$$(echo $@ | sed -E -e "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\2,") && \
+	    stem=$* && src=$${stem%-to-*} dst=$${stem#*-to-} && \
 	    test -e $(KEYCODEMAP_GEN) && \
 	    $(PYTHON) $(KEYCODEMAP_GEN) \
 	          --lang glib2 \
-- 
2.14.3

Re: [Qemu-devel] [PULL 0/7] Qio next patches

[Peter Maydell]

On 15 February 2018 at 17:50, Daniel P. Berrangé <berrange@redhat.com> wrote:
> The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:
>
>   Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' into staging (2018-02-15 13:00:44 +0000)
>
> are available in the Git repository at:
>
>   ssh://git@github.com/berrange/qemu tags/qio-next-pull-request

This isn't a publicly usable git reference; did you mean
   git://github.com/berrange/qemu.git tags/qio-next-pull-request

? (I'm testing the pullreq on the assumption that you did.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Qio next patches

[Daniel P. Berrangé]

On Thu, Feb 15, 2018 at 07:43:39PM +0000, Peter Maydell wrote:
> On 15 February 2018 at 17:50, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:
> >
> >   Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' into staging (2018-02-15 13:00:44 +0000)
> >
> > are available in the Git repository at:
> >
> >   ssh://git@github.com/berrange/qemu tags/qio-next-pull-request
> 
> This isn't a publicly usable git reference; did you mean
>    git://github.com/berrange/qemu.git tags/qio-next-pull-request
> 
> ? (I'm testing the pullreq on the assumption that you did.)

Sigh, yes. This is my first time trying git-publish, and I guess I missed
some key config parameter, to distinguish the URL it must push too, vs
the URL it must document in the cover letter.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 0/7] Qio next patches

[Peter Maydell]

On 15 February 2018 at 17:50, Daniel P. Berrangé <berrange@redhat.com> wrote:
> The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:
>
>   Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' into staging (2018-02-15 13:00:44 +0000)
>
> are available in the Git repository at:
>
>   ssh://git@github.com/berrange/qemu tags/qio-next-pull-request
>
> for you to fetch changes up to 6809df1df036840d41a0cc9ca77cc6a0214fb1b5:
>
>   allow to build with older sed (2018-02-15 16:54:57 +0000)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM