All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
@ 2015-05-14 11:11 Fabio Fantoni
  2015-05-18 15:53 ` Wei Liu
  2015-05-18 16:16 ` Ian Jackson
  0 siblings, 2 replies; 18+ messages in thread
From: Fabio Fantoni @ 2015-05-14 11:11 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	Fabio Fantoni, Paul.Durrant, anthony.perard

Usage:
ahci=0|1 (default=0)

If enabled adds ich9 disk controller in ahci mode and uses it with
upstream qemu to emulate disks instead of ide.
It doesn't support cdroms which still use ide (cdroms will use "-device
ide-cd" as new qemu parameters)
Ahci requires new qemu parameters but for now other emulated disks cases
remains with old ones because automatic bus selection seems bugged in
qemu using new parameters. (I'll retry)

NOTES:
This patch is a only a fast draft for testing.
Tested with 1 and 6 disks on ubuntu 15.04 hvm, windows 7 and windows 8
domUs.
Doc entry and libxl.h define should be added, I'll do.
Other emulated disks cases should be converted to use new qemu
parameters but probably a fix in qemu is needed.

Any comment is appreciated.

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>

Changes in v2:
- libxl_dm.c: manual bus and unit selection (as workaround to qemu bug)
to have multiple disks working.
---
 tools/libxl/libxl_create.c  |  1 +
 tools/libxl/libxl_dm.c      | 10 +++++++++-
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c    |  1 +
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f0da7dc..fcfe24a 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -322,6 +322,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
         libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
+        libxl_defbool_setdefault(&b_info->u.hvm.ahci,               false);
 
         libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
         if (!libxl_defbool_val(b_info->u.hvm.spice.enable) &&
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..4bec5ba 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -804,6 +804,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        if (libxl_defbool_val(b_info->u.hvm.ahci))
+            flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0");
         for (i = 0; i < num_disks; i++) {
             int disk, part;
             int dev_number =
@@ -858,7 +860,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
                          pdev_path, disk, format);
-                else if (disk < 4)
+                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",
+                        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);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 023b21e..6326429 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -433,6 +433,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("nested_hvm",       libxl_defbool),
                                        ("smbios_firmware",  string),
                                        ("acpi_firmware",    string),
+                                       ("ahci",             libxl_defbool),
                                        ("nographic",        libxl_defbool),
                                        ("vga",              libxl_vga_interface_info),
                                        ("vnc",              libxl_vnc_info),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 526a1f6..9475a16 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2195,6 +2195,7 @@ skip_vfb:
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);
+        xlu_cfg_get_defbool(config, "ahci", &b_info->u.hvm.ahci, 0);
 
         if(b_info->u.hvm.vnc.listen
            && b_info->u.hvm.vnc.display
-- 
1.9.1

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-14 11:11 [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu Fabio Fantoni
@ 2015-05-18 15:53 ` Wei Liu
  2015-05-18 17:22   ` Fabio Fantoni
  2015-05-18 16:16 ` Ian Jackson
  1 sibling, 1 reply; 18+ messages in thread
From: Wei Liu @ 2015-05-18 15:53 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, wei.liu2, Ian.Campbell, Stefano.Stabellini,
	Ian.Jackson, Paul.Durrant, anthony.perard

On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
> Usage:
> ahci=0|1 (default=0)
> 
> If enabled adds ich9 disk controller in ahci mode and uses it with
> upstream qemu to emulate disks instead of ide.

Is ICH9 available in our default setup?

Why do we not always enable AHCI?

> It doesn't support cdroms which still use ide (cdroms will use "-device
> ide-cd" as new qemu parameters)
> Ahci requires new qemu parameters but for now other emulated disks cases
> remains with old ones because automatic bus selection seems bugged in
> qemu using new parameters. (I'll retry)
> 

Buggy as in? Have you reported to QEMU upstream?

> NOTES:
> This patch is a only a fast draft for testing.
> Tested with 1 and 6 disks on ubuntu 15.04 hvm, windows 7 and windows 8
> domUs.
> Doc entry and libxl.h define should be added, I'll do.
> Other emulated disks cases should be converted to use new qemu
> parameters but probably a fix in qemu is needed.
> 
> Any comment is appreciated.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> 
> Changes in v2:
> - libxl_dm.c: manual bus and unit selection (as workaround to qemu bug)
> to have multiple disks working.

What's the relation between this patch the other patch you posted later?

> ---
>  tools/libxl/libxl_create.c  |  1 +
>  tools/libxl/libxl_dm.c      | 10 +++++++++-
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c    |  1 +
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index f0da7dc..fcfe24a 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -322,6 +322,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
>          libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.ahci,               false);
>  
>          libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
>          if (!libxl_defbool_val(b_info->u.hvm.spice.enable) &&
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..4bec5ba 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -804,6 +804,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>      flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
>  
>      if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> +        if (libxl_defbool_val(b_info->u.hvm.ahci))
> +            flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0");
>          for (i = 0; i < num_disks; i++) {
>              int disk, part;
>              int dev_number =
> @@ -858,7 +860,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>                      drive = libxl__sprintf
>                          (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
>                           pdev_path, disk, format);
> -                else if (disk < 4)
> +                else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){

And you choose 6 because?

Wei.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-14 11:11 [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu Fabio Fantoni
  2015-05-18 15:53 ` Wei Liu
@ 2015-05-18 16:16 ` Ian Jackson
  2015-05-18 17:28   ` Fabio Fantoni
  1 sibling, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2015-05-18 16:16 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, wei.liu2, Ian.Campbell, Stefano.Stabellini,
	Paul.Durrant, anthony.perard

Fabio Fantoni writes ("[PATCH v2][RFC] libxl: Add AHCI support for upstream qemu"):
> If enabled adds ich9 disk controller in ahci mode and uses it with
> upstream qemu to emulate disks instead of ide.

I'm sorry for perhaps querying the obvious, but why is this a good
idea ?  I think the underlying motivation should be explained in the
commit message.

That will also help us understand whether we ought to be changing the
default.

Ian.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-18 15:53 ` Wei Liu
@ 2015-05-18 17:22   ` Fabio Fantoni
  2015-05-19 10:40     ` Wei Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:22 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	Paul.Durrant, anthony.perard

Il 18/05/2015 17:53, Wei Liu ha scritto:
> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>> Usage:
>> ahci=0|1 (default=0)
>>
>> If enabled adds ich9 disk controller in ahci mode and uses it with
>> upstream qemu to emulate disks instead of ide.
> Is ICH9 available in our default setup?
>
> Why do we not always enable AHCI?

ahci seems require ich9 controller (default in q35 but not in older used 
by xen), I think that change it ide->ahci automatically for all cases 
may causes problems, probably in save/restore and more probably in 
windows <=7 without pv where change ide<->ahci require a registry change 
for not have blue screen.

>
>> It doesn't support cdroms which still use ide (cdroms will use "-device
>> ide-cd" as new qemu parameters)
>> Ahci requires new qemu parameters but for now other emulated disks cases
>> remains with old ones because automatic bus selection seems bugged in
>> qemu using new parameters. (I'll retry)
>>
> Buggy as in? Have you reported to QEMU upstream?

Already reported long time ago in xen-devel and qemu-devel, the only 
reply from qemu-devel was to use fixed bus and is what I did in v2 of 
this patch.
Can someone tell me if can be a problem fixed bus?

>
>> NOTES:
>> This patch is a only a fast draft for testing.
>> Tested with 1 and 6 disks on ubuntu 15.04 hvm, windows 7 and windows 8
>> domUs.
>> Doc entry and libxl.h define should be added, I'll do.
>> Other emulated disks cases should be converted to use new qemu
>> parameters but probably a fix in qemu is needed.
>>
>> Any comment is appreciated.
>>
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>
>> Changes in v2:
>> - libxl_dm.c: manual bus and unit selection (as workaround to qemu bug)
>> to have multiple disks working.
> What's the relation between this patch the other patch you posted later?
>
>> ---
>>   tools/libxl/libxl_create.c  |  1 +
>>   tools/libxl/libxl_dm.c      | 10 +++++++++-
>>   tools/libxl/libxl_types.idl |  1 +
>>   tools/libxl/xl_cmdimpl.c    |  1 +
>>   4 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index f0da7dc..fcfe24a 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -322,6 +322,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>>           libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
>>           libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
>>           libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
>> +        libxl_defbool_setdefault(&b_info->u.hvm.ahci,               false);
>>   
>>           libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
>>           if (!libxl_defbool_val(b_info->u.hvm.spice.enable) &&
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 0c6408d..4bec5ba 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -804,6 +804,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>       flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
>>   
>>       if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
>> +        if (libxl_defbool_val(b_info->u.hvm.ahci))
>> +            flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0");
>>           for (i = 0; i < num_disks; i++) {
>>               int disk, part;
>>               int dev_number =
>> @@ -858,7 +860,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>                       drive = libxl__sprintf
>>                           (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
>>                            pdev_path, disk, format);
>> -                else if (disk < 4)
>> +                else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){
> And you choose 6 because?

Because the ich9 ahci controller have 6 channel, so is the max, ide 
instead have 2 channel with master/slave.

>
> Wei.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-18 16:16 ` Ian Jackson
@ 2015-05-18 17:28   ` Fabio Fantoni
  0 siblings, 0 replies; 18+ messages in thread
From: Fabio Fantoni @ 2015-05-18 17:28 UTC (permalink / raw)
  To: Ian Jackson
  Cc: xen-devel, wei.liu2, Ian.Campbell, Stefano.Stabellini,
	Paul.Durrant, anthony.perard

Il 18/05/2015 18:16, Ian Jackson ha scritto:
> Fabio Fantoni writes ("[PATCH v2][RFC] libxl: Add AHCI support for upstream qemu"):
>> If enabled adds ich9 disk controller in ahci mode and uses it with
>> upstream qemu to emulate disks instead of ide.
> I'm sorry for perhaps querying the obvious, but why is this a good
> idea ?  I think the underlying motivation should be explained in the
> commit message.
>
> That will also help us understand whether we ought to be changing the
> default.
>
> Ian.

 From another mails:

see this: http://lists.xen.org/archives/html/xen-devel/2015-05/msg01277.html
and...
>>   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!!! (20%) 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 (33%), in the worst it seems to gain 
> only 10-20% of the total time.
> With windows 8.1 the gain is only 5-10% of the total 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. 

And another details probably useful:
http://lists.xen.org/archives/html/xen-devel/2015-05/msg02327.html

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-18 17:22   ` Fabio Fantoni
@ 2015-05-19 10:40     ` Wei Liu
  2015-05-19 12:13       ` Fabio Fantoni
  0 siblings, 1 reply; 18+ messages in thread
From: Wei Liu @ 2015-05-19 10:40 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Wei Liu, Ian.Campbell, Stefano.Stabellini,
	Ian.Jackson, Paul.Durrant, anthony.perard

On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
> Il 18/05/2015 17:53, Wei Liu ha scritto:
> >On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
> >>Usage:
> >>ahci=0|1 (default=0)
> >>
> >>If enabled adds ich9 disk controller in ahci mode and uses it with
> >>upstream qemu to emulate disks instead of ide.
> >Is ICH9 available in our default setup?
> >
> >Why do we not always enable AHCI?
> 
> ahci seems require ich9 controller (default in q35 but not in older used by
> xen), I think that change it ide->ahci automatically for all cases may
> causes problems, probably in save/restore and more probably in windows <=7
> without pv where change ide<->ahci require a registry change for not have
> blue screen.
> 

I guess we now have a QEMU with Q35. But migrating from previous
previous versions will indeed need to be taken care of.

> >
> >>It doesn't support cdroms which still use ide (cdroms will use "-device
> >>ide-cd" as new qemu parameters)
> >>Ahci requires new qemu parameters but for now other emulated disks cases
> >>remains with old ones because automatic bus selection seems bugged in
> >>qemu using new parameters. (I'll retry)
> >>
> >Buggy as in? Have you reported to QEMU upstream?
> 
> Already reported long time ago in xen-devel and qemu-devel, the only reply
> from qemu-devel was to use fixed bus and is what I did in v2 of this patch.
> Can someone tell me if can be a problem fixed bus?
> 

I don't have enough knowledge on this. Maybe Anthony and Stefano have
more insight.

Wei.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-19 10:40     ` Wei Liu
@ 2015-05-19 12:13       ` Fabio Fantoni
  2015-05-25 10:41         ` Fabio Fantoni
  2015-06-08 14:00         ` Stefano Stabellini
  0 siblings, 2 replies; 18+ messages in thread
From: Fabio Fantoni @ 2015-05-19 12:13 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian.Campbell, Stefano Stabellini, Ian.Jackson,
	Paul.Durrant, anthony.perard

Il 19/05/2015 12:40, Wei Liu ha scritto:
> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>> Usage:
>>>> ahci=0|1 (default=0)
>>>>
>>>> If enabled adds ich9 disk controller in ahci mode and uses it with
>>>> upstream qemu to emulate disks instead of ide.
>>> Is ICH9 available in our default setup?
>>>
>>> Why do we not always enable AHCI?
>> ahci seems require ich9 controller (default in q35 but not in older used by
>> xen), I think that change it ide->ahci automatically for all cases may
>> causes problems, probably in save/restore and more probably in windows <=7
>> without pv where change ide<->ahci require a registry change for not have
>> blue screen.
>>
> I guess we now have a QEMU with Q35. But migrating from previous
> previous versions will indeed need to be taken care of.

This patch don't add q35 cipset support but only ich9 ahci disk 
controller in older and only cipset supported in xen.
Time ago I tried to add q35 support in xen but I had some problems I was 
unable to solves and nobody helped me.
Q35 have newer ich9 ahci disk controller as default, on older cipset 
must be added (done in this patch).
qemu, kvm ovmf already support q35, xen need changes at least in 
hvmloader (that I'm probably unable to do) and libxl (that I started 
time ago without good result), Add ahci support and switch all qemu 
parameters in libxl to new -device is a good start also for future q35 
support (are both part needed based on my old q35 tests on xen)

>
>>>> It doesn't support cdroms which still use ide (cdroms will use "-device
>>>> ide-cd" as new qemu parameters)
>>>> Ahci requires new qemu parameters but for now other emulated disks cases
>>>> remains with old ones because automatic bus selection seems bugged in
>>>> qemu using new parameters. (I'll retry)
>>>>
>>> Buggy as in? Have you reported to QEMU upstream?
>> Already reported long time ago in xen-devel and qemu-devel, the only reply
>> from qemu-devel was to use fixed bus and is what I did in v2 of this patch.
>> Can someone tell me if can be a problem fixed bus?
>>
> I don't have enough knowledge on this. Maybe Anthony and Stefano have
> more insight.
>
> Wei.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-19 12:13       ` Fabio Fantoni
@ 2015-05-25 10:41         ` Fabio Fantoni
  2015-05-25 10:48           ` Wei Liu
  2015-06-08 14:00         ` Stefano Stabellini
  1 sibling, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-05-25 10:41 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian.Campbell, Stefano.Stabellini, Ian.Jackson,
	Paul.Durrant, anthony.perard

Il 19/05/2015 14:13, Fabio Fantoni ha scritto:
> Il 19/05/2015 12:40, Wei Liu ha scritto:
>> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>>> Usage:
>>>>> ahci=0|1 (default=0)
>>>>>
>>>>> If enabled adds ich9 disk controller in ahci mode and uses it with
>>>>> upstream qemu to emulate disks instead of ide.
>>>> Is ICH9 available in our default setup?
>>>>
>>>> Why do we not always enable AHCI?
>>> ahci seems require ich9 controller (default in q35 but not in older 
>>> used by
>>> xen), I think that change it ide->ahci automatically for all cases may
>>> causes problems, probably in save/restore and more probably in 
>>> windows <=7
>>> without pv where change ide<->ahci require a registry change for not 
>>> have
>>> blue screen.
>>>
>> I guess we now have a QEMU with Q35. But migrating from previous
>> previous versions will indeed need to be taken care of.
>
> This patch don't add q35 cipset support but only ich9 ahci disk 
> controller in older and only cipset supported in xen.
> Time ago I tried to add q35 support in xen but I had some problems I 
> was unable to solves and nobody helped me.
> Q35 have newer ich9 ahci disk controller as default, on older cipset 
> must be added (done in this patch).
> qemu, kvm ovmf already support q35, xen need changes at least in 
> hvmloader (that I'm probably unable to do) and libxl (that I started 
> time ago without good result), Add ahci support and switch all qemu 
> parameters in libxl to new -device is a good start also for future q35 
> support (are both part needed based on my old q35 tests on xen)
>
>>
>>>>> It doesn't support cdroms which still use ide (cdroms will use 
>>>>> "-device
>>>>> ide-cd" as new qemu parameters)
>>>>> Ahci requires new qemu parameters but for now other emulated disks 
>>>>> cases
>>>>> remains with old ones because automatic bus selection seems bugged in
>>>>> qemu using new parameters. (I'll retry)
>>>>>
>>>> Buggy as in? Have you reported to QEMU upstream?
>>> Already reported long time ago in xen-devel and qemu-devel, the only 
>>> reply
>>> from qemu-devel was to use fixed bus and is what I did in v2 of this 
>>> patch.
>>> Can someone tell me if can be a problem fixed bus?
>>>
>> I don't have enough knowledge on this. Maybe Anthony and Stefano have
>> more insight.
>>
>> Wei.
>

Ping...
Any review and/or comment about AHCI support patch is appreciated.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-25 10:41         ` Fabio Fantoni
@ 2015-05-25 10:48           ` Wei Liu
  2015-05-27  9:52             ` George Dunlap
  0 siblings, 1 reply; 18+ messages in thread
From: Wei Liu @ 2015-05-25 10:48 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Wei Liu, Ian.Campbell, Stefano.Stabellini,
	Ian.Jackson, Paul.Durrant, anthony.perard

On Mon, May 25, 2015 at 12:41:48PM +0200, Fabio Fantoni wrote:
> 
> Ping...
> Any review and/or comment about AHCI support patch is appreciated.

Stefano and Anthony were away last week and today is a UK public
holiday, so please be patient.

Wei.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-25 10:48           ` Wei Liu
@ 2015-05-27  9:52             ` George Dunlap
  0 siblings, 0 replies; 18+ messages in thread
From: George Dunlap @ 2015-05-27  9:52 UTC (permalink / raw)
  To: Wei Liu
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	Fabio Fantoni, Paul Durrant, Anthony PERARD

On Mon, May 25, 2015 at 11:48 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Mon, May 25, 2015 at 12:41:48PM +0200, Fabio Fantoni wrote:
>>
>> Ping...
>> Any review and/or comment about AHCI support patch is appreciated.
>
> Stefano and Anthony were away last week and today is a UK public
> holiday, so please be patient.

Just to be clear, pinging is helpful, but maybe a 2-3 week timeout
would be a bit better than 1 week.

 -George

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-05-19 12:13       ` Fabio Fantoni
  2015-05-25 10:41         ` Fabio Fantoni
@ 2015-06-08 14:00         ` Stefano Stabellini
  2015-06-08 14:17           ` Fabio Fantoni
  1 sibling, 1 reply; 18+ messages in thread
From: Stefano Stabellini @ 2015-06-08 14:00 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Wei Liu, Ian.Campbell, Stefano Stabellini,
	Ian.Jackson, Paul.Durrant, anthony.perard

On Tue, 19 May 2015, Fabio Fantoni wrote:
> Il 19/05/2015 12:40, Wei Liu ha scritto:
> > On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
> > > Il 18/05/2015 17:53, Wei Liu ha scritto:
> > > > On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
> > > > > Usage:
> > > > > ahci=0|1 (default=0)
> > > > > 
> > > > > If enabled adds ich9 disk controller in ahci mode and uses it with
> > > > > upstream qemu to emulate disks instead of ide.
> > > > Is ICH9 available in our default setup?
> > > > 
> > > > Why do we not always enable AHCI?
> > > ahci seems require ich9 controller (default in q35 but not in older used
> > > by
> > > xen), I think that change it ide->ahci automatically for all cases may
> > > causes problems, probably in save/restore and more probably in windows <=7
> > > without pv where change ide<->ahci require a registry change for not have
> > > blue screen.
> > > 
> > I guess we now have a QEMU with Q35. But migrating from previous
> > previous versions will indeed need to be taken care of.
> 
> This patch don't add q35 cipset support but only ich9 ahci disk controller in
> older and only cipset supported in xen.

This is pretty nice actually, thanks for the patch!


> Time ago I tried to add q35 support in xen but I had some problems I was
> unable to solves and nobody helped me.
> Q35 have newer ich9 ahci disk controller as default, on older cipset must be
> added (done in this patch).
> qemu, kvm ovmf already support q35, xen need changes at least in hvmloader
> (that I'm probably unable to do) and libxl (that I started time ago without
> good result), Add ahci support and switch all qemu parameters in libxl to new
> -device is a good start also for future q35 support (are both part needed
> based on my old q35 tests on xen)

Yeah, going q35 opens a whole new can of worms. I think that adding the
ich9 chipset to the existing machine is a good compromise.


> > 
> > > > > It doesn't support cdroms which still use ide (cdroms will use
> > > > > "-device
> > > > > ide-cd" as new qemu parameters)
> > > > > Ahci requires new qemu parameters but for now other emulated disks
> > > > > cases
> > > > > remains with old ones because automatic bus selection seems bugged in
> > > > > qemu using new parameters. (I'll retry)
> > > > > 
> > > > Buggy as in? Have you reported to QEMU upstream?
> > > Already reported long time ago in xen-devel and qemu-devel, the only reply
> > > from qemu-devel was to use fixed bus and is what I did in v2 of this
> > > patch.
> > > Can someone tell me if can be a problem fixed bus?
> > > 
> > I don't have enough knowledge on this. Maybe Anthony and Stefano have
> > more insight.

Let me get this straight: cdrom still works but goes via ide. In
addition AHCI disks can be attached. Seems good to me.

Have you tested Windows?

If this works with Windows too, I would expose an ahci option
(default=off) in the config file. If ahci=on, then all disks could be
exposed via ahci. Does this seem sensible? In a couple of releases, if
it works well, we could make ahci=on the default.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-08 14:00         ` Stefano Stabellini
@ 2015-06-08 14:17           ` Fabio Fantoni
  2015-06-11 10:06             ` Zir Blazer
  0 siblings, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-06-08 14:17 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Wei Liu, Ian.Campbell, Ian.Jackson, Paul.Durrant,
	anthony.perard

Il 08/06/2015 16:00, Stefano Stabellini ha scritto:
> On Tue, 19 May 2015, Fabio Fantoni wrote:
>> Il 19/05/2015 12:40, Wei Liu ha scritto:
>>> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>>>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>>>> Usage:
>>>>>> ahci=0|1 (default=0)
>>>>>>
>>>>>> If enabled adds ich9 disk controller in ahci mode and uses it with
>>>>>> upstream qemu to emulate disks instead of ide.
>>>>> Is ICH9 available in our default setup?
>>>>>
>>>>> Why do we not always enable AHCI?
>>>> ahci seems require ich9 controller (default in q35 but not in older used
>>>> by
>>>> xen), I think that change it ide->ahci automatically for all cases may
>>>> causes problems, probably in save/restore and more probably in windows <=7
>>>> without pv where change ide<->ahci require a registry change for not have
>>>> blue screen.
>>>>
>>> I guess we now have a QEMU with Q35. But migrating from previous
>>> previous versions will indeed need to be taken care of.
>> This patch don't add q35 cipset support but only ich9 ahci disk controller in
>> older and only cipset supported in xen.
> This is pretty nice actually, thanks for the patch!
>
>
>> Time ago I tried to add q35 support in xen but I had some problems I was
>> unable to solves and nobody helped me.
>> Q35 have newer ich9 ahci disk controller as default, on older cipset must be
>> added (done in this patch).
>> qemu, kvm ovmf already support q35, xen need changes at least in hvmloader
>> (that I'm probably unable to do) and libxl (that I started time ago without
>> good result), Add ahci support and switch all qemu parameters in libxl to new
>> -device is a good start also for future q35 support (are both part needed
>> based on my old q35 tests on xen)
> Yeah, going q35 opens a whole new can of worms. I think that adding the
> ich9 chipset to the existing machine is a good compromise.
>
>
>>>>>> It doesn't support cdroms which still use ide (cdroms will use
>>>>>> "-device
>>>>>> ide-cd" as new qemu parameters)
>>>>>> Ahci requires new qemu parameters but for now other emulated disks
>>>>>> cases
>>>>>> remains with old ones because automatic bus selection seems bugged in
>>>>>> qemu using new parameters. (I'll retry)
>>>>>>
>>>>> Buggy as in? Have you reported to QEMU upstream?
>>>> Already reported long time ago in xen-devel and qemu-devel, the only reply
>>>> from qemu-devel was to use fixed bus and is what I did in v2 of this
>>>> patch.
>>>> Can someone tell me if can be a problem fixed bus?
>>>>
>>> I don't have enough knowledge on this. Maybe Anthony and Stefano have
>>> more insight.
> Let me get this straight: cdrom still works but goes via ide. In
> addition AHCI disks can be attached. Seems good to me.
>
> Have you tested Windows?
>
> If this works with Windows too, I would expose an ahci option
> (default=off) in the config file. If ahci=on, then all disks could be
> exposed via ahci. Does this seem sensible? In a couple of releases, if
> it works well, we could make ahci=on the default.

I tested both linux and windows, mainly windows domUs (7 and 8.1).
Windows < 8 FWIK needs a registry key change for switch ide<->ahci 
without bluescreen (this is the reason because I did ahci=0|1 instead 
change it by default) but trying with windows 7 with pv is possible 
change without bluescreen, probably the problem is only without pv but 
ahci option default disabled resolve it (for the users don't set ahci=0 
on xen upgrade process).
Based on what I know and all tests I did this patch seems the best way 
for add ahci support but I think that a experts (like you) review is better.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-08 14:17           ` Fabio Fantoni
@ 2015-06-11 10:06             ` Zir Blazer
  2015-06-11 10:28               ` Fabio Fantoni
  0 siblings, 1 reply; 18+ messages in thread
From: Zir Blazer @ 2015-06-11 10:06 UTC (permalink / raw)
  To: xen-devel, fabio.fantoni

Since I'm not a developer I may be peeking my nose a bit too far, but based on what I know, I think that enabling AHCI by default would be a compatibility suicide. I'm not sure about Linux and Windows Vista/7/8+, but at least for Windows XP based VMs, it would be a terrible idea.


Back during WXP years, the vanilla install ISO didn't had any AHCI Drivers integrated at all. If you set the SATA Controller to AHCI, the WXP installation wouldn't detect any Hard Disk at all. If you really wanted to use AHCI during its early days, you would need a Floppy Disk with the AHCI Drivers of your SATA Controller (Chipset/South Bridge or third party controller), that you could use by pressing the F6 key right after booting the install CD:
http://www.pc-tips-and-tricks.com/images/Install-windows-xp-02.jpg

If you didn't had a Floppy Disk Driver or were lazy, the typical choice was to set the SATA Controller to IDE and install WXP, skipping anything AHCI related.

At some point an application called nLite appeared, which allowed you to make your custom Windows XP ISO. You could easily modify a lot of default options, integrate hotfixes, and even Drivers, and after you were happy with your changes, you could then make your custom ISO and burn it to a CD (Or from a USB Flash Drive with other tools to make it booteable). So, with nLite, it was rather easy to integrated the AHCI Drivers of your SATA Controller to the install ISO. This way, if you were doing a fresh install, reinstall or repair, there was no excuse to not use AHCI.
However, since for VM usage AHCI was not available at all until QEMU implemented it recently, by default you will be always using IDE, so there was no actual need to make a customized WXP install ISO with AHCI Drivers that you were never going to use. Besides, Windows XP doesn't even install nor have ready any AHCI Drivers if you're in IDE mode, so if you're going to switch to use AHCI by default, every existing WXP VM would greet you with a BSOD after upgrading Xen.


So, with this proposal you would have two issues: First, doing a new install of Windows XP on a VM , since by default it shouldn't detect any HD at all in AHCI. Second, that existing WXP VMs will BSOD on boot. For as long as you can switch to IDE mode everything should still be working as always, but it would mean that at the very least you would have to modify the DomU config file to replace default AHCI with old IDE.

Regarding using Windows XP with AHCI:
First, as stated before, if you want to install it on a new VM, you need the AHCI Drivers. Since it seems that at some point support for Floppy Disk got removed from Xen (I tried recently to use a Floppy Disk image with fda= as stated somewhere on the wiki, but it did nothing. Nor I found any recent documentation claiming that you can mount a Floppy Disk image at all), the only choice is to make a custom ISO with nLite with the AHCI Drivers integrated in it. Since the emulated Southbridge is an Intel ICH9, I believe that the Drivers that should be needed are these:
http://www.win-raid.com/t22f23-Guide-Integration-of-Intels-AHCI-RAID-drivers-into-a-Windows-XP-W-k-W-k-CD.html
Check for the a) and c) options for WXP/2003 in 32 and 64 Bits variants.

Also, it is important to note that the plain ICH9 Southbridge according to Intel DOES NOT OFFICIALLY HAVE AHCI SUPPORT, ICH9R does.
http://en.wikipedia.org/wiki/I/O_Controller_Hub#ICH9

The previous Drivers are claimed to have been modified to force support for ICH9 since AHCI is there, just that the original Drivers doesn't want to use it. Typical artificial market segmentation strategies. Still, if QEMU is emulating the plain ICH9 and you can't get Windows to see drives in AHCI mode after integrating the AHCI Drivers, that may be why.


Second, it is possible to switch from IDE to AHCI on WXP without reinstalling, but doing it is rather hacky and instructions are very Chipset and vendor dependant, which may (Or not) include editing Windows registry. Here are instructions that claim to work for Intel Southbridges, which since we're using ICH9, may possibily work for Xen based WXP VMs:
http://www.prime-expert.com/articles/a11/change-from-ide-to-ahci-without-reinstalling-windows-xp.php

But should these not work, chances are that you will need to add a rescue ISO to your DomU config file to repair the broken install, since I'm not 100% sure if you can even get it to boot in Safe Mode if it BSODs at boot.


I think that's all what I could figure out. If you're using the GPLPV Drivers I don't know if there are any changes regarding running in AHCI mode.



----------------------------------------
> Date: Mon, 8 Jun 2015 16:17:39 +0200
> From: fabio.fantoni@m2r.biz
> To: stefano.stabellini@eu.citrix.com
> CC: xen-devel@lists.xensource.com; wei.liu2@citrix.com; Ian.Campbell@citrix.com; Ian.Jackson@eu.citrix.com; Paul.Durrant@citrix.com; anthony.perard@citrix.com
> Subject: Re: [Xen-devel] [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
>
> Il 08/06/2015 16:00, Stefano Stabellini ha scritto:
>> On Tue, 19 May 2015, Fabio Fantoni wrote:
>>> Il 19/05/2015 12:40, Wei Liu ha scritto:
>>>> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>>>>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>>>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>>>>> Usage:
>>>>>>> ahci=0|1 (default=0)
>>>>>>>
>>>>>>> If enabled adds ich9 disk controller in ahci mode and uses it with
>>>>>>> upstream qemu to emulate disks instead of ide.
>>>>>> Is ICH9 available in our default setup?
>>>>>>
>>>>>> Why do we not always enable AHCI?
>>>>> ahci seems require ich9 controller (default in q35 but not in older used
>>>>> by
>>>>> xen), I think that change it ide->ahci automatically for all cases may
>>>>> causes problems, probably in save/restore and more probably in windows <=7
>>>>> without pv where change ide<->ahci require a registry change for not have
>>>>> blue screen.
>>>>>
>>>> I guess we now have a QEMU with Q35. But migrating from previous
>>>> previous versions will indeed need to be taken care of.
>>> This patch don't add q35 cipset support but only ich9 ahci disk controller in
>>> older and only cipset supported in xen.
>> This is pretty nice actually, thanks for the patch!
>>
>>
>>> Time ago I tried to add q35 support in xen but I had some problems I was
>>> unable to solves and nobody helped me.
>>> Q35 have newer ich9 ahci disk controller as default, on older cipset must be
>>> added (done in this patch).
>>> qemu, kvm ovmf already support q35, xen need changes at least in hvmloader
>>> (that I'm probably unable to do) and libxl (that I started time ago without
>>> good result), Add ahci support and switch all qemu parameters in libxl to new
>>> -device is a good start also for future q35 support (are both part needed
>>> based on my old q35 tests on xen)
>> Yeah, going q35 opens a whole new can of worms. I think that adding the
>> ich9 chipset to the existing machine is a good compromise.
>>
>>
>>>>>>> It doesn't support cdroms which still use ide (cdroms will use
>>>>>>> "-device
>>>>>>> ide-cd" as new qemu parameters)
>>>>>>> Ahci requires new qemu parameters but for now other emulated disks
>>>>>>> cases
>>>>>>> remains with old ones because automatic bus selection seems bugged in
>>>>>>> qemu using new parameters. (I'll retry)
>>>>>>>
>>>>>> Buggy as in? Have you reported to QEMU upstream?
>>>>> Already reported long time ago in xen-devel and qemu-devel, the only reply
>>>>> from qemu-devel was to use fixed bus and is what I did in v2 of this
>>>>> patch.
>>>>> Can someone tell me if can be a problem fixed bus?
>>>>>
>>>> I don't have enough knowledge on this. Maybe Anthony and Stefano have
>>>> more insight.
>> Let me get this straight: cdrom still works but goes via ide. In
>> addition AHCI disks can be attached. Seems good to me.
>>
>> Have you tested Windows?
>>
>> If this works with Windows too, I would expose an ahci option
>> (default=off) in the config file. If ahci=on, then all disks could be
>> exposed via ahci. Does this seem sensible? In a couple of releases, if
>> it works well, we could make ahci=on the default.
>
> I tested both linux and windows, mainly windows domUs (7 and 8.1).
> Windows < 8 FWIK needs a registry key change for switch ide<->ahci
> without bluescreen (this is the reason because I did ahci=0|1 instead
> change it by default) but trying with windows 7 with pv is possible
> change without bluescreen, probably the problem is only without pv but
> ahci option default disabled resolve it (for the users don't set ahci=0
> on xen upgrade process).
> Based on what I know and all tests I did this patch seems the best way
> for add ahci support but I think that a experts (like you) review is better.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
 		 	   		  

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-11 10:06             ` Zir Blazer
@ 2015-06-11 10:28               ` Fabio Fantoni
  2015-06-19 11:18                 ` Fabio Fantoni
  0 siblings, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-06-11 10:28 UTC (permalink / raw)
  To: Zir Blazer, xen-devel

Il 11/06/2015 12:06, Zir Blazer ha scritto:
> Since I'm not a developer I may be peeking my nose a bit too far, but based on what I know, I think that enabling AHCI by default would be a compatibility suicide. I'm not sure about Linux and Windows Vista/7/8+, but at least for Windows XP based VMs, it would be a terrible idea.

Also use windows xp without security updates (support ended one year 
ago) is a "suicide".

I already did this patch considering windows domU problems (I'm using 
mainly them for now), ahci used with option (ahci=0|1) instead replace 
and default is disabled.
I tried it with different windows (excluding xp...abandoned)
I also tried with new winpv drivers 
(http://www.xenproject.org/developers/teams/windows-pv-drivers.html)

With this patch applied ahci will be not used and will be used only 
setting ahci=1, is it a good idea or is there problem also in this case?


Thanks for any reply and sorry for my bad english.

>
>
> Back during WXP years, the vanilla install ISO didn't had any AHCI Drivers integrated at all. If you set the SATA Controller to AHCI, the WXP installation wouldn't detect any Hard Disk at all. If you really wanted to use AHCI during its early days, you would need a Floppy Disk with the AHCI Drivers of your SATA Controller (Chipset/South Bridge or third party controller), that you could use by pressing the F6 key right after booting the install CD:
> http://www.pc-tips-and-tricks.com/images/Install-windows-xp-02.jpg
>
> If you didn't had a Floppy Disk Driver or were lazy, the typical choice was to set the SATA Controller to IDE and install WXP, skipping anything AHCI related.
>
> At some point an application called nLite appeared, which allowed you to make your custom Windows XP ISO. You could easily modify a lot of default options, integrate hotfixes, and even Drivers, and after you were happy with your changes, you could then make your custom ISO and burn it to a CD (Or from a USB Flash Drive with other tools to make it booteable). So, with nLite, it was rather easy to integrated the AHCI Drivers of your SATA Controller to the install ISO. This way, if you were doing a fresh install, reinstall or repair, there was no excuse to not use AHCI.
> However, since for VM usage AHCI was not available at all until QEMU implemented it recently, by default you will be always using IDE, so there was no actual need to make a customized WXP install ISO with AHCI Drivers that you were never going to use. Besides, Windows XP doesn't even install nor have ready any AHCI Drivers if you're in IDE mode, so if you're going to switch to use AHCI by default, every existing WXP VM would greet you with a BSOD after upgrading Xen.
>
>
> So, with this proposal you would have two issues: First, doing a new install of Windows XP on a VM , since by default it shouldn't detect any HD at all in AHCI. Second, that existing WXP VMs will BSOD on boot. For as long as you can switch to IDE mode everything should still be working as always, but it would mean that at the very least you would have to modify the DomU config file to replace default AHCI with old IDE.
>
> Regarding using Windows XP with AHCI:
> First, as stated before, if you want to install it on a new VM, you need the AHCI Drivers. Since it seems that at some point support for Floppy Disk got removed from Xen (I tried recently to use a Floppy Disk image with fda= as stated somewhere on the wiki, but it did nothing. Nor I found any recent documentation claiming that you can mount a Floppy Disk image at all), the only choice is to make a custom ISO with nLite with the AHCI Drivers integrated in it. Since the emulated Southbridge is an Intel ICH9, I believe that the Drivers that should be needed are these:
> http://www.win-raid.com/t22f23-Guide-Integration-of-Intels-AHCI-RAID-drivers-into-a-Windows-XP-W-k-W-k-CD.html
> Check for the a) and c) options for WXP/2003 in 32 and 64 Bits variants.
>
> Also, it is important to note that the plain ICH9 Southbridge according to Intel DOES NOT OFFICIALLY HAVE AHCI SUPPORT, ICH9R does.
> http://en.wikipedia.org/wiki/I/O_Controller_Hub#ICH9
>
> The previous Drivers are claimed to have been modified to force support for ICH9 since AHCI is there, just that the original Drivers doesn't want to use it. Typical artificial market segmentation strategies. Still, if QEMU is emulating the plain ICH9 and you can't get Windows to see drives in AHCI mode after integrating the AHCI Drivers, that may be why.
>
>
> Second, it is possible to switch from IDE to AHCI on WXP without reinstalling, but doing it is rather hacky and instructions are very Chipset and vendor dependant, which may (Or not) include editing Windows registry. Here are instructions that claim to work for Intel Southbridges, which since we're using ICH9, may possibily work for Xen based WXP VMs:
> http://www.prime-expert.com/articles/a11/change-from-ide-to-ahci-without-reinstalling-windows-xp.php
>
> But should these not work, chances are that you will need to add a rescue ISO to your DomU config file to repair the broken install, since I'm not 100% sure if you can even get it to boot in Safe Mode if it BSODs at boot.
>
>
> I think that's all what I could figure out. If you're using the GPLPV Drivers I don't know if there are any changes regarding running in AHCI mode.
>
>
>
> ----------------------------------------
>> Date: Mon, 8 Jun 2015 16:17:39 +0200
>> From: fabio.fantoni@m2r.biz
>> To: stefano.stabellini@eu.citrix.com
>> CC: xen-devel@lists.xensource.com; wei.liu2@citrix.com; Ian.Campbell@citrix.com; Ian.Jackson@eu.citrix.com; Paul.Durrant@citrix.com; anthony.perard@citrix.com
>> Subject: Re: [Xen-devel] [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
>>
>> Il 08/06/2015 16:00, Stefano Stabellini ha scritto:
>>> On Tue, 19 May 2015, Fabio Fantoni wrote:
>>>> Il 19/05/2015 12:40, Wei Liu ha scritto:
>>>>> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>>>>>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>>>>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>>>>>> Usage:
>>>>>>>> ahci=0|1 (default=0)
>>>>>>>>
>>>>>>>> If enabled adds ich9 disk controller in ahci mode and uses it with
>>>>>>>> upstream qemu to emulate disks instead of ide.
>>>>>>> Is ICH9 available in our default setup?
>>>>>>>
>>>>>>> Why do we not always enable AHCI?
>>>>>> ahci seems require ich9 controller (default in q35 but not in older used
>>>>>> by
>>>>>> xen), I think that change it ide->ahci automatically for all cases may
>>>>>> causes problems, probably in save/restore and more probably in windows <=7
>>>>>> without pv where change ide<->ahci require a registry change for not have
>>>>>> blue screen.
>>>>>>
>>>>> I guess we now have a QEMU with Q35. But migrating from previous
>>>>> previous versions will indeed need to be taken care of.
>>>> This patch don't add q35 cipset support but only ich9 ahci disk controller in
>>>> older and only cipset supported in xen.
>>> This is pretty nice actually, thanks for the patch!
>>>
>>>
>>>> Time ago I tried to add q35 support in xen but I had some problems I was
>>>> unable to solves and nobody helped me.
>>>> Q35 have newer ich9 ahci disk controller as default, on older cipset must be
>>>> added (done in this patch).
>>>> qemu, kvm ovmf already support q35, xen need changes at least in hvmloader
>>>> (that I'm probably unable to do) and libxl (that I started time ago without
>>>> good result), Add ahci support and switch all qemu parameters in libxl to new
>>>> -device is a good start also for future q35 support (are both part needed
>>>> based on my old q35 tests on xen)
>>> Yeah, going q35 opens a whole new can of worms. I think that adding the
>>> ich9 chipset to the existing machine is a good compromise.
>>>
>>>
>>>>>>>> It doesn't support cdroms which still use ide (cdroms will use
>>>>>>>> "-device
>>>>>>>> ide-cd" as new qemu parameters)
>>>>>>>> Ahci requires new qemu parameters but for now other emulated disks
>>>>>>>> cases
>>>>>>>> remains with old ones because automatic bus selection seems bugged in
>>>>>>>> qemu using new parameters. (I'll retry)
>>>>>>>>
>>>>>>> Buggy as in? Have you reported to QEMU upstream?
>>>>>> Already reported long time ago in xen-devel and qemu-devel, the only reply
>>>>>> from qemu-devel was to use fixed bus and is what I did in v2 of this
>>>>>> patch.
>>>>>> Can someone tell me if can be a problem fixed bus?
>>>>>>
>>>>> I don't have enough knowledge on this. Maybe Anthony and Stefano have
>>>>> more insight.
>>> Let me get this straight: cdrom still works but goes via ide. In
>>> addition AHCI disks can be attached. Seems good to me.
>>>
>>> Have you tested Windows?
>>>
>>> If this works with Windows too, I would expose an ahci option
>>> (default=off) in the config file. If ahci=on, then all disks could be
>>> exposed via ahci. Does this seem sensible? In a couple of releases, if
>>> it works well, we could make ahci=on the default.
>> I tested both linux and windows, mainly windows domUs (7 and 8.1).
>> Windows < 8 FWIK needs a registry key change for switch ide<->ahci
>> without bluescreen (this is the reason because I did ahci=0|1 instead
>> change it by default) but trying with windows 7 with pv is possible
>> change without bluescreen, probably the problem is only without pv but
>> ahci option default disabled resolve it (for the users don't set ahci=0
>> on xen upgrade process).
>> Based on what I know and all tests I did this patch seems the best way
>> for add ahci support but I think that a experts (like you) review is better.
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>   		 	   		

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-11 10:28               ` Fabio Fantoni
@ 2015-06-19 11:18                 ` Fabio Fantoni
  2015-06-22 10:34                   ` George Dunlap
  0 siblings, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-06-19 11:18 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, Stefano Stabellini, Ian.Jackson,
	Paul Durrant, anthony.perard, Zir Blazer

Il 11/06/2015 12:28, Fabio Fantoni ha scritto:
> Il 11/06/2015 12:06, Zir Blazer ha scritto:
>> Since I'm not a developer I may be peeking my nose a bit too far, but 
>> based on what I know, I think that enabling AHCI by default would be 
>> a compatibility suicide. I'm not sure about Linux and Windows 
>> Vista/7/8+, but at least for Windows XP based VMs, it would be a 
>> terrible idea.
>
> Also use windows xp without security updates (support ended one year 
> ago) is a "suicide".
>
> I already did this patch considering windows domU problems (I'm using 
> mainly them for now), ahci used with option (ahci=0|1) instead replace 
> and default is disabled.
> I tried it with different windows (excluding xp...abandoned)
> I also tried with new winpv drivers 
> (http://www.xenproject.org/developers/teams/windows-pv-drivers.html)
>
> With this patch applied ahci will be not used and will be used only 
> setting ahci=1, is it a good idea or is there problem also in this case?
>

I did many other tests in different linux hvm domUs (fedora and ubuntu) 
and windows (7, 8.1, 10) without found problems.
Is this patch acceptable for xen 4.6?

>
> Thanks for any reply and sorry for my bad english.
>
>>
>>
>> Back during WXP years, the vanilla install ISO didn't had any AHCI 
>> Drivers integrated at all. If you set the SATA Controller to AHCI, 
>> the WXP installation wouldn't detect any Hard Disk at all. If you 
>> really wanted to use AHCI during its early days, you would need a 
>> Floppy Disk with the AHCI Drivers of your SATA Controller 
>> (Chipset/South Bridge or third party controller), that you could use 
>> by pressing the F6 key right after booting the install CD:
>> http://www.pc-tips-and-tricks.com/images/Install-windows-xp-02.jpg
>>
>> If you didn't had a Floppy Disk Driver or were lazy, the typical 
>> choice was to set the SATA Controller to IDE and install WXP, 
>> skipping anything AHCI related.
>>
>> At some point an application called nLite appeared, which allowed you 
>> to make your custom Windows XP ISO. You could easily modify a lot of 
>> default options, integrate hotfixes, and even Drivers, and after you 
>> were happy with your changes, you could then make your custom ISO and 
>> burn it to a CD (Or from a USB Flash Drive with other tools to make 
>> it booteable). So, with nLite, it was rather easy to integrated the 
>> AHCI Drivers of your SATA Controller to the install ISO. This way, if 
>> you were doing a fresh install, reinstall or repair, there was no 
>> excuse to not use AHCI.
>> However, since for VM usage AHCI was not available at all until QEMU 
>> implemented it recently, by default you will be always using IDE, so 
>> there was no actual need to make a customized WXP install ISO with 
>> AHCI Drivers that you were never going to use. Besides, Windows XP 
>> doesn't even install nor have ready any AHCI Drivers if you're in IDE 
>> mode, so if you're going to switch to use AHCI by default, every 
>> existing WXP VM would greet you with a BSOD after upgrading Xen.
>>
>>
>> So, with this proposal you would have two issues: First, doing a new 
>> install of Windows XP on a VM , since by default it shouldn't detect 
>> any HD at all in AHCI. Second, that existing WXP VMs will BSOD on 
>> boot. For as long as you can switch to IDE mode everything should 
>> still be working as always, but it would mean that at the very least 
>> you would have to modify the DomU config file to replace default AHCI 
>> with old IDE.
>>
>> Regarding using Windows XP with AHCI:
>> First, as stated before, if you want to install it on a new VM, you 
>> need the AHCI Drivers. Since it seems that at some point support for 
>> Floppy Disk got removed from Xen (I tried recently to use a Floppy 
>> Disk image with fda= as stated somewhere on the wiki, but it did 
>> nothing. Nor I found any recent documentation claiming that you can 
>> mount a Floppy Disk image at all), the only choice is to make a 
>> custom ISO with nLite with the AHCI Drivers integrated in it. Since 
>> the emulated Southbridge is an Intel ICH9, I believe that the Drivers 
>> that should be needed are these:
>> http://www.win-raid.com/t22f23-Guide-Integration-of-Intels-AHCI-RAID-drivers-into-a-Windows-XP-W-k-W-k-CD.html 
>>
>> Check for the a) and c) options for WXP/2003 in 32 and 64 Bits variants.
>>
>> Also, it is important to note that the plain ICH9 Southbridge 
>> according to Intel DOES NOT OFFICIALLY HAVE AHCI SUPPORT, ICH9R does.
>> http://en.wikipedia.org/wiki/I/O_Controller_Hub#ICH9
>>
>> The previous Drivers are claimed to have been modified to force 
>> support for ICH9 since AHCI is there, just that the original Drivers 
>> doesn't want to use it. Typical artificial market segmentation 
>> strategies. Still, if QEMU is emulating the plain ICH9 and you can't 
>> get Windows to see drives in AHCI mode after integrating the AHCI 
>> Drivers, that may be why.
>>
>>
>> Second, it is possible to switch from IDE to AHCI on WXP without 
>> reinstalling, but doing it is rather hacky and instructions are very 
>> Chipset and vendor dependant, which may (Or not) include editing 
>> Windows registry. Here are instructions that claim to work for Intel 
>> Southbridges, which since we're using ICH9, may possibily work for 
>> Xen based WXP VMs:
>> http://www.prime-expert.com/articles/a11/change-from-ide-to-ahci-without-reinstalling-windows-xp.php 
>>
>>
>> But should these not work, chances are that you will need to add a 
>> rescue ISO to your DomU config file to repair the broken install, 
>> since I'm not 100% sure if you can even get it to boot in Safe Mode 
>> if it BSODs at boot.
>>
>>
>> I think that's all what I could figure out. If you're using the GPLPV 
>> Drivers I don't know if there are any changes regarding running in 
>> AHCI mode.
>>
>>
>>
>> ----------------------------------------
>>> Date: Mon, 8 Jun 2015 16:17:39 +0200
>>> From: fabio.fantoni@m2r.biz
>>> To: stefano.stabellini@eu.citrix.com
>>> CC: xen-devel@lists.xensource.com; wei.liu2@citrix.com; 
>>> Ian.Campbell@citrix.com; Ian.Jackson@eu.citrix.com; 
>>> Paul.Durrant@citrix.com; anthony.perard@citrix.com
>>> Subject: Re: [Xen-devel] [PATCH v2][RFC] libxl: Add AHCI support for 
>>> upstream qemu
>>>
>>> Il 08/06/2015 16:00, Stefano Stabellini ha scritto:
>>>> On Tue, 19 May 2015, Fabio Fantoni wrote:
>>>>> Il 19/05/2015 12:40, Wei Liu ha scritto:
>>>>>> On Mon, May 18, 2015 at 07:22:01PM +0200, Fabio Fantoni wrote:
>>>>>>> Il 18/05/2015 17:53, Wei Liu ha scritto:
>>>>>>>> On Thu, May 14, 2015 at 01:11:13PM +0200, Fabio Fantoni wrote:
>>>>>>>>> Usage:
>>>>>>>>> ahci=0|1 (default=0)
>>>>>>>>>
>>>>>>>>> If enabled adds ich9 disk controller in ahci mode and uses it 
>>>>>>>>> with
>>>>>>>>> upstream qemu to emulate disks instead of ide.
>>>>>>>> Is ICH9 available in our default setup?
>>>>>>>>
>>>>>>>> Why do we not always enable AHCI?
>>>>>>> ahci seems require ich9 controller (default in q35 but not in 
>>>>>>> older used
>>>>>>> by
>>>>>>> xen), I think that change it ide->ahci automatically for all 
>>>>>>> cases may
>>>>>>> causes problems, probably in save/restore and more probably in 
>>>>>>> windows <=7
>>>>>>> without pv where change ide<->ahci require a registry change for 
>>>>>>> not have
>>>>>>> blue screen.
>>>>>>>
>>>>>> I guess we now have a QEMU with Q35. But migrating from previous
>>>>>> previous versions will indeed need to be taken care of.
>>>>> This patch don't add q35 cipset support but only ich9 ahci disk 
>>>>> controller in
>>>>> older and only cipset supported in xen.
>>>> This is pretty nice actually, thanks for the patch!
>>>>
>>>>
>>>>> Time ago I tried to add q35 support in xen but I had some problems 
>>>>> I was
>>>>> unable to solves and nobody helped me.
>>>>> Q35 have newer ich9 ahci disk controller as default, on older 
>>>>> cipset must be
>>>>> added (done in this patch).
>>>>> qemu, kvm ovmf already support q35, xen need changes at least in 
>>>>> hvmloader
>>>>> (that I'm probably unable to do) and libxl (that I started time 
>>>>> ago without
>>>>> good result), Add ahci support and switch all qemu parameters in 
>>>>> libxl to new
>>>>> -device is a good start also for future q35 support (are both part 
>>>>> needed
>>>>> based on my old q35 tests on xen)
>>>> Yeah, going q35 opens a whole new can of worms. I think that adding 
>>>> the
>>>> ich9 chipset to the existing machine is a good compromise.
>>>>
>>>>
>>>>>>>>> It doesn't support cdroms which still use ide (cdroms will use
>>>>>>>>> "-device
>>>>>>>>> ide-cd" as new qemu parameters)
>>>>>>>>> Ahci requires new qemu parameters but for now other emulated 
>>>>>>>>> disks
>>>>>>>>> cases
>>>>>>>>> remains with old ones because automatic bus selection seems 
>>>>>>>>> bugged in
>>>>>>>>> qemu using new parameters. (I'll retry)
>>>>>>>>>
>>>>>>>> Buggy as in? Have you reported to QEMU upstream?
>>>>>>> Already reported long time ago in xen-devel and qemu-devel, the 
>>>>>>> only reply
>>>>>>> from qemu-devel was to use fixed bus and is what I did in v2 of 
>>>>>>> this
>>>>>>> patch.
>>>>>>> Can someone tell me if can be a problem fixed bus?
>>>>>>>
>>>>>> I don't have enough knowledge on this. Maybe Anthony and Stefano 
>>>>>> have
>>>>>> more insight.
>>>> Let me get this straight: cdrom still works but goes via ide. In
>>>> addition AHCI disks can be attached. Seems good to me.
>>>>
>>>> Have you tested Windows?
>>>>
>>>> If this works with Windows too, I would expose an ahci option
>>>> (default=off) in the config file. If ahci=on, then all disks could be
>>>> exposed via ahci. Does this seem sensible? In a couple of releases, if
>>>> it works well, we could make ahci=on the default.
>>> I tested both linux and windows, mainly windows domUs (7 and 8.1).
>>> Windows < 8 FWIK needs a registry key change for switch ide<->ahci
>>> without bluescreen (this is the reason because I did ahci=0|1 instead
>>> change it by default) but trying with windows 7 with pv is possible
>>> change without bluescreen, probably the problem is only without pv but
>>> ahci option default disabled resolve it (for the users don't set ahci=0
>>> on xen upgrade process).
>>> Based on what I know and all tests I did this patch seems the best way
>>> for add ahci support but I think that a experts (like you) review is 
>>> better.
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>
>

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-19 11:18                 ` Fabio Fantoni
@ 2015-06-22 10:34                   ` George Dunlap
  2015-06-22 11:42                     ` Fabio Fantoni
  0 siblings, 1 reply; 18+ messages in thread
From: George Dunlap @ 2015-06-22 10:34 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, Ian Jackson,
	xen-devel, Paul Durrant, Anthony PERARD, Zir Blazer

On Fri, Jun 19, 2015 at 12:18 PM, Fabio Fantoni
<fabio.fantoni@m2r.biz> wrote:> Il 11/06/2015 12:28, Fabio Fantoni ha
scritto:
>>
>> Il 11/06/2015 12:06, Zir Blazer ha scritto:
>>>
>>> Since I'm not a developer I may be peeking my nose a bit too far, but
>>> based on what I know, I think that enabling AHCI by default would be a
>>> compatibility suicide. I'm not sure about Linux and Windows Vista/7/8+, but
>>> at least for Windows XP based VMs, it would be a terrible idea.
>>
>>
>> Also use windows xp without security updates (support ended one year ago)
>> is a "suicide".
>>
>> I already did this patch considering windows domU problems (I'm using
>> mainly them for now), ahci used with option (ahci=0|1) instead replace and
>> default is disabled.
>> I tried it with different windows (excluding xp...abandoned)
>> I also tried with new winpv drivers
>> (http://www.xenproject.org/developers/teams/windows-pv-drivers.html)
>>
>> With this patch applied ahci will be not used and will be used only
>> setting ahci=1, is it a good idea or is there problem also in this case?
>>
>
> I did many other tests in different linux hvm domUs (fedora and ubuntu) and
> windows (7, 8.1, 10) without found problems.
> Is this patch acceptable for xen 4.6?

Well maybe I missed something, but:

1. The most recent version of this patch (v2) has RFC in the title;
this is a specific request *not* to apply this patch.

2. The most recent version of this patch has the following in the
changelog: "NOTES: This patch is a only a fast draft for testing."
That also sounds like you're asking people not to apply the patch.

3. After reading the changelog, many people were still unclear what
the purpose of the patch is.  You answered their questions by e-mail,
but that information needs to be in the changelog.

So you need to resend the patch 1) with RFC removed from the title;
and 2) with a proper changelog that doesn't say "just a draft", but
that 3) explains what the purpose of the change for people reading
through the revision history.

 -George

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-22 10:34                   ` George Dunlap
@ 2015-06-22 11:42                     ` Fabio Fantoni
  2015-06-22 13:14                       ` George Dunlap
  0 siblings, 1 reply; 18+ messages in thread
From: Fabio Fantoni @ 2015-06-22 11:42 UTC (permalink / raw)
  To: George Dunlap
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, Ian Jackson,
	xen-devel, Paul Durrant, Anthony PERARD, Zir Blazer

Il 22/06/2015 12:34, George Dunlap ha scritto:
> On Fri, Jun 19, 2015 at 12:18 PM, Fabio Fantoni
> <fabio.fantoni@m2r.biz> wrote:> Il 11/06/2015 12:28, Fabio Fantoni ha
> scritto:
>>> Il 11/06/2015 12:06, Zir Blazer ha scritto:
>>>> Since I'm not a developer I may be peeking my nose a bit too far, but
>>>> based on what I know, I think that enabling AHCI by default would be a
>>>> compatibility suicide. I'm not sure about Linux and Windows Vista/7/8+, but
>>>> at least for Windows XP based VMs, it would be a terrible idea.
>>>
>>> Also use windows xp without security updates (support ended one year ago)
>>> is a "suicide".
>>>
>>> I already did this patch considering windows domU problems (I'm using
>>> mainly them for now), ahci used with option (ahci=0|1) instead replace and
>>> default is disabled.
>>> I tried it with different windows (excluding xp...abandoned)
>>> I also tried with new winpv drivers
>>> (http://www.xenproject.org/developers/teams/windows-pv-drivers.html)
>>>
>>> With this patch applied ahci will be not used and will be used only
>>> setting ahci=1, is it a good idea or is there problem also in this case?
>>>
>> I did many other tests in different linux hvm domUs (fedora and ubuntu) and
>> windows (7, 8.1, 10) without found problems.
>> Is this patch acceptable for xen 4.6?
> Well maybe I missed something, but:
>
> 1. The most recent version of this patch (v2) has RFC in the title;
> this is a specific request *not* to apply this patch.
>
> 2. The most recent version of this patch has the following in the
> changelog: "NOTES: This patch is a only a fast draft for testing."
> That also sounds like you're asking people not to apply the patch.
>
> 3. After reading the changelog, many people were still unclear what
> the purpose of the patch is.  You answered their questions by e-mail,
> but that information needs to be in the changelog.
>
> So you need to resend the patch 1) with RFC removed from the title;
> and 2) with a proper changelog that doesn't say "just a draft", but
> that 3) explains what the purpose of the change for people reading
> through the revision history.
>
>   -George

Thanks for reply, I did RFC initially more than 1 month ago when I was 
less certainty requiring comment and experts review.
Is RFC=request for comments or I'm wrong?
Now after many tests on many systems and keep using it for weeks without 
problem seems ok.
I'll post v3 including missing doc, libxl.h entry and improving changelog.

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

* Re: [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu
  2015-06-22 11:42                     ` Fabio Fantoni
@ 2015-06-22 13:14                       ` George Dunlap
  0 siblings, 0 replies; 18+ messages in thread
From: George Dunlap @ 2015-06-22 13:14 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, Ian Jackson,
	xen-devel, Paul Durrant, Anthony PERARD, Zir Blazer

On Mon, Jun 22, 2015 at 12:42 PM, Fabio Fantoni <fabio.fantoni@m2r.biz> wrote:
> Il 22/06/2015 12:34, George Dunlap ha scritto:
>>
>> On Fri, Jun 19, 2015 at 12:18 PM, Fabio Fantoni
>> <fabio.fantoni@m2r.biz> wrote:> Il 11/06/2015 12:28, Fabio Fantoni ha
>> scritto:
>>>>
>>>> Il 11/06/2015 12:06, Zir Blazer ha scritto:
>>>>>
>>>>> Since I'm not a developer I may be peeking my nose a bit too far, but
>>>>> based on what I know, I think that enabling AHCI by default would be a
>>>>> compatibility suicide. I'm not sure about Linux and Windows Vista/7/8+,
>>>>> but
>>>>> at least for Windows XP based VMs, it would be a terrible idea.
>>>>
>>>>
>>>> Also use windows xp without security updates (support ended one year
>>>> ago)
>>>> is a "suicide".
>>>>
>>>> I already did this patch considering windows domU problems (I'm using
>>>> mainly them for now), ahci used with option (ahci=0|1) instead replace
>>>> and
>>>> default is disabled.
>>>> I tried it with different windows (excluding xp...abandoned)
>>>> I also tried with new winpv drivers
>>>> (http://www.xenproject.org/developers/teams/windows-pv-drivers.html)
>>>>
>>>> With this patch applied ahci will be not used and will be used only
>>>> setting ahci=1, is it a good idea or is there problem also in this case?
>>>>
>>> I did many other tests in different linux hvm domUs (fedora and ubuntu)
>>> and
>>> windows (7, 8.1, 10) without found problems.
>>> Is this patch acceptable for xen 4.6?
>>
>> Well maybe I missed something, but:
>>
>> 1. The most recent version of this patch (v2) has RFC in the title;
>> this is a specific request *not* to apply this patch.
>>
>> 2. The most recent version of this patch has the following in the
>> changelog: "NOTES: This patch is a only a fast draft for testing."
>> That also sounds like you're asking people not to apply the patch.
>>
>> 3. After reading the changelog, many people were still unclear what
>> the purpose of the patch is.  You answered their questions by e-mail,
>> but that information needs to be in the changelog.
>>
>> So you need to resend the patch 1) with RFC removed from the title;
>> and 2) with a proper changelog that doesn't say "just a draft", but
>> that 3) explains what the purpose of the change for people reading
>> through the revision history.
>>
>>   -George
>
>
> Thanks for reply, I did RFC initially more than 1 month ago when I was less
> certainty requiring comment and experts review.
> Is RFC=request for comments or I'm wrong?

Yes, RFC literally means "request for comments"; but of course, you
should expect people to read and comment on *any* patch that you
submit. :-)

Normally when you submit a patch, there is an implied "Please check
this in."  Adding RFC changes that to, "What do you guys think about
this?"  In other words, you're just looking for feeback, not asking
this to be checked into the tree just yet.  That changes the way the
patch is reviewed.  Once you have something you're ready to check in,
you should drop the RFC.

> Now after many tests on many systems and keep using it for weeks without
> problem seems ok.
> I'll post v3 including missing doc, libxl.h entry and improving changelog.

Just as a note, normally when you change from RFC -> non-RFC, you
reset the version numbers.  (i.e., you should be able to just submit
this patch with no 'v' attached).

That's not too big a deal either way though.

 -George

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

end of thread, other threads:[~2015-06-22 13:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 11:11 [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu Fabio Fantoni
2015-05-18 15:53 ` Wei Liu
2015-05-18 17:22   ` Fabio Fantoni
2015-05-19 10:40     ` Wei Liu
2015-05-19 12:13       ` Fabio Fantoni
2015-05-25 10:41         ` Fabio Fantoni
2015-05-25 10:48           ` Wei Liu
2015-05-27  9:52             ` George Dunlap
2015-06-08 14:00         ` Stefano Stabellini
2015-06-08 14:17           ` Fabio Fantoni
2015-06-11 10:06             ` Zir Blazer
2015-06-11 10:28               ` Fabio Fantoni
2015-06-19 11:18                 ` Fabio Fantoni
2015-06-22 10:34                   ` George Dunlap
2015-06-22 11:42                     ` Fabio Fantoni
2015-06-22 13:14                       ` George Dunlap
2015-05-18 16:16 ` Ian Jackson
2015-05-18 17:28   ` 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.