qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] console: screendump improvements
@ 2019-11-27 11:51 Marc-André Lureau
  2019-11-27 11:51 ` [PATCH 1/7] console: add graphic_hw_update_done() Marc-André Lureau
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Hi,

The following patches have been extracted from the "[PATCH v6 00/25]
monitor: add asynchronous command type", as they are
reviewable/mergeable independantly.

They introduce some internal API changes, and fix
qemu_open()/qemu_close()/unlink() misusages which should be quite
harmless.

Marc-André Lureau (7):
  console: add graphic_hw_update_done()
  ppm-save: pass opened fd
  ui: add pixman image g_autoptr support
  object: add g_autoptr support
  screendump: replace FILE with QIOChannel and fix close()/qemu_close()
  osdep: add qemu_unlink()
  screendump: use qemu_unlink()

 hw/display/qxl-render.c  |  9 +++--
 hw/display/qxl.c         |  1 +
 include/qemu/osdep.h     |  1 +
 include/qom/object.h     |  3 ++
 include/ui/console.h     |  2 ++
 include/ui/qemu-pixman.h |  2 ++
 ui/console.c             | 74 +++++++++++++++++++++-------------------
 ui/trace-events          |  2 +-
 util/osdep.c             | 15 ++++++++
 9 files changed, 71 insertions(+), 38 deletions(-)

-- 
2.24.0



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] console: add graphic_hw_update_done()
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
@ 2019-11-27 11:51 ` Marc-André Lureau
  2019-11-28 14:34   ` Daniel P. Berrangé
  2019-11-27 11:51 ` [PATCH 2/7] ppm-save: pass opened fd Marc-André Lureau
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Add a function to be called when a graphic update is done.

Declare the QXL renderer as async: render_update_cookie_num counts the
number of outstanding updates, and graphic_hw_update_done() is called
when it reaches none.

(note: this is preliminary work for asynchronous screendump support)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/qxl-render.c | 9 +++++++--
 hw/display/qxl.c        | 1 +
 include/ui/console.h    | 2 ++
 ui/console.c            | 9 +++++++++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f7fdc4901e..3ce2e57b8f 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -109,7 +109,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
                                                 qxl->guest_primary.surface.mem,
                                                 MEMSLOT_GROUP_GUEST);
         if (!qxl->guest_primary.data) {
-            return;
+            goto end;
         }
         qxl_set_rect_to_surface(qxl, &qxl->dirty[0]);
         qxl->num_dirty_rects = 1;
@@ -137,7 +137,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
     }
 
     if (!qxl->guest_primary.data) {
-        return;
+        goto end;
     }
     for (i = 0; i < qxl->num_dirty_rects; i++) {
         if (qemu_spice_rect_is_empty(qxl->dirty+i)) {
@@ -158,6 +158,11 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
                        qxl->dirty[i].bottom - qxl->dirty[i].top);
     }
     qxl->num_dirty_rects = 0;
+
+end:
+    if (qxl->render_update_cookie_num == 0) {
+        graphic_hw_update_done(qxl->ssd.dcl.con);
+    }
 }
 
 /*
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index cd7eb39d20..6d43b7433c 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1181,6 +1181,7 @@ static const QXLInterface qxl_interface = {
 
 static const GraphicHwOps qxl_ops = {
     .gfx_update  = qxl_hw_update,
+    .gfx_update_async = true,
 };
 
 static void qxl_enter_vga_mode(PCIQXLDevice *d)
diff --git a/include/ui/console.h b/include/ui/console.h
index f981696848..281f9c145b 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -365,6 +365,7 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 typedef struct GraphicHwOps {
     void (*invalidate)(void *opaque);
     void (*gfx_update)(void *opaque);
+    bool gfx_update_async; /* if true, calls graphic_hw_update_done() */
     void (*text_update)(void *opaque, console_ch_t *text);
     void (*update_interval)(void *opaque, uint64_t interval);
     int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info);
@@ -380,6 +381,7 @@ void graphic_console_set_hwops(QemuConsole *con,
 void graphic_console_close(QemuConsole *con);
 
 void graphic_hw_update(QemuConsole *con);
+void graphic_hw_update_done(QemuConsole *con);
 void graphic_hw_invalidate(QemuConsole *con);
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
 void graphic_hw_gl_block(QemuConsole *con, bool block);
diff --git a/ui/console.c b/ui/console.c
index 82d1ddac9c..3c941528d2 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -259,13 +259,22 @@ static void gui_setup_refresh(DisplayState *ds)
     ds->have_text = have_text;
 }
 
+void graphic_hw_update_done(QemuConsole *con)
+{
+}
+
 void graphic_hw_update(QemuConsole *con)
 {
+    bool async = false;
     if (!con) {
         con = active_console;
     }
     if (con && con->hw_ops->gfx_update) {
         con->hw_ops->gfx_update(con->hw);
+        async = con->hw_ops->gfx_update_async;
+    }
+    if (!async) {
+        graphic_hw_update_done(con);
     }
 }
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] ppm-save: pass opened fd
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
  2019-11-27 11:51 ` [PATCH 1/7] console: add graphic_hw_update_done() Marc-André Lureau
@ 2019-11-27 11:51 ` Marc-André Lureau
  2019-12-02 11:40   ` Daniel P. Berrangé
  2019-11-27 11:51 ` [PATCH 3/7] ui: add pixman image g_autoptr support Marc-André Lureau
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

This will allow to pre-open the file before running the async finish
handler and avoid potential monitor fdset races.

(note: this is preliminary work for asynchronous screendump support)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c    | 45 ++++++++++++++++++++++-----------------------
 ui/trace-events |  2 +-
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 3c941528d2..77d62fe76d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -193,6 +193,7 @@ static void dpy_refresh(DisplayState *s);
 static DisplayState *get_alloc_displaystate(void);
 static void text_console_update_cursor_timer(void);
 static void text_console_update_cursor(void *opaque);
+static bool ppm_save(int fd, DisplaySurface *ds, Error **errp);
 
 static void gui_update(void *opaque)
 {
@@ -308,29 +309,22 @@ void graphic_hw_invalidate(QemuConsole *con)
     }
 }
 
-static void ppm_save(const char *filename, DisplaySurface *ds,
-                     Error **errp)
+static bool ppm_save(int fd, DisplaySurface *ds, Error **errp)
 {
     int width = pixman_image_get_width(ds->image);
     int height = pixman_image_get_height(ds->image);
-    int fd;
     FILE *f;
     int y;
     int ret;
     pixman_image_t *linebuf;
+    bool success = false;
 
-    trace_ppm_save(filename, ds);
-    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
-    if (fd == -1) {
-        error_setg(errp, "failed to open file '%s': %s", filename,
-                   strerror(errno));
-        return;
-    }
+    trace_ppm_save(fd, ds);
     f = fdopen(fd, "wb");
     ret = fprintf(f, "P6\n%d %d\n%d\n", width, height, 255);
     if (ret < 0) {
         linebuf = NULL;
-        goto write_err;
+        goto end;
     }
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
     for (y = 0; y < height; y++) {
@@ -339,21 +333,16 @@ static void ppm_save(const char *filename, DisplaySurface *ds,
         ret = fwrite(pixman_image_get_data(linebuf), 1,
                      pixman_image_get_stride(linebuf), f);
         (void)ret;
-        if (ferror(f)) {
-            goto write_err;
-        }
+        success = !ferror(f);
     }
 
-out:
+end:
+    if (!success) {
+        error_setg(errp, "failed to write to PPM file: %s", strerror(errno));
+    }
     qemu_pixman_image_unref(linebuf);
     fclose(f);
-    return;
-
-write_err:
-    error_setg(errp, "failed to write to file '%s': %s", filename,
-               strerror(errno));
-    unlink(filename);
-    goto out;
+    return success;
 }
 
 void qmp_screendump(const char *filename, bool has_device, const char *device,
@@ -361,6 +350,7 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
 {
     QemuConsole *con;
     DisplaySurface *surface;
+    int fd;
 
     if (has_device) {
         con = qemu_console_lookup_by_device_name(device, has_head ? head : 0,
@@ -387,7 +377,16 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
         return;
     }
 
-    ppm_save(filename, surface, errp);
+    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
+    if (fd == -1) {
+        error_setg(errp, "failed to open file '%s': %s", filename,
+                   strerror(errno));
+        return;
+    }
+
+    if (!ppm_save(fd, surface, errp)) {
+        unlink(filename);
+    }
 }
 
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata)
diff --git a/ui/trace-events b/ui/trace-events
index 63de72a798..0dcda393c1 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -15,7 +15,7 @@ displaysurface_create_pixman(void *display_surface) "surface=%p"
 displaysurface_free(void *display_surface) "surface=%p"
 displaychangelistener_register(void *dcl, const char *name) "%p [ %s ]"
 displaychangelistener_unregister(void *dcl, const char *name) "%p [ %s ]"
-ppm_save(const char *filename, void *display_surface) "%s surface=%p"
+ppm_save(int fd, void *display_surface) "fd=%d surface=%p"
 
 # gtk.c
 # gtk-gl-area.c
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] ui: add pixman image g_autoptr support
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
  2019-11-27 11:51 ` [PATCH 1/7] console: add graphic_hw_update_done() Marc-André Lureau
  2019-11-27 11:51 ` [PATCH 2/7] ppm-save: pass opened fd Marc-André Lureau
@ 2019-11-27 11:51 ` Marc-André Lureau
  2019-12-02 11:41   ` Daniel P. Berrangé
  2019-11-27 11:51 ` [PATCH 4/7] object: add " Marc-André Lureau
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/qemu-pixman.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 0668109305..3b7cf70157 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -90,4 +90,6 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
                               pixman_color_t *bgcol,
                               int x, int y, int cw, int ch);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
+
 #endif /* QEMU_PIXMAN_H */
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] object: add g_autoptr support
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (2 preceding siblings ...)
  2019-11-27 11:51 ` [PATCH 3/7] ui: add pixman image g_autoptr support Marc-André Lureau
@ 2019-11-27 11:51 ` Marc-André Lureau
  2019-12-02 11:41   ` Daniel P. Berrangé
  2019-11-27 11:52 ` [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close() Marc-André Lureau
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qom/object.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 128d00c77f..f96a44be64 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1747,4 +1747,7 @@ Object *container_get(Object *root, const char *path);
  * Returns the instance_size of the given @typename.
  */
 size_t object_type_get_instance_size(const char *typename);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Object, object_unref)
+
 #endif
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close()
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (3 preceding siblings ...)
  2019-11-27 11:51 ` [PATCH 4/7] object: add " Marc-André Lureau
@ 2019-11-27 11:52 ` Marc-André Lureau
  2019-12-02 11:45   ` Daniel P. Berrangé
  2019-11-27 11:52 ` [PATCH 6/7] osdep: add qemu_unlink() Marc-André Lureau
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

The file opened for ppm_save() may be a /dev/fdset, in which case a
dup fd is added to the fdset. It should be removed by calling
qemu_close(), instead of the implicit close() on fclose().

I don't see a convenient way to solve that with stdio streams, so I
switched the code to QIOChannel which uses qemu_close().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 77d62fe76d..587edf4ed4 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -33,6 +33,7 @@
 #include "chardev/char-fe.h"
 #include "trace.h"
 #include "exec/memory.h"
+#include "io/channel-file.h"
 
 #define DEFAULT_BACKSCROLL 512
 #define CONSOLE_CURSOR_PERIOD 500
@@ -313,36 +314,31 @@ static bool ppm_save(int fd, DisplaySurface *ds, Error **errp)
 {
     int width = pixman_image_get_width(ds->image);
     int height = pixman_image_get_height(ds->image);
-    FILE *f;
+    g_autoptr(Object) ioc = OBJECT(qio_channel_file_new_fd(fd));
+    g_autofree char *header = NULL;
+    g_autoptr(pixman_image_t) linebuf = NULL;
+    g_autoptr(GError) error = NULL;
     int y;
-    int ret;
-    pixman_image_t *linebuf;
-    bool success = false;
 
     trace_ppm_save(fd, ds);
-    f = fdopen(fd, "wb");
-    ret = fprintf(f, "P6\n%d %d\n%d\n", width, height, 255);
-    if (ret < 0) {
-        linebuf = NULL;
-        goto end;
+
+    header = g_strdup_printf("P6\n%d %d\n%d\n", width, height, 255);
+    if (qio_channel_write_all(QIO_CHANNEL(ioc),
+                              header, strlen(header), errp) < 0) {
+        return false;
     }
+
     linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
     for (y = 0; y < height; y++) {
         qemu_pixman_linebuf_fill(linebuf, ds->image, width, 0, y);
-        clearerr(f);
-        ret = fwrite(pixman_image_get_data(linebuf), 1,
-                     pixman_image_get_stride(linebuf), f);
-        (void)ret;
-        success = !ferror(f);
+        if (qio_channel_write_all(QIO_CHANNEL(ioc),
+                                  (char *)pixman_image_get_data(linebuf),
+                                  pixman_image_get_stride(linebuf), errp) < 0) {
+            return false;
+        }
     }
 
-end:
-    if (!success) {
-        error_setg(errp, "failed to write to PPM file: %s", strerror(errno));
-    }
-    qemu_pixman_image_unref(linebuf);
-    fclose(f);
-    return success;
+    return true;
 }
 
 void qmp_screendump(const char *filename, bool has_device, const char *device,
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] osdep: add qemu_unlink()
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (4 preceding siblings ...)
  2019-11-27 11:52 ` [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close() Marc-André Lureau
@ 2019-11-27 11:52 ` Marc-André Lureau
  2019-12-02 11:46   ` Daniel P. Berrangé
  2019-11-27 11:52 ` [PATCH 7/7] screendump: use qemu_unlink() Marc-André Lureau
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Add a helper function to match qemu_open() which may return files
under the /dev/fdset prefix. Those shouldn't be removed, since it's
only a qemu namespace.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/osdep.h |  1 +
 util/osdep.c         | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0f97d68586..9bd3dcfd13 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -462,6 +462,7 @@ int qemu_mprotect_none(void *addr, size_t size);
 
 int qemu_open(const char *name, int flags, ...);
 int qemu_close(int fd);
+int qemu_unlink(const char *name);
 #ifndef _WIN32
 int qemu_dup(int fd);
 #endif
diff --git a/util/osdep.c b/util/osdep.c
index 3f04326040..f7d06050f7 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -370,6 +370,21 @@ int qemu_close(int fd)
     return close(fd);
 }
 
+/*
+ * Delete a file from the filesystem, unless the filename is /dev/fdset/...
+ *
+ * Returns: On success, zero is returned.  On error, -1 is returned,
+ * and errno is set appropriately.
+ */
+int qemu_unlink(const char *name)
+{
+    if (g_str_has_prefix(name, "/dev/fdset/")) {
+        return 0;
+    }
+
+    return unlink(name);
+}
+
 /*
  * A variant of write(2) which handles partial write.
  *
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] screendump: use qemu_unlink()
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (5 preceding siblings ...)
  2019-11-27 11:52 ` [PATCH 6/7] osdep: add qemu_unlink() Marc-André Lureau
@ 2019-11-27 11:52 ` Marc-André Lureau
  2019-12-02 11:49   ` Daniel P. Berrangé
  2019-11-27 12:25 ` [PATCH 0/7] console: screendump improvements no-reply
  2019-12-20 14:36 ` Marc-André Lureau
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-11-27 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	Eduardo Habkost, Marc-André Lureau, Gerd Hoffmann

Don't attempt to remove /dev/fdset files.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/console.c b/ui/console.c
index 587edf4ed4..e6ac462aa0 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -381,7 +381,7 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
     }
 
     if (!ppm_save(fd, surface, errp)) {
-        unlink(filename);
+        qemu_unlink(filename);
     }
 }
 
