CA-300103 Avoid duplicated tools SRs on pool join #3760
Conversation
| try | ||
| Client.SR.get_all_records ~rpc ~session_id | ||
| |> List.find (fun (_, sr) -> sr.API.sR_is_tools_sr) | ||
| |> (fun (ref,_) -> ref) |
There was a problem hiding this comment.
Should we check that the device config is the same and what should happen if it isn't?
What would happen if you start a VM with the tools ISO plugged in, and migrate it to a host which has a newer tools SR? (e.g. during an RPU)
If the contents of the tools ISO is different then I think the SRs should have different UUIDs to avoid accidentally changing the contents of a plugged ISO in a guest.
There was a problem hiding this comment.
This is a good point. Should we scan for VMs that have a tools ISO attached?
There was a problem hiding this comment.
The host that is joining a pool cannot have any running VMs (another pre-join check further up in this file), so this should not be a problem. This function is not related to RPU either.
ocaml/xapi/xapi_pool.ml
Outdated
| Client.SR.get_by_uuid ~rpc ~session_id ~uuid:my_uuid | ||
| with _ -> | ||
| if sr.API.sR_is_tools_sr then | ||
| (* find the tools ISO and return it *) |
ocaml/xapi/xapi_pool.ml
Outdated
| |> List.find (fun (_, sr) -> sr.API.sR_is_tools_sr) | ||
| |> (fun (ref,_) -> ref) | ||
| with Not_found -> | ||
| let msg = Printf.sprintf "can't find SR of tools iso %s" my_uuid in |
ocaml/xapi/dbsync_master.ml
Outdated
| ] in | ||
| let destroy self = | ||
| try Db.SR.destroy ~__context ~self with e -> | ||
| info "failed to destroy bogus tools SR %s: %s" |
| Db.SR.set_is_tools_sr ~__context ~self:sr ~value:true; sr | ||
| | sr :: others -> | ||
| Db.SR.set_is_tools_sr ~__context ~self:sr ~value:true; | ||
| List.iter destroy others; (* destroy bogus Tool SRs CA-300103 *) |
There was a problem hiding this comment.
We want to do this cleanup of the "old" and "bogus" Tools SRs also if we did find a Tools SR (with is_tools_sr = true), so in the | sr :: [] case.
By the way, the commit message is not quite right: this is not executed on pool join, but when the master xapi starts. Pool join is fixed by the previous commit already. This code is meant to fix things up for pools that already have the bogus tools SRs due the bug that the previous commit fixes.
"tools SRs" |
|
I have addressed to comments in a new commit but would merge this into one coherent commit before we merge this. |
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
We need to make sure that only a single Tools SR exists. This commit ensures this by: * use sR_is_tools_sr to find all Tools SR rather than a heuristic * if multiple such SRs exists, destroy all but one and destroy all legacy SRs * if only legacy SRs exists, select one, promote it, and destroy the others * if none exist, create one * make sure that the elected Tool ISO SR has the correct device config - especially when it was imported. * remove update_tools_sr_pbd_device_config from upgrade rules. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
|
It may be worth adding some unit tests to test_dbsync_master.ml. That would give us more confidence that the corner cases are working. |
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
|
I have added two unit tests. |
robhoes
changed the title
4 participants
This is work in progress. On joining a pool, the SR for the Guest Tools ISO is a special case. This series of commits tries to avoid that multiple such SRs are present in the final pool.