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

[Cole Robinson]

On 04/28/2015 07:50 AM, Ján Tomko wrote:
> 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.
> 

Hmm, I got my error codes confused :)

I added the EPERM check, fixed the error message and the commit message, and
pushed

Thanks,
Cole