Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make setattr silently fail on remote inodes #3

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Conversation

pawalt
Copy link
Member

@pawalt pawalt commented Aug 14, 2024

This commit changes the setattr behavior so that if an inode is remote, the operation is a no-op. This means that programs like wget that used to fail on our s3 mounts will now succeed, but some of their operations will not do anything.

This commit changes the setattr behavior so that if an inode is remote,
the operation is a no-op. This means that programs like `wget` that used
to fail on our s3 mounts will now succeed, but some of their operations
will not do anything.
@pawalt
Copy link
Member Author

pawalt commented Aug 15, 2024

I have tested that this fixes our wget issue on cloud bucket mounts

root@modal:/my-mount# wget http://google.com
--2024-08-15 15:48:26--  http://google.com/
Resolving google.com (google.com)... 142.251.111.113, 142.251.111.102, 142.251.111.101, ...
Connecting to google.com (google.com)|142.251.111.113|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://www.google.com/ [following]
--2024-08-15 15:48:26--  http://www.google.com/
Resolving www.google.com (www.google.com)... 142.251.167.147, 142.251.167.106, 142.251.167.105, ...
Connecting to www.google.com (www.google.com)|142.251.167.147|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/html]
Saving to: ‘index.html’

index.html                                                                        [ <=>                                                                                                                                                                                           ]  19.98K  --.-KB/s    in 0.04s   

2024-08-15 15:48:27 (454 KB/s) - ‘index.html’ saved [20458]

if sync.write_status == WriteStatus::Remote {
return Err(InodeError::SetAttrNotPermittedOnRemoteInode(inode.err()));
let stat = sync.stat.clone();
drop(sync);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this drop needed to address a compiler complaint? It gets dropped when this setattr returns doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's for the compiler. Not sure how else to handle it.

@pawalt pawalt merged commit 0c344de into main Aug 16, 2024
9 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants