Re: [libvirt] [PATCH v3 4/4] Build QEMU command line for pcihole64

On 08/27/2013 03:39 PM, Daniel P. Berrange wrote: > On Thu, Aug 15, 2013 at 01:30:21PM +0200, Ján Tomko wrote: >> QEMU commit 3984890 introduced the "pci-hole64-size" property, >> to i440FX-pcihost and q35-pcihost with a default setting of 2 GB. >> >> Translate <pcihole64>x<pcihole64/> to: >> -global q35-pcihost.pci-hole64-size=x for q35 machines and >> -global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines. >> >> Error out on other machine types or if the size was specified >> but the pcihost device lacks 'pci-hole64-size' property. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=990418 >> --- >> src/qemu/qemu_capabilities.c | 14 ++++++ >> src/qemu/qemu_capabilities.h | 2 + >> src/qemu/qemu_command.c | 58 ++++++++++++++++++++++ >> .../qemuxml2argv-pcihole64-none.args | 4 ++ >> .../qemuxml2argv-pcihole64-q35.args | 9 ++++ >> tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 ++ >> tests/qemuxml2argvtest.c | 10 ++++ >> 7 files changed, 102 insertions(+) >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args > > >> + for (i = 0; i < def->ncontrollers; i++) { >> + virDomainControllerDefPtr cont = def->controllers[i]; >> + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && >> + cont->opts.pciopts.pcihole64) { > > Hmm, by doing 'if (cont->opts.pciopts.pcihole64)' we loose the ability > to set pcihole64=0, to completely disable the PCI hole. Is this something > we need to worry about ? If so, then we'll need a way to distinguish a > hole value of 0, from the "no attribute set" scenario. > pciopts.pcihole64 is a bool meaning "the attribute is set", the size is stored in pciopts.pcihole64size.