NearNodeFlash / NearNodeFlash.github.io

View this document https://nearnodeflash.github.io/
Apache License 2.0
3 stars 3 forks source link

Allow umask to be configured for data movement #99

Open behlendorf opened 1 year ago

behlendorf commented 1 year ago

It should be possible to set a default umask to be used during data movement. There are also use cases which would benefit from being able to override the default for a given data movement request.

bdevcich commented 1 year ago

I think we could add a umask value to the nnf-dm-config configmap and allow that to be overridden via the Copy Offload API's CreateRequest.

@behlendorf: do you know if mpirun will honor the current session's umask? I'm curious how this work would when running mpirun dcp .... I'll do some experimenting.

bdevcich commented 1 year ago

I think we may have to update dcp again (like we did for UID/GID) to account for this.

behlendorf commented 1 year ago

@bdevcich-hpe I believe mpirun is expected to propagate the umask, but I haven't tested this myself.

bdevcich commented 1 year ago

Seems you are right, @behlendorf. However, it looks like dcp doesn't account for it. I've tried the -X all, -X none, -p options with it, to no avail. I might be missing something.

Here's my test results. cp --no-preserve=all works with or without mpirun and we get 666 with a umask of 0 on a src file that originates with 644 (umask 0022). But as soon as I introduce dcp it appears that it uses the default umask of 0022 rather than 0.

root@nnf-dm-controller-manager-798dbbc9cd-z2kfg:~/blake# ./test.sh
creating file with 0022
using umask: 0000 for copies

test-22: 644
dest/test-22-cp: 666
dest/test-22-mpirun-cp: 666
dest/test-22-mpirun-dcp: 644
dest/test-22-mpirun-dcp-p: 644
dest/test-22-mpirun-dcp-x-all: 644
dest/test-22-mpirun-dcp-x-none: 644
root@nnf-dm-controller-manager-798dbbc9cd-z2kfg:~/blake#

Here's the script to reproduce this. I'm running it on the DM controller pod:

#!/usr/bin/env bash

