[libvirt] [PATCH RFC 01/22] qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage

[Eric Blake]

On 05/06/2014 07:36 AM, Peter Krempa wrote:
> Refactor the function to accept a virStorageSourcePtr instead of just
> the path, add a check to run it only on local storage and fix callers
> (possibly by using a newly introduced wrapper that wraps a path in the
>  virStorageSource struct for legacy code)
> ---
>  src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 28 deletions(-)
> 

>  {
>      /* The easiest way to label a single file with the same
>       * permissions it would have as if part of the disk chain is to
>       * temporarily modify the disk in place.  */

Actually, the easiest way is probably to modify the audit and security
code to operate on a virStorageSourcePtr rather than a
virDomainDiskDefPtr - but that's refactoring that can be done independently.

> -    char *origsrc = disk->src.path;
> -    int origformat = disk->src.format;
> -    virStorageSourcePtr origchain = disk->src.backingStore;
> +    virStorageSource origdisk;
>      bool origreadonly = disk->readonly;
> +    virQEMUDriverConfigPtr cfg = NULL;
>      int ret = -1;
> -    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> 
> -    disk->src.path = (char *) file; /* casting away const is safe here */
> -    disk->src.format = VIR_STORAGE_FILE_RAW;
> -    disk->src.backingStore = NULL;
> +    /* XXX Labelling of non-local files isn't currently supported */
> +    if (virStorageSourceGetActualType(&disk->src) == VIR_STORAGE_TYPE_NETWORK)
> +        return 0;
> +
> +    cfg = virQEMUDriverGetConfig(driver);
> +
> +    /* XXX This will be easier when disk->src will be a pointer */

"will be" as in later patches make the conversion? Might read better as
"would be easier if disk->were a pointer"

> +    memcpy(&origdisk, &disk->src, sizeof(origdisk));
> +    memcpy(&disk->src, elem, sizeof(*elem));

And indeed, doing a pass over the source tree to use a storageSource via
pointer rather than inline struct would be a fairly mechanical (but
probably large) patch worth doing.

> @@ -12061,6 +12063,25 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
> 
> 
>  static int
> +qemuDomainPrepareDiskChainElementPath(virQEMUDriverPtr driver,
> +                                      virDomainObjPtr vm,
> +                                      virDomainDiskDefPtr disk,
> +                                      const char *file,
> +                                      qemuDomainDiskChainMode mode)
> +{
> +    virStorageSource src;
> +
> +    memset(&src, 0, sizeof(src));
> +
> +    src.type = VIR_STORAGE_TYPE_FILE;
> +    src.format = VIR_STORAGE_FILE_RAW;
> +    src.path = (char *) file; /* casting away const is safe here */

Indeed; we can later add a VIR_STRDUP and VIR_FREE if the internals make
it no longer safe.

ACK with comment cleanup.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140506/71772598/attachment-0001.sig>