All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros
Date: Wed, 25 Jun 2014 08:56:43 +0300	[thread overview]
Message-ID: <20140625055643.GG21297@redhat.com> (raw)
In-Reply-To: <1403661885-28619-3-git-send-email-ehabkost@redhat.com>

On Tue, Jun 24, 2014 at 11:04:45PM -0300, Eduardo Habkost wrote:
> The QEMU_COMPAT_* macros will contain compat properties that are not
> specific to PC, and may be reused by other machine-types.
> 
> The compat properties for PC-specific devices were moved to
> QEMU_COMPAT_* too, because they are simply not going to be applied to
> any device if they are not instantiated.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I don't see value in this.
If some target wants to start versioning their machine types,
let them start from 2.0 (or even 2.1 - does any non-PC target
plan to support cross-version compatibility for 2.0?).
Why carry around useless code for < 2.0 for non PC platforms?

> ---
>  include/hw/compat.h  | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h | 206 +++-----------------------------------------------
>  2 files changed, 221 insertions(+), 195 deletions(-)
>  create mode 100644 include/hw/compat.h

And we have lost all we have gained in 1/2 :(

> 
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> new file mode 100644
> index 0000000..dff55c8
> --- /dev/null
> +++ b/include/hw/compat.h
> @@ -0,0 +1,210 @@
> +/* Common compat properties that can be used by machine-types to keep guest ABI
> + * stable between QEMU versions.
> + */
> +
> +#ifndef HW_COMPAT_H
> +#define HW_COMPAT_H
> +
> +#define QEMU_COMPAT_2_0 \
> +    {\
> +        .driver   = "virtio-scsi-pci",\
> +        .property = "any_layout",\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "PIIX4_PM",\
> +        .property = "memory-hotplug-support",\
> +        .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "apic",\
> +        .property = "version",\
> +        .value    = stringify(0x11),\
> +    },\
> +    {\
> +        .driver   = "nec-usb-xhci",\
> +        .property = "superspeed-ports-first",\
> +        .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "pci-serial",\
> +        .property = "prog_if",\
> +        .value    = stringify(0),\
> +    },\
> +    {\
> +        .driver   = "pci-serial-2x",\
> +        .property = "prog_if",\
> +        .value    = stringify(0),\
> +    },\
> +    {\
> +        .driver   = "pci-serial-4x",\
> +        .property = "prog_if",\
> +        .value    = stringify(0),\
> +    },\
> +    {\
> +        .driver   = "virtio-net-pci",\
> +        .property = "guest_announce",\
> +        .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "ICH9-LPC",\
> +        .property = "memory-hotplug-support",\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "xio3130-downstream",\
> +        .property = COMPAT_PROP_PCP,\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "ioh3420",\
> +        .property = COMPAT_PROP_PCP,\
> +        .value    = "off",\
> +    }
> +
> +#define QEMU_COMPAT_1_7 \
> +    {\
> +        .driver   = TYPE_USB_DEVICE,\
> +        .property = "msos-desc",\
> +        .value    = "no",\
> +    },\
> +    {\
> +        .driver   = "PIIX4_PM",\
> +        .property = "acpi-pci-hotplug-with-bridge-support",\
> +        .value    = "off",\
> +    },\
> +    {\
> +        .driver   = "hpet",\
> +        .property = HPET_INTCAP,\
> +        .value    = stringify(4),\
> +    }
> +
> +#define QEMU_COMPAT_1_6 \
> +    {\
> +        .driver   = "e1000",\
> +        .property = "mitigation",\
> +        .value    = "off",\
> +    },{\
> +        .driver   = "qemu64-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "qemu32-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(3),\
> +    },{\
> +        .driver   = "i440FX-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(1),\
> +    },{\
> +        .driver   = "q35-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(1),\
> +    }
> +
> +#define QEMU_COMPAT_1_5 \
> +    {\
> +        .driver   = "Conroe-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "Conroe-" TYPE_X86_CPU,\
> +        .property = "level",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "Penryn-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "Penryn-" TYPE_X86_CPU,\
> +        .property = "level",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "Nehalem-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "Nehalem-" TYPE_X86_CPU,\
> +        .property = "level",\
> +        .value    = stringify(2),\
> +    },{\
> +        .driver   = "virtio-net-pci",\
> +        .property = "any_layout",\
> +        .value    = "off",\
> +    },{\
> +        .driver = TYPE_X86_CPU,\
> +        .property = "pmu",\
> +        .value = "on",\
> +    },{\
> +        .driver   = "i440FX-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "q35-pcihost",\
> +        .property = "short_root_bus",\
> +        .value    = stringify(0),\
> +    }
> +
> +#define QEMU_COMPAT_1_4 \
> +    {\
> +        .driver   = "scsi-hd",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "scsi-cd",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "scsi-disk",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "ide-hd",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "ide-cd",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "ide-drive",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "virtio-blk-pci",\
> +        .property = "discard_granularity",\
> +        .value    = stringify(0),\
> +    },{\
> +        .driver   = "virtio-serial-pci",\
> +        .property = "vectors",\
> +        /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> +        .value    = stringify(0xFFFFFFFF),\
> +    },{ \
> +        .driver   = "virtio-net-pci", \
> +        .property = "ctrl_guest_offloads", \
> +        .value    = "off", \
> +    },{\
> +        .driver   = "e1000",\
> +        .property = "romfile",\
> +        .value    = "pxe-e1000.rom",\
> +    },{\
> +        .driver   = "ne2k_pci",\
> +        .property = "romfile",\
> +        .value    = "pxe-ne2k_pci.rom",\
> +    },{\
> +        .driver   = "pcnet",\
> +        .property = "romfile",\
> +        .value    = "pxe-pcnet.rom",\
> +    },{\
> +        .driver   = "rtl8139",\
> +        .property = "romfile",\
> +        .value    = "pxe-rtl8139.rom",\
> +    },{\
> +        .driver   = "virtio-net-pci",\
> +        .property = "romfile",\
> +        .value    = "pxe-virtio.rom",\
> +    },{\
> +        .driver   = "486-" TYPE_X86_CPU,\
> +        .property = "model",\
> +        .value    = stringify(0),\
> +    }
> +
> +

Two empty lines.

> +#endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1c0c382..5907e99 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -4,6 +4,7 @@
>  #include "qemu-common.h"
>  #include "exec/memory.h"
>  #include "hw/boards.h"
> +#include "hw/compat.h"
>  #include "hw/isa/isa.h"
>  #include "hw/block/fdc.h"
>  #include "net/net.h"
> @@ -295,209 +296,24 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_0 \
> -        {\
> -            .driver   = "virtio-scsi-pci",\
> -            .property = "any_layout",\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "PIIX4_PM",\
> -            .property = "memory-hotplug-support",\
> -            .value    = "off",\
> -        },\
> -        {\
> -            .driver   = "apic",\
> -            .property = "version",\
> -            .value    = stringify(0x11),\
> -        },\
> -        {\
> -            .driver   = "nec-usb-xhci",\
> -            .property = "superspeed-ports-first",\
> -            .value    = "off",\
> -        },\
> -        {\
> -            .driver   = "pci-serial",\
> -            .property = "prog_if",\
> -            .value    = stringify(0),\
> -        },\
> -        {\
> -            .driver   = "pci-serial-2x",\
> -            .property = "prog_if",\
> -            .value    = stringify(0),\
> -        },\
> -        {\
> -            .driver   = "pci-serial-4x",\
> -            .property = "prog_if",\
> -            .value    = stringify(0),\
> -        },\
> -        {\
> -            .driver   = "virtio-net-pci",\
> -            .property = "guest_announce",\
> -            .value    = "off",\
> -        },\
> -        {\
> -            .driver   = "ICH9-LPC",\
> -            .property = "memory-hotplug-support",\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "xio3130-downstream",\
> -            .property = COMPAT_PROP_PCP,\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "ioh3420",\
> -            .property = COMPAT_PROP_PCP,\
> -            .value    = "off",\
> -        }
> +    QEMU_COMPAT_2_0
>  
>  #define PC_COMPAT_1_7 \
> -        PC_COMPAT_2_0, \
> -        {\
> -            .driver   = TYPE_USB_DEVICE,\
> -            .property = "msos-desc",\
> -            .value    = "no",\
> -        },\
> -        {\
> -            .driver   = "PIIX4_PM",\
> -            .property = "acpi-pci-hotplug-with-bridge-support",\
> -            .value    = "off",\
> -        },\
> -        {\
> -            .driver   = "hpet",\
> -            .property = HPET_INTCAP,\
> -            .value    = stringify(4),\
> -        }
> +    PC_COMPAT_2_0,\
> +    QEMU_COMPAT_1_7
>  
>  #define PC_COMPAT_1_6 \
> -        PC_COMPAT_1_7, \
> -        {\
> -            .driver   = "e1000",\
> -            .property = "mitigation",\
> -            .value    = "off",\
> -        },{\
> -            .driver   = "qemu64-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "qemu32-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(3),\
> -        },{\
> -            .driver   = "i440FX-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(1),\
> -        },{\
> -            .driver   = "q35-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(1),\
> -        }
> +    PC_COMPAT_1_7,\
> +    QEMU_COMPAT_1_6
>  
>  #define PC_COMPAT_1_5 \
> -        PC_COMPAT_1_6, \
> -        {\
> -            .driver   = "Conroe-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "Conroe-" TYPE_X86_CPU,\
> -            .property = "level",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "Penryn-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "Penryn-" TYPE_X86_CPU,\
> -            .property = "level",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "Nehalem-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "Nehalem-" TYPE_X86_CPU,\
> -            .property = "level",\
> -            .value    = stringify(2),\
> -        },{\
> -            .driver   = "virtio-net-pci",\
> -            .property = "any_layout",\
> -            .value    = "off",\
> -        },{\
> -            .driver = TYPE_X86_CPU,\
> -            .property = "pmu",\
> -            .value = "on",\
> -        },{\
> -            .driver   = "i440FX-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(0),\
> -        },{\
> -            .driver   = "q35-pcihost",\
> -            .property = "short_root_bus",\
> -            .value    = stringify(0),\
> -        }
> +    PC_COMPAT_1_6,\
> +    QEMU_COMPAT_1_5
>  
>  #define PC_COMPAT_1_4 \
> -        PC_COMPAT_1_5, \
> -        {\
> -            .driver   = "scsi-hd",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "scsi-cd",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "scsi-disk",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "ide-hd",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "ide-cd",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "ide-drive",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -        },{\
> -            .driver   = "virtio-blk-pci",\
> -            .property = "discard_granularity",\
> -            .value    = stringify(0),\
> -	},{\
> -            .driver   = "virtio-serial-pci",\
> -            .property = "vectors",\
> -            /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> -            .value    = stringify(0xFFFFFFFF),\
> -        },{ \
> -            .driver   = "virtio-net-pci", \
> -            .property = "ctrl_guest_offloads", \
> -            .value    = "off", \
> -        },{\
> -            .driver   = "e1000",\
> -            .property = "romfile",\
> -            .value    = "pxe-e1000.rom",\
> -        },{\
> -            .driver   = "ne2k_pci",\
> -            .property = "romfile",\
> -            .value    = "pxe-ne2k_pci.rom",\
> -        },{\
> -            .driver   = "pcnet",\
> -            .property = "romfile",\
> -            .value    = "pxe-pcnet.rom",\
> -        },{\
> -            .driver   = "rtl8139",\
> -            .property = "romfile",\
> -            .value    = "pxe-rtl8139.rom",\
> -        },{\
> -            .driver   = "virtio-net-pci",\
> -            .property = "romfile",\
> -            .value    = "pxe-virtio.rom",\
> -        },{\
> -            .driver   = "486-" TYPE_X86_CPU,\
> -            .property = "model",\
> -            .value    = stringify(0),\
> -        }
> +    PC_COMPAT_1_5,\
> +    QEMU_COMPAT_1_4
> +
>  

Two emoty lines.

>  #define PC_COMMON_MACHINE_OPTIONS \
>      .default_boot_order = "cad"
> -- 
> 1.9.3

  reply	other threads:[~2014-06-25  5:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-25  2:04 [Qemu-devel] [PATCH v2 0/2] Introduce common QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25  2:04 ` [Qemu-devel] [PATCH v2 1/2] pc: Move q35 compat props to PC_COMPAT_* Eduardo Habkost
2014-06-25  5:53   ` Michael S. Tsirkin
2014-06-25  2:04 ` [Qemu-devel] [PATCH v2 2/2] machine: Introduce QEMU_COMPAT_* macros Eduardo Habkost
2014-06-25  5:56   ` Michael S. Tsirkin [this message]
2014-06-25 13:23     ` Eduardo Habkost
2014-06-25 14:18       ` Michael S. Tsirkin
2014-06-25  5:58 ` [Qemu-devel] [PATCH v2 0/2] Introduce common " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140625055643.GG21297@redhat.com \
    --to=mst@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.