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

--use-fs-snapshot failed on windows/386 #3087

Closed
DRON-666 opened this issue Nov 11, 2020 · 9 comments · Fixed by #3090
Closed

--use-fs-snapshot failed on windows/386 #3087

DRON-666 opened this issue Nov 11, 2020 · 9 comments · Fixed by #3090

Comments

@DRON-666
Copy link
Contributor

Output of restic version

restic 0.11.0 compiled with go1.15.2 on windows/386

How did you run restic exactly?

restic --use-fs-snapshot d:\test

What backend/server/service did you use to store the repository?

local filesystem repository

Expected behavior

VSS don't failed

Actual behavior

repository 43e035ae opened successfully, password is correct
creating VSS snapshot for [d:\]
error: failed to create snapshot for [d:\]: VSS error: GetSnapshotProperties() failed: E_INVALIDARG (0x80070057)
.....

Steps to reproduce the behavior

Use windows/386 restic version on 32-bit Windows with --use-fs-snapshot flag.

Do you have any idea what may have caused this?

Nobody uses 32-bit Windows, and syscalls from vss_windows.go are hardly tested.
I was absolute sure this bug was fixed by @fgma, but this piece of code seems to be cursed.

Do you have an idea how to solve the issue?

diff --git a/internal/fs/vss_windows.go b/internal/fs/vss_windows.go
index e24dade..8cee09f 100644
--- a/internal/fs/vss_windows.go
+++ b/internal/fs/vss_windows.go
@@ -482,7 +482,7 @@ func (vss *IVssBackupComponents) DeleteSnapshots(snapshotID ole.GUID) (int32, ol
        var result uintptr = 0
 
        if runtime.GOARCH == "386" {
-               id := (*[4]uintptr)(unsafe.Pointer(ole.IID_NULL))
+               id := (*[4]uintptr)(unsafe.Pointer(&snapshotID))
 
                result, _, _ = syscall.Syscall9(vss.getVTable().deleteSnapshots, 9,
                        uintptr(unsafe.Pointer(vss)), id[0], id[1], id[2], id[3],
@@ -506,7 +506,7 @@ func (vss *IVssBackupComponents) GetSnapshotProperties(snapshotID ole.GUID,
        var result uintptr = 0
 
        if runtime.GOARCH == "386" {
-               id := (*[4]uintptr)(unsafe.Pointer(ole.IID_NULL))
+               id := (*[4]uintptr)(unsafe.Pointer(&snapshotID))
 
                result, _, _ = syscall.Syscall6(vss.getVTable().getSnapshotProperties, 6,
                        uintptr(unsafe.Pointer(vss)), id[0], id[1], id[2], id[3],

I think restic needs integration tests with real testing of VSS. Perhaps carefully prepared VHD image with many partitions of different types, broken mount points and other edge cases.

Did restic help you today? Did it make you happy in any way?

Yes! Yes!

@rawtaz rawtaz changed the title --use-fs-snapshot failed on windows/386 --use-fs-snapshot failed on windows/386 Nov 11, 2020
@rawtaz
Copy link
Contributor

rawtaz commented Nov 11, 2020

Related comment: #2274 (comment)

@DRON-666
Copy link
Contributor Author

No @rawtaz it's totally different issue.

@DRON-666
Copy link
Contributor Author

This comment (and below) seemed more related.

@rawtaz
Copy link
Contributor

rawtaz commented Nov 11, 2020

I was mostly thinking about not supporting 32-bit Windows due to the reasons in the comment i linked to and that we'll mostly just handle the failures on it gracefully.

@DRON-666
Copy link
Contributor Author

32-bit restic supports VSS on 32-bit Windows, but don't supports it on 64-bit Windows due to VSS limitations.

@fgma
Copy link
Contributor

fgma commented Nov 12, 2020

@DRON-666 I'm going to test this in a VM. Are you going to create a PR?

@DRON-666
Copy link
Contributor Author

Are you going to create a PR?

Not now.

@fgma
Copy link
Contributor

fgma commented Nov 12, 2020

Just tested it. Works as expected. Should I create a PR for it?

@DRON-666
Copy link
Contributor Author

Yes. Please.

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 a pull request may close this issue.

3 participants