[libvirt] [PATCH v3 0/4] Add support for adjusting the 64-bit PCI hole size
v3: use <pcihole64> sub-element of the root PCI controller with 'unit' attribute, defaulting to KiB v2: https://www.redhat.com/archives/libvir-list/2013-August/msg00565.html Use 'pcihole64' attribute of the root PCI controller instead of <pcihole64> element in domain features. v1: https://www.redhat.com/archives/libvir-list/2013-August/msg00510.html https://bugzilla.redhat.com/show_bug.cgi?id=990418 Ján Tomko (4): Move virDomainParseScaledValue earlier Allow controller XML parsing to use XPath context Add pcihole64 element to root PCI controllers Build QEMU command line for pcihole64 docs/formatdomain.html.in | 9 ++ docs/schemas/domaincommon.rng | 32 +++-- src/conf/domain_conf.c | 140 +++++++++++++-------- src/conf/domain_conf.h | 8 ++ src/qemu/qemu_capabilities.c | 14 +++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 58 +++++++++ .../qemuxml2argv-pcihole64-gib.xml | 23 ++++ .../qemuxml2argv-pcihole64-none.args | 4 + .../qemuxml2argv-pcihole64-none.xml | 23 ++++ .../qemuxml2argv-pcihole64-q35.args | 9 ++ .../qemuxml2argv-pcihole64-q35.xml | 33 +++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 + tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 23 ++++ tests/qemuxml2argvtest.c | 10 ++ .../qemuxml2xmlout-pcihole64-gib.xml | 23 ++++ tests/qemuxml2xmltest.c | 5 + 17 files changed, 358 insertions(+), 63 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml -- 1.8.1.5
Let virDomainControllerDefParseXML use it without a forward declaration. --- src/conf/domain_conf.c | 105 +++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7309877..4e24101 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5587,6 +5587,59 @@ error: } +/* Parse a value located at XPATH within CTXT, and store the + * result into val. If REQUIRED, then the value must exist; + * otherwise, the value is optional. The value is in bytes. + * Return 1 on success, 0 if the value was not present and + * is not REQUIRED, -1 on failure after issuing error. */ +static int +virDomainParseScaledValue(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned long long *val, + unsigned long long scale, + unsigned long long max, + bool required) +{ + char *xpath_full = NULL; + char *unit = NULL; + int ret = -1; + unsigned long long bytes; + + *val = 0; + if (virAsprintf(&xpath_full, "string(%s)", xpath) < 0) + goto cleanup; + ret = virXPathULongLong(xpath_full, ctxt, &bytes); + if (ret < 0) { + if (ret == -2) + virReportError(VIR_ERR_XML_ERROR, + _("could not parse element %s"), + xpath); + else if (required) + virReportError(VIR_ERR_XML_ERROR, + _("missing element %s"), + xpath); + else + ret = 0; + goto cleanup; + } + VIR_FREE(xpath_full); + + if (virAsprintf(&xpath_full, "string(%s/@unit)", xpath) < 0) + goto cleanup; + unit = virXPathString(xpath_full, ctxt); + + if (virScaleInteger(&bytes, unit, scale, max) < 0) + goto cleanup; + + *val = bytes; + ret = 1; +cleanup: + VIR_FREE(xpath_full); + VIR_FREE(unit); + return ret; +} + + static int virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, const char *model) @@ -5777,58 +5830,6 @@ virDomainNetGenerateMAC(virDomainXMLOptionPtr xmlopt, } -/* Parse a value located at XPATH within CTXT, and store the - * result into val. If REQUIRED, then the value must exist; - * otherwise, the value is optional. The value is in bytes. - * Return 0 on success, -1 on failure after issuing error. */ -static int -virDomainParseScaledValue(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long long *val, - unsigned long long scale, - unsigned long long max, - bool required) -{ - char *xpath_full = NULL; - char *unit = NULL; - int ret = -1; - unsigned long long bytes; - - *val = 0; - if (virAsprintf(&xpath_full, "string(%s)", xpath) < 0) - goto cleanup; - ret = virXPathULongLong(xpath_full, ctxt, &bytes); - if (ret < 0) { - if (ret == -2) - virReportError(VIR_ERR_XML_ERROR, - _("could not parse element %s"), - xpath); - else if (required) - virReportError(VIR_ERR_XML_ERROR, - _("missing element %s"), - xpath); - else - ret = 0; - goto cleanup; - } - VIR_FREE(xpath_full); - - if (virAsprintf(&xpath_full, "string(%s/@unit)", xpath) < 0) - goto cleanup; - unit = virXPathString(xpath_full, ctxt); - - if (virScaleInteger(&bytes, unit, scale, max) < 0) - goto cleanup; - - *val = bytes; - ret = 0; -cleanup: - VIR_FREE(xpath_full); - VIR_FREE(unit); - return ret; -} - - /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ -- 1.8.1.5
On Thu, Aug 15, 2013 at 01:30:18PM +0200, Ján Tomko wrote:
Let virDomainControllerDefParseXML use it without a forward declaration. --- src/conf/domain_conf.c | 105 +++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 52 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
virDomainParseScaledValue requires it. --- src/conf/domain_conf.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4e24101..fece93a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5659,6 +5659,7 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, */ static virDomainControllerDefPtr virDomainControllerDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, unsigned int flags) { virDomainControllerDefPtr def; @@ -5667,6 +5668,9 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *idx = NULL; char *model = NULL; char *queues = NULL; + xmlNodePtr saved = ctxt->node; + + ctxt->node = node; if (VIR_ALLOC(def) < 0) return NULL; @@ -5808,6 +5812,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, } cleanup: + ctxt->node = saved; VIR_FREE(type); VIR_FREE(idx); VIR_FREE(model); @@ -9479,7 +9484,8 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_CONTROLLER: - if (!(dev->data.controller = virDomainControllerDefParseXML(node, flags))) + if (!(dev->data.controller = virDomainControllerDefParseXML(node, ctxt, + flags))) goto error; break; case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -11690,6 +11696,7 @@ virDomainDefParseXML(xmlDocPtr xml, for (i = 0; i < n; i++) { virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i], + ctxt, flags); if (!controller) goto error; -- 1.8.1.5
On Thu, Aug 15, 2013 at 01:30:19PM +0200, Ján Tomko wrote:
virDomainParseScaledValue requires it. --- src/conf/domain_conf.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
<controller type='pci' index='0' model='pci-root'> <pcihole64 unit='KiB'>1048576</pcihole64> </controller> It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in kilobytes (different unit can be specified on input), but it gets rounded up to the nearest GB by QEMU. Disabling it will be needed for guests that crash with the 64-bit PCI hole (like Windows XP), see: https://bugzilla.redhat.com/show_bug.cgi?id=990418 --- docs/formatdomain.html.in | 9 ++++++ docs/schemas/domaincommon.rng | 32 +++++++++++++++------ src/conf/domain_conf.c | 26 +++++++++++++++-- src/conf/domain_conf.h | 8 ++++++ .../qemuxml2argv-pcihole64-gib.xml | 23 +++++++++++++++ .../qemuxml2argv-pcihole64-none.xml | 23 +++++++++++++++ .../qemuxml2argv-pcihole64-q35.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 23 +++++++++++++++ .../qemuxml2xmlout-pcihole64-gib.xml | 23 +++++++++++++++ tests/qemuxml2xmltest.c | 5 ++++ 10 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 83d551a..91ac5f6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2344,6 +2344,15 @@ PCI controllers have an optional <code>model</code> attribute with possible values <code>pci-root</code>, <code>pcie-root</code>, <code>pci-bridge</code>, or <code>dmi-to-pci-bridge</code>. + The root controllers (<code>pci-root</code> and <code>pcie-root</code>) + have an optional <code>pcihole64</code> element specifying how big + (in kilobytes, or in the unit specified by <code>pcihole64</code>'s + <code>unit</code> attribute) the 64-bit PCI hole should be. Some guests (like + Windows XP or Windows Server 2003) might crash when QEMU and Seabios + are recent enough to support 64-bit PCI holes, unless this is disabled + (set to 0). <span class="since">Since 1.1.2 (QEMU only)</span> + </p> + <p> For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. pci-root has no address. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac807e6..cb6246c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1541,14 +1541,30 @@ <attribute name="type"> <value>pci</value> </attribute> - <attribute name="model"> - <choice> - <value>pci-root</value> - <value>pcie-root</value> - <value>pci-bridge</value> - <value>dmi-to-pci-bridge</value> - </choice> - </attribute> + <!-- *-root controllers have an optional element "pcihole64"--> + <choice> + <group> + <attribute name="model"> + <choice> + <value>pci-root</value> + <value>pcie-root</value> + </choice> + </attribute> + <optional> + <element name="pcihole64"> + <ref name="scaledInteger"/> + </element> + </optional> + </group> + <group> + <attribute name="model"> + <choice> + <value>pci-bridge</value> + <value>dmi-to-pci-bridge</value> + </choice> + </attribute> + </group> + </choice> </group> <!-- virtio-serial has optional "ports" and "vectors" --> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fece93a..f14805f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5669,6 +5669,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *model = NULL; char *queues = NULL; xmlNodePtr saved = ctxt->node; + int rc; ctxt->node = node; @@ -5781,7 +5782,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: switch (def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: { + unsigned long long bytes; if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_XML_ERROR, "%s", _("pci-root and pcie-root controllers should not " @@ -5794,7 +5796,15 @@ virDomainControllerDefParseXML(xmlNodePtr node, "should have index 0")); goto error; } + if ((rc = virDomainParseScaledValue("./pcihole64", ctxt, + &bytes, 1024, + 1024ULL * ULONG_MAX, false)) < 0) + goto error; + if (rc == 1) + def->opts.pciopts.pcihole64 = true; + def->opts.pciopts.pcihole64size = VIR_DIV_UP(bytes, 1024); + } } default: @@ -14462,6 +14472,7 @@ virDomainControllerDefFormat(virBufferPtr buf, { const char *type = virDomainControllerTypeToString(def->type); const char *model = NULL; + bool pcihole64 = false; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -14499,11 +14510,17 @@ virDomainControllerDefFormat(virBufferPtr buf, } break; + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + if (def->opts.pciopts.pcihole64) + pcihole64 = true; + break; + default: break; } - if (def->queues || virDomainDeviceInfoIsSet(&def->info, flags)) { + if (def->queues || virDomainDeviceInfoIsSet(&def->info, flags) || + pcihole64) { virBufferAddLit(buf, ">\n"); if (def->queues) @@ -14513,6 +14530,11 @@ virDomainControllerDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; + if (pcihole64) { + virBufferAsprintf(buf, " <pcihole64 unit='KiB'>%lu</" + "pcihole64>\n", def->opts.pciopts.pcihole64size); + } + virBufferAddLit(buf, " </controller>\n"); } else { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3e118d6..168d03f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -811,6 +811,13 @@ struct _virDomainVirtioSerialOpts { int vectors; /* -1 == undef */ }; +typedef struct _virDomainPciControllerOpts virDomainPciControllerOpts; +typedef virDomainPciControllerOpts *virDomainPciControllerOptsPtr; +struct _virDomainPciControllerOpts { + bool pcihole64; + unsigned long pcihole64size; +}; + /* Stores the virtual disk controller configuration */ struct _virDomainControllerDef { int type; @@ -819,6 +826,7 @@ struct _virDomainControllerDef { unsigned int queues; union { virDomainVirtioSerialOpts vioserial; + virDomainPciControllerOpts pciopts; } opts; virDomainDeviceInfo info; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml new file mode 100644 index 0000000..13eb2dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>c84fc647-6198-4ff9-bf81-d65a1f8f5ec0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'> + <pcihole64 unit='GiB'>1</pcihole64> + </controller> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml new file mode 100644 index 0000000..a181d6c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>c84fc647-6198-4ff9-bf81-d65a1f8f5ec0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'> + <pcihole64 unit='KiB'>0</pcihole64> + </controller> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml new file mode 100644 index 0000000..ee151be --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'> + <pcihole64 unit='KiB'>1048576</pcihole64> + </controller> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <controller type='sata' index='0'/> + <video> + <model type='qxl' ram='65536' vram='18432' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml new file mode 100644 index 0000000..60da238 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>3c7c30b5-7866-4b05-8a29-efebccba52a0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'> + <pcihole64 unit='KiB'>1048576</pcihole64> + </controller> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml new file mode 100644 index 0000000..6d949ac --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>c84fc647-6198-4ff9-bf81-d65a1f8f5ec0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'> + <pcihole64 unit='KiB'>1048576</pcihole64> + </controller> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5c6730d..7302043 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -309,6 +309,11 @@ mymain(void) DO_TEST_DIFFERENT("s390-defaultconsole"); + DO_TEST("pcihole64"); + DO_TEST_DIFFERENT("pcihole64-gib"); + DO_TEST("pcihole64-none"); + DO_TEST("pcihole64-q35"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.1.5
On Thu, Aug 15, 2013 at 01:30:20PM +0200, Ján Tomko wrote:
<controller type='pci' index='0' model='pci-root'> <pcihole64 unit='KiB'>1048576</pcihole64> </controller>
It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in kilobytes (different unit can be specified on input), but it gets rounded up to the nearest GB by QEMU.
Disabling it will be needed for guests that crash with the 64-bit PCI hole (like Windows XP), see: https://bugzilla.redhat.com/show_bug.cgi?id=990418 --- docs/formatdomain.html.in | 9 ++++++ docs/schemas/domaincommon.rng | 32 +++++++++++++++------ src/conf/domain_conf.c | 26 +++++++++++++++-- src/conf/domain_conf.h | 8 ++++++ .../qemuxml2argv-pcihole64-gib.xml | 23 +++++++++++++++ .../qemuxml2argv-pcihole64-none.xml | 23 +++++++++++++++ .../qemuxml2argv-pcihole64-q35.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 23 +++++++++++++++ .../qemuxml2xmlout-pcihole64-gib.xml | 23 +++++++++++++++ tests/qemuxml2xmltest.c | 5 ++++ 10 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-gib.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcihole64-gib.xml
ACK though it'd be good to get a second ack on the XML design before pushing. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 47cc07a..87c9a96 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -235,6 +235,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vnc-share-policy", /* 150 */ "device-del-event", "dmi-to-pci-bridge", + "i440fx-pci-hole64-size", + "q35-pci-hole64-size", ); struct _virQEMUCaps { @@ -1436,6 +1438,14 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsScsiGeneric[] = { { "bootindex", QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsI440FXPciHost[] = { + { "pci-hole64-size", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE }, +}; + +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PciHost[] = { + { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE }, +}; + struct virQEMUCapsObjectTypeProps { const char *type; struct virQEMUCapsStringFlags *props; @@ -1473,6 +1483,10 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbHost) }, { "scsi-generic", virQEMUCapsObjectPropsScsiGeneric, ARRAY_CARDINALITY(virQEMUCapsObjectPropsScsiGeneric) }, + { "i440FX-pcihost", virQEMUCapsObjectPropsI440FXPciHost, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPciHost) }, + { "q35-pcihost", virQEMUCapsObjectPropsQ35PciHost, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PciHost) }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 074e55d..69f3395 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -191,6 +191,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */ + QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size */ + QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8b628d6..5ad73fd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2399,6 +2399,17 @@ qemuDomainMachineIsQ35(virDomainDefPtr def) } +static bool +qemuDomainMachineIsI440FX(virDomainDefPtr def) +{ + return (STREQ(def->os.machine, "pc") || + STRPREFIX(def->os.machine, "pc-0.") || + STRPREFIX(def->os.machine, "pc-1.") || + STRPREFIX(def->os.machine, "pc-i440") || + STRPREFIX(def->os.machine, "rhel")); +} + + static int qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, @@ -7919,6 +7930,53 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); } + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + cont->opts.pciopts.pcihole64) { + const char *hoststr = NULL; + bool cap = false; + bool machine = false; + + switch (cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + hoststr = "i440FX-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + machine = qemuDomainMachineIsI440FX(def); + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + hoststr = "q35-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); + machine = qemuDomainMachineIsQ35(def); + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("64-bit PCI hole setting is only for root" + " PCI controllers")); + goto error; + } + + if (!machine) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Setting the 64-bit PCI hole size is not " + "supported for machine '%s'"), def->os.machine); + goto error; + } + if (!cap) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("64-bit PCI hole size setting is not supported " + "with this QEMU binary")); + goto error; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr, + cont->opts.pciopts.pcihole64size); + } + } + for (i = 0; i < def->ndisks; i++) { virDomainDiskDefPtr disk = def->disks[i]; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args new file mode 100644 index 0000000..e878d2f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M pc-1.2 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args new file mode 100644 index 0000000..6d33b65 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi \ +-boot c -global q35-pcihost.pci-hole64-size=1048576K \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,unit=0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args new file mode 100644 index 0000000..3165139 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M pc-1.2 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi \ +-boot c -global i440FX-pcihost.pci-hole64-size=1048576K -usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 679124e..f274ced 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1037,6 +1037,16 @@ mymain(void) DO_TEST_PARSE_ERROR("pci-root-address", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("pcihole64", QEMU_CAPS_DEVICE, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + DO_TEST_FAILURE("pcihole64-none", QEMU_CAPS_DEVICE); + DO_TEST("pcihole64-q35", + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_Q35_PCI_HOLE64_SIZE); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.1.5
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. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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. Jan
On Tue, Aug 27, 2013 at 03:44:44PM +0200, Ján Tomko wrote:
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.
Oh, whoops, I missed that. ACK in that case. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/15/2013 01:30 PM, Ján Tomko wrote:
v3: use <pcihole64> sub-element of the root PCI controller with 'unit' attribute, defaulting to KiB
v2: https://www.redhat.com/archives/libvir-list/2013-August/msg00565.html Use 'pcihole64' attribute of the root PCI controller instead of <pcihole64> element in domain features.
v1: https://www.redhat.com/archives/libvir-list/2013-August/msg00510.html
https://bugzilla.redhat.com/show_bug.cgi?id=990418
Ján Tomko (4): Move virDomainParseScaledValue earlier Allow controller XML parsing to use XPath context Add pcihole64 element to root PCI controllers Build QEMU command line for pcihole64
*gentle ping* Jan
On 23.08.2013 09:16, Ján Tomko wrote:
On 08/15/2013 01:30 PM, Ján Tomko wrote:
v3: use <pcihole64> sub-element of the root PCI controller with 'unit' attribute, defaulting to KiB
v2: https://www.redhat.com/archives/libvir-list/2013-August/msg00565.html Use 'pcihole64' attribute of the root PCI controller instead of <pcihole64> element in domain features.
v1: https://www.redhat.com/archives/libvir-list/2013-August/msg00510.html
https://bugzilla.redhat.com/show_bug.cgi?id=990418
Ján Tomko (4): Move virDomainParseScaledValue earlier Allow controller XML parsing to use XPath context Add pcihole64 element to root PCI controllers Build QEMU command line for pcihole64
*gentle ping*
*gentle ACK* Michal
On 08/15/2013 01:30 PM, Ján Tomko wrote:
v3: use <pcihole64> sub-element of the root PCI controller with 'unit' attribute, defaulting to KiB
v2: https://www.redhat.com/archives/libvir-list/2013-August/msg00565.html Use 'pcihole64' attribute of the root PCI controller instead of <pcihole64> element in domain features.
v1: https://www.redhat.com/archives/libvir-list/2013-August/msg00510.html
https://bugzilla.redhat.com/show_bug.cgi?id=990418
Ján Tomko (4): Move virDomainParseScaledValue earlier Allow controller XML parsing to use XPath context Add pcihole64 element to root PCI controllers Build QEMU command line for pcihole64
I've pushed the series now. Thank you for the reviews! Jan
participants (3)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik