[Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Aaron Lindsay]

I get the following error when building on an NFSv3 filesystem:

% make -j8
  GEN     aarch64-softmmu/config-devices.mak.tmp
  GEN     config-host.h
[snip]
  GEN     qmp-marshal.c
  GEN     aarch64-softmmu/config-devices.mak
cp: preserving permissions for ‘aarch64-softmmu/config-devices.mak.old’: Operation not supported
make: *** Deleting file `aarch64-softmmu/config-devices.mak'
  GEN     qapi-types.c
[snip]
  CC      scsi/qemu-pr-helper.o
make: *** No rule to make target `config-all-devices.mak', needed by `subdir-aarch64-softmmu'.  Stop.
make: *** Waiting for unfinished jobs....

Ideally you would only build on a filesystem with proper support, but I haven't
been able to find a reason why preserving exact permissions is important in
this case.

Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 9372742..952b6df 100644
--- a/Makefile
+++ b/Makefile
@@ -287,7 +287,7 @@ endif
 	$(call quiet-command, if test -f $@; then \
 	  if cmp -s $@.old $@; then \
 	    mv $@.tmp $@; \
-	    cp -p $@ $@.old; \
+	    cp $@ $@.old; \
 	  else \
 	    if test -f $@.old; then \
 	      echo "WARNING: $@ (user modified) out of date.";\
@@ -299,7 +299,7 @@ endif
 	  fi; \
 	 else \
 	  mv $@.tmp $@; \
-	  cp -p $@ $@.old; \
+	  cp $@ $@.old; \
 	 fi,"GEN","$@");
 
 defconfig:
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Peter Maydell]

On 19 October 2017 at 21:04, Aaron Lindsay <alindsay@codeaurora.org> wrote:
> I get the following error when building on an NFSv3 filesystem:
>
> % make -j8
>   GEN     aarch64-softmmu/config-devices.mak.tmp
>   GEN     config-host.h
> [snip]
>   GEN     qmp-marshal.c
>   GEN     aarch64-softmmu/config-devices.mak
> cp: preserving permissions for ‘aarch64-softmmu/config-devices.mak.old’: Operation not supported
> make: *** Deleting file `aarch64-softmmu/config-devices.mak'
>   GEN     qapi-types.c
> [snip]
>   CC      scsi/qemu-pr-helper.o
> make: *** No rule to make target `config-all-devices.mak', needed by `subdir-aarch64-softmmu'.  Stop.
> make: *** Waiting for unfinished jobs....
>
> Ideally you would only build on a filesystem with proper support, but I haven't
> been able to find a reason why preserving exact permissions is important in
> this case.

Do we even need this code at all? As far as I can tell from
the git logs, the idea is to support users who hand-modify
config-devices.mak. But do we want to support that? I would
think of config-devices.mak as an internal part of the build
machinery, and the bit you can edit as a user is the stuff
in default-configs/.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[alindsay]

On 2017-10-20 05:27, Peter Maydell wrote:
> On 19 October 2017 at 21:04, Aaron Lindsay <alindsay@codeaurora.org> 
> wrote:
>> I get the following error when building on an NFSv3 filesystem:
>> 
>> % make -j8
>>   GEN     aarch64-softmmu/config-devices.mak.tmp
>>   GEN     config-host.h
>> [snip]
>>   GEN     qmp-marshal.c
>>   GEN     aarch64-softmmu/config-devices.mak
>> cp: preserving permissions for 
>> ‘aarch64-softmmu/config-devices.mak.old’: Operation not supported
>> make: *** Deleting file `aarch64-softmmu/config-devices.mak'
>>   GEN     qapi-types.c
>> [snip]
>>   CC      scsi/qemu-pr-helper.o
>> make: *** No rule to make target `config-all-devices.mak', needed by 
>> `subdir-aarch64-softmmu'.  Stop.
>> make: *** Waiting for unfinished jobs....
>> 
>> Ideally you would only build on a filesystem with proper support, but 
>> I haven't
>> been able to find a reason why preserving exact permissions is 
>> important in
>> this case.
> 
> Do we even need this code at all? As far as I can tell from
> the git logs, the idea is to support users who hand-modify
> config-devices.mak. But do we want to support that? I would
> think of config-devices.mak as an internal part of the build
> machinery, and the bit you can edit as a user is the stuff
> in default-configs/.

I haven't ever found a reason to modify config-devices.mak and
just assumed others had. Its existence doesn't bother me, but I
can also see the argument to simplify if it's unused. Would you
prefer I resubmit a patch removing it instead?

-Aaron

-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Stefan Weil]

Am 20.10.2017 um 20:24 schrieb alindsay@codeaurora.org:
> On 2017-10-20 05:27, Peter Maydell wrote:
>> On 19 October 2017 at 21:04, Aaron Lindsay <alindsay@codeaurora.org>
>> wrote:
>>> I get the following error when building on an NFSv3 filesystem:
>>>
>>> % make -j8
>>>   GEN     aarch64-softmmu/config-devices.mak.tmp
>>>   GEN     config-host.h
>>> [snip]
>>>   GEN     qmp-marshal.c
>>>   GEN     aarch64-softmmu/config-devices.mak
>>> cp: preserving permissions for
>>> ‘aarch64-softmmu/config-devices.mak.old’: Operation not supported
>>> make: *** Deleting file `aarch64-softmmu/config-devices.mak'
>>>   GEN     qapi-types.c
>>> [snip]
>>>   CC      scsi/qemu-pr-helper.o
>>> make: *** No rule to make target `config-all-devices.mak', needed by
>>> `subdir-aarch64-softmmu'.  Stop.
>>> make: *** Waiting for unfinished jobs....
>>>
>>> Ideally you would only build on a filesystem with proper support, but
>>> I haven't
>>> been able to find a reason why preserving exact permissions is
>>> important in
>>> this case.
>>
>> Do we even need this code at all? As far as I can tell from
>> the git logs, the idea is to support users who hand-modify
>> config-devices.mak. But do we want to support that? I would
>> think of config-devices.mak as an internal part of the build
>> machinery, and the bit you can edit as a user is the stuff
>> in default-configs/.