mkdir -p dest
rm dest/*

umask 0022
echo "creating file with $(umask)"
touch test-22

umask 0000
echo "using umask: $(umask) for copies"
echo ""

cp --no-preserve=all test-22 dest/test-22-cp
mpirun --allow-run-as-root --host localhost cp --no-preserve=all test-22 dest/test-22-mpirun-cp
mpirun --allow-run-as-root --host localhost dcp -q test-22 dest/test-22-mpirun-dcp
mpirun --allow-run-as-root --host localhost dcp -q test-22 -p dest/test-22-mpirun-dcp-p
mpirun --allow-run-as-root --host localhost dcp -q test-22 -X all dest/test-22-mpirun-dcp-x-all
mpirun --allow-run-as-root --host localhost dcp -q test-22 -X none dest/test-22-mpirun-dcp-x-none

echo "test-22: $(stat -c "%a" test-22)"
echo "dest/test-22-cp: $(stat -c "%a" dest/test-22-cp)"
echo "dest/test-22-mpirun-cp: $(stat -c "%a" dest/test-22-mpirun-cp)"
echo "dest/test-22-mpirun-dcp: $(stat -c "%a" dest/test-22-mpirun-dcp)"
echo "dest/test-22-mpirun-dcp-p: $(stat -c "%a" dest/test-22-mpirun-dcp-p)"
echo "dest/test-22-mpirun-dcp-x-all: $(stat -c "%a" dest/test-22-mpirun-dcp-x-all)"
echo "dest/test-22-mpirun-dcp-x-none: $(stat -c "%a" dest/test-22-mpirun-dcp-x-none)"
ofaaland commented 1 year ago

Hi Blake,

In mpifileutils, the "-X" options are for extended attributes (see man xattr and man attr), not permissions. So you're right those won't help.

I'll look at the dcp code to see how it handles umask.

-Olaf


From: Blake Devcich @.***> Sent: Tuesday, April 18, 2023 12:31 PM To: NearNodeFlash/nnf-dm Cc: Subscribed Subject: Re: [NearNodeFlash/nnf-dm] Allow umask to be configured for data movement (Issue NearNodeFlash/NearNodeFlash.github.io#99)

Seems you are right, @behlendorfhttps://urldefense.us/v3/__https://github.com/behlendorf__;!!G2kpM7uM-TzIFchu!gheu5qmkIguETolou-HzMiy1xeModG91hha9oPgxfJAUY6k5IwFKvOx-Y-g--ljDtQ$. However, it looks like dcp doesn't account for it. I've tried the -X all, -X none, -p options with it, to no avail. I might be missing something.

Here's my test results. cp --no-preserve=all works with or without mpirun and we get 666 with a umask of 0 on a src file that originates with 644 (umask 0022). But as soon as I introduce dcp it appears that it uses the default umask of 0022 rather than 0.

@.***:~/blake# ./test.sh creating file with 0022 using umask: 0000 for copies

test-22: 644 dest/test-22-cp: 666 dest/test-22-mpirun-cp: 666 dest/test-22-mpirun-dcp: 644 dest/test-22-mpirun-dcp-p: 644 dest/test-22-mpirun-dcp-x-all: 644 dest/test-22-mpirun-dcp-x-none: 644 @.***:~/blake#

Here's the script to reproduce this. I'm running it on the DM controller pod:

!/usr/bin/env bash

mkdir -p dest rm dest/*

umask 0022 echo "creating file with $(umask)" touch test-22

umask 0000 echo "using umask: $(umask) for copies" echo ""

cp --no-preserve=all test-22 dest/test-22-cp mpirun --allow-run-as-root --host localhost cp --no-preserve=all test-22 dest/test-22-mpirun-cp mpirun --allow-run-as-root --host localhost dcp -q test-22 dest/test-22-mpirun-dcp mpirun --allow-run-as-root --host localhost dcp -q test-22 -p dest/test-22-mpirun-dcp-p mpirun --allow-run-as-root --host localhost dcp -q test-22 -X all dest/test-22-mpirun-dcp-x-all mpirun --allow-run-as-root --host localhost dcp -q test-22 -X none dest/test-22-mpirun-dcp-x-none

echo "test-22: $(stat -c "%a" test-22)" echo "dest/test-22-cp: $(stat -c "%a" dest/test-22-cp)" echo "dest/test-22-mpirun-cp: $(stat -c "%a" dest/test-22-mpirun-cp)" echo "dest/test-22-mpirun-dcp: $(stat -c "%a" dest/test-22-mpirun-dcp)" echo "dest/test-22-mpirun-dcp-p: $(stat -c "%a" dest/test-22-mpirun-dcp-p)" echo "dest/test-22-mpirun-dcp-x-all: $(stat -c "%a" dest/test-22-mpirun-dcp-x-all)" echo "dest/test-22-mpirun-dcp-x-none: $(stat -c "%a" dest/test-22-mpirun-dcp-x-none)"

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/NearNodeFlash/NearNodeFlash.github.io/issues/99*issuecomment-1513690461__;Iw!!G2kpM7uM-TzIFchu!gheu5qmkIguETolou-HzMiy1xeModG91hha9oPgxfJAUY6k5IwFKvOx-Y-hbCLEMdg$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AB73C457XSRMNRCPZXM2UILXB3TYFANCNFSM6AAAAAAVMLB4IU__;!!G2kpM7uM-TzIFchu!gheu5qmkIguETolou-HzMiy1xeModG91hha9oPgxfJAUY6k5IwFKvOx-Y-h22gquUA$. You are receiving this because you are subscribed to this thread.Message ID: @.***>

bdevcich commented 1 year ago

@ofaaland any updates/guidance on this?

ofaaland commented 1 year ago

Hi Blake,

I've reproduced the issue and begun looking through the dcp code. I still don't quite understand how it happens, but I think I'm probably close.


From: Blake Devcich @.***> Sent: Monday, May 1, 2023 7:48 AM To: NearNodeFlash/nnf-dm Cc: Faaland, Olaf P.; Mention Subject: Re: [NearNodeFlash/nnf-dm] Allow umask to be configured for data movement (Issue NearNodeFlash/NearNodeFlash.github.io#99)

@ofaalandhttps://urldefense.us/v3/__https://github.com/ofaaland__;!!G2kpM7uM-TzIFchu!wLH-vYMEjhdc8M0fLs7JbtAUqgBx0rk0xloN4F8Tl7FF7UjGUtNgDaP5rPX-OX1o3-zEYn_t5MESiNo5JmTCapbD7Q$ any updates/guidance on this?

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/NearNodeFlash/NearNodeFlash.github.io/issues/99*issuecomment-1529791396__;Iw!!G2kpM7uM-TzIFchu!wLH-vYMEjhdc8M0fLs7JbtAUqgBx0rk0xloN4F8Tl7FF7UjGUtNgDaP5rPX-OX1o3-zEYn_t5MESiNo5JmRPr3cVdQ$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AB73C4Z7Z6TRDSF2LMI55F3XD7EL7ANCNFSM6AAAAAAVMLB4IU__;!!G2kpM7uM-TzIFchu!wLH-vYMEjhdc8M0fLs7JbtAUqgBx0rk0xloN4F8Tl7FF7UjGUtNgDaP5rPX-OX1o3-zEYn_t5MESiNo5JmQ0CRhugQ$. You are receiving this because you were mentioned.Message ID: @.***>