Re: [Xen-devel] Re: [PATCH] libxl: virtio-blk-pci support for FV domain

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 10 May 2011, Wei Liu wrote:
> Refine the patch according to Stefano's advice.
> 
> 1. Writing to Xenstore in libxl_device_disk_add is left as-it-is.
> 2. Reserved encoding for virtio disk.
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ccf6518..6943b9d 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1028,6 +1028,9 @@ int libxl_device_disk_add(libxl_ctx *ctx,
> uint32_t domid, libxl_device_disk *dis
> 
> libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
>              device.backend_kind = DEVICE_QDISK;
>              break;
> +        case LIBXL_DISK_BACKEND_NONE:
> +         /* Nothing to do, not a Xen VBD */
> +         break;
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk
> backend type: %d\n", disk->backend);
>              rc = ERROR_INVAL;
> diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
> index a5be66f..6b27eae 100644
> --- a/tools/libxl/libxl.idl
> +++ b/tools/libxl/libxl.idl
> @@ -54,6 +54,7 @@ libxl_disk_backend = Enumeration("disk_backend", [
>      (1, "PHY"),
>      (2, "TAP"),
>      (3, "QDISK"),
> +    (4, "NONE"),
>      ])
> 
>  libxl_nic_type = Enumeration("nic_type", [
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 5d85822..d8de1b7 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -239,6 +239,13 @@ int libxl__device_disk_dev_number(char *virtpath,
> int *pdisk, int *ppartition)
>          if (ppartition) *ppartition = partition;
>          return (8 << 8) | (disk << 4) | partition;
>      }
> +    if (device_virtdisk_matches(virtpath, "vd",
> +                             &disk, 15,
> +                             &partition, 15)) {
> +        if (pdisk) *pdisk = disk;
> +        if (ppartition) *ppartition = partition;
> +        return (2 << 28) | (disk << 8) | partition;
> +    }
>      return -1;
>  }

libxl__device_disk_dev_number needs to return an error here, possibily
different from -1 to identify that the disk is not a xen VBD.
The caller needs to check for errors and return in case the disk doesn't
need a xen backend setup.


> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 76479fe..3403b5e 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -423,6 +423,10 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
>                      drive = libxl__sprintf
>                          (gc, "file=%s,if=ide,index=%d,media=disk,format=%s",
>                           disks[i].pdev_path, disk, format);
> +             else if (strncmp(disks[i].vdev, "vd", 2) == 0)
> +                    drive = libxl__sprintf
> +                        (gc, 
> "file=%s,if=virtio,index=%d,media=disk,format=%s",
> +                         disks[i].pdev_path, disk, format);
>                  else
>                      continue; /* Do not emulate this disk */
>              }
> @@ -976,6 +980,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
> 
>              case LIBXL_DISK_BACKEND_PHY:
>              case LIBXL_DISK_BACKEND_UNKNOWN:
> +         case LIBXL_DISK_BACKEND_NONE:
>                  break;
>              }
>          }
> 

the rest of the patch looks fine (apart from few coding style issues)
but IanJ is quite close to have his refactoring ready so I am not sure
if he wants to accept this patch right now...
right now.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel