Skip to content

Added disk selection and exclusion of deactivated disks - #87

Open
GMasta2000 wants to merge 2 commits into
wefixit-AT:masterfrom
GMasta2000:oVirt4.4-FilterDisks
Open

Added disk selection and exclusion of deactivated disks#87
GMasta2000 wants to merge 2 commits into
wefixit-AT:masterfrom
GMasta2000:oVirt4.4-FilterDisks

Conversation

@GMasta2000

Copy link
Copy Markdown

I have transferred the changes that I made in 4.3 for disk filtering and additionally added default values for these parameters in config.py.

@kobihk kobihk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GMasta2000 Thanks for creating this PR for the master branch
to fix issue #86

Comment thread backup.py Outdated

# Get the Attachments Disk
disk_attachments = vms_service.vm_service(vm.id).disk_attachments_service().list()
disksBackup = []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please use the python convention
https://peps.python.org/pep-0008/#function-and-variable-names
like:

Suggested change
disksBackup = []
disks_backup = []

Comment thread backup.py Outdated
# Get the Attachments Disk
disk_attachments = vms_service.vm_service(vm.id).disk_attachments_service().list()
disksBackup = []
BootableOnly = config.get_bootable_only()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here ^^

Suggested change
BootableOnly = config.get_bootable_only()
bootable_only = config.get_bootable_only()

Comment thread backup.py Outdated
Comment on lines +378 to +380
# print disk_attachment.id
# break
# print disks_id[0].__dict__

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if this code is not necessary maybe you can remove it

Suggested change
# print disk_attachment.id
# break
# print disks_id[0].__dict__

Comment thread backup.py Outdated

for disk_attachment in disksBackup:
if disk_attachment.bootable == True:
logger.info("Finding bootable disk: %s", disk_attachment.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe it will be better to use the new python3 format like:

Suggested change
logger.info("Finding bootable disk: %s", disk_attachment.id)
logger.info(f"Finding bootable disk: {disk_attachment.id}")

IMHO it looks better and more readable.

Comment thread backup.py Outdated
if disk_attachment.bootable == True:
logger.info("Finding bootable disk: %s", disk_attachment.id)
else:
logger.info("Finding disk: %s", disk_attachment.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same ^^

Suggested change
logger.info("Finding disk: %s", disk_attachment.id)
logger.info(f"Finding disk: {disk_attachment.id}")

Comment thread config_example.cfg Outdated
# Backup deactivated disks, set to False to skip deactivated disks backup (Recommend).
# This True - causes an error deleting a snapshot of the VM when the state is on.
with_disks_deactivated=False
# an array of exclude disks id,exampel ["6d811aa25-52a1-45c3-9f40-1c7c868182c9","6e78d7c2-5786-4dz2-a966-fd308c23f08e"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo

Suggested change
# an array of exclude disks id,exampel ["6d811aa25-52a1-45c3-9f40-1c7c868182c9","6e78d7c2-5786-4dz2-a966-fd308c23f08e"]
# an array of exclude disks id, example ["6d811aa25-52a1-45c3-9f40-1c7c868182c9","6e78d7c2-5786-4dz2-a966-fd308c23f08e"]

@GMasta2000

Copy link
Copy Markdown
Author

@kobihk Thanks, I changed the variables.

@kobihk kobihk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @GMasta2000

@kobihk

kobihk commented Sep 19, 2022

Copy link
Copy Markdown
Contributor

Hi @wefixit-AT
Could you please CR this PR?
Thanks in advance,
Kobi

@kobihk

kobihk commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

Hi @wefixit-AT
Could you please take a look at this PR?
Thanks in advance,
Kobi

@kobihk

kobihk commented Oct 30, 2022

Copy link
Copy Markdown
Contributor

Hi @wefixit-AT ^^???

richard-tns added a commit to richard-tns/oVirtBackup that referenced this pull request Dec 15, 2022
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