-- 
2.24.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] console: screendump improvements
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (6 preceding siblings ...)
  2019-11-27 11:52 ` [PATCH 7/7] screendump: use qemu_unlink() Marc-André Lureau
@ 2019-11-27 12:25 ` no-reply
  2019-12-20 14:36 ` Marc-André Lureau
  8 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2019-11-27 12:25 UTC (permalink / raw)
  To: marcandre.lureau
  Cc: berrange, ehabkost, qemu-devel, kraxel, marcandre.lureau, pbonzini

Patchew URL: https://patchew.org/QEMU/20191127115202.375107-1-marcandre.lureau@redhat.com/



Hi,

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

Subject: [PATCH 0/7] console: screendump improvements
Type: series
Message-id: 20191127115202.375107-1-marcandre.lureau@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: git fetch_pack: expected ACK/NAK, got 'ERR upload-pack: not our ref 4aa8cc3583eabafc01c50020f7887c3509a8f0c4'
fatal: The remote end hung up unexpectedly
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 521, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo
    stdout=logf, stderr=logf)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1.



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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] console: add graphic_hw_update_done()
  2019-11-27 11:51 ` [PATCH 1/7] console: add graphic_hw_update_done() Marc-André Lureau
@ 2019-11-28 14:34   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-11-28 14:34 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:51:56PM +0400, Marc-André Lureau wrote:
> Add a function to be called when a graphic update is done.
> 
> Declare the QXL renderer as async: render_update_cookie_num counts the
> number of outstanding updates, and graphic_hw_update_done() is called
> when it reaches none.
> 
> (note: this is preliminary work for asynchronous screendump support)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/qxl-render.c | 9 +++++++--
>  hw/display/qxl.c        | 1 +
>  include/ui/console.h    | 2 ++
>  ui/console.c            | 9 +++++++++
>  4 files changed, 19 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] ppm-save: pass opened fd
  2019-11-27 11:51 ` [PATCH 2/7] ppm-save: pass opened fd Marc-André Lureau
@ 2019-12-02 11:40   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:40 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:51:57PM +0400, Marc-André Lureau wrote:
> This will allow to pre-open the file before running the async finish
> handler and avoid potential monitor fdset races.
> 
> (note: this is preliminary work for asynchronous screendump support)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c    | 45 ++++++++++++++++++++++-----------------------
>  ui/trace-events |  2 +-
>  2 files changed, 23 insertions(+), 24 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/7] ui: add pixman image g_autoptr support
  2019-11-27 11:51 ` [PATCH 3/7] ui: add pixman image g_autoptr support Marc-André Lureau
