[libvirt] [PATCH] domain controller index check
Ján Tomko
jtomko at redhat.com
Fri Jul 12 12:59:03 UTC 2013
On 07/09/2013 06:13 AM, Jincheng Miao wrote:
> The index of the controller should not be limited in [zero, INT_MAX].
> So use virStrToLong_ui() and check the limit of the controller
> index in virDomainControllerDefParseXML().
> ---
> src/conf/domain_conf.c | 6 +++---
> src/conf/domain_conf.h | 2 +-
> src/qemu/qemu_command.c | 2 +-
> src/vmx/vmx.c | 3 +--
> 4 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 402e6e9..d4c1054 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2655,7 +2655,7 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
>
> for (i = 0; i < def->ncontrollers; i++) {
> cont = def->controllers[i];
> - if (cont->idx > max_idx[cont->type])
> + if ((int)cont->idx > max_idx[cont->type])
^ missing space
> max_idx[cont->type] = cont->idx;
> }
>
> @@ -2663,7 +2663,7 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
> max_idx[VIR_DOMAIN_CONTROLLER_TYPE_USB] = -1;
>
> for (i = 0; i < VIR_DOMAIN_CONTROLLER_TYPE_LAST; i++) {
> - if (max_idx[i] >= 0 && !(bitmaps[i] = virBitmapNew(max_idx[i] + 1)))
> + if (max_idx[i] >= 0 && !(bitmaps[i] = virBitmapNew((size_t)max_idx[i] + 1)))
This cast seems unnecessary.
> goto no_memory;
> nbitmaps++;
> }
> @@ -5593,7 +5593,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>
> idx = virXMLPropString(node, "index");
> if (idx) {
> - if (virStrToLong_i(idx, NULL, 10, &def->idx) < 0) {
> + if (virStrToLong_ui(idx, NULL, 10, &def->idx) < 0 || def->idx > INT_MAX) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Cannot parse controller index %s"), idx);
> goto error;
If we parse it as unsigned, we should print it as unsigned too. I've split the
line over 80 columns and squashed this in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b515887..354131e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14382,7 +14382,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
virBufferAsprintf(buf,
- " <controller type='%s' index='%d'",
+ " <controller type='%s' index='%u'",
type, def->idx);
if (model) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index da83eb6..7897b4b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -790,7 +790,7 @@ struct _virDomainVirtioSerialOpts {
> /* Stores the virtual disk controller configuration */
> struct _virDomainControllerDef {
> int type;
> - int idx;
> + unsigned int idx;
> int model; /* -1 == undef */
> unsigned int queues;
> union {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 46db28a..7fd1cbf 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1585,7 +1585,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>
> for (i = 0; i < def->ncontrollers; i++) {
> if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
> - if (def->controllers[i]->idx > max_idx)
> + if ((int)def->controllers[i]->idx > max_idx)
^ missing space
> max_idx = def->controllers[i]->idx;
> }
> }
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 5464d13..deddfaa 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -1664,8 +1664,7 @@ virVMXParseConfig(virVMXContext *ctx,
>
> for (controller = 0; controller < def->ncontrollers; ++controller) {
> if (def->controllers[controller]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
> - if (def->controllers[controller]->idx < 0 ||
> - def->controllers[controller]->idx > 3) {
> + if (def->controllers[controller]->idx > 3) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("SCSI controller index %d out of [0..3] range"),
> def->controllers[controller]->idx);
>
ACK and pushed with a test added:
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
new file mode 100644
index 0000000..be3d8f2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge-negative-index-invalid.xml
@@ -0,0 +1,15 @@
+<domain type='qemu'>
+ <name>fdr-br</name>
+ <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>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='pci' index='-1' model='pci-bridge'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d0d9cad..7d7332f 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1015,6 +1015,9 @@ mymain(void)
DO_TEST("mlock-off", QEMU_CAPS_MLOCK);
DO_TEST("mlock-unsupported", NONE);
+ DO_TEST_PARSE_ERROR("pci-bridge-negative-index-invalid",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
+
virObjectUnref(driver.config);
virObjectUnref(driver.caps);
virObjectUnref(driver.xmlopt);
Jan
More information about the libvir-list
mailing list