On 03/31/2014 03:08 PM, Daniel P. Berrange wrote:
> On Mon, Mar 31, 2014 at 02:50:48PM -0400, Cole Robinson wrote:
>> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
>> index e02d17f..4d44897 100644
>> --- a/src/storage/storage_backend_fs.c
>> +++ b/src/storage/storage_backend_fs.c
>> @@ -904,8 +907,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
>> * have logged a similar message for the same problem, but only
>> * if AUTO format detection was used. */
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("cannot probe backing volume info: %s"),
>> - vol->backingStore.path);
>> + _("cannot probe backing volume path '%s': %s"),
>> + vol->backingStore.path,
>> + virGetLastErrorMessage());
>
> [snip]
>
>> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>> index 4c2484d..51404ff 100644
>> --- a/src/storage/storage_backend_scsi.c
>> +++ b/src/storage/storage_backend_scsi.c
>> @@ -202,8 +202,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>> if (virStorageBackendUpdateVolInfo(vol, true, true,
>> VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("Failed to update volume for '%s'"),
>> - devpath);
>> + _("Failed to update volume for '%s': %s"),
>> + devpath, virGetLastErrorMessage());
>> retval = -1;
>> goto free_vol;
>
> The fact that you can call virGetLastErrorMessage() here is showing that
> we're overwriting earlier errors. IMHO use of virGetLastErrorMessage()
> is exclusivly for client applicatons / test suites. We should just not
> overwrite the original errors here.
>
> Regards,
> Daniel
>
Indeed in this case it's not that useful, I was just preserving original
behavior. I'll drop it in the next round.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list