Re: [libvirt] [libvirt PATCH v2 39/44] Deprecate QEMU_CAPS_MACHINE_OPT

2018-04-17 Thread Ján Tomko 

On Tue, Apr 17, 2018 at 01:16:41PM +0200, Andrea Bolognani wrote:

On Mon, 2018-04-09 at 17:20 +0200, Ján Tomko wrote:

diff --git a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args 
b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
index 6f332941ce..6a25e53175 100644
--- a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
+++ b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-aarch64 \
 -name aarch64test \
 -S \
--M virt \
+-machine virt,accel=tcg \
 -cpu cortex-a53 \
 -m 1024 \
 -smp 1,sockets=1,cores=1,threads=1 \

[ ... etc ...]


One of the hunks you snipped looks pretty interesting:


diff --git a/tests/qemuxml2argvdata/controller-order.args 
b/tests/qemuxml2argvdata/controller-order.args
index 276f42fdce..7de96d5620 100644
--- a/tests/qemuxml2argvdata/controller-order.args
+++ b/tests/qemuxml2argvdata/controller-order.args
@@ -7,8 +7,7 @@ QEMU_AUDIO_DRV=spice \
 /usr/bin/qemu-system-x86_64 \
 -name fdr \
 -S \
--M rhel6.1.0 \
--enable-kvm \
+-machine rhel6.1.0,accel=kvm \
 -m 4096 \
 -smp 4,sockets=4,cores=1,threads=1 \
 -uuid d091ea82-29e6-2e34-3005-f02617b36e87 \


Unless I'm mistaken, we only use qemuBuildObsoleteAccelArg() (the
function that adds -enable-kvm) if we don't have -machine, which is
never going to be the case now, or we are asked for an accelerator
which is neither KVM or TCG, in which case we'll just error out.

As a follow-up cleanup patch, we should get rid of that function
altogether and turn the

   if (def->virtType == VIR_DOMAIN_VIRT_QEMU)
   virBufferAddLit(, ",accel=tcg");
   else if (def->virtType == VIR_DOMAIN_VIRT_KVM)
   virBufferAddLit(, ",accel=kvm");
   else
   obsoleteAccel = true;

from qemuBuildMachineCommandLine() into

   switch ((virDomainVirtType) def->virtType) {
   case VIR_DOMAIN_VIRT_QEMU:
   virBufferAddLit(, ",accel=tcg");
   break;
   case VIR_DOMAIN_VIRT_KVM:
   virBufferAddLit(, ",accel=kvm");
   break;
   default:
   virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  _("the QEMU binary does not support %s"),
  virDomainVirtTypeToString(def->virtType));
   goto cleanup;
   }



Right, it seems the other option for AccelArg was VIR_DOMAIN_VIRT_XEN:
commit 1f17ce215f8db809a2e5b77ef27412b7352e1451
   qemu: Remove remnants of xenner support

(One of the many commits removing xenner support)

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt PATCH v2 39/44] Deprecate QEMU_CAPS_MACHINE_OPT

2018-04-17 Thread Andrea Bolognani 
On Mon, 2018-04-09 at 17:20 +0200, Ján Tomko wrote:
> diff --git a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args 
> b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
> index 6f332941ce..6a25e53175 100644
> --- a/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
> +++ b/tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args
> @@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
>  /usr/bin/qemu-system-aarch64 \
>  -name aarch64test \
>  -S \
> --M virt \
> +-machine virt,accel=tcg \
>  -cpu cortex-a53 \
>  -m 1024 \
>  -smp 1,sockets=1,cores=1,threads=1 \
> 
> [ ... etc ...]

One of the hunks you snipped looks pretty interesting:

> diff --git a/tests/qemuxml2argvdata/controller-order.args 
> b/tests/qemuxml2argvdata/controller-order.args
> index 276f42fdce..7de96d5620 100644
> --- a/tests/qemuxml2argvdata/controller-order.args
> +++ b/tests/qemuxml2argvdata/controller-order.args
> @@ -7,8 +7,7 @@ QEMU_AUDIO_DRV=spice \
>  /usr/bin/qemu-system-x86_64 \
>  -name fdr \
>  -S \
> --M rhel6.1.0 \
> --enable-kvm \
> +-machine rhel6.1.0,accel=kvm \
>  -m 4096 \
>  -smp 4,sockets=4,cores=1,threads=1 \
>  -uuid d091ea82-29e6-2e34-3005-f02617b36e87 \

Unless I'm mistaken, we only use qemuBuildObsoleteAccelArg() (the
function that adds -enable-kvm) if we don't have -machine, which is
never going to be the case now, or we are asked for an accelerator
which is neither KVM or TCG, in which case we'll just error out.

As a follow-up cleanup patch, we should get rid of that function
altogether and turn the

if (def->virtType == VIR_DOMAIN_VIRT_QEMU)
virBufferAddLit(, ",accel=tcg");
else if (def->virtType == VIR_DOMAIN_VIRT_KVM)
virBufferAddLit(, ",accel=kvm");
else
obsoleteAccel = true;

from qemuBuildMachineCommandLine() into