@ 2019-12-02 11:41   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:41 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:51:58PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/qemu-pixman.h | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/7] object: add g_autoptr support
  2019-11-27 11:51 ` [PATCH 4/7] object: add " Marc-André Lureau
@ 2019-12-02 11:41   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:41 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:51:59PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qom/object.h | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close()
  2019-11-27 11:52 ` [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close() Marc-André Lureau
@ 2019-12-02 11:45   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:45 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:52:00PM +0400, Marc-André Lureau wrote:
> The file opened for ppm_save() may be a /dev/fdset, in which case a
> dup fd is added to the fdset. It should be removed by calling
> qemu_close(), instead of the implicit close() on fclose().
> 
> I don't see a convenient way to solve that with stdio streams, so I
> switched the code to QIOChannel which uses qemu_close().

The only way is to duplicate what qemu_close() does in the
ppm_save method by calling  monitor_fdset_dup_fd_remove(fd).
Using QIOChannel is fine too though.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/7] osdep: add qemu_unlink()
  2019-11-27 11:52 ` [PATCH 6/7] osdep: add qemu_unlink() Marc-André Lureau
@ 2019-12-02 11:46   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:46 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:52:01PM +0400, Marc-André Lureau wrote:
> Add a helper function to match qemu_open() which may return files
> under the /dev/fdset prefix. Those shouldn't be removed, since it's
> only a qemu namespace.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/osdep.h |  1 +
>  util/osdep.c         | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/7] screendump: use qemu_unlink()
  2019-11-27 11:52 ` [PATCH 7/7] screendump: use qemu_unlink() Marc-André Lureau
