On 11/20/2012 03:27 PM, Peter Krempa wrote:
> On 11/20/12 14:55, Martin Kletzander wrote:
>> The error "... but the cause is unknown" appeared for XMLs similar to
>> this:
>>
>> <disk type='file' device='cdrom'>
>> <driver name='qemu' type='raw'/>
>> <source file='/dev/zero'/>
>> <target dev='sr0'/>
>> </disk>
>>
>> Notice unsupported disk type (for the driver), but also no address
>> specified. The first part is not a problem and we should not abort
>> immediately because of that, but the combination with the address
>> unknown was causing an unspecified error.
>> ---
>> src/util/util.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/util.c b/src/util/util.c
>> index 75b18c1..d5b2c97 100644
>> --- a/src/util/util.c
>> +++ b/src/util/util.c
>> @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) {
>> }
>> }
>>
>> - if (!ptr)
>> + if (!ptr) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Unknown disk name '%s' and no address
>> specified"),
>> + name);
>> return -1;
>> + }
>
> Some of the call paths of this function the callers do their own error
> reporting (see src/qemu/qemu_command.c) based on the return value. This
> should be consolidated.
>
> In case of this function I'd probably rather go for adding the error
> message on the appropriate places as the inability to parse the disk ID
> is ignored on some calls (maybe that should be fixed in the first place
> and than the error reporting is okay here.)
>
You're right, I went through the code just looking for the "un-errored"
negative return and haven't thought about that deeply enough. I figured
when the error gets set on two places, it'll be simply shadowed.
Sending a v2 in a minute.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list