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