[libvirt] [PATCH 6/6] storage:dir: adapts .refreshVol .refreshPool for ploop volumes

[Olga Krishtal]

On 18/02/16 17:04, Maxim Nestratov wrote:
> 18.02.2016 16:46, Ján Tomko пишет:
>> On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
>>> To update information about ploop volumes inside the a single pool 
>>> we need
>>> to be sure that it is the ploop directory and not some other directory.
>>> Ploop volume directory obligatory contains root.hds - image file and 
>>> disk
>>> descriptor - DiskDescriptor.xml. If path to a volume is a path to some
>>> directory, we check the existance of this files.
>>>
>> With each ploop volume being a directory with a ploop disk image and the
>> XML, I think they deserve a separate pool type.
>> The ploop image (root.hds) could be detected as such by the fs pool,
>> but creating and deleting the directories feels out of place in this
>> backend.
>
> Actually one of the main intention of implementing ploop disk support 
> in storage pool was
> ability to make it possible to work with existing storage pools like 
> NFS, DIR, FS. Creating a
> separate  storage pool makes it impossible. Also, our future plans to 
> expand storage pools
> with new pool type like VZ storage and CEPH FS becomes worthless either.
>
>>
>> Also, this should be documented in docs/storage.html.in, maybe with some
>> examples.
>>
Ok
>>> The capacity of a ploop volume is obtained via offset
>>> in the header file:
>>> https://openvz.org/Ploop/format
>>>
>>> Signed-off-by: Olga Krishtal <okrishtal at virtuozzo.com>
>>> ---
>>>   src/storage/storage_backend.c         | 92 
>>> ++++++++++++++++++++++++++---------
>>>   src/storage/storage_backend.h         |  2 +-
>>>   src/storage/storage_backend_fs.c      |  6 ++-
>>>   src/storage/storage_backend_logical.c |  2 +-
>>>   src/util/virstoragefile.c             |  9 +++-
>>>   5 files changed, 84 insertions(+), 27 deletions(-)
>>>
>>> @@ -1636,6 +1661,18 @@ virStorageBackendVolOpen(const char *path, 
>>> struct stat *sb,
>>>           virReportError(VIR_ERR_INTERNAL_ERROR,
>>>                          _("Volume path '%s' is a socket"), path);
>>>           return -1;
>>> +    } else if (S_ISDIR(sb->st_mode)) {
>>> +        if (virStorageBackendIsPloopDir(path)) {
>>> +            if (virAsprintf(&ploop_path, "%s/%s", target->path, 
>>> "root.hds") < 0)
>>> +                return -1;
>>> +            path = ploop_path;
>>> +            target->format = VIR_STORAGE_FILE_PLOOP;
>> virStorageBackendVolOpen should be just opening the volume, not probing
>> its format.
>>
How about virStorageBackendProbeTarget function:
I won't touch VirStorageBackendVolOpen, but
in
     if (S_ISDIR(sb.st_mode)) {
         if(this is ploop directory) {
             do some stuff
     }
         target->format = VIR_STORAGE_FILE_DIR;
         ret = 0;
         goto cleanup;
     }

>>
>> Also, this is not really a 'ploop file', but a 'ploop dir' - wouldn't
>> the lone disk images matching the magic in virstoragefile.c be
>> indistinguishable form the directories?
The difference between ploop volume and dir, that directory volume is 
not mounted yet.
In ploop volume will contain root.hds at least. So, we are able to read 
its header and get
basic information.
>>
>>> +            if (lstat(path, sb) < 0) {
>>> +                FAILED_STAT(path, ret);
>>> +                VIR_FREE(ploop_path);
>>> +                return ret;
>>> +            }
>>> +        }
>>>       }
>>>         /* O_NONBLOCK should only matter during open() for fifos and
>>> diff --git a/src/storage/storage_backend.h 
>>> b/src/storage/storage_backend.h
>>> index 65e91dc..de48012 100644
>>> --- a/src/storage/storage_backend.h
>>> +++ b/src/storage/storage_backend.h
>>> @@ -203,7 +203,7 @@ enum {
>>>   # define VIR_STORAGE_VOL_OPEN_DEFAULT 
>>> (VIR_STORAGE_VOL_OPEN_REG      |\
>>> VIR_STORAGE_VOL_OPEN_BLOCK)
>>>   -int virStorageBackendVolOpen(const char *path, struct stat *sb,
>>> +int virStorageBackendVolOpen(virStorageSourcePtr target, struct 
>>> stat *sb,
>>>                                unsigned int flags)
>> This should not be needed if the format is detected outside
>> virStorageBackendVolOpen.
>>
>> Jan
>