Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI.
This requires two test updates in order to set the correct capability
bit for an xml2xml test as well as setting up the similar capability
for the pseries memlocktest.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/qemu/qemu_command.c | 70 +------------------------------------------
src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
tests/qemumemlocktest.c | 14 +++++++++
tests/qemuxml2xmltest.c | 5 +++-
4 files changed, 97 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0dbc73399..7a138f921 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2726,23 +2726,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pci-bridge controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
modelName, pciopts->chassisNr,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
modelName, pciopts->busNr,
def->info.alias);
@@ -2751,65 +2739,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the dmi-to-pci-bridge (i82801b11-bridge) "
- "controller is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- if ((pciopts->modelName ==
- VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (ioh3420) "
- "controller is not supported in this QEMU binary"));
- goto error;
- }
- if ((pciopts->modelName ==
- VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (pcie-root-port) "
- "controller is not supported in this QEMU binary"));
- goto error;
- }
-
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
modelName, pciopts->port,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-switch-upstream-port (x3130-upstream) "
- "controller is not supported in this QEMU binary"));
- goto error;
- }
-
virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The pcie-switch-downstream-port "
- "(xio3130-downstream) controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
modelName, pciopts->port,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb-pcie controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
modelName, pciopts->busNr,
def->info.alias);
@@ -2822,25 +2767,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
if (pciopts->targetIndex == 0)
goto done;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,index=%d,id=%s",
modelName, pciopts->targetIndex,
def->info.alias);
- if (pciopts->numaNode != -1) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller "
- "doesn't support numa_node on this QEMU binary"));
- goto error;
- }
+ if (pciopts->numaNode != -1)
virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode);
- }
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8cbb29e40..55750615c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4119,7 +4119,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
static int
qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
- const virDomainDef *def)
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
virDomainControllerModelPCI model = controller->model;
const virDomainPCIControllerOpts *pciopts;
@@ -4196,6 +4197,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pci-bridge controller is not supported "
+ "in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
@@ -4213,6 +4221,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pxb controller is not supported in this "
+ "QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
@@ -4224,6 +4239,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the dmi-to-pci-bridge (i82801b11-bridge) "
+ "controller is not supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
@@ -4242,6 +4264,22 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-root-port (ioh3420) controller "
+ "is not supported in this QEMU binary"));
+ return -1;
+ }
+
+ if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-root-port (pcie-root-port) controller "
+ "is not supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
@@ -4253,6 +4291,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-switch-upstream-port (x3130-upstream) "
+ "controller is not supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
@@ -4271,6 +4316,14 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-switch-downstream-port "
+ "(xio3130-downstream) controller is not "
+ "supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
@@ -4288,6 +4341,13 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pxb-pcie controller is not supported "
+ "in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
@@ -4309,6 +4369,21 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
return 0;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the spapr-pci-host-bridge controller is not "
+ "supported in this QEMU binary"));
+ return -1;
+ }
+
+ if (pciopts->numaNode != -1 &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the spapr-pci-host-bridge controller doesn't "
+ "support numa_node in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
@@ -4353,7 +4428,8 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
- ret = qemuDomainDeviceDefValidateControllerPCI(controller, def);
+ ret = qemuDomainDeviceDefValidateControllerPCI(controller, def,
+ qemuCaps);
break;
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
index 62bd25450..bc0b53e6f 100644
--- a/tests/qemumemlocktest.c
+++ b/tests/qemumemlocktest.c
@@ -63,6 +63,7 @@ mymain(void)
{
int ret = 0;
char *fakerootdir;
+ virQEMUCapsPtr qemuCaps = NULL;
if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
fprintf(stderr, "Out of memory\n");
@@ -127,6 +128,16 @@ mymain(void)
DO_TEST("pc-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64);
+ if (!(qemuCaps = virQEMUCapsNew())) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+ if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0) {
+ ret = -1;
+ goto cleanup;
+ };
DO_TEST("pseries-kvm", 20971520);
DO_TEST("pseries-tcg", 0);
@@ -140,6 +151,9 @@ mymain(void)
DO_TEST("pseries-hardlimit+locked+hostdev", 2147483648);
DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
+ cleanup:
+ virObjectUnref(qemuCaps);
+
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2be8eb2c1..e866fb724 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1310,7 +1310,10 @@ mymain(void)
DO_TEST("intel-iommu-machine",
QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACHINE_IOMMU);
- DO_TEST("intel-iommu-caching-mode", NONE);
+ DO_TEST("intel-iommu-caching-mode",
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420);
DO_TEST("intel-iommu-eim", NONE);
DO_TEST("intel-iommu-device-iotlb", NONE);
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/06/2018 12:47 AM, John Ferlan wrote:
> Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI.
>
> This requires two test updates in order to set the correct capability
> bit for an xml2xml test as well as setting up the similar capability
> for the pseries memlocktest.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> src/qemu/qemu_command.c | 70 +------------------------------------------
> src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
> tests/qemumemlocktest.c | 14 +++++++++
> tests/qemuxml2xmltest.c | 5 +++-
> 4 files changed, 97 insertions(+), 72 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0dbc73399..7a138f921 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2726,23 +2726,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>
> switch ((virDomainControllerModelPCI) def->model) {
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("the pci-bridge controller "
> - "is not supported in this QEMU binary"));
> - goto error;
> - }
> virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
> modelName, pciopts->chassisNr,
> def->info.alias);
> break;
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("the pxb controller "
> - "is not supported in this QEMU binary"));
> - goto error;
> - }
> virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
> modelName, pciopts->busNr,
> def->info.alias);
I'm worried that we cannot do this. As I say in one of of comments to
previous patches - TOCTOU problem. What if somebody downgrades qemu
between define & start times? In this light, I no longer think we can do
03/16, can we?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/28/2018 03:48 AM, Michal Privoznik wrote:
> On 01/06/2018 12:47 AM, John Ferlan wrote:
>> Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI.
>>
>> This requires two test updates in order to set the correct capability
>> bit for an xml2xml test as well as setting up the similar capability
>> for the pseries memlocktest.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/qemu/qemu_command.c | 70 +------------------------------------------
>> src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
>> tests/qemumemlocktest.c | 14 +++++++++
>> tests/qemuxml2xmltest.c | 5 +++-
>> 4 files changed, 97 insertions(+), 72 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 0dbc73399..7a138f921 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -2726,23 +2726,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>>
>> switch ((virDomainControllerModelPCI) def->model) {
>> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
>> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("the pci-bridge controller "
>> - "is not supported in this QEMU binary"));
>> - goto error;
>> - }
>> virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
>> modelName, pciopts->chassisNr,
>> def->info.alias);
>> break;
>> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
>> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("the pxb controller "
>> - "is not supported in this QEMU binary"));
>> - goto error;
>> - }
>> virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
>> modelName, pciopts->busNr,
>> def->info.alias);
>
> I'm worried that we cannot do this. As I say in one of of comments to
> previous patches - TOCTOU problem. What if somebody downgrades qemu
> between define & start times? In this light, I no longer think we can do
> 03/16, can we?
>
> Michal
>
I assume you mean 2 & 3 then? Still TOCTOU would be true for any
Validate vs. Run checks.
Long ago, there was some review or comment made where there was a desire
to move many of the domain validation checks from command line building
into a separate phase so that the only thing that fails during command
line building would be something that really caused the command line
build to fail, not some configuration error.
Doing that was tricky due to the domains disappearing problem. IIRC
that's essentially why the Validation code was created.
As for someone downgrading - well they're either going to fail on their
next edit of their guest or they're going to fail when they try to run,
right? It won't be a libvirt failure, it'll be a qemu failure. Still, if
the run fails after a qemu downgrade because someone didn't validate
that the guest that they changed to take advantage of some feature in
the qemu that they now have downgraded from, then who's "problem" is
that? <tongue-in-cheek> Ooohh - excellent qemu now supports X, let's
upgrade and use X. So update qemu and alter the guest to use X. After
using X it's determined, damn, I don't like X, so I'm going to downgrade
to ensure X isn't used. Now if they don't update their guest to remove X
as well, then the next start will fail in qemu because X doesn't exist.
Of course they'll blame and curse libvirt, but maybe, just maybe,
they'll recall they forgot to remove X from the guest. Who's fault is
that? </tongue-in-cheek>
Look if the desire is to just keep all the controller validation checks
at run time, then fine - I can drop this series. That's fine.
Personally, it felt better doing things during validate.
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jan 05, 2018 at 06:47:37PM -0500, John Ferlan wrote: >Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI. > >This requires two test updates in order to set the correct capability >bit for an xml2xml test as well as setting up the similar capability >for the pseries memlocktest. > >Signed-off-by: John Ferlan <jferlan@redhat.com> >--- > src/qemu/qemu_command.c | 70 +------------------------------------------ > src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++-- > tests/qemumemlocktest.c | 14 +++++++++ > tests/qemuxml2xmltest.c | 5 +++- > 4 files changed, 97 insertions(+), 72 deletions(-) > ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.