All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-15 11:54 ` Fabio Fantoni
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-15 11:54 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	qemu-devel, Fabio Fantoni, Paul.Durrant, anthony.perard

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.

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)){
                     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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-15 11:54 ` Fabio Fantoni
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-15 11:54 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	qemu-devel, Fabio Fantoni, Paul.Durrant, anthony.perard

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.

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)){
                     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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
  2015-05-15 11:54 ` Fabio Fantoni
@ 2015-05-18 11:24   ` George Dunlap
  -1 siblings, 0 replies; 10+ messages in thread
From: George Dunlap @ 2015-05-18 11:24 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Wei Liu, Ian Campbell, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD

On Fri, May 15, 2015 at 12:54 PM, Fabio Fantoni <fabio.fantoni@m2r.biz> 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 think what's missing in this changelog is why we want this patch --
what's the advantage?  Was some of the functionality above not working
before, for example?

 -George

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-18 11:24   ` George Dunlap
  0 siblings, 0 replies; 10+ messages in thread
From: George Dunlap @ 2015-05-18 11:24 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Wei Liu, Ian Campbell, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD

On Fri, May 15, 2015 at 12:54 PM, Fabio Fantoni <fabio.fantoni@m2r.biz> 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 think what's missing in this changelog is why we want this patch --
what's the advantage?  Was some of the functionality above not working
before, for example?

 -George

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
  2015-05-15 11:54 ` Fabio Fantoni
@ 2015-05-18 15:43   ` Wei Liu
  -1 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2015-05-18 15:43 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, wei.liu2, Ian.Campbell, Stefano.Stabellini,
	Ian.Jackson, qemu-devel, Paul.Durrant, anthony.perard

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-18 15:43   ` Wei Liu
  0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2015-05-18 15:43 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, wei.liu2, Ian.Campbell, Stefano.Stabellini,
	Ian.Jackson, qemu-devel, Paul.Durrant, anthony.perard

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
  2015-05-18 11:24   ` George Dunlap
@ 2015-05-18 17:02     ` Fabio Fantoni
  -1 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:02 UTC (permalink / raw)
  To: George Dunlap
  Cc: xen-devel, Wei Liu, Ian Campbell, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD

Il 18/05/2015 13:24, George Dunlap ha scritto:
> On Fri, May 15, 2015 at 12:54 PM, Fabio Fantoni <fabio.fantoni@m2r.biz> 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 think what's missing in this changelog is why we want this patch --
> what's the advantage?  Was some of the functionality above not working
> before, for example?
>
>   -George

Add of ahci support require new -device parameters, I tried the change 
also all other cases to have the same.
Is there also rare case of possible problem using old parameters on part 
of things (disks, network, audio and older usb if I remember good) but I 
not remember the example case, was found by another user.
With this patch I'm trying to maintan full compatibility but probably 
there is something that I don't know that must be changed or may changes 
with this.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Xen-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-18 17:02     ` Fabio Fantoni
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:02 UTC (permalink / raw)
  To: George Dunlap
  Cc: xen-devel, Wei Liu, Ian Campbell, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD

Il 18/05/2015 13:24, George Dunlap ha scritto:
> On Fri, May 15, 2015 at 12:54 PM, Fabio Fantoni <fabio.fantoni@m2r.biz> 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 think what's missing in this changelog is why we want this patch --
> what's the advantage?  Was some of the functionality above not working
> before, for example?
>
>   -George

Add of ahci support require new -device parameters, I tried the change 
also all other cases to have the same.
Is there also rare case of possible problem using old parameters on part 
of things (disks, network, audio and older usb if I remember good) but I 
not remember the example case, was found by another user.
With this patch I'm trying to maintan full compatibility but probably 
there is something that I don't know that must be changed or may changes 
with this.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
  2015-05-18 15:43   ` Wei Liu
@ 2015-05-18 17:13     ` Fabio Fantoni
  -1 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:13 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	qemu-devel, Paul.Durrant, anthony.perard

Il 18/05/2015 17:43, Wei Liu ha scritto:
> 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?
Already used many times with many domUs.
>   Do you notice improvement using
> AHCI when booting a guest?

The more significant is with lubuntu 15.04 hvm where time boot time to 
login is only about a fifth!!! in comparison to without.
With windows 7 pro 64 bit is different in many case, in better case the 
boot time is about a third, in the worst it seems to gain only 10-20% of 
the time.
With windows 8.1 the gain is only 5-10% but with win8 the boot time is 
very long in any case, I don't know if caused by unexpected case in xen 
or windows 8 is simply bad or weighty.

The ahci support patch is here:
http://lists.xen.org/archives/html/xen-devel/2015-05/msg01804.html

This is about changes qemu parameters for other disk cases.

>
>> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
@ 2015-05-18 17:13     ` Fabio Fantoni
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:13 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	qemu-devel, Paul.Durrant, anthony.perard

Il 18/05/2015 17:43, Wei Liu ha scritto:
> 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?
Already used many times with many domUs.
>   Do you notice improvement using
> AHCI when booting a guest?

The more significant is with lubuntu 15.04 hvm where time boot time to 
login is only about a fifth!!! in comparison to without.
With windows 7 pro 64 bit is different in many case, in better case the 
boot time is about a third, in the worst it seems to gain only 10-20% of 
the time.
With windows 8.1 the gain is only 5-10% but with win8 the boot time is 
very long in any case, I don't know if caused by unexpected case in xen 
or windows 8 is simply bad or weighty.

The ahci support patch is here:
http://lists.xen.org/archives/html/xen-devel/2015-05/msg01804.html

This is about changes qemu parameters for other disk cases.

>
>> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-05-18 17:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-15 11:54 [Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks Fabio Fantoni
2015-05-15 11:54 ` Fabio Fantoni
2015-05-18 11:24 ` [Qemu-devel] [Xen-devel] " George Dunlap
2015-05-18 11:24   ` George Dunlap
2015-05-18 17:02   ` [Qemu-devel] " Fabio Fantoni
2015-05-18 17:02     ` Fabio Fantoni
2015-05-18 15:43 ` [Qemu-devel] " Wei Liu
2015-05-18 15:43   ` Wei Liu
2015-05-18 17:13   ` [Qemu-devel] " Fabio Fantoni
2015-05-18 17:13     ` Fabio Fantoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.