[libvirt] [PATCH 6/7] Support NBD backed disks/filesystems in LXC driver

[John Ferlan]

On 03/15/2013 12:32 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> The LXC driver can already configure <disk> or <filesystem>
> devices to use the loop device. This extends it to also allow
> for use of the NBD device, to support non-raw formats.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/lxc/lxc_controller.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index 8f3ca6a..c433fb1 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -403,6 +403,46 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk)
>  }
>  
>  
> +static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
> +{
> +    char *dev;
> +
> +    if (virFileNBDDeviceAssociate(fs->src, &dev,
> +                                  !!fs->readonly) < 0)
> +        return -1;
> +
> +    /*
> +     * We now change it into a block device type, so that
> +     * the rest of container setup 'just works'
> +     */
> +    fs->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +    VIR_FREE(fs->src);
> +    fs->src = dev;
> +
> +    return 0;
> +}
> +
> +
> +static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk)
> +{
> +    char *dev;
> +
> +    if (virFileNBDDeviceAssociate(disk->src, &dev,
> +                                  !!disk->readonly) < 0)
> +        return -1;
> +
> +    /*
> +     * We now change it into a block device type, so that
> +     * the rest of container setup 'just works'
> +     */
> +    disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +    VIR_FREE(disk->src);
> +    disk->src = dev;
> +
> +    return 0;
> +}
> +
> +
>  static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
>  {
>      size_t i;
> @@ -435,6 +475,9 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
>                  goto cleanup;
>              }
>              ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd;
> +        } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) {
> +            if (virLXCControllerSetupNBDDeviceFS(fs) < 0)
> +                goto cleanup;
>          } else {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("fs driver %s is not supported"),
> @@ -449,8 +492,14 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
>          if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE)
>              continue;
>  
> -        if (!disk->driverName ||
> -            STREQ(disk->driverName, "loop")) {

This was if no driverName or if driverName is loop, then make sure RAW
or NONE for format...  That logic doesn't seem to change with the new
checks; however...

> +        /* If no driverName is set, we prefer 'loop' for
> +         * dealing with raw or undefined formats. Only
> +         * default to 'nbd' for non-raw formats.
> +         */
> +        if ((disk->driverName && STREQ(disk->driverName, "loop")) ||
> +            (!disk->driverName &&
> +             (disk->format == VIR_STORAGE_FILE_RAW ||
> +              disk->format == VIR_STORAGE_FILE_NONE))) {
>              if (disk->format != VIR_STORAGE_FILE_RAW &&
>                  disk->format != VIR_STORAGE_FILE_NONE) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -474,6 +523,17 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
>                  goto cleanup;
>              }
>              ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd;
> +        } else if (!disk->driverName ||
> +                   STREQ(disk->driverName, "nbd")) {

if driverName is not provided we fall into here?  Which doesn't seem to
be what was intended.  Seems this should be if driverName && nbd...

> +            if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT &&
> +                disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("Disk cache mode %s is not supported"),
> +                               virDomainDiskCacheTypeToString(disk->cachemode));
> +                goto cleanup;
> +            }
> +            if (virLXCControllerSetupNBDDeviceDisk(disk) < 0)
> +                goto cleanup;
>          } else {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("disk driver %s is not supported"),
> 

Which potentially causes issues hereif driverName == NULL, right?

Also might be good to document the nbd format on the formatdomain page.

John

John