Re: [Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks

[Wei Liu <wei.liu2@citrix.com>]

On Fri, May 15, 2015 at 01:54:32PM +0200, Fabio Fantoni wrote:
> NOTES:
> This patch is a only a fast draft for testing.
> 
> Some tests result:
> At xl create cdrom empty or not  are both working, xl cd-insert is
> working, xl cd-eject seems working but on xl command in linux hvm domU
> return qmp error of "Device 'ide-N' is locked", in windows 7 instead
> don't show the errror.
> xl block-attach seems working correctly and xl block-detach works
> correctly with linux hvm but not with windows 7 (seems block the disk
> remove, I don't know if do the same without this patch)
> Scsi disk case not tested for now.
> 
> Any comment is appreciated.
> 

I presume you're trying to use AHCI?  Do you notice improvement using
AHCI when booting a guest?

> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>  tools/libxl/libxl_dm.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 4bec5ba..6d00e38 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -811,7 +811,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>              int dev_number =
>                  libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
>              const char *format = qemu_disk_format_string(disks[i].format);
> -            char *drive;
>              const char *pdev_path;
>  
>              if (dev_number == -1) {
> @@ -822,13 +821,14 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>  
>              if (disks[i].is_cdrom) {
>                  if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
> -                    drive = libxl__sprintf
> -                        (gc, "if=ide,index=%d,media=cdrom,cache=writeback,id=ide-%i",
> -                         disk, dev_number);
> +                    flexarray_vappend(dm_args, "-drive",
> +                        GCSPRINTF("if=none,id=ide-%i,cache=writeback", dev_number), "-device",
> +                        GCSPRINTF("ide-cd,drive=ide-%i", dev_number), NULL);
>                  else
> -                    drive = libxl__sprintf
> -                        (gc, "file=%s,if=ide,index=%d,media=cdrom,format=%s,cache=writeback,id=ide-%i",
> -                         disks[i].pdev_path, disk, format, dev_number);
> +                    flexarray_vappend(dm_args, "-drive",
> +                        GCSPRINTF("file=%s,if=none,id=ide-%i,format=%s,cache=writeback",
> +                        disks[i].pdev_path, dev_number, format), "-device",
> +                        GCSPRINTF("ide-cd,drive=ide-%i", dev_number), NULL);
>              } else {
>                  if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
>                      LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "cannot support"
> @@ -857,25 +857,26 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                   * hd[a-d] and ignore the rest.
>                   */
>                  if (strncmp(disks[i].vdev, "sd", 2) == 0)
> -                    drive = libxl__sprintf
> -                        (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
> -                         pdev_path, disk, format);
> +                    flexarray_vappend(dm_args, "-drive",
> +                        GCSPRINTF("file=%s,if=none,id=scsidisk-%d,format=%s,cache=writeback",
> +                        pdev_path, disk, format), "-device",
> +                        GCSPRINTF("scsi-hd,drive=scsidisk-%d,scsi-id=%d",
> +                        disk, disk), NULL);
>                  else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){

I don't see a "ahci" field in libxl idl. Did you forget to commit that
part?

Another question is that do we always want to enable AHCI? Do we want to
make it user tunable? This affects if you actually need a new field in
idl.

Wei.

>                      flexarray_vappend(dm_args, "-drive",
>                          GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
> -                        pdev_path, disk, format), "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
> +                        pdev_path, disk, format), "-device",
> +                        GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
>                          disk, disk), NULL);
> -                    continue;
>                  }else if (disk < 4)
> -                    drive = libxl__sprintf
> -                        (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback",
> -                         pdev_path, disk, format);
> +                    flexarray_vappend(dm_args, "-drive",
> +                        GCSPRINTF("file=%s,if=none,id=idedisk-%d,format=%s,cache=writeback",
> +                        pdev_path, disk, format), "-device", GCSPRINTF("ide-hd,drive=idedisk-%d",
> +                        disk), NULL);
>                  else
>                      continue; /* Do not emulate this disk */
>              }
>  
> -            flexarray_append(dm_args, "-drive");
> -            flexarray_append(dm_args, drive);
>          }
>  
>          switch (b_info->u.hvm.vendor_device) {
> -- 
> 1.9.1