On Thu, Apr 26, 2018 at 11:15:42AM +0200, Peter Krempa wrote:
> 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é <berra...@redhat.com>
> > ---
> > 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?
Sure will change it.
>
> >
> > - 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
Depends on your POV - this is standard indentation after a new
line - it would only line up following lines if there was a
opening round bracket on first line. That said I'll change it
to avoid affecting pre-existing code alignment.
>
> > }
> >
> >
> > @@ -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
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