@ 2019-12-02 11:49   ` Daniel P. Berrangé
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrangé @ 2019-12-02 11:49 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Paolo Bonzini, Gerd Hoffmann, qemu-devel, Eduardo Habkost

On Wed, Nov 27, 2019 at 03:52:02PM +0400, Marc-André Lureau wrote:
> Don't attempt to remove /dev/fdset files.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] console: screendump improvements
  2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
                   ` (7 preceding siblings ...)
  2019-11-27 12:25 ` [PATCH 0/7] console: screendump improvements no-reply
@ 2019-12-20 14:36 ` Marc-André Lureau
  2020-01-02  9:52   ` Gerd Hoffmann
  8 siblings, 1 reply; 18+ messages in thread
From: Marc-André Lureau @ 2019-12-20 14:36 UTC (permalink / raw)
  To: QEMU, Gerd Hoffmann; +Cc: Daniel P. Berrangé

Hi Gerd,

On Wed, Nov 27, 2019 at 3:52 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi,
>
> The following patches have been extracted from the "[PATCH v6 00/25]
> monitor: add asynchronous command type", as they are
> reviewable/mergeable independantly.
>
> They introduce some internal API changes, and fix
> qemu_open()/qemu_close()/unlink() misusages which should be quite
> harmless.
>
> Marc-André Lureau (7):
>   console: add graphic_hw_update_done()
>   ppm-save: pass opened fd
>   ui: add pixman image g_autoptr support
>   object: add g_autoptr support
>   screendump: replace FILE with QIOChannel and fix close()/qemu_close()
>   osdep: add qemu_unlink()
>   screendump: use qemu_unlink()

The series has been reviewed by Daniel. Can I send a PR or do you plan
to take a look and make the PR yourself?

thanks

>
>  hw/display/qxl-render.c  |  9 +++--
>  hw/display/qxl.c         |  1 +
>  include/qemu/osdep.h     |  1 +
>  include/qom/object.h     |  3 ++
>  include/ui/console.h     |  2 ++
>  include/ui/qemu-pixman.h |  2 ++
>  ui/console.c             | 74 +++++++++++++++++++++-------------------
>  ui/trace-events          |  2 +-
>  util/osdep.c             | 15 ++++++++
>  9 files changed, 71 insertions(+), 38 deletions(-)
>
> --
> 2.24.0
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] console: screendump improvements
  2019-12-20 14:36 ` Marc-André Lureau
@ 2020-01-02  9:52   ` Gerd Hoffmann
  0 siblings, 0 replies; 18+ messages in thread
From: Gerd Hoffmann @ 2020-01-02  9:52 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Daniel P. Berrangé, QEMU

On Fri, Dec 20, 2019 at 06:36:41PM +0400, Marc-André Lureau wrote:
> Hi Gerd,
> 
> On Wed, Nov 27, 2019 at 3:52 PM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
> >
> > Hi,
> >
> > The following patches have been extracted from the "[PATCH v6 00/25]
> > monitor: add asynchronous command type", as they are
> > reviewable/mergeable independantly.
> >
> > They introduce some internal API changes, and fix
> > qemu_open()/qemu_close()/unlink() misusages which should be quite
> > harmless.
> >
> > Marc-André Lureau (7):
> >   console: add graphic_hw_update_done()
> >   ppm-save: pass opened fd
> >   ui: add pixman image g_autoptr support
> >   object: add g_autoptr support
> >   screendump: replace FILE with QIOChannel and fix close()/qemu_close()
> >   osdep: add qemu_unlink()
> >   screendump: use qemu_unlink()
> 
> The series has been reviewed by Daniel. Can I send a PR or do you plan
> to take a look and make the PR yourself?

Looks all sane.  Feel free to send a PR.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-01-02  9:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 11:51 [PATCH 0/7] console: screendump improvements Marc-André Lureau
2019-11-27 11:51 ` [PATCH 1/7] console: add graphic_hw_update_done() Marc-André Lureau
2019-11-28 14:34   ` Daniel P. Berrangé
2019-11-27 11:51 ` [PATCH 2/7] ppm-save: pass opened fd Marc-André Lureau
2019-12-02 11:40   ` Daniel P. Berrangé
2019-11-27 11:51 ` [PATCH 3/7] ui: add pixman image g_autoptr support Marc-André Lureau
2019-12-02 11:41   ` Daniel P. Berrangé
2019-11-27 11:51 ` [PATCH 4/7] object: add " Marc-André Lureau
2019-12-02 11:41   ` Daniel P. Berrangé
2019-11-27 11:52 ` [PATCH 5/7] screendump: replace FILE with QIOChannel and fix close()/qemu_close() Marc-André Lureau
2019-12-02 11:45   ` Daniel P. Berrangé
2019-11-27 11:52 ` [PATCH 6/7] osdep: add qemu_unlink() Marc-André Lureau
2019-12-02 11:46   ` Daniel P. Berrangé
2019-11-27 11:52 ` [PATCH 7/7] screendump: use qemu_unlink() Marc-André Lureau
2019-12-02 11:49   ` Daniel P. Berrangé
2019-11-27 12:25 ` [PATCH 0/7] console: screendump improvements no-reply
2019-12-20 14:36 ` Marc-André Lureau
2020-01-02  9:52   ` Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).