Re: [Xen-devel] [PATCH v2][RFC] libxl: Add AHCI support for upstream qemu

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Jun 22, 2015 at 12:42 PM, Fabio Fantoni <fabio.fantoni@xxxxxxx> wrote:
> Il 22/06/2015 12:34, George Dunlap ha scritto:
>>
>> On Fri, Jun 19, 2015 at 12:18 PM, Fabio Fantoni
>> <fabio.fantoni@xxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel