[FIX #1006] destroy/libvirt: cannot destroy domain if it is already shutoff

[abhinavdahiya]

https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDestroy

Destroy the domain object. The running instance is shutdown if not down already and all resources used by it are given back to the hypervisor.

Calling destroy on a shutoff/shutdown domain returns Requested operation is not valid: domain is not running error.
Therefore, skipping calling destroy when domain is already in those 2 states.

Fixes #1006

[crawford]

/lgtm

[crawford]

/hold

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [abhinavdahiya,crawford]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[abhinavdahiya]

@wking i remember (can't find the issue) where somebody had similar issue and decided to not fix it.
Is this fix okay?
/assign @wking

[wking]

Previous "already shutoff" discussion here. I'm reading through that again now...

[wking]

After re-reading #660, it looks like my previous position was "folks will have to unstick these manually anyway, so don't bother doing anything fancy". But if we can just skip domain.Destroy for shut-down domains and continue on to delete them, that sounds good to me.

With your 6374a90, there's a small race with the potential for:

1. domain.GetState() says the domain is running
2. domain stops running
3. the installer things the domain is running, so it calls domain.Undefine(), which fails because the domain is stopped.

It would be more robust to just attempt the domain.Destroy() and then ignore (with debug-level logs?) any resulting "already close enough for a successful domain.Undefine()" errors. But I don't know how easy it is to classify the returned errors. If it would take searching for substrings, it may be better to go with the race ;).

[abhinavdahiya]

With your 6374a90, there's a small race with the potential for:

1. `domain.GetState()` says the domain is running

2. domain stops running

3. the installer things the domain is running, so it calls `domain.Undefine()`, which fails because the domain is stopped.

atleast it will complete in the next destroy cluster.. :P

[crawford]

/hold cancel
/retest

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.