Re: [libvirt] [PATCH 5/6] util: refactor storage file checks to allow error reporting

On Wed, Apr 25, 2018 at 16:52:42 +0100, Daniel Berrange wrote: > The virStorageFileSupportsSecurityDriver and > virStorageFileSupportsAccess currently just return a boolean > value. This is ok because they don't have any failure scenarios > but a subsequent patch is going to introduce potential failure > scenario. This changes their return type from a boolean to an > int with values -1, 0, 1. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 21 +++++++++------ > src/qemu/qemu_driver.c | 6 +++-- > src/util/virstoragefile.c | 66 +++++++++++++++++++++++++++++++---------------- > src/util/virstoragefile.h | 4 +-- > 4 files changed, 63 insertions(+), 34 deletions(-) [...] > index f09035cd4a..da13d51d32 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -4098,34 +4098,46 @@ virStorageFileIsInitialized(const virStorageSource *src) [...] > -static bool > +static int > virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) > { > virStorageFileBackendPtr backend; > + int ret; Hmm, isn't 'rv' better in the case when this variable actually is not returned?

> > - if (!(backend = virStorageFileGetBackendForSupportCheck(src))) > - return false; > + ret = virStorageFileGetBackendForSupportCheck(src, &backend); > + if (ret < 0) > + return -1; > + > + if (!backend) > + return 0; > > return backend->storageFileGetUniqueIdentifier && > - backend->storageFileRead && > - backend->storageFileAccess; > + backend->storageFileRead && > + backend->storageFileAccess ? 1 : 0; Alignment looks off

> } > > > @@ -4137,15 +4149,19 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) > * Check if a storage file supports operations needed by the security > * driver to perform labelling > */ > -bool > +int > virStorageFileSupportsSecurityDriver(const virStorageSource *src) > { > virStorageFileBackendPtr backend; > + int ret; As in above hunk. > > - if (!(backend = virStorageFileGetBackendForSupportCheck(src))) > - return false; > + ret = virStorageFileGetBackendForSupportCheck(src, &backend); > + if (ret < 0) > + return -1; > + if (backend == NULL) > + return 0; > > - return !!backend->storageFileChown; > + return backend->storageFileChown ? 1 : 0; ACK