[from-REQ-477] allow SMAPIv3 VDI snapshots to run on hosts other than the master #3466
Conversation
ocaml/xapi/storage_utils.ml
Outdated
| let result = f call in | ||
| if result.Rpc.success then result | ||
| else begin | ||
| debug "Got failure: checking for redirect"; |
There was a problem hiding this comment.
This spreads log messages that cannot be identified individually over several log lines which might no longer be next to each other in the log. Log lines should be self-contained.
edwintorok
changed the title
|
I've collapsed some of the mutliple debug calls to a single debug call, and repeated the rpc on some of the other debug messages. Not sure what the best approach would be here, lets keep this in mind as a use-case for the logging best practices seminar. |
|
Please squash the fixup commits and we will merge it. |
edwintorok
force-pushed
the
CP-26970-master-smapiv3
branch
from
9687161 to
34b83a4
Compare
| [ "del"; Db.VDI.get_uuid ~__context ~self:vdi ]) | ||
| [ "detach"; vdi_uuid ]); | ||
| ignore(Helpers.call_script !Xapi_globs.static_vdis | ||
| [ "del"; vdi_uuid ]) |
There was a problem hiding this comment.
What happens if any of these raises an exception? Is it fine to propagate it without performing del for example?
There was a problem hiding this comment.
I pushed a change to use log_and_ignore_exn, that should preserve the original behaviour wrt to exceptions.
According to the comment the VDI might have already been destroyed, and then detach can fail.
ocaml/xapi/static_vdis.ml
Outdated
| warn "Ignoring exception calling SM vdi_detach for VDI uuid %s: %s (possibly VDI has been deleted while we were offline" uuid (ExnHelper.string_of_exn e) | ||
| end; | ||
| (* This might fail because the VDI has been destroyed *) | ||
| ignore(Helpers.call_script !Xapi_globs.static_vdis [ "detach"; uuid ]); |
There was a problem hiding this comment.
As above, the previous version ignored the erros, this one would fail and reraise before reaching "del". Is that ok?
There was a problem hiding this comment.
Right, I think log_and_ignore_exn might be better here.
|
I think the new version preserving the old behaviour is safer. Please, suqash the commits before merging |
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
…PIv3
After deactivating HA, tapdisk for the statefile-vdi isn't stopped
and the following error is seen xensource.log:
Ignoring exception calling SM vdi_deactivate for VDI uuid
<UUID>: INTERNAL_ERROR: [ Sm.Unknown_driver("gfs2") ]
(possibly VDI has been deleted while we were offline)
To fix this, adopt the same mechanism as in permanent_vdi_attach - use
Xapi_globs.static_vdis instead of Sm.call_sm_vdi_functions in order to
ensure compatibility with both the legacy SM and SMAPIv3.
permanent_vdi_deactivate_by_uuid has the same problem. However, we
don't need to fix this now, as detach calls deactivate implicitly.
Arguably permanent_vdi_deactivate_by_uuid should be removed on some
point, as it's now obsolete. See CA-274332.
Signed-off-by: Robert Breker <robert.breker@citrix.com>
But only if the SM backends asks us to do it. The SM backend redirects based on host UUID, lookup the UUID here and transform it to an IP address, it should be a host from the same pool. If this gets called during storage migration it should still work as long as the VM was activate on a host in this same pool, but that is untested. Signed-off-by: Edwin Török <edvin.torok@citrix.com>
edwintorok
force-pushed
the
CP-26970-master-smapiv3
branch
from
5cef35a to
df89e10
Compare
All applied cleanly on master.