2010/8/23 Eduardo Otubo <ot...@linux.vnet.ibm.com>:
> I fixed the way I check for invalid values and changed the way I report
> errors (from VIR_ERROR0 to PHYP_ERROR). I'll change the VIR_WARNs in
> another different patch.
Okay, that's fine.
> ---
> src/phyp/phyp_driver.c | 23 +++++++++++++++++++++++
> 1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index 251111d..a74eedf 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -3701,6 +3701,29 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
> int exit_status = 0;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
>
> + if (def->memory > 0) {
> + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s",
> + _("Field \"<memory>\" on the domain XML file is missing or
> has "
> + "invalid value."));
> + goto err;
> + }
> +
> + if (def->maxmem > 0) {
> + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s",
> + _("Field \"<currentMemory>\" on the domain XML file is
> missing or"
> + " has invalid value."));
> + goto err;
> + }
Sorry for confusing you about "comparing > 0", now the check is wrong
here and you report an error when the input is actually valid. Please
change this back to "if (!def->memory)".
> + if (def->ndisks > 0) {
> + if (!def->disks[0]->src) {
> + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s",
> + _("Field \"<src>\" under \"<disk>\" on the domain XML file
> is "
> + "missing."));
> + goto err;
> + }
> + }
Okay, no potential segfault here anymore. But a few lines below you
use def->disks[0]->src in a virBufferVSprintf call, without checking
if the first disk element is there.
So you either need to make the first disk element mandatory by
checking for def->ndisks being 0 and reporting an error in that case,
or by altering the virBufferVSprintf call to only output the
virtual_scsi_adapters=%s part when the domain XML contains a disk
element.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
