Fix #45, #43, #36, #39, #46

[lancefogle]

Incorporates changes from #39 and #46 but actually implements a complete fix for the exclude when using regexp.

The all_vms list was being updated inside the loop that is iterating over that same list. This causes the loop to break which is why even a exclude=.* will not exclude more than one match. The fix was to move the updating of the all_vms array after the loop and only when found_match is true.

This code also removes the all_vms.remove(vm) line completely from the save_to_config_export function which shouldn't be removing anything as it is for adding to the array, not removing.

[NAUbackup]

Thanks much, @lancefogl, I had noticed some exclude issues. Did you find the same for the VDI exclude section or have you not looked at that code?

[lancefogle]

Haven't had a chance yet. I will look at that shortly to see if I can suggest fixes for that as well.

[lancefogle]

@NAUbackup I have added two more fixes to this pull request that fix false positive situations for both the exclude and the vdi-export parms in the config that could lead to matching all VMs if specified but empty.

In regards to checking the code for the vdi-export exclusions, I tested and the vdi-export parm is working as expected (taking precedence over any VMs also matched by vm-export) but there is no (nor has there been) any function to explicitly check for duplicates and remove them during the config save functions.

It wouldn't be hard to add a check there based on the key to force it to just not include the match if it already exists in vdi-export values but it would be reliant on vdi-exports absolutely being listed first in the config so that they apply first. This isn't so bad because you already have a fallback with warning messages if there are duplicates in the config between vm-export and vdi-export with vdi-export winning all ties which makes it safe if someone doesn't follow the config order.

[lancefogle]

@NAUbackup I added a commit to check for duplicate vms that match vm-export but already exist as matches to vdi-export. It will continue through the loop without adding to vm-export key if duplicate so that (so long as the vdi-export entries are loaded into the config first before vm-export) there will be no duplicates or warnings produced. If for some reason they are loaded in the reverse order this won't catch the duplicates but the already existing check during backup will and will throw warnings to let you know.

This makes the vdi-export have multiple functions which I think was the original intent (?):

• Serves as an exclusion list so you don't backup the whole VM if you just want to do disks
• Allows regex matches so you can easily configure overlapping groups with varying specificity between vm-export and vdi-export without duplicates and without erroneous warnings
• You can still specify nothing for vm-export which will backup all VMs unless you have specified some or all of them as vdi-exports in which case it will skip those particular matches for full backups

[NAUbackup]

You got it, man, that covers all the cases! Again, many thanks for all this, Lance!

--Tobias

[lancefogle]

No problem @NAUbackup

See my fix for an edge case I just caught for when all VMs match prior to vm-export being applied that caused an erroneous warning. I also made a cosmetic change to effect how the config output appears making it in alphabetical order instead of reverse.

[NAUbackup]

Great, many thanks, Lance. Really appreciate your time and efforts here! --Tobias

[lancefogle]

No problem at all, Tobias.

If you merge this pull request after you are done testing the changes it will automatically close all the pending issues and PRs that are related to the fixes here for you.

Also, I noticed several issues that are still "open" but have already been fixed in the current version by you a long time ago. If you could go through and close those that will help show myself and other contributors what is still out there requested to be done or fixed more clearly.

If you can do that, I will see how else I can contribute, and maybe even implement some of the new features requested out there for you. This is a very useful tool and I am happy to give back to it as able.

[NAUbackup]

Thanks, Lance. Call it very unconventional, but we don't do merges as such on GitHub but patch the code externally and instead, bundle everything with a new distribution as a standalone package. You're right that several issues have indeed been already addressed and should be closed. I've started on a new distribution, but the start of the semester has us hopping here and currently, I'm the only one with any time at all to spend on this until possible a restructuring later on. We'll see. Anything you're willing to contribute is appreciated and acknowledged within the distribution. Some stuff is actually pretty easy, like adding an option for dom0 backup, for example. Others, like block tracking, not so much, in particular because of keeping tabs on what all is current and the whole history, etc. I'm working with someone from Citrix on improved vm-export mechanisms, as well, so never a dull moment! Cheers, --Tobias

[NAUbackup]

Hi there, Lance:

Am testing your code now with the multiple fixes -- nice job! Found a typo and a spelling issue:

Fail fast if exclude param given but empty to prevent from exluding all VMs

if vm_name = "":
    return

should read: => # Fail fast if exclude param given but empty to prevent from excluding all VMs => if vm_name == "": => return

Thank you for merging a number of the issues. I would like to redistribute soon as V3.2 and so I wanted to ask if you had anything else you wanted to add soon, or should I go ahead and release the patched and updated version rather sooner than later? Cheers, --Tobias

[NAUbackup]

Ignore now deleted note; things are running OK (I had a typo in the cfg file). Looking very good so far. I had an error a while back that seems to have now been taken care of. Still testing... :-)

[lancefogle]

Fixed the mentioned typo and made a change to the handling to more accurately describe whether the config file was specified or not (cosmetic)

[lancefogle]

Feel free to release v3.2 whenever you are done testing. I will submit a later pull request if I have anything additional to add at a later date. I think I have covered most of the bug-fix small stuff that can be remedied easily for now.

[NAUbackup]

Greetings and thanks fo rthe latest feedback.

Couple of quick things:

log('Running with default config)

=> missing single quote.

Also, if vm_name = "":

=> missing additional "=" still seems present.

--Tobias

[lancefogle]

Fixed both

[NAUbackup]

Many thanks!