[PATCH v2 0/2] Misc: build fixes for Fedora 35, Ubuntu et al

[PATCH v2 0/2] Misc: build fixes for Fedora 35, Ubuntu et al

[John Snow]

I didn't push this through in time for 6.2, so this series just worries
about the little fixes necessary for building QEMU under Fedora 35 and
the latest Ubuntu distributions.

The actual container changes have been cut off of this series in favor
of Dan's larger series that switches us over to using lcitool.

John Snow (2):
  spice: Update QXLInterface for spice >= 0.15.0
  ui/clipboard: Don't use g_autoptr just to free a variable

 include/ui/qemu-spice.h |  6 ++++++
 hw/display/qxl.c        | 14 +++++++++++++-
 ui/clipboard.c          |  3 +--
 ui/spice-display.c      | 11 +++++++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] spice: Update QXLInterface for spice >= 0.15.0

[John Snow]

spice updated the spelling (and arguments) of "attache_worker" in
0.15.0. Update QEMU to match, preventing -Wdeprecated-declarations
compilations from reporting build errors.

See also:
https://gitlab.freedesktop.org/spice/spice/-/commit/974692bda1e77af92b71ed43b022439448492cb9

Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/qemu-spice.h |  6 ++++++
 hw/display/qxl.c        | 14 +++++++++++++-
 ui/spice-display.c      | 11 +++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 71ecd6cfd1..21fe195e18 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -40,6 +40,12 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
 #define SPICE_NEEDS_SET_MM_TIME 0
 #endif
 
+#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
+#define SPICE_HAS_ATTACHED_WORKER 1
+#else
+#define SPICE_HAS_ATTACHED_WORKER 0
+#endif
+
 #else  /* CONFIG_SPICE */
 
 #include "qemu/error-report.h"
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 29c80b4289..1da6703e44 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -517,13 +517,20 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
 
 /* spice display interface callbacks */
 
-static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
+static void interface_attached_worker(QXLInstance *sin)
 {
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
 
     trace_qxl_interface_attach_worker(qxl->id);
 }
 
