Re: [Qemu-devel] [PATCH] Require spice-server 0.12.0 and spice-protocol 0.12.2

[Paolo Bonzini]

Il 04/10/2012 19:37, Søren Sandmann ha scritto:
> From: Søren Sandmann Pedersen <address@hidden>
> 
> Set the default PCI revision to QXL_REVISION_STABLE_V12, and remove
> the various #ifs on various subversions of spice-server 0.11.x.
> 
> Also set the revision to 3 for the PC 1.2 machine for compatibility.
> 
> Signed-off-by: Soren Sandmann <address@hidden>
> ---
>  configure    |   10 ++--------
>  hw/pc_piix.c |    8 ++++++++
>  hw/qxl.c     |   29 -----------------------------
>  hw/qxl.h     |    5 -----
>  4 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/configure b/configure
> index 8f99b7b..317a6b0 100755
> --- a/configure
> +++ b/configure
> @@ -2685,20 +2685,14 @@ 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.8.2 spice-server >/dev/null 2>&1 && \
> -     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
> +  if $pkg_config --atleast-version=0.12.0 spice-server >/dev/null 2>&1 && \
> +     $pkg_config --atleast-version=0.12.2 spice-protocol > /dev/null 2>&1 && 
> \
>       compile_prog "$spice_cflags" "$spice_libs" ; then
>      spice="yes"
>      libs_softmmu="$libs_softmmu $spice_libs"
>      QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
>      spice_protocol_version=$($pkg_config --modversion spice-protocol)
>      spice_server_version=$($pkg_config --modversion spice-server)
> -    if $pkg_config --atleast-version=0.12.0 spice-protocol >/dev/null 2>&1; 
> then
> -        spice_qxl_io_monitors_config_async="yes"
> -    fi
> -    if $pkg_config --atleast-version=0.12.2 spice-protocol > /dev/null 2>&1; 
> then
> -        spice_qxl_client_monitors_config="yes"
> -    fi
>    else
>      if test "$spice" = "yes" ; then
>        feature_not_found "spice"
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index fd5898f..82364ab 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -371,6 +371,14 @@ static QEMUMachine pc_machine_v1_3 = {
>              .driver   = "ivshmem",\
>              .property = "use64",\
>              .value    = "0",\
> +        },{\
> +            .driver   = "qxl",\
> +            .property = "revision",\
> +            .value    = stringify(3),\
> +        },{\
> +            .driver   = "qxl-vga",\
> +            .property = "revision",\
> +            .value    = stringify(3),\
>          }
>  
>  static QEMUMachine pc_machine_v1_2 = {
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 33169f3..c256bda 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -29,11 +29,6 @@
>  
>  #include "qxl.h"
>  
> -#ifndef CONFIG_QXL_IO_MONITORS_CONFIG_ASYNC
> -/* spice-protocol is too old, add missing definitions */
> -#define QXL_IO_MONITORS_CONFIG_ASYNC (QXL_IO_FLUSH_RELEASE + 1)
> -#endif
> -
>  /*
>   * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
>   * such can be changed by the guest, so to avoid a guest trigerrable
> @@ -262,9 +257,6 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, 
> qxl_async_io async)
>  static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
>  {
>      trace_qxl_spice_monitors_config(qxl->id);
> -/* 0x000b01 == 0.11.1 */
> -#if SPICE_SERVER_VERSION >= 0x000b01 && \
> -    defined(CONFIG_QXL_IO_MONITORS_CONFIG_ASYNC)
>      if (replay) {
>          /*
>           * don't use QXL_COOKIE_TYPE_IO:
> @@ -286,10 +278,6 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice 
> *qxl, int replay)
>                  (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
>                                            QXL_IO_MONITORS_CONFIG_ASYNC));
>      }
> -#else
> -    fprintf(stderr, "qxl: too old spice-protocol/spice-server for "
> -            "QXL_IO_MONITORS_CONFIG_ASYNC\n");
> -#endif
>  }
>  
>  void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
> @@ -948,8 +936,6 @@ static void interface_async_complete(QXLInstance *sin, 
> uint64_t cookie_token)
>      }
>  }
>  
> -#if SPICE_SERVER_VERSION >= 0x000b04
> -
>  /* called from spice server thread context only */
>  static void interface_set_client_capabilities(QXLInstance *sin,
>                                                uint8_t client_present,
> @@ -971,11 +957,6 @@ static void 
> interface_set_client_capabilities(QXLInstance *sin,
>      qxl_send_events(qxl, QXL_INTERRUPT_CLIENT);
>  }
>  
> -#endif
> -
> -#if defined(CONFIG_QXL_CLIENT_MONITORS_CONFIG) \
> -    && SPICE_SERVER_VERSION >= 0x000b05
> -
>  static uint32_t qxl_crc32(const uint8_t *p, unsigned len)
>  {
>      /*
> @@ -1044,7 +1025,6 @@ static int interface_client_monitors_config(QXLInstance 
> *sin,
>      qxl_send_events(qxl, QXL_INTERRUPT_CLIENT_MONITORS_CONFIG);
>      return 1;
>  }
> -#endif
>  
>  static const QXLInterface qxl_interface = {
>      .base.type               = SPICE_INTERFACE_QXL,
> @@ -1067,13 +1047,8 @@ static const QXLInterface qxl_interface = {
>      .flush_resources         = interface_flush_resources,
>      .async_complete          = interface_async_complete,
>      .update_area_complete    = interface_update_area_complete,
> -#if SPICE_SERVER_VERSION >= 0x000b04
>      .set_client_capabilities = interface_set_client_capabilities,
> -#endif
> -#if SPICE_SERVER_VERSION >= 0x000b05 && \
> -    defined(CONFIG_QXL_CLIENT_MONITORS_CONFIG)
>      .client_monitors_config = interface_client_monitors_config,
> -#endif
>  };
>  
>  static void qxl_enter_vga_mode(PCIQXLDevice *d)
> @@ -1971,14 +1946,10 @@ static int qxl_init_common(PCIQXLDevice *qxl)
>          pci_device_rev = QXL_REVISION_STABLE_V10;
>          io_size = 32; /* PCI region size must be pow2 */
>          break;
> -/* 0x000b01 == 0.11.1 */
> -#if SPICE_SERVER_VERSION >= 0x000b01 && \
> -        defined(CONFIG_QXL_IO_MONITORS_CONFIG_ASYNC)
>      case 4: /* qxl-4 */
>          pci_device_rev = QXL_REVISION_STABLE_V12;
>          io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
>          break;
> -#endif
>      default:
>          error_report("Invalid revision %d for qxl device (max %d)",
>                       qxl->revision, QXL_DEFAULT_REVISION);
> diff --git a/hw/qxl.h b/hw/qxl.h
> index 5553824..e583cfb 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -129,12 +129,7 @@ typedef struct PCIQXLDevice {
>          }                                                               \
>      } while (0)
>  
> -#if 0
> -/* spice-server 0.12 is still in development */
>  #define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12
> -#else
> -#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
> -#endif
>  
>  /* qxl.c */
>  void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
> 

Nice cleanup, thanks!

As a followup, there is some confusion between values of the revision
property (1-4) and PCI device revisions (QXL_REVISION_STABLE_Vxy).  They
are the same right now, but this means that you can do one of the following:

1) if they are always going to be the same, eliminate pci_device_rev
from qxl_init_common, just use qxl->revision instead;

2) if they might diverge, change the value of QXL_DEFAULT_REVISION from
QXL_REVISION_STABLE_V12 to 4.

Paolo