On Sat, Oct 20, 2012 at 03:47:10PM -0600, Eric Blake wrote:
> Yet another instance of where using plain open() mishandles files
> that live on root-squash NFS, and where improving the API can
> improve the chance of a successful probe.
>
> * src/util/storage_file.h (virStorageFileProbeFormat): Alter
> signature.
> * src/util/storage_file.c (virStorageFileProbeFormat): Use better
> method for opening file.
> * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Update caller.
> * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
> Likewise.
> ---
>
> v8: new patch
>
> src/qemu/qemu_driver.c | 3 ++-
> src/storage/storage_backend_fs.c | 2 +-
> src/util/storage_file.c | 4 ++--
> src/util/storage_file.h | 2 +-
> 4 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index feda4d9..dc9c62c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9309,7 +9309,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
> format = disk->format;
> } else {
> if (driver->allowDiskFormatProbing) {
> - if ((format = virStorageFileProbeFormat(disk->src)) < 0)
> + if ((format = virStorageFileProbeFormat(disk->src, driver->user,
> + driver->group)) < 0)
> goto cleanup;
> } else {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index cecc2d2..30e203c 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -104,7 +104,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
> meta->backingStore = NULL;
> if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO &&
> meta->backingStoreIsFile) {
> - if ((ret = virStorageFileProbeFormat(*backingStore)) < 0) {
> + if ((ret = virStorageFileProbeFormat(*backingStore, -1, -1)) < 0) {
> /* If the backing file is currently unavailable, only log an error,
> * but continue. Returning -1 here would disable the whole storage
> * pool, making it unavailable for even maintenance. */
> diff --git a/src/util/storage_file.c b/src/util/storage_file.c
> index 882df6e..e0b4178 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -830,11 +830,11 @@ cleanup:
> * Best option: Don't use this function
> */
> int
> -virStorageFileProbeFormat(const char *path)
> +virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
> {
> int fd, ret;
>
> - if ((fd = open(path, O_RDONLY)) < 0) {
> + if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {
> virReportSystemError(errno, _("cannot open file '%s'"), path);
> return -1;
> }
> diff --git a/src/util/storage_file.h b/src/util/storage_file.h
> index 9b00dba..abfaca9 100644
> --- a/src/util/storage_file.h
> +++ b/src/util/storage_file.h
> @@ -66,7 +66,7 @@ struct _virStorageFileMetadata {
> # define DEV_BSIZE 512
> # endif
>
> -int virStorageFileProbeFormat(const char *path);
> +int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
> int virStorageFileProbeFormatFromFD(const char *path,
> int fd);
>
I don't see how passing more context informations could be a bad idea
and usually we can't predict when something may break various NFS
scenario except by testing the code, ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list