CA-402921: Relax VIF constraint for PVS proxy #6189
Conversation
The current constraint is that the VIF used for PVS proxy must have device number 0. It turned out that this can be relaxed. It is sufficient to enforce that the VIF is the one with the lowest device number for the VM. Signed-off-by: Rob Hoes <[email protected]>
Signed-off-by: Rob Hoes <[email protected]>
ocaml/xapi/xapi_vif_helpers.ml
Outdated
| let vm_has_pvs_proxy = | ||
| List.exists | ||
| (fun vif -> | ||
| Pvs_proxy_control.find_proxy_for_vif ~__context ~vif <> None | ||
| ) | ||
| all | ||
| in |
There was a problem hiding this comment.
Is it not enough to check that just the min_device's VIF has a PVS proxy - since otherwise this code wouldn't let a VIF get created? Perhaps the fold_left above would return vif * vif_device instead?
There was a problem hiding this comment.
You're right, I can simplify this.
There was a problem hiding this comment.
Also, this compares strings rather than integers, so have to fix that.
| let vm = Db.VIF.get_VM ~__context ~self:vIF in | ||
| Db.VIF.get_records_where ~__context | ||
| ~expr:(Eq (Field "VM", Literal (Ref.string_of vm))) | ||
| |> List.fold_left |
There was a problem hiding this comment.
It might be clearer to define a function minimum that takes a non-empty list of integers.
There was a problem hiding this comment.
Then we'd have to separately handle the conversion from strings and the empy list case?
There was a problem hiding this comment.
val minimum: min:int -> int list -> int (** minimum of the list or min if the list is empty *)
...
client code like
| hd::tl -> minimum min:hd tl
..
Maybe it's nice but verbose.
There was a problem hiding this comment.
IMO the current fold_left is more to the point.
When creating a new VIF and there is already a VIF with PVS_proxy, check that the new VIF does not have a lower device number than the PVS_proxy VIF. Signed-off-by: Rob Hoes <[email protected]>
Signed-off-by: Rob Hoes <[email protected]>
The current constraint is that the VIF used for PVS proxy must have
device number 0. It turned out that this can be relaxed. It is
sufficient to enforce that the VIF is the one with the lowest device
number for the VM.