Re: [Xen-devel] [Qemu-devel] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.13 at 13:10, Markus Armbruster <armbru@xxxxxxxxxx> wrote:
> Ian Campbell <Ian.Campbell@xxxxxxxxxx> writes:
> 
>> On Wed, 2013-10-16 at 12:49 +0200, Markus Armbruster wrote:
>>> Ian Campbell <Ian.Campbell@xxxxxxxxxx> writes:
>>> 
>>> > On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
>>> >> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@xxxxxxxxxx> 
>>> >> >>> wrote:
>>> >> > --- hvmloader/acpi/Makefile    2013-10-16 11:51:53.000000000 +0800
>>> >> > +++ hvmloader_new//acpi/Makefile       2013-10-16 11:51:58.000000000 
>>> >> > +0800
>>> >> > @@ -36,18 +36,34 @@
>>> >> >  
>>> >> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>>> >> >        awk 'NR > 1 {print s} {s=$$0}' $< > $@
>>> >> > -      ./mk_dsdt --dm-version qemu-xen >> $@
>>> >> > +      sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
>>> >> 
>>> >> This must never be done - if someone hits Ctrl-C in the middle of
>>> >> this, you'll have a modified but incomplete generated file. You
>>> >> either need to use properly chained rules, or do all output to a
>>> >> temporary file which you rename as the last step.
>>> >> 
>>> >> I realize that the problem existed before your change, but you
>>> >> making it worse requires doing it properly now.
>>> >
>>> > The correct way to do this is to generate to a temporary file and then
>>> > use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
>>> > tools/libxl/Makefile and elsewhere.
>>> 
>>> Quoting the GNU make manual[*]:
>>> 
>>>     So generally the right thing to do is to delete the target file if
>>>     the recipe fails after beginning to change the file.  make will do
>>>     this if .DELETE_ON_ERROR appears as a target.  This is almost always
>>>     what you want make to do, but it is not historical practice; so for
>>>     compatibility, you must explicitly request it.
>>> 
>>> In my opinion, every Makefile should request it.
>>> 
>>> [*] https://www.gnu.org/software/make/manual/html_node/Errors.html 
>>
>> I think this is somewhat orthogonal to the use of move-if-changed which
>> is there to ensure that the timestamp doesn't change gratuitously and
>> cause a load of knock on rebuilds even though the regenerated content is
>> identical.
> 
> Yes.  I was latching onto the "if someone hits Ctrl-C in the middle of
> this, you'll have a modified but incomplete generated file" phrase.

Right, but Ctrl-C is only the soft way of failing. If the system
happens to crash at the right moment, make won't be able to go
and delete the file anymore. IOW the move-if-changed approach
covers a wider range of situations _and_ had the benefit of
avoiding unnecessary rebuilds.

Jan


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