Skip to content

external_snapshot(create,list,revert)_draft#75

Draft
Jbchan01 wants to merge 1 commit intomainfrom
external_snap
Draft

external_snapshot(create,list,revert)_draft#75
Jbchan01 wants to merge 1 commit intomainfrom
external_snap

Conversation

@Jbchan01
Copy link
Contributor

외부 스냅샷 복구 시 발생하던 Permission denied 문제가 일단 해결되어

생성, 리스트, 복구가 가능해졌습니다.

  • 안정성도 다소 미흡한 점이 있어서 임시 완료라고 보면 좋습니다.
  • 현재 가독성이 좀 떨어져 코드 리펙토링 등이 추후 진행될 것 같습니다.

if name == "" {
name = param.UUID + "-extsnap"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 param.uuid 는 vm uuid 인가요?
"+extsnap" 이 무슨 의미인지 궁금합니다.
하드 코딩된 스트링이라면 충돌문제가 있을지
아니면 extsnap 포스트픽스가 내부에서 특정 인덱스를 부여하는지?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. param.uuid는 vm uuid가 맞습니다.
  2. 이름이 없으면 string으로 붙여버려서 uuid+extsnap으로 이름이 생성됩니다.

현재 api json 요청형식에 uuid, name 등등의 정보를 받는데. 만약 name이 비어있을 경우, "uuid +extsnap"으로 설정되게 되어있습니다. 다시 보니 name이 없으면 생성 실패로 오류처리가 더 나을 것 같네요.

Copy link
Contributor

Choose a reason for hiding this comment

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

좋습니당
그리고 여러 스냅샷을 만들땨 이름에서 충돌이 날지도 한번 봐주세용

resp.ResponseWriteErr(w, fmt.Errorf("libvirt not initialized"), http.StatusInternalServerError)
i.Logger.Error("libvirt not initialized")
return
}
Copy link
Contributor

@kwonkwonn kwonkwonn Feb 18, 2026

Choose a reason for hiding this comment

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

i.LibvirtInst 가 nil 이라는것은 부팅에서 오류가 있었다는 의미같은데,
이건 프로그램을 종료하는게 맞을 것 같습니다.
추후에 이런 경우에는 어떻게 안전하게 종료하게 할지 합의하면 좋을 듯 합니다

미들웨어로 빼서 매 오퍼레이션에서 검증하거나 시간단위로 해도 괜찮을듯?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libvirt 접근이 반드시 필요한 엔드포인트에 대해서 묶어서 정리를 해보는 것이 좋을 것 같습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

인정합니다..
시간날때 이슈 만들어놓을게요

}

snaps, err := domain.Domain.ListAllSnapshots(0)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

snap, err := domain.Domain.CreateSnapshotXML(snapXML, flags) <--이것처럼 enum의 경우 숫자말고 enumType을 가져다 박는게 좋을거 가타요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

flag type 코드 가독성을 높이기 위해서 변경하는걸까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

네 정확하십니다
그렇지 않다면 다음번에 개발할때 매번 함수 들어가서 인자를 확인해야될 거에여


snapName, err := snap.GetName()
if err != nil {
return "", fmt.Errorf("snapshot created but failed to read name: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt.Erorrf대신 VirError 이나 ErrorDescriptor 를 쓰면 좋을 것 같습니다.

ErrorGen<-- fmt.Errorf 와 동일한 효과
ErrorJoin<-- 아래함수에서 올라온 Erorr 와 현재 깊이에서의 에러를 합성함.

원인 함수에서 ErrorGen을 쓰고, 그 함수를 호출한 함수에서 ErrorJoin을 사용합니다.
해당 함수같으면,
"From CreateExternalSnapshotAPI, failed creating External snapshot : From snapshotExpternalSnapshotservice Error creating snapthot, snapshotCreated but failed to read name"
처럼 원인을 파악한느데 도움이 될 것 같아요

Driver string
DriverName string
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Type 이랑 함수를 분리하는게 어떨까요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

대찬성

Copy link
Contributor

Choose a reason for hiding this comment

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

좋읍니더

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

Comments