+#if !(SPICE_HAS_ATTACHED_WORKER)
+static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
+{
+    interface_attached_worker(sin);
+}
+#endif
+
 static void interface_set_compression_level(QXLInstance *sin, int level)
 {
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
@@ -1131,7 +1138,12 @@ static const QXLInterface qxl_interface = {
     .base.major_version      = SPICE_INTERFACE_QXL_MAJOR,
     .base.minor_version      = SPICE_INTERFACE_QXL_MINOR,
 
+#if SPICE_HAS_ATTACHED_WORKER
+    .attached_worker         = interface_attached_worker,
+#else
     .attache_worker          = interface_attach_worker,
+#endif
+
     .set_compression_level   = interface_set_compression_level,
 #if SPICE_NEEDS_SET_MM_TIME
     .set_mm_time             = interface_set_mm_time,
diff --git a/ui/spice-display.c b/ui/spice-display.c
index f59c69882d..1a60cebb7d 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -500,10 +500,17 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 
 /* spice display interface callbacks */
 
+#if SPICE_HAS_ATTACHED_WORKER
+static void interface_attached_worker(QXLInstance *sin)
+{
+    /* nothing to do */
+}
+#else
 static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
 {
     /* nothing to do */
 }
+#endif
 
 static void interface_set_compression_level(QXLInstance *sin, int level)
 {
@@ -702,7 +709,11 @@ static const QXLInterface dpy_interface = {
     .base.major_version      = SPICE_INTERFACE_QXL_MAJOR,
     .base.minor_version      = SPICE_INTERFACE_QXL_MINOR,
 
+#if SPICE_HAS_ATTACHED_WORKER
+    .attached_worker         = interface_attached_worker,
+#else
     .attache_worker          = interface_attach_worker,
+#endif
     .set_compression_level   = interface_set_compression_level,
 #if SPICE_NEEDS_SET_MM_TIME
     .set_mm_time             = interface_set_mm_time,
-- 
2.31.1

Re: [PATCH v2 1/2] spice: Update QXLInterface for spice >= 0.15.0

[Daniel P. Berrangé]

On Tue, Dec 07, 2021 at 03:40:37PM -0500, John Snow wrote:
> spice updated the spelling (and arguments) of "attache_worker" in
> 0.15.0. Update QEMU to match, preventing -Wdeprecated-declarations
> compilations from reporting build errors.
> 
> See also:
> https://gitlab.freedesktop.org/spice/spice/-/commit/974692bda1e77af92b71ed43b022439448492cb9
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/qemu-spice.h |  6 ++++++
>  hw/display/qxl.c        | 14 +++++++++++++-
>  ui/spice-display.c      | 11 +++++++++++
>  3 files changed, 30 insertions(+), 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 :|

Re: [PATCH v2 1/2] spice: Update QXLInterface for spice >= 0.15.0

[Philippe Mathieu-Daudé]

On 12/7/21 21:40, John Snow wrote:
> spice updated the spelling (and arguments) of "attache_worker" in
> 0.15.0. Update QEMU to match, preventing -Wdeprecated-declarations
> compilations from reporting build errors.
> 
> See also:
> https://gitlab.freedesktop.org/spice/spice/-/commit/974692bda1e77af92b71ed43b022439448492cb9
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/qemu-spice.h |  6 ++++++
>  hw/display/qxl.c        | 14 +++++++++++++-
>  ui/spice-display.c      | 11 +++++++++++
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index 71ecd6cfd1..21fe195e18 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -40,6 +40,12 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
>  #define SPICE_NEEDS_SET_MM_TIME 0
>  #endif
>  
> +#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
> +#define SPICE_HAS_ATTACHED_WORKER 1
> +#else
> +#define SPICE_HAS_ATTACHED_WORKER 0
> +#endif
> +
>  #else  /* CONFIG_SPICE */
>  
>  #include "qemu/error-report.h"
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 29c80b4289..1da6703e44 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -517,13 +517,20 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
>  
>  /* spice display interface callbacks */
>  
> -static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
> +static void interface_attached_worker(QXLInstance *sin)
>  {
>      PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
>  
>      trace_qxl_interface_attach_worker(qxl->id);
>  }
>  
> +#if !(SPICE_HAS_ATTACHED_WORKER)
> +static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
> +{
> +    interface_attached_worker(sin);
> +}
> +#endif
> +
>  static void interface_set_compression_level(QXLInstance *sin, int level)
>  {
>      PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
> @@ -1131,7 +1138,12 @@ static const QXLInterface qxl_interface = {
>      .base.major_version      = SPICE_INTERFACE_QXL_MAJOR,
>      .base.minor_version      = SPICE_INTERFACE_QXL_MINOR,
>  
> +#if SPICE_HAS_ATTACHED_WORKER
> +    .attached_worker         = interface_attached_worker,
> +#else
>      .attache_worker          = interface_attach_worker,
> +#endif
> +
>      .set_compression_level   = interface_set_compression_level,
>  #if SPICE_NEEDS_SET_MM_TIME
>      .set_mm_time             = interface_set_mm_time,
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index f59c69882d..1a60cebb7d 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -500,10 +500,17 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
>  
>  /* spice display interface callbacks */
>  
> +#if SPICE_HAS_ATTACHED_WORKER
> +static void interface_attached_worker(QXLInstance *sin)
> +{
> +    /* nothing to do */
> +}
> +#else
>  static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
>  {
>      /* nothing to do */
>  }
> +#endif
>  
>  static void interface_set_compression_level(QXLInstance *sin, int level)
>  {
> @@ -702,7 +709,11 @@ static const QXLInterface dpy_interface = {
>      .base.major_version      = SPICE_INTERFACE_QXL_MAJOR,
>      .base.minor_version      = SPICE_INTERFACE_QXL_MINOR,
>  
> +#if SPICE_HAS_ATTACHED_WORKER
> +    .attached_worker         = interface_attached_worker,
> +#else
>      .attache_worker          = interface_attach_worker,
> +#endif
>      .set_compression_level   = interface_set_compression_level,
>  #if SPICE_NEEDS_SET_MM_TIME
>      .set_mm_time             = interface_set_mm_time,
> 

Could we get this patch directly applied as a buildfix?

[PATCH v2 2/2] ui/clipboard: Don't use g_autoptr just to free a variable

[John Snow]

Clang doesn't recognize that the variable is being "used" and will emit
a warning:

  ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
      g_autoptr(QemuClipboardInfo) old = NULL;
                                 ^
  1 error generated.

OK, fine. Just do things the old way.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 ui/clipboard.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index d7b008d62a..9ab65efefb 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
-    g_autoptr(QemuClipboardInfo) old = NULL;
     assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
 
     notifier_list_notify(&clipboard_notifiers, info);
 
-    old = cbinfo[info->selection];
+    g_free(cbinfo[info->selection]);
     cbinfo[info->selection] = qemu_clipboard_info_ref(info);
 }
 
-- 
2.31.1

Re: [PATCH v2 2/2] ui/clipboard: Don't use g_autoptr just to free a variable

[Philippe Mathieu-Daudé]

On 12/7/21 21:40, John Snow wrote:
> Clang doesn't recognize that the variable is being "used" and will emit
> a warning:
> 
>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>       g_autoptr(QemuClipboardInfo) old = NULL;
>                                  ^
>   1 error generated.
> 
> OK, fine. Just do things the old way.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  ui/clipboard.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 2/2] ui/clipboard: Don't use g_autoptr just to free a variable

[Daniel P. Berrangé]

On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
> Clang doesn't recognize that the variable is being "used" and will emit
> a warning:
> 
>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>       g_autoptr(QemuClipboardInfo) old = NULL;
>                                  ^
>   1 error generated.
> 
> OK, fine. Just do things the old way.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  ui/clipboard.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ui/clipboard.c b/ui/clipboard.c
> index d7b008d62a..9ab65efefb 100644
> --- a/ui/clipboard.c
> +++ b/ui/clipboard.c
> @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
>  
>  void qemu_clipboard_update(QemuClipboardInfo *info)
>  {
> -    g_autoptr(QemuClipboardInfo) old = NULL;
>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>  
>      notifier_list_notify(&clipboard_notifiers, info);
>  
> -    old = cbinfo[info->selection];
> +    g_free(cbinfo[info->selection]);

This is a ref counted data type - it can't use g_free:

  https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html

>      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
>  }

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: [PATCH v2 2/2] ui/clipboard: Don't use g_autoptr just to free a variable

[Philippe Mathieu-Daudé]

On 12/8/21 10:11, Daniel P. Berrangé wrote:
> On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
>> Clang doesn't recognize that the variable is being "used" and will emit
>> a warning:
>>
>>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>>       g_autoptr(QemuClipboardInfo) old = NULL;
>>                                  ^
>>   1 error generated.
>>
>> OK, fine. Just do things the old way.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  ui/clipboard.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/ui/clipboard.c b/ui/clipboard.c
>> index d7b008d62a..9ab65efefb 100644
>> --- a/ui/clipboard.c
>> +++ b/ui/clipboard.c
>> @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
>>  
>>  void qemu_clipboard_update(QemuClipboardInfo *info)
>>  {
>> -    g_autoptr(QemuClipboardInfo) old = NULL;
>>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>>  
>>      notifier_list_notify(&clipboard_notifiers, info);
>>  
>> -    old = cbinfo[info->selection];
>> +    g_free(cbinfo[info->selection]);
> 
> This is a ref counted data type - it can't use g_free:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html

Doh, I vaguely remembered having already reviewed this then only
found John's v1 and finally mis-reviewed it :/ Thanks for noticing.

Re: [PATCH v2 2/2] ui/clipboard: Don't use g_autoptr just to free a variable

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Wed, Dec 8, 2021, 4:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
> > Clang doesn't recognize that the variable is being "used" and will emit
> > a warning:
> >
> >   ../ui/clipboard.c:47:34: error: variable 'old' set but not used
> [-Werror,-Wunused-but-set-variable]
> >       g_autoptr(QemuClipboardInfo) old = NULL;
> >                                  ^
> >   1 error generated.
> >
> > OK, fine. Just do things the old way.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  ui/clipboard.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/ui/clipboard.c b/ui/clipboard.c
> > index d7b008d62a..9ab65efefb 100644
> > --- a/ui/clipboard.c
> > +++ b/ui/clipboard.c
> > @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer
> *peer,
> >
> >  void qemu_clipboard_update(QemuClipboardInfo *info)
> >  {
> > -    g_autoptr(QemuClipboardInfo) old = NULL;
> >      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
> >
> >      notifier_list_notify(&clipboard_notifiers, info);
> >
> > -    old = cbinfo[info->selection];
> > +    g_free(cbinfo[info->selection]);
>
> This is a ref counted data type - it can't use g_free:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html
>
> >      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
> >  }
>
> Regards,
> Daniel
>

Whoops, sorry. I saw free being used elsewhere and assumed it was safe.
That's what I get for assuming things...

[-- Attachment #2: Type: text/html, Size: 2428 bytes --]