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

Re: [libvirt] [PATCH] storage: fs: Ignore volumes that fail to open with EPERM



The summary says 'EPERM', but the code checks for EACCES.

On Mon, Apr 27, 2015 at 11:57:53AM -0400, Cole Robinson wrote:
> Trying to use qemu:///session to create a storage pool pointing at
> /tmp will usually fail with something like:
> 
> $ virsh pool-start tmp
> error: Failed to start pool tmp
> error: cannot open volume '/tmp/systemd-private-c38cf0418d7a4734a66a8175996c384f-colord.service-kEyiTA': Permission denied
> 
> If any volume in an FS pool can't be opened by the daemon, the refresh
> fails, and the pool can't be used.
> 
> This causes pain for virt-install/virt-manager though. Imaging a user
> downloads a disk image to /tmp. virt-manager wants to import /tmp as
> a storage pool, so we can detect what disk format it is, and set the
> XML correctly. However this case will likely fail as explained above.
> 
> Change the logic here to skip volumes that fail to open. This could
> conceivably cause user complaints along the lines of 'why doesn't
> libvirt show $ROOT-OWNED-VOLUME-FOO', but figuring that currently
> the pool won't even startup, I don't think there are any current
> users that care about that case.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1103308
> ---
>  src/storage/storage_backend.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index e0311e1..186013c 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -1445,6 +1445,10 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
>              VIR_WARN("ignoring missing fifo '%s'", path);
>              return -2;
>          }
> +        if (errno == EACCES && noerror) {

Looking at open(2) we are unlikely to hit EPERM, but maybe it would be
better to look for both?

> +            VIR_WARN("ignore permission error for '%s'", path);

s/ignore/ignoring/ to match the other warnings

ACK whether you check EPERM or not, as long as you tweak the commit
summary.

Jan

Attachment: signature.asc
Description: Digital signature


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