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

feat(backing encryption): backing image encryption support #251

Merged

Conversation

ChanYiLin
Copy link
Contributor

@ChanYiLin ChanYiLin self-assigned this Jun 14, 2024
Copy link

mergify bot commented Jun 14, 2024

This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏

@ChanYiLin
Copy link
Contributor Author

This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏

Fixed.

@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch from 8634f3f to eb5b2e4 Compare June 20, 2024 08:56
Copy link

mergify bot commented Jun 25, 2024

This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏

@derekbit
Copy link
Member

@ChanYiLin Is it ready for review?

@ChanYiLin
Copy link
Contributor Author

@ChanYiLin Is it ready for review?

Yes it is ready for review.

pkg/sync/handler.go Show resolved Hide resolved
pkg/sync/handler.go Outdated Show resolved Hide resolved
pkg/sync/sync_file.go Outdated Show resolved Hide resolved
pkg/sync/sync_file.go Outdated Show resolved Hide resolved
Comment on lines 612 to 614
if imgInfo.Format == "qcow2" {
tmpRawFile := fmt.Sprintf("%v-raw.tmp", sourceFile)
if err := util.ConvertFromQcow2ToRaw(sourceFile, tmpRawFile); err != nil {
return 0, errors.Wrapf(err, "failed to create raw image from qcow2 image %v", sourceFile)
}
// use the raw image as source when doing encryption
sourceFile = tmpRawFile
defer os.RemoveAll(tmpRawFile)
}
Copy link
Member

Choose a reason for hiding this comment

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

We don't have any progress for the image conversion, right?
Can we create a ticket for exposing the progress?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes
sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wonder how do we get the progress of the conversion

	if _, err := Execute([]string{}, QemuImgBinary, "convert", "-f", "qcow2", "-O", "raw", sourcePath, targetPath); err != nil {
		return err
	}

The conversion is part of the backing image preparation
But the current progress is about 100% data copying
how do we define and integrate the progress of conversion into data copying?

Copy link
Member

Choose a reason for hiding this comment

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

A simple example
0-20%: image conversion
20-80%: encryption or decryption

pkg/sync/service.go Show resolved Hide resolved
pkg/sync/service.go Outdated Show resolved Hide resolved
pkg/sync/service.go Show resolved Hide resolved
pkg/client/sync_client.go Outdated Show resolved Hide resolved
pkg/client/sync_client.go Outdated Show resolved Hide resolved
@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch 4 times, most recently from 3bccfd1 to a3e46ee Compare June 27, 2024 09:15
@ChanYiLin ChanYiLin requested a review from derekbit June 27, 2024 09:43
derekbit
derekbit previously approved these changes Jun 27, 2024
Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/sync/handler.go Outdated Show resolved Hide resolved
pkg/util/util.go Show resolved Hide resolved
pkg/crypto/crypto.go Outdated Show resolved Hide resolved
pkg/sync/sync_file.go Show resolved Hide resolved
pkg/sync/sync_file.go Outdated Show resolved Hide resolved
pkg/sync/sync_file.go Show resolved Hide resolved
shuo-wu
shuo-wu previously approved these changes Jul 1, 2024
Copy link
Contributor

@shuo-wu shuo-wu left a comment

Choose a reason for hiding this comment

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

LGTM

ref: longhorn/longhorn 7051

Signed-off-by: Jack Lin <[email protected]>
@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch from 1b6e8bb to dd3736d Compare July 2, 2024 07:16
@derekbit derekbit merged commit 73da985 into longhorn:master Jul 2, 2024
6 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.

3 participants