switch ((virDomainVirtType) def->virtType) {
case VIR_DOMAIN_VIRT_QEMU:
virBufferAddLit(, ",accel=tcg");
break;
case VIR_DOMAIN_VIRT_KVM:
virBufferAddLit(, ",accel=kvm");
break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("the QEMU binary does not support %s"),
   virDomainVirtTypeToString(def->virtType));
goto cleanup;
}

[...]
> -DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT);
> -DO_TEST("default-kvm-host-arch", QEMU_CAPS_MACHINE_OPT);
> -DO_TEST("default-qemu-host-arch", QEMU_CAPS_MACHINE_OPT);
> -DO_TEST("x86-kvm-32-on-64", QEMU_CAPS_MACHINE_OPT);
> +DO_TEST("kvm", NONE);

This one can definitely be dropped.

[...]
> @@ -1795,11 +1792,10 @@ mymain(void)
>  
>  DO_TEST("pseries-features",
>  QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
> -QEMU_CAPS_MACHINE_OPT,
>  QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
>  DO_TEST_FAILURE("pseries-features",
>  QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
> -QEMU_CAPS_MACHINE_OPT);
> +NONE);

Don't add NONE :)

[...]
> @@ -2552,110 +2546,110 @@ mymain(void)
>  DO_TEST("memory-hotplug-ppc64-nonuma", QEMU_CAPS_KVM, 
> QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA,
>  QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>  QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> -DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_MACHINE_OPT, 
> QEMU_CAPS_DEVICE_NVDIMM,
> +DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_DEVICE_NVDIMM,
>  QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, 
> QEMU_CAPS_OBJECT_MEMORY_FILE);
> -DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_MACHINE_OPT, 
> QEMU_CAPS_DEVICE_NVDIMM,
> +DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_DEVICE_NVDIMM,
>  QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, 
> QEMU_CAPS_OBJECT_MEMORY_FILE);
> -DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_MACHINE_OPT, 
> QEMU_CAPS_DEVICE_NVDIMM,
> +DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM,
>  QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, 
> QEMU_CAPS_OBJECT_MEMORY_FILE);

Since you're touching these anyway, might as well move
QEMU_CAPS_DEVICE_NVDIMM to its own line.

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [libvirt PATCH v2 39/44] Deprecate QEMU_CAPS_MACHINE_OPT

2018-04-09 Thread Ján Tomko 
Implied by QEMU >= 1.2.0.

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_capabilities.c   |   1 -
 src/qemu/qemu_capabilities.h   |   2 +-
 src/qemu/qemu_command.c| 317 +
 tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml |   1 -
 tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   1 -
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 -
 tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml  |   1 -
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml|   1 -
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml|   1 -
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml|   1 -
 tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |   1 -
 tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml|   1 -
 tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml|   1 -
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |   1 -
 .../aarch64-aavmf-virtio-mmio.args |   2 +-
 tests/qemuxml2argvdata/aarch64-acpi-uefi.args  |   2 +-
 [ ... etc ... ]
 tests/qemuxml2argvdata/watchdog-injectnmi.args |   2 +-
 tests/qemuxml2argvdata/watchdog.args   |   2 +-
 tests/qemuxml2argvtest.c   | 142 +
 tests/qemuxml2xmltest.c|   2 -
 597 files changed, 773 insertions(+), 853 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ce9fad52a3..6af3cc9d61 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3629,7 +3629,6 @@ static qemuMonitorCallbacks callbacks = {
 static void
 virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps)
 {
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT);
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE);
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY);
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_HOST_PCI_MULTIDOMAIN);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c57554db4f..f1a55c421d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -238,7 +238,7 @@ typedef enum {
 
 /* 135 */
 X_QEMU_CAPS_IPV6_MIGRATION, /* -incoming [::] */
-QEMU_CAPS_MACHINE_OPT, /* -machine */
+X_QEMU_CAPS_MACHINE_OPT, /* -machine */
 QEMU_CAPS_MACHINE_USB_OPT, /* -machine xxx,usb=on/off */
 QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, /* -tpmdev passthrough */
 QEMU_CAPS_DEVICE_TPM_TIS, /* -device tpm_tis */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45722aa1fe..e6c70f84bf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6996,6 +6996,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 const virDomainDef *def,
 virQEMUCapsPtr qemuCaps)
 {
+virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
+virTristateSwitch smm = def->features[VIR_DOMAIN_FEATURE_SMM];
+virCPUDefPtr cpu = def->cpu;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 bool obsoleteAccel = false;
 size_t i;
@@ -7008,228 +7011,186 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 if (!def->os.machine)
 return 0;
 
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_OPT)) {
-/* if no parameter to the machine type is needed, we still use
- * '-M' to keep the most of the compatibility with older versions.
- */
-virCommandAddArgList(cmd, "-M", def->os.machine, NULL);
-if (def->mem.dump_core) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("dump-guest-core is not available "
- "with this QEMU binary"));
-return -1;
-}
-
-if (def->mem.nosharepages) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("disable shared memory is not available "
- "with this QEMU binary"));
-return -1;
-}
+virCommandAddArg(cmd, "-machine");
+virBufferAdd(, def->os.machine, -1);
 
+if (def->virtType == VIR_DOMAIN_VIRT_QEMU)
+