On 07/24/2015 12:14 PM, Martin Kletzander wrote:
> On Fri, Jul 17, 2015 at 02:43:38PM -0400, Laine Stump wrote:
>> This is backed by the qemu device ioh3420.
>>
>> chassis and port from the <model> subelement are used to store/set the
>> respective qemu device options for the ioh3420. Currently, chassis is
>> set to be the index of the controller, and port is set to
>> "(slot << 3) + function" (per suggestion from Alex Williamson).
>> ---
>> src/qemu/qemu_command.c | 40
>> ++++++++++++++++++++++
>> .../qemuxml2argv-pcie-root-port.args | 10 ++++++
>> tests/qemuxml2argvtest.c | 7 ++++
>> 3 files changed, 57 insertions(+)
>> create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 53f5317..1b86f1d 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4687,6 +4704,29 @@ qemuBuildControllerDevStr(virDomainDefPtr
>> domainDef,
>> virBufferAsprintf(&buf, "%s,id=%s",
>> def->opts.pciopts.type, def->info.alias);
>> break;
>> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
>> + if (!def->opts.pciopts.type) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("autogenerated pcie-root-port
>> options not set"));
>> + goto error;
>> + }
>
> I wonder how these errors can happen when the parsing itself will
> always set them up.
They can happen if somebody screws up the code. That's why they're
logged as "INTERNAL_ERROR" - they should never happen, and are just
there as a sanity check.
>
>> + if (STREQ(def->opts.pciopts.type, "ioh3420")) {
>> + if (!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;
>> + }
>> + } else {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("unknown pcie-root-port device '%s'"),
>> + def->opts.pciopts.type);
>> + goto error;
>> + }
>> + virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
>> + def->opts.pciopts.type,
>> def->opts.pciopts.port,
>> + def->opts.pciopts.chassis,
>> def->info.alias);
>> + break;
>
> This whole hunk would be way shorter with the type stored as an enum
Actually it ends up not being any shorter, since we have to check for
internal inconsistencies. It's definitely faster though (for whatever
that's worth).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list