Fix extended attributes handling for VSS snapshots#4998
Fix extended attributes handling for VSS snapshots#4998MichaelEischer merged 4 commits intorestic:masterfrom
Conversation
MichaelEischer
left a comment
There was a problem hiding this comment.
Just a few small nits, see below
Gracefully handle errors while checking for EA and add debug logs.
| debug.Log("Error checking if extended attributes are supported for prepared volume name %s: %v", volumeName, err) | ||
| // There can be multiple errors like path does not exist, bad network path, etc. | ||
| // We just gracefully disallow extended attributes for cases. | ||
| return false, nil |
There was a problem hiding this comment.
I think it will be much less intrusive to gracefully handle the errors and simple skip extended attributes since this is a side functionality. The other parts of the code can handle the path and file existence related errors instead.
There can be multiple errors like path does not exist, bad network path, etc.
Maybe we should show Warnings?
There was a problem hiding this comment.
I agree, extended attributes aren't that important on Windows. Skipping them if we cannot detect whether they are supported is fine for me.
Maybe we should show Warnings?
Hmm, there's nothing a user can do about such warnings. A debug log entry should be enough for now.
MichaelEischer
left a comment
There was a problem hiding this comment.
LGTM. Thanks a lot for the fixes!
| debug.Log("Error checking if extended attributes are supported for prepared volume name %s: %v", volumeName, err) | ||
| // There can be multiple errors like path does not exist, bad network path, etc. | ||
| // We just gracefully disallow extended attributes for cases. | ||
| return false, nil |
There was a problem hiding this comment.
I agree, extended attributes aren't that important on Windows. Skipping them if we cannot detect whether they are supported is fine for me.
Maybe we should show Warnings?
Hmm, there's nothing a user can do about such warnings. A debug log entry should be enough for now.
What does this PR change? What problem does it solve?
After the change in #4980 support for extended attributes for VSS snapshot based backups was disabled. This change allows extended attributes to be backed up for VSS snapshots as well.
Was the change previously discussed in an issue or on the forum?
Follow up to - #4980
Checklist
changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.