[Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5

Marc-André Lureau posted 1 patch 5 years, 4 months ago
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181128155932.16171-1-marcandre.lureau@redhat.com
include/ui/qemu-spice.h |  6 ------
chardev/spice.c         | 10 ----------
hw/display/qxl.c        |  2 --
ui/spice-core.c         |  8 --------
configure               |  4 ++--
5 files changed, 2 insertions(+), 28 deletions(-)
[Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Marc-André Lureau 5 years, 4 months ago
Looking at chardev/spice.c code, I realize compilation was broken for
a while with spice-server < 0.12.3. Let's bump required version
to 0.12.5, released May 19 2014, instead of adding more #ifdef.

(this patch combines changes from an early version and some of
Frediano "[PATCH 2/2] spice: Bump required spice-server version to
0.12.6")

According to repology, all the distros that are build target platforms
for QEMU include it:

      RHEL-7: 0.14.0
      Debian (Stretch): 0.12.8
      Debian (Jessie): 0.12.5
      FreeBSD (ports): 0.14.0
      OpenSUSE Leap 15: 0.14.0
      Ubuntu (Xenial): 0.12.6

Note that a previous version of this patch was bumping version to
0.12.6. Unfortunately, Debian Jessie (oldstable) is stuck with spice
server 0.12.5, and QEMU should keep building until after 2y of current
stable (Stretch), which will be around June 17th 2019. Qemu 4.1
should thus be free of bumping to spice-server 0.12.6 during 4.1
development cycle.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/qemu-spice.h |  6 ------
 chardev/spice.c         | 10 ----------
 hw/display/qxl.c        |  2 --
 ui/spice-core.c         |  8 --------
 configure               |  4 ++--
 5 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index c6d50eb87a..8c23dfe717 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -46,13 +46,7 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
 #else
 #define SPICE_NEEDS_SET_MM_TIME 0
 #endif
-
-#if SPICE_SERVER_VERSION >= 0x000c02
 void qemu_spice_register_ports(void);
-#else
-static inline Chardev *qemu_chr_open_spice_port(const char *name)
-{ return NULL; }
-#endif
 
 #else  /* CONFIG_SPICE */
 
diff --git a/chardev/spice.c b/chardev/spice.c
index e66e3ad568..173c257949 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -77,7 +77,6 @@ static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
     return bytes;
 }
 
-#if SPICE_SERVER_VERSION >= 0x000c02
 static void vmc_event(SpiceCharDeviceInstance *sin, uint8_t event)
 {
     SpiceChardev *scd = container_of(sin, SpiceChardev, sin);
@@ -95,7 +94,6 @@ static void vmc_event(SpiceCharDeviceInstance *sin, uint8_t event)
     trace_spice_vmc_event(chr_event);
     qemu_chr_be_event(chr, chr_event);
 }
-#endif
 
 static void vmc_state(SpiceCharDeviceInstance *sin, int connected)
 {
@@ -119,9 +117,7 @@ static SpiceCharDeviceInterface vmc_interface = {
     .state              = vmc_state,
     .write              = vmc_write,
     .read               = vmc_read,
-#if SPICE_SERVER_VERSION >= 0x000c02
     .event              = vmc_event,
-#endif
 #if SPICE_SERVER_VERSION >= 0x000c06
     .flags              = SPICE_CHAR_DEVICE_NOTIFY_WRITABLE,
 #endif
@@ -223,9 +219,7 @@ static void char_spice_finalize(Object *obj)
     }
 
     g_free((char *)s->sin.subtype);
-#if SPICE_SERVER_VERSION >= 0x000c02
     g_free((char *)s->sin.portname);
-#endif
 }
 
 static void spice_vmc_set_fe_open(struct Chardev *chr, int fe_open)
@@ -240,7 +234,6 @@ static void spice_vmc_set_fe_open(struct Chardev *chr, int fe_open)
 
 static void spice_port_set_fe_open(struct Chardev *chr, int fe_open)
 {
-#if SPICE_SERVER_VERSION >= 0x000c02
     SpiceChardev *s = SPICE_CHARDEV(chr);
 
     if (fe_open) {
@@ -248,7 +241,6 @@ static void spice_port_set_fe_open(struct Chardev *chr, int fe_open)
     } else {
         spice_server_port_event(&s->sin, SPICE_PORT_EVENT_CLOSED);
     }
-#endif
 }
 
 static void spice_chr_accept_input(struct Chardev *chr)
@@ -298,7 +290,6 @@ static void qemu_chr_open_spice_vmc(Chardev *chr,
     chr_open(chr, type);
 }
 
-#if SPICE_SERVER_VERSION >= 0x000c02
 static void qemu_chr_open_spice_port(Chardev *chr,
                                      ChardevBackend *backend,
                                      bool *be_opened,
@@ -331,7 +322,6 @@ void qemu_spice_register_ports(void)
         vmc_register_interface(s);
     }
 }
-#endif
 
 static void qemu_chr_parse_spice_vmc(QemuOpts *opts, ChardevBackend *backend,
                                      Error **errp)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 9087db5dee..8e9a65e75b 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1189,9 +1189,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
         return;
     }
     trace_qxl_enter_vga_mode(d->id);
-#if SPICE_SERVER_VERSION >= 0x000c03 /* release 0.12.3 */
     spice_qxl_driver_unload(&d->ssd.qxl);
-#endif
     graphic_console_set_hwops(d->ssd.dcl.con, d->vga.hw_ops, &d->vga);
     update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_DEFAULT);
     qemu_spice_create_host_primary(&d->ssd);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ebaae24643..fc850b3f50 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -745,13 +745,7 @@ void qemu_spice_init(void)
     }
 
     if (qemu_opt_get_bool(opts, "disable-agent-file-xfer", 0)) {
-#if SPICE_SERVER_VERSION >= 0x000c04
         spice_server_set_agent_file_xfer(spice_server, false);
-#else
-        error_report("this qemu build does not support the "
-                     "\"disable-agent-file-xfer\" option");
-        exit(1);
-#endif
     }
 
     compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
@@ -817,9 +811,7 @@ void qemu_spice_init(void)
     g_free(x509_cert_file);
     g_free(x509_cacert_file);
 
-#if SPICE_SERVER_VERSION >= 0x000c02
     qemu_spice_register_ports();
-#endif
 
 #ifdef HAVE_SPICE_GL
     if (qemu_opt_get_bool(opts, "gl", 0)) {
diff --git a/configure b/configure
index 8c292ef994..97288044c7 100755
--- a/configure
+++ b/configure
@@ -4560,7 +4560,7 @@ int main(void) { spice_server_new(); return 0; }
 EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
-  if $pkg_config --atleast-version=0.12.0 spice-server && \
+  if $pkg_config --atleast-version=0.12.5 spice-server && \
      $pkg_config --atleast-version=0.12.3 spice-protocol && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
@@ -4571,7 +4571,7 @@ EOF
   else
     if test "$spice" = "yes" ; then
       feature_not_found "spice" \
-          "Install spice-server(>=0.12.0) and spice-protocol(>=0.12.3) devel"
+          "Install spice-server(>=0.12.5) and spice-protocol(>=0.12.3) devel"
     fi
     spice="no"
   fi
-- 
2.20.0.rc1


Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Frediano Ziglio 5 years, 4 months ago
> 
> Looking at chardev/spice.c code, I realize compilation was broken for
> a while with spice-server < 0.12.3. Let's bump required version
> to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> 
> (this patch combines changes from an early version and some of
> Frediano "[PATCH 2/2] spice: Bump required spice-server version to
> 0.12.6")
> 

Minor, I didn't do much, you can remove this last paragraph

> According to repology, all the distros that are build target platforms
> for QEMU include it:
> 
>       RHEL-7: 0.14.0
>       Debian (Stretch): 0.12.8
>       Debian (Jessie): 0.12.5
>       FreeBSD (ports): 0.14.0
>       OpenSUSE Leap 15: 0.14.0
>       Ubuntu (Xenial): 0.12.6
> 
> Note that a previous version of this patch was bumping version to
> 0.12.6. Unfortunately, Debian Jessie (oldstable) is stuck with spice
> server 0.12.5, and QEMU should keep building until after 2y of current
> stable (Stretch), which will be around June 17th 2019. Qemu 4.1
> should thus be free of bumping to spice-server 0.12.6 during 4.1
> development cycle.
> 

I would not refer to previous patch version, kind of starting with
the "Debian Jessie "...

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/qemu-spice.h |  6 ------
>  chardev/spice.c         | 10 ----------
>  hw/display/qxl.c        |  2 --
>  ui/spice-core.c         |  8 --------
>  configure               |  4 ++--
>  5 files changed, 2 insertions(+), 28 deletions(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index c6d50eb87a..8c23dfe717 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -46,13 +46,7 @@ int qemu_spice_migrate_info(const char *hostname, int
> port, int tls_port,
>  #else
>  #define SPICE_NEEDS_SET_MM_TIME 0
>  #endif
> -

Really minor, an empty line would not be bad.

> -#if SPICE_SERVER_VERSION >= 0x000c02
>  void qemu_spice_register_ports(void);
> -#else
> -static inline Chardev *qemu_chr_open_spice_port(const char *name)
> -{ return NULL; }
> -#endif
>  
>  #else  /* CONFIG_SPICE */
>  
> diff --git a/chardev/spice.c b/chardev/spice.c
> index e66e3ad568..173c257949 100644
> --- a/chardev/spice.c
> +++ b/chardev/spice.c
> @@ -77,7 +77,6 @@ static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t
> *buf, int len)
>      return bytes;
>  }
>  
> -#if SPICE_SERVER_VERSION >= 0x000c02
>  static void vmc_event(SpiceCharDeviceInstance *sin, uint8_t event)
>  {
>      SpiceChardev *scd = container_of(sin, SpiceChardev, sin);
> @@ -95,7 +94,6 @@ static void vmc_event(SpiceCharDeviceInstance *sin, uint8_t
> event)
>      trace_spice_vmc_event(chr_event);
>      qemu_chr_be_event(chr, chr_event);
>  }
> -#endif
>  
>  static void vmc_state(SpiceCharDeviceInstance *sin, int connected)
>  {
> @@ -119,9 +117,7 @@ static SpiceCharDeviceInterface vmc_interface = {
>      .state              = vmc_state,
>      .write              = vmc_write,
>      .read               = vmc_read,
> -#if SPICE_SERVER_VERSION >= 0x000c02
>      .event              = vmc_event,
> -#endif
>  #if SPICE_SERVER_VERSION >= 0x000c06
>      .flags              = SPICE_CHAR_DEVICE_NOTIFY_WRITABLE,
>  #endif
> @@ -223,9 +219,7 @@ static void char_spice_finalize(Object *obj)
>      }
>  
>      g_free((char *)s->sin.subtype);
> -#if SPICE_SERVER_VERSION >= 0x000c02
>      g_free((char *)s->sin.portname);
> -#endif
>  }
>  
>  static void spice_vmc_set_fe_open(struct Chardev *chr, int fe_open)
> @@ -240,7 +234,6 @@ static void spice_vmc_set_fe_open(struct Chardev *chr,
> int fe_open)
>  
>  static void spice_port_set_fe_open(struct Chardev *chr, int fe_open)
>  {
> -#if SPICE_SERVER_VERSION >= 0x000c02
>      SpiceChardev *s = SPICE_CHARDEV(chr);
>  
>      if (fe_open) {
> @@ -248,7 +241,6 @@ static void spice_port_set_fe_open(struct Chardev *chr,
> int fe_open)
>      } else {
>          spice_server_port_event(&s->sin, SPICE_PORT_EVENT_CLOSED);
>      }
> -#endif
>  }
>  
>  static void spice_chr_accept_input(struct Chardev *chr)
> @@ -298,7 +290,6 @@ static void qemu_chr_open_spice_vmc(Chardev *chr,
>      chr_open(chr, type);
>  }
>  
> -#if SPICE_SERVER_VERSION >= 0x000c02
>  static void qemu_chr_open_spice_port(Chardev *chr,
>                                       ChardevBackend *backend,
>                                       bool *be_opened,
> @@ -331,7 +322,6 @@ void qemu_spice_register_ports(void)
>          vmc_register_interface(s);
>      }
>  }
> -#endif
>  
>  static void qemu_chr_parse_spice_vmc(QemuOpts *opts, ChardevBackend
>  *backend,
>                                       Error **errp)
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 9087db5dee..8e9a65e75b 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1189,9 +1189,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
>          return;
>      }
>      trace_qxl_enter_vga_mode(d->id);
> -#if SPICE_SERVER_VERSION >= 0x000c03 /* release 0.12.3 */
>      spice_qxl_driver_unload(&d->ssd.qxl);
> -#endif
>      graphic_console_set_hwops(d->ssd.dcl.con, d->vga.hw_ops, &d->vga);
>      update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_DEFAULT);
>      qemu_spice_create_host_primary(&d->ssd);
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index ebaae24643..fc850b3f50 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -745,13 +745,7 @@ void qemu_spice_init(void)
>      }
>  
>      if (qemu_opt_get_bool(opts, "disable-agent-file-xfer", 0)) {
> -#if SPICE_SERVER_VERSION >= 0x000c04
>          spice_server_set_agent_file_xfer(spice_server, false);
> -#else
> -        error_report("this qemu build does not support the "
> -                     "\"disable-agent-file-xfer\" option");
> -        exit(1);
> -#endif
>      }
>  
>      compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
> @@ -817,9 +811,7 @@ void qemu_spice_init(void)
>      g_free(x509_cert_file);
>      g_free(x509_cacert_file);
>  
> -#if SPICE_SERVER_VERSION >= 0x000c02
>      qemu_spice_register_ports();
> -#endif
>  
>  #ifdef HAVE_SPICE_GL
>      if (qemu_opt_get_bool(opts, "gl", 0)) {
> diff --git a/configure b/configure
> index 8c292ef994..97288044c7 100755
> --- a/configure
> +++ b/configure
> @@ -4560,7 +4560,7 @@ int main(void) { spice_server_new(); return 0; }
>  EOF
>    spice_cflags=$($pkg_config --cflags spice-protocol spice-server
>    2>/dev/null)
>    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
> -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> +  if $pkg_config --atleast-version=0.12.5 spice-server && \
>       $pkg_config --atleast-version=0.12.3 spice-protocol && \
>       compile_prog "$spice_cflags" "$spice_libs" ; then
>      spice="yes"
> @@ -4571,7 +4571,7 @@ EOF
>    else
>      if test "$spice" = "yes" ; then
>        feature_not_found "spice" \
> -          "Install spice-server(>=0.12.0) and spice-protocol(>=0.12.3)
> devel"
> +          "Install spice-server(>=0.12.5) and spice-protocol(>=0.12.3)
> devel"
>      fi
>      spice="no"
>    fi

Otherwise fine for me,

Frediano

Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Gerd Hoffmann 5 years, 4 months ago
On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> Looking at chardev/spice.c code, I realize compilation was broken for
> a while with spice-server < 0.12.3. Let's bump required version
> to 0.12.5, released May 19 2014, instead of adding more #ifdef.

Oh, you did the 0.12.5 patch already.  Scratch my other reply then.

> -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> +  if $pkg_config --atleast-version=0.12.5 spice-server && \
>       $pkg_config --atleast-version=0.12.3 spice-protocol && \

I think we should adjust spice-protocol too to whatever 0.12.5 requires
to build.

cheers,
  Gerd


Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Frediano Ziglio 5 years, 4 months ago
> 
> On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > Looking at chardev/spice.c code, I realize compilation was broken for
> > a while with spice-server < 0.12.3. Let's bump required version
> > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> 
> Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
> 
> > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
> 
> I think we should adjust spice-protocol too to whatever 0.12.5 requires
> to build.
> 
> cheers,
>   Gerd
> 

Was 0.12.8

Frediano

Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Marc-André Lureau 5 years, 4 months ago
On Thu, Nov 29, 2018 at 12:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > Looking at chardev/spice.c code, I realize compilation was broken for
> > a while with spice-server < 0.12.3. Let's bump required version
> > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
>
> Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
>
> > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
>
> I think we should adjust spice-protocol too to whatever 0.12.5 requires
> to build.
>

Why not leave that responsibility to pkg-config, and only require in
qemu what is required there?

Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Frediano Ziglio 5 years, 4 months ago
> On Thu, Nov 29, 2018 at 12:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > > Looking at chardev/spice.c code, I realize compilation was broken for
> > > a while with spice-server < 0.12.3. Let's bump required version
> > > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> >
> > Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
> >
> > > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> > >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
> >
> > I think we should adjust spice-protocol too to whatever 0.12.5 requires
> > to build.
> >
> 
> Why not leave that responsibility to pkg-config, and only require in
> qemu what is required there?
> 
> 

That is remove explicit requirement in configure script?
I can see that spice-core.h (spice-server, one of the mail include) is
including spice-protocol headers.
Looking at configure both are required so would make sense to check
only spice-server, unless packaging has some bugs if you have spice-server
(devel) installed you also have spice-protocol.

Frediano

Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Gerd Hoffmann 5 years, 4 months ago
On Thu, Nov 29, 2018 at 03:32:28AM -0500, Frediano Ziglio wrote:
> > On Thu, Nov 29, 2018 at 12:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > > > Looking at chardev/spice.c code, I realize compilation was broken for
> > > > a while with spice-server < 0.12.3. Let's bump required version
> > > > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> > >
> > > Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
> > >
> > > > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > > > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> > > >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
> > >
> > > I think we should adjust spice-protocol too to whatever 0.12.5 requires
> > > to build.
> > >
> > 
> > Why not leave that responsibility to pkg-config, and only require in
> > qemu what is required there?
> > 
> > 
> 
> That is remove explicit requirement in configure script?
> I can see that spice-core.h (spice-server, one of the mail include) is
> including spice-protocol headers.
> Looking at configure both are required so would make sense to check
> only spice-server, unless packaging has some bugs if you have spice-server
> (devel) installed you also have spice-protocol.

seems the spice-protocol dep is there due to qemu itself needing it:

commit 358689fe299c306f1d81bea57a5067d0abb56699
Author: Michal Privoznik <mprivozn@redhat.com>
Date:   Fri Mar 1 08:43:18 2013 +0100

    configure: Require at least spice-protocol-0.12.3
    
    As of 5a49d3e9 we assume SPICE_PORT_EVENT_BREAK to be defined.
    However, it is defined not in 0.12.2 what we require now, but in
    0.12.3.  Therefore in order to prevent build failure we must
    adjust our minimal requirements.
    
    Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

That makes sense.  So, when spice-server 0.12.5 requires spice-protocol
0.12.8+ anyway I think we can savely drop the spice-protocol check.

cheers,
  Gerd


Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Frediano Ziglio 5 years, 4 months ago
> 
> On Thu, Nov 29, 2018 at 03:32:28AM -0500, Frediano Ziglio wrote:
> > > On Thu, Nov 29, 2018 at 12:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > >
> > > > On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > > > > Looking at chardev/spice.c code, I realize compilation was broken for
> > > > > a while with spice-server < 0.12.3. Let's bump required version
> > > > > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> > > >
> > > > Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
> > > >
> > > > > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > > > > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> > > > >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
> > > >
> > > > I think we should adjust spice-protocol too to whatever 0.12.5 requires
> > > > to build.
> > > >
> > > 
> > > Why not leave that responsibility to pkg-config, and only require in
> > > qemu what is required there?
> > > 
> > > 
> > 
> > That is remove explicit requirement in configure script?
> > I can see that spice-core.h (spice-server, one of the mail include) is
> > including spice-protocol headers.
> > Looking at configure both are required so would make sense to check
> > only spice-server, unless packaging has some bugs if you have spice-server
> > (devel) installed you also have spice-protocol.
> 
> seems the spice-protocol dep is there due to qemu itself needing it:
> 
> commit 358689fe299c306f1d81bea57a5067d0abb56699
> Author: Michal Privoznik <mprivozn@redhat.com>
> Date:   Fri Mar 1 08:43:18 2013 +0100
> 
>     configure: Require at least spice-protocol-0.12.3
>     
>     As of 5a49d3e9 we assume SPICE_PORT_EVENT_BREAK to be defined.
>     However, it is defined not in 0.12.2 what we require now, but in
>     0.12.3.  Therefore in order to prevent build failure we must
>     adjust our minimal requirements.
>     
>     Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> That makes sense.  So, when spice-server 0.12.5 requires spice-protocol
> 0.12.8+ anyway I think we can savely drop the spice-protocol check.
> 
> cheers,
>   Gerd
> 

My idea was more to revert (kind of) 7e3efdac75caca0b283f8e76ad24c924b4718e7b:


commit 7e3efdac75caca0b283f8e76ad24c924b4718e7b
Author: Alon Levy <alevy@redhat.com>
Date:   Wed Mar 7 16:19:03 2012 +0200

    spice: require spice-protocol >= 0.8.1
    
    Requiring spice-server >= 0.8.2 is not enough since spice-server.pc
    doesn't require spice-protocol (any version). Until that is fixed
    upstream an explicit requirement in qemu fixes compilation broken since
    
    commit 2e1a98c9c1b90ca093278c6b43244dc46604d7b7
    Author: Alon Levy <alevy@redhat.com>
    Date:   Fri Feb 24 23:19:30 2012 +0200
    
        qxl: introduce QXLCookie
    
    Reported-by: Peter Maydell <peter.maydell@linaro.org>
    
    Signed-off-by: Alon Levy <alevy@redhat.com>
    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/configure b/configure
index 0111774cb0..62aa7609e1 100755
--- a/configure
+++ b/configure
@@ -2592,6 +2592,7 @@ EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
   if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
+     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
     libs_softmmu="$libs_softmmu $spice_libs"


The rationale does not apply any more, spice-server depends on spice-protocol.

Frediano

Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Gerd Hoffmann 5 years, 4 months ago
> > seems the spice-protocol dep is there due to qemu itself needing it:
> > 
> > commit 358689fe299c306f1d81bea57a5067d0abb56699
> > Author: Michal Privoznik <mprivozn@redhat.com>
> > Date:   Fri Mar 1 08:43:18 2013 +0100
> > 
> >     configure: Require at least spice-protocol-0.12.3
> >     
> >     As of 5a49d3e9 we assume SPICE_PORT_EVENT_BREAK to be defined.
> >     However, it is defined not in 0.12.2 what we require now, but in
> >     0.12.3.  Therefore in order to prevent build failure we must
> >     adjust our minimal requirements.
> >     
> >     Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > That makes sense.  So, when spice-server 0.12.5 requires spice-protocol
> > 0.12.8+ anyway I think we can savely drop the spice-protocol check.
> > 
> > cheers,
> >   Gerd
> > 
> 
> My idea was more to revert (kind of) 7e3efdac75caca0b283f8e76ad24c924b4718e7b:
> 
> 
> commit 7e3efdac75caca0b283f8e76ad24c924b4718e7b
> Author: Alon Levy <alevy@redhat.com>
> Date:   Wed Mar 7 16:19:03 2012 +0200
> 
>     spice: require spice-protocol >= 0.8.1
>     
>     Requiring spice-server >= 0.8.2 is not enough since spice-server.pc
>     doesn't require spice-protocol (any version). Until that is fixed
>     upstream an explicit requirement in qemu fixes compilation broken since
>     
>     commit 2e1a98c9c1b90ca093278c6b43244dc46604d7b7
>     Author: Alon Levy <alevy@redhat.com>
>     Date:   Fri Feb 24 23:19:30 2012 +0200
>     
>         qxl: introduce QXLCookie
>     
>     Reported-by: Peter Maydell <peter.maydell@linaro.org>
>     
>     Signed-off-by: Alon Levy <alevy@redhat.com>
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> diff --git a/configure b/configure
> index 0111774cb0..62aa7609e1 100755
> --- a/configure
> +++ b/configure
> @@ -2592,6 +2592,7 @@ EOF
>    spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
>    spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
>    if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
> +     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
>       compile_prog "$spice_cflags" "$spice_libs" ; then
>      spice="yes"
>      libs_softmmu="$libs_softmmu $spice_libs"
> 
> 
> The rationale does not apply any more, spice-server depends on spice-protocol.

Fine with me too.

cheers,
  Gerd


Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Gerd Hoffmann 5 years, 4 months ago
On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> Looking at chardev/spice.c code, I realize compilation was broken for
> a while with spice-server < 0.12.3. Let's bump required version
> to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> 
> (this patch combines changes from an early version and some of
> Frediano "[PATCH 2/2] spice: Bump required spice-server version to
> 0.12.6")
> 
> According to repology, all the distros that are build target platforms
> for QEMU include it:
> 
>       RHEL-7: 0.14.0
>       Debian (Stretch): 0.12.8
>       Debian (Jessie): 0.12.5
>       FreeBSD (ports): 0.14.0
>       OpenSUSE Leap 15: 0.14.0
>       Ubuntu (Xenial): 0.12.6
> 
> Note that a previous version of this patch was bumping version to
> 0.12.6. Unfortunately, Debian Jessie (oldstable) is stuck with spice
> server 0.12.5, and QEMU should keep building until after 2y of current
> stable (Stretch), which will be around June 17th 2019. Qemu 4.1
> should thus be free of bumping to spice-server 0.12.6 during 4.1
> development cycle.

Added to UI queue.

thanks,
  Gerd


Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Posted by Daniel P. Berrangé 5 years, 4 months ago
On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> Looking at chardev/spice.c code, I realize compilation was broken for
> a while with spice-server < 0.12.3. Let's bump required version
> to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> 
> (this patch combines changes from an early version and some of
> Frediano "[PATCH 2/2] spice: Bump required spice-server version to
> 0.12.6")
> 
> According to repology, all the distros that are build target platforms
> for QEMU include it:
> 
>       RHEL-7: 0.14.0
>       Debian (Stretch): 0.12.8
>       Debian (Jessie): 0.12.5
>       FreeBSD (ports): 0.14.0
>       OpenSUSE Leap 15: 0.14.0
>       Ubuntu (Xenial): 0.12.6
> 
> Note that a previous version of this patch was bumping version to
> 0.12.6. Unfortunately, Debian Jessie (oldstable) is stuck with spice
> server 0.12.5, and QEMU should keep building until after 2y of current
> stable (Stretch), which will be around June 17th 2019. Qemu 4.1
> should thus be free of bumping to spice-server 0.12.6 during 4.1
> development cycle.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/qemu-spice.h |  6 ------
>  chardev/spice.c         | 10 ----------
>  hw/display/qxl.c        |  2 --
>  ui/spice-core.c         |  8 --------
>  configure               |  4 ++--
>  5 files changed, 2 insertions(+), 28 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 :|