Re: Storage Backend Error Handling Consistency

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



  Richard Laager wrote:

> These two blocks exist in different storage backends' createVol
> implementations:
> 
>     if (vol->target.format != VIR_STORAGE_FILE_RAW) {
>         virReportError(VIR_ERR_NO_SUPPORT, "%s",
>                        _("only RAW volumes are supported by this storage pool"));
>         return -VIR_ERR_NO_SUPPORT;
>     }
> 
>     if (vol->target.encryption != NULL) {
>         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                        "%s", _("storage pool does not support encrypted "
>                                "volumes"));
>         return -1;
>     }
> 
> My hunch is that the second example (VIR_ERR_CONFIG_UNSUPPORTED and
> returning -1) is correct in both of these cases.

I think the second example is more correct and
VIR_ERR_CONFIG_UNSUPPORTED should be used is such cases, because
VIR_ERR_NO_SUPPORT is supposed to mean a function unsupported by the
driver itself, not a situation when a user is requesting some
configuration that is not possible.

> Related... Which backends should be restricted to only raw? That code
> is from storage_backend_rbd.c, which seems to be the only one
> restricted in that way. But does it make sense to allow !raw on LVM (or
> ZFS), for example?

As for ZFS, I don't see how non-raw could be used. I didn't add a check
for that because I just did not think about that. Not sure about the
other backends though.

> Whatever the correct answer, I'm looking to submit one or more patches.
> 
> -- 
> Richard
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Roman Bogorodskiy

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]