[libvirt] [PATCH 0/3] More OOM check fixes
From: "Daniel P. Berrange" <berrange@redhat.com> More OOM fixes. These didn't cause crashes, rather invalid XML / cli arg generation. Daniel P. Berrange (3): Add missing check for OOM when building boot menu args Honour error returned by virBitmapFormat Check return value of virDomainControllerInsert when parsing QEMU args src/conf/domain_conf.h | 9 ++++++--- src/qemu/qemu_command.c | 38 ++++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 19 deletions(-) -- 1.8.3.1
From: "Daniel P. Berrange" <berrange@redhat.com> When building boot menu args, if OOM occurred the CLI args would end up containing 'order=(null)' due to a missing call to 'virBufferError'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 850ccea..9900b41 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8057,6 +8057,11 @@ qemuBuildCommandLine(virConnectPtr conn, if (boot_nparams > 0) { virCommandAddArg(cmd, "-boot"); + if (virBufferError(&boot_buf)) { + virReportOOMError(); + goto error; + } + if (boot_nparams < 2 || emitBootindex) { virCommandAddArgBuffer(cmd, &boot_buf); virBufferFreeAndReset(&boot_buf); -- 1.8.3.1
From: "Daniel P. Berrange" <berrange@redhat.com> The code formatting NUMA args was ignoring the return value of virBitmapFormat, so on OOM, it would silently drop the NUMA cpumask arg. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9900b41..fe3d353 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6861,22 +6861,22 @@ qemuBuildNumaArgStr(const virDomainDefPtr def, virCommandPtr cmd) virCommandAddArg(cmd, "-numa"); virBufferAsprintf(&buf, "node,nodeid=%d", def->cpu->cells[i].cellid); virBufferAddLit(&buf, ",cpus="); - cpumask = virBitmapFormat(def->cpu->cells[i].cpumask); - if (cpumask) { - /* Up through qemu 1.4, -numa does not accept a cpus - * argument any more complex than start-stop. - * - * XXX For qemu 1.5, the syntax has not yet been decided; - * but when it is, we need a capability bit and - * translation of our cpumask into the qemu syntax. */ - if (strchr(cpumask, ',')) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disjoint NUMA cpu ranges are not supported " - "with this QEMU")); - goto cleanup; - } - virBufferAdd(&buf, cpumask, -1); + if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) + goto cleanup; + + /* Up through qemu 1.4, -numa does not accept a cpus + * argument any more complex than start-stop. + * + * XXX For qemu 1.5, the syntax has not yet been decided; + * but when it is, we need a capability bit and + * translation of our cpumask into the qemu syntax. */ + if (strchr(cpumask, ',')) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disjoint NUMA cpu ranges are not supported " + "with this QEMU")); + goto cleanup; } + virBufferAdd(&buf, cpumask, -1); def->cpu->cells[i].mem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024) * 1024; virBufferAsprintf(&buf, ",mem=%d", def->cpu->cells[i].mem / 1024); -- 1.8.3.1
From: "Daniel P. Berrange" <berrange@redhat.com> The parsing of '-usb' did not check for failure of the virDomainControllerInsert method. As a result on OOM, the parser mistakenly attached USB disks to the IDE controller. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/domain_conf.h | 9 ++++++--- src/qemu/qemu_command.c | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9414ebf..5c33e08 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2361,7 +2361,8 @@ int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); const char *virDomainDiskPathByName(virDomainDefPtr, const char *name); int virDomainDiskInsert(virDomainDefPtr def, - virDomainDiskDefPtr disk); + virDomainDiskDefPtr disk) + ATTRIBUTE_RETURN_CHECK; void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, @@ -2415,7 +2416,8 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); int virDomainControllerInsert(virDomainDefPtr def, - virDomainControllerDefPtr controller); + virDomainControllerDefPtr controller) + ATTRIBUTE_RETURN_CHECK; void virDomainControllerInsertPreAlloced(virDomainDefPtr def, virDomainControllerDefPtr controller); int virDomainControllerFind(virDomainDefPtr def, int type, int idx); @@ -2425,7 +2427,8 @@ int virDomainLeaseIndex(virDomainDefPtr def, virDomainLeaseDefPtr lease); int virDomainLeaseInsert(virDomainDefPtr def, virDomainLeaseDefPtr lease); -int virDomainLeaseInsertPreAlloc(virDomainDefPtr def); +int virDomainLeaseInsertPreAlloc(virDomainDefPtr def) + ATTRIBUTE_RETURN_CHECK; void virDomainLeaseInsertPreAlloced(virDomainDefPtr def, virDomainLeaseDefPtr lease); virDomainLeaseDefPtr diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe3d353..0376611 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11624,7 +11624,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, ctldef->type = VIR_DOMAIN_CONTROLLER_TYPE_USB; ctldef->idx = 0; ctldef->model = -1; - virDomainControllerInsert(def, ctldef); + if (virDomainControllerInsert(def, ctldef) < 0) + goto error; } else if (STREQ(arg, "-pidfile")) { WANT_VALUE(); if (pidfile) -- 1.8.3.1
On 09/24/2013 12:10 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
More OOM fixes. These didn't cause crashes, rather invalid XML / cli arg generation.
Daniel P. Berrange (3): Add missing check for OOM when building boot menu args Honour error returned by virBitmapFormat Check return value of virDomainControllerInsert when parsing QEMU args
src/conf/domain_conf.h | 9 ++++++--- src/qemu/qemu_command.c | 38 ++++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 19 deletions(-)
ACK
participants (2)
-
Daniel P. Berrange -
Ján Tomko