[libvirt] [PATCH] qemu: fix no error settings if fail to find a disk match path

lhuang lhuang at redhat.com
Thu Jul 9 06:43:53 UTC 2015


On 07/09/2015 02:37 PM, Martin Kletzander wrote:
> On Thu, Jul 09, 2015 at 11:49:15AM +0800, Luyao Huang wrote:
>> When we use get blockjob info to a unexist disk path, we will
>> get a error like this:
>>
>> # virsh blockjob r7 vdc
>> error: An error occurred, but the cause is unknown
>>
>> This is because we do not set the error when jump to endjob.
>> As virDomainDiskByName won't set the error, we need set them
>> in the callers function.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 900740e..f134248 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
>>     if (qemuDomainSupportsBlockJobs(vm, NULL) < 0)
>>         goto endjob;
>>
>> -    if (!(disk = virDomainDiskByName(vm->def, path, true)))
>> +    if (!(disk = virDomainDiskByName(vm->def, path, true))) {
>> +        virReportError(VIR_ERR_INVALID_ARG,
>> +                       _("invalid path %s not assigned to domain"), 
>> path);
>
> Since the 'path' parameter can be both path and a disk name
> (e.g. 'vdc' in your example), I wouldn't use "path" here because it
> looks weird when you get an error saying:
>
>  invalid path vdc not assigned to domain
>
> I'd change it to something more abstract like:
>
>  Disk '%s' not found in the domain
>
> or anything else but with such specification.  If you're OK with that
> I'll push it with the modification (also fixing the commit message).
>

Okay, i think it will be ok for me as it sounds more friendly.

Thanks a lot for your quick review and help.

Luyao
>>         goto endjob;
>> +    }
>>
>>     qemuDomainObjEnterMonitor(driver, vm);
>>     ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
>> -- 
>> 1.8.3.1
>>
>> -- 
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list