From 07fc13c2fc2c4e5098f70be5fe659226e844f4b6 Mon Sep 17 00:00:00 2001 From: Siddharth Vinothkumar Date: Tue, 30 Oct 2012 12:58:29 +0000 Subject: [PATCH] CA-90857: VIF Port Locking Persistence Issue: While importing a VM through XenCenter, the VIF imported is destroyed and recreated. The locking-mode field for VIF was of type DynamicRO and the VIF recreated by XenCenter always used to set it to network default. Solution: The locking-mode field for VIF was changed to type StaticRO, the constructor signature modified and then the vif record was constructed locally to preserve this field. Signed-off-by: Siddharth Vinothkumar --- ocaml/idl/datamodel.ml | 6 +++--- ocaml/multipathrt/iscsi_utils.ml | 2 +- ocaml/perftest/createVM.ml | 6 +++--- ocaml/perftest/createpool.ml | 2 +- ocaml/xapi/cancel_tests.ml | 2 +- ocaml/xapi/cli_operations.ml | 2 +- ocaml/xapi/cli_operations_geneva.ml | 4 ++-- ocaml/xapi/debug_populate.ml | 2 +- ocaml/xapi/import.ml | 23 +++++++++++++---------- ocaml/xapi/quicktest.ml | 4 ++-- ocaml/xapi/xapi_vif.ml | 9 ++------- ocaml/xapi/xapi_vif.mli | 5 ++++- ocaml/xapi/xapi_vif_helpers.ml | 7 +++++++ ocaml/xapi/xapi_vif_helpers.mli | 3 +++ 14 files changed, 44 insertions(+), 33 deletions(-) diff --git a/ocaml/idl/datamodel.ml b/ocaml/idl/datamodel.ml index 20c3d2dc83..be4bf25654 100644 --- a/ocaml/idl/datamodel.ml +++ b/ocaml/idl/datamodel.ml @@ -4954,9 +4954,9 @@ let vif = [ namespace ~name:"qos" ~contents:(qos "VIF") (); ] @ [ field ~qualifier:DynamicRO ~ty:(Ref _vif_metrics) "metrics" "metrics associated with this VIF"; field ~qualifier:DynamicRO ~in_product_since:rel_george ~default_value:(Some (VBool false)) ~ty:Bool "MAC_autogenerated" "true if the MAC was autogenerated; false indicates it was set manually"; - field ~qualifier:DynamicRO ~in_product_since:rel_tampa ~default_value:(Some (VEnum "network_default")) ~ty:vif_locking_mode "locking_mode" "current locking mode of the VIF"; - field ~qualifier:DynamicRO ~in_product_since:rel_tampa ~default_value:(Some (VSet [])) ~ty:(Set (String)) "ipv4_allowed" "A list of IPv4 addresses which can be used to filter traffic passing through this VIF"; - field ~qualifier:DynamicRO ~in_product_since:rel_tampa ~default_value:(Some (VSet [])) ~ty:(Set (String)) "ipv6_allowed" "A list of IPv6 addresses which can be used to filter traffic passing through this VIF"; + field ~qualifier:StaticRO ~in_product_since:rel_tampa ~default_value:(Some (VEnum "network_default")) ~ty:vif_locking_mode "locking_mode" "current locking mode of the VIF"; + field ~qualifier:StaticRO ~in_product_since:rel_tampa ~default_value:(Some (VSet [])) ~ty:(Set (String)) "ipv4_allowed" "A list of IPv4 addresses which can be used to filter traffic passing through this VIF"; + field ~qualifier:StaticRO ~in_product_since:rel_tampa ~default_value:(Some (VSet [])) ~ty:(Set (String)) "ipv6_allowed" "A list of IPv6 addresses which can be used to filter traffic passing through this VIF"; ]) () diff --git a/ocaml/multipathrt/iscsi_utils.ml b/ocaml/multipathrt/iscsi_utils.ml index b931e8871b..b47eb6fdf9 100644 --- a/ocaml/multipathrt/iscsi_utils.ml +++ b/ocaml/multipathrt/iscsi_utils.ml @@ -59,7 +59,7 @@ let make_iscsi rpc session_id iscsi_luns num_vifs sr_disk_size key network = Client.VM.set_HVM_boot_policy rpc session_id newvm ""; for i = 0 to num_vifs - 1 do - ignore (Client.VIF.create rpc session_id (string_of_int i) network newvm "" 1500L [oc_key,key] "" []) + ignore (Client.VIF.create rpc session_id (string_of_int i) network newvm "" 1500L [oc_key,key] "" [] `network_default [] [] ) done; Client.VM.add_to_other_config rpc session_id newvm oc_key key; diff --git a/ocaml/perftest/createVM.ml b/ocaml/perftest/createVM.ml index e32bce8598..b5561d7e6f 100644 --- a/ocaml/perftest/createVM.ml +++ b/ocaml/perftest/createVM.ml @@ -52,7 +52,7 @@ let make_iscsi session_id pool network = Client.VM.set_PV_bootloader rpc session_id newvm "pygrub"; Client.VM.set_PV_args rpc session_id newvm (Printf.sprintf "net_ip=%s net_mask=255.255.255.0" (make_iscsi_ip pool)); Client.VM.set_HVM_boot_policy rpc session_id newvm ""; - let (_ : API.ref_VIF) = Client.VIF.create rpc session_id "0" network newvm "" 1500L [oc_key,pool.key] "" [] in + let (_ : API.ref_VIF) = Client.VIF.create rpc session_id "0" network newvm "" 1500L [oc_key,pool.key] "" [] `network_default [] [] in Client.VM.add_to_other_config rpc session_id newvm oc_key pool.key; let localhost_uuid = Xapi_inventory.lookup "INSTALLATION_UUID" in Client.VM.start_on rpc session_id newvm (Client.Host.get_by_uuid rpc session_id localhost_uuid) false false; @@ -81,8 +81,8 @@ let make ~rpc ~session_id ~pool ~vm ~networks ~storages = done; Client.VM.provision ~rpc ~session_id ~vm:clone; for device = 0 to (min vm.vifs (Array.length networks)) - 1 do - ignore(Client.VIF.create ~rpc ~session_id ~device:(string_of_int device) ~network:networks.(device) - ~vM:clone ~mAC:"" ~mTU:1500L ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[]) + ignore(Client.VIF.create ~rpc ~session_id ~device:(string_of_int device) ~network:networks.(device) ~vM:clone ~mAC:"" + ~mTU:1500L ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[] ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[]) done; Client.VM.set_memory_static_min ~rpc ~session_id ~self:clone ~value:16777216L; Client.VM.set_memory_dynamic_min ~rpc ~session_id ~self:clone ~value:16777216L; diff --git a/ocaml/perftest/createpool.ml b/ocaml/perftest/createpool.ml index 8abf4736d5..01c7083cdc 100644 --- a/ocaml/perftest/createpool.ml +++ b/ocaml/perftest/createpool.ml @@ -58,7 +58,7 @@ let initialise session_id template pool = let interfaces = Array.init pool.interfaces_per_host (fun i -> let net = networks.(get_network_num_from_interface pool i) in Client.VIF.create ~rpc ~session_id ~device:(string_of_int i) ~network:net ~vM:template ~mAC:"" ~mTU:1500L - ~other_config:[oc_key,pool.key] ~qos_algorithm_type:"" ~qos_algorithm_params:[]) + ~other_config:[oc_key,pool.key] ~qos_algorithm_type:"" ~qos_algorithm_params:[] ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[]) in (* Create a disk for local storage *) diff --git a/ocaml/xapi/cancel_tests.ml b/ocaml/xapi/cancel_tests.ml index 114ca30f81..2f0cead070 100644 --- a/ocaml/xapi/cancel_tests.ml +++ b/ocaml/xapi/cancel_tests.ml @@ -105,7 +105,7 @@ let find_or_create_vif { session_id = session_id; vm = vm; net = net } = let vif, _ = List.find (fun (_, r) -> r.API.vIF_device = vif_idx) (List.combine vifs vif_records) in vif with Not_found -> - Client.VIF.create ~rpc ~session_id ~vM:vm ~network:net ~mAC:"" ~device:vif_idx ~mTU:1500L ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[] + Client.VIF.create ~rpc ~session_id ~vM:vm ~network:net ~mAC:"" ~device:vif_idx ~mTU:1500L ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[] ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[] let find_or_create_vbd { session_id = session_id; vm = vm; vdi = vdi } = let rpc = make_rpc () in diff --git a/ocaml/xapi/cli_operations.ml b/ocaml/xapi/cli_operations.ml index 72780455f4..4b40c2f68b 100644 --- a/ocaml/xapi/cli_operations.ml +++ b/ocaml/xapi/cli_operations.ml @@ -1481,7 +1481,7 @@ let vif_create printer rpc session_id params = let vm=Client.VM.get_by_uuid rpc session_id vm_uuid in let network=Client.Network.get_by_uuid rpc session_id network_uuid in let mtu = Client.Network.get_MTU rpc session_id network in - let vif = Client.VIF.create rpc session_id device network vm mac mtu [] "" [] in + let vif = Client.VIF.create rpc session_id device network vm mac mtu [] "" [] `network_default [] [] in let uuid = Client.VIF.get_uuid rpc session_id vif in printer (Cli_printer.PList [uuid]) diff --git a/ocaml/xapi/cli_operations_geneva.ml b/ocaml/xapi/cli_operations_geneva.ml index 97d5afabe8..1c4ef5bd40 100644 --- a/ocaml/xapi/cli_operations_geneva.ml +++ b/ocaml/xapi/cli_operations_geneva.ml @@ -368,7 +368,7 @@ let vm_install fd printer rpc session_id params = let add_vif net = let mac = Record_util.random_mac_local () in marshal fd (Command (Print ("Adding VIF, device "^(string_of_int !device)^" to network '"^(Client.Network.get_name_label rpc session_id net)^"' mac="^mac))); - ignore(Client.VIF.create rpc session_id (string_of_int !device) net new_vm mac 1500L [] "" []); + ignore(Client.VIF.create rpc session_id (string_of_int !device) net new_vm mac 1500L [] "" [] `network_default [] [] ); device := !device + 1 in List.iter add_vif filtered_nets; @@ -481,7 +481,7 @@ let vm_vif_add printer rpc session_id params = | [] -> failwith "Bridge not found" | n::ns -> begin - let vif = Client.VIF.create rpc session_id device n vm mac 1500L [] "" [] in + let vif = Client.VIF.create rpc session_id device n vm mac 1500L [] "" [] `network_default [] [] in if List.mem_assoc "rate" params then (Client.VIF.set_qos_algorithm_type rpc session_id vif "ratelimit"; Client.VIF.add_to_qos_algorithm_params rpc session_id vif "kbs" (List.assoc "rate" params)) diff --git a/ocaml/xapi/debug_populate.ml b/ocaml/xapi/debug_populate.ml index 55915739c1..f72367b6d3 100644 --- a/ocaml/xapi/debug_populate.ml +++ b/ocaml/xapi/debug_populate.ml @@ -81,7 +81,7 @@ let rec make_vifs __context vmref i = else begin ignore(Xapi_vif.create ~__context ~device:(string_of_int i) ~network:(get_random nws) ~vM:vmref - ~mAC:"de:ad:be:ef:99:88" ~mTU:Int64.zero ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[]); + ~mAC:"de:ad:be:ef:99:88" ~mTU:Int64.zero ~other_config:[] ~qos_algorithm_type:"" ~qos_algorithm_params:[] ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[]); make_vifs __context vmref (i-1) end diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml index ef6b0130ac..3ae376667f 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -871,6 +871,19 @@ module VIF : HandlerTools = struct vif_record.API.vIF_other_config in (* Construct the VIF record we're going to try to create locally. *) + let vif_record = if (Pool_features.is_enabled ~__context Features.VIF_locking) then + vif_record + else begin + if vif_record.API.vIF_locking_mode = `locked then + { vif_record with API.vIF_locking_mode = `network_default; + API.vIF_ipv4_allowed = []; + API.vIF_ipv6_allowed = []; + } + else + { vif_record with API.vIF_ipv4_allowed = []; + API.vIF_ipv6_allowed = []; + } + end in let vif_record = { vif_record with API.vIF_VM = vm; API.vIF_network = net; @@ -900,16 +913,6 @@ module VIF : HandlerTools = struct if config.full_restore then Db.VIF.set_uuid ~__context ~self:vif ~value:value.API.vIF_uuid; vif) vif_record in - (* Make a best-effort attempt to persist the port locking fields - this may fail due to licensing. *) - (* The VIF is not currently_attached at this stage, so setup-vif-rules will not be called yet. *) - begin - try - Client.VIF.set_locking_mode ~rpc ~session_id ~self:vif ~value:vif_record.API.vIF_locking_mode; - Client.VIF.set_ipv4_allowed ~rpc ~session_id ~self:vif ~value:vif_record.API.vIF_ipv4_allowed; - Client.VIF.set_ipv6_allowed ~rpc ~session_id ~self:vif ~value:vif_record.API.vIF_ipv6_allowed; - with e -> - debug "Could not persist port locking fields for this VIF - caught %s" (Printexc.to_string e) - end; state.cleanup <- (fun __context rpc session_id -> Client.VIF.destroy rpc session_id vif) :: state.cleanup; (* Now that we can import/export suspended VMs we need to preserve the currently_attached flag *) diff --git a/ocaml/xapi/quicktest.ml b/ocaml/xapi/quicktest.ml index 0c4e16effe..7249238f75 100644 --- a/ocaml/xapi/quicktest.ml +++ b/ocaml/xapi/quicktest.ml @@ -748,12 +748,12 @@ let vm_powercycle_test s vm = (* Try to add some VIFs *) let (guest_installer_network: API.ref_network) = find_guest_installer_network s in debug test (Printf.sprintf "Adding VIF to guest installer network (%s)" (Client.Network.get_uuid !rpc s guest_installer_network)); - let (_: API.ref_VIF) = make_vif ~session_id:s ~vM:vm ~network:guest_installer_network ~device:"0" in + let (_: API.ref_VIF) = make_vif ~session_id:s ~vM:vm ~network:guest_installer_network ~device:"0" ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[] in begin match Client.PIF.get_all !rpc s with | pif :: _ -> let net = Client.PIF.get_network !rpc s pif in debug test (Printf.sprintf "Adding VIF to physical network (%s)" (Client.Network.get_uuid !rpc s net)); - let (_: API.ref_VIF) = make_vif ~session_id:s ~vM:vm ~network:net ~device:"1" in + let (_: API.ref_VIF) = make_vif ~session_id:s ~vM:vm ~network:net ~device:"1" ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[] in () | _ -> () end; diff --git a/ocaml/xapi/xapi_vif.ml b/ocaml/xapi/xapi_vif.ml index 5fd1bb699a..c1d8daed6d 100644 --- a/ocaml/xapi/xapi_vif.ml +++ b/ocaml/xapi/xapi_vif.ml @@ -33,10 +33,9 @@ let unplug_force ~__context ~self = Xapi_xenops.vif_unplug ~__context ~self true let create ~__context ~device ~network ~vM - ~mAC ~mTU ~other_config ~qos_algorithm_type ~qos_algorithm_params : API.ref_VIF = + ~mAC ~mTU ~other_config ~qos_algorithm_type ~qos_algorithm_params ~locking_mode ~ipv4_allowed ~ipv6_allowed : API.ref_VIF = create ~__context ~device ~network ~vM ~currently_attached:false - ~mAC ~mTU ~other_config ~qos_algorithm_type ~qos_algorithm_params - ~locking_mode:`network_default ~ipv4_allowed:[] ~ipv6_allowed:[] + ~mAC ~mTU ~other_config ~qos_algorithm_type ~qos_algorithm_params ~locking_mode ~ipv4_allowed ~ipv6_allowed let destroy ~__context ~self = destroy ~__context ~self @@ -60,10 +59,6 @@ let move ~__context ~network vif = if device_active ~__context ~self:vif then Xapi_xenops.vif_move ~__context ~self:vif network -let assert_locking_licensed ~__context = - if (not (Pool_features.is_enabled ~__context Features.VIF_locking)) then - raise (Api_errors.Server_error(Api_errors.license_restriction, [])) - let change_locking_config ~__context ~self ~licence_check f = if licence_check then assert_locking_licensed ~__context; f (); diff --git a/ocaml/xapi/xapi_vif.mli b/ocaml/xapi/xapi_vif.mli index 086ff20e48..17f74a0104 100644 --- a/ocaml/xapi/xapi_vif.mli +++ b/ocaml/xapi/xapi_vif.mli @@ -50,7 +50,10 @@ val create : mTU:int64 -> other_config:(string * string) list -> qos_algorithm_type:string -> - qos_algorithm_params:(string * string) list -> API.ref_VIF + qos_algorithm_params:(string * string) list -> + locking_mode:API.vif_locking_mode -> + ipv4_allowed:string list -> + ipv6_allowed:string list -> API.ref_VIF (** Destroy the specified VIF instance *) val destroy : __context:Context.t -> self:[ `VIF ] Ref.t -> unit diff --git a/ocaml/xapi/xapi_vif_helpers.ml b/ocaml/xapi/xapi_vif_helpers.ml index fd30a4b85e..43c242593d 100644 --- a/ocaml/xapi/xapi_vif_helpers.ml +++ b/ocaml/xapi/xapi_vif_helpers.ml @@ -144,6 +144,10 @@ let gen_mac(dev, seed) = take_byte 1 mac_data_2; take_byte 2 mac_data_2; |] +let assert_locking_licensed ~__context = + if (not (Pool_features.is_enabled ~__context Features.VIF_locking)) then + raise (Api_errors.Server_error(Api_errors.license_restriction, [])) + let m = Mutex.create () (* prevents duplicate VIFs being created by accident *) let create ~__context ~device ~network ~vM @@ -151,6 +155,9 @@ let create ~__context ~device ~network ~vM ~currently_attached ~locking_mode ~ipv4_allowed ~ipv6_allowed : API.ref_VIF = let () = debug "VIF.create running" in + if locking_mode = `locked || ipv4_allowed <> [] || ipv6_allowed <> [] then + assert_locking_licensed ~__context; + let uuid = Uuid.make_uuid () in let ref = Ref.make () in diff --git a/ocaml/xapi/xapi_vif_helpers.mli b/ocaml/xapi/xapi_vif_helpers.mli index 96e4935b5d..facd0590cf 100644 --- a/ocaml/xapi/xapi_vif_helpers.mli +++ b/ocaml/xapi/xapi_vif_helpers.mli @@ -33,6 +33,9 @@ val cancel_tasks : val clear_current_operations : __context:Context.t -> self:[ `VIF ] Ref.t -> unit +val assert_locking_licensed : + __context:Context.t -> unit + (** Create a VIF object in the database. *) val create : __context:Context.t ->