It's a long time since I wrote that code, but when I look at
the commit message for my commit 012f0879234, it was written
for users who do _not_ hand-modify config-devices.mak. They
had a problem when they updated the code from git and the
new version had changed some of the device configurations
which were used to build config-devices.mak.

So maybe that code is still needed because device configurations
change sometimes.

Regards,
Stefan


> I haven't ever found a reason to modify config-devices.mak and
> just assumed others had. Its existence doesn't bother me, but I
> can also see the argument to simplify if it's unused. Would you
> prefer I resubmit a patch removing it instead?
> 
> -Aaron
> 

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Peter Maydell]

On 20 October 2017 at 20:08, Stefan Weil <sw@weilnetz.de> wrote:
> Am 20.10.2017 um 20:24 schrieb alindsay@codeaurora.org:
>> On 2017-10-20 05:27, Peter Maydell wrote:
>>> Do we even need this code at all? As far as I can tell from
>>> the git logs, the idea is to support users who hand-modify
>>> config-devices.mak. But do we want to support that? I would
>>> think of config-devices.mak as an internal part of the build
>>> machinery, and the bit you can edit as a user is the stuff
>>> in default-configs/.
>
> It's a long time since I wrote that code, but when I look at
> the commit message for my commit 012f0879234, it was written
> for users who do _not_ hand-modify config-devices.mak. They
> had a problem when they updated the code from git and the
> new version had changed some of the device configurations
> which were used to build config-devices.mak.

Right, but it's only this complicated set of conditions
because we seem to be also trying to support the hand-modify
case. Otherwise we could just generate the new version
and copy it into place if it's changed, unconditionally...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 20 October 2017 at 20:08, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 20.10.2017 um 20:24 schrieb alindsay@codeaurora.org:
>>> On 2017-10-20 05:27, Peter Maydell wrote:
>>>> Do we even need this code at all? As far as I can tell from
>>>> the git logs, the idea is to support users who hand-modify
>>>> config-devices.mak. But do we want to support that? I would
>>>> think of config-devices.mak as an internal part of the build
>>>> machinery, and the bit you can edit as a user is the stuff
>>>> in default-configs/.
>>
>> It's a long time since I wrote that code, but when I look at
>> the commit message for my commit 012f0879234, it was written
>> for users who do _not_ hand-modify config-devices.mak. They
>> had a problem when they updated the code from git and the
>> new version had changed some of the device configurations
>> which were used to build config-devices.mak.
>
> Right, but it's only this complicated set of conditions
> because we seem to be also trying to support the hand-modify
> case. Otherwise we could just generate the new version
> and copy it into place if it's changed, unconditionally...

So, do we need this patch?  If yes, who's going to merge it?  If no, do
we need some other patch?

Re: [Qemu-devel] [PATCH] build: Don't force preserving permissions on config-devices.mak.old

[Stefan Hajnoczi]

On Thu, Oct 19, 2017 at 04:04:43PM -0400, Aaron Lindsay wrote:
> I get the following error when building on an NFSv3 filesystem:
> 
> % make -j8
>   GEN     aarch64-softmmu/config-devices.mak.tmp
>   GEN     config-host.h
> [snip]
>   GEN     qmp-marshal.c
>   GEN     aarch64-softmmu/config-devices.mak
> cp: preserving permissions for ‘aarch64-softmmu/config-devices.mak.old’: Operation not supported
> make: *** Deleting file `aarch64-softmmu/config-devices.mak'
>   GEN     qapi-types.c
> [snip]
>   CC      scsi/qemu-pr-helper.o
> make: *** No rule to make target `config-all-devices.mak', needed by `subdir-aarch64-softmmu'.  Stop.
> make: *** Waiting for unfinished jobs....
> 
> Ideally you would only build on a filesystem with proper support, but I haven't
> been able to find a reason why preserving exact permissions is important in
> this case.
> 
> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I cannot see a reason why $@.old must preserve timestamp/mode/ownership
either:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>