Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

CA-96541/SCTX-1188: Fix race condition when creating metrics for VLAN PIFs #1083

Merged
merged 1 commit into from

2 participants

@robhoes
Owner

This fixes a race condition between the thread that copies bonds and VLANs from
the master when a new slave joins the pool, and the thread that creates/updates
the PIF_metrics (the "monitor thread"). Or in fact a race between anything that
creates a bond and very soon after that a VLAN on top of it.

Since XCP1.6/XS6.1, VLANs no longer have their own PIF_metrics. Rather, they point
to the PIF_metrics of the underlying PIF, which can be either a physical PIF
(e.g. for eth0), or a bond PIF. Therefore, the carrier of a VLAN PIF is the same
as the carrier of the underlying PIF.

When a physical PIF or bond PIF is created, it does not immediately have
PIF_metrics (PIF.metrics = "OpaqueRef:NULL"). The PIF_metrics object is created
and assigned to the PIF by the monitor thread a short while after that. If you
create a VLAN before the PIF_metrics have been created, the PIF.metrics field from
the underlying PIF will still be copied, which is "OpaqueRef:NULL". This will never
get updated, because the monitor thread ignores VLANs.

The fix is simply to immediately create an (empty) metrics object when creating a
bond.

Signed-off-by: Rob Hoes rob.hoes@citrix.com

@robhoes robhoes CA-96541/SCTX-1188: Fix race condition when creating metrics for VLAN…
… PIFs

This fixes a race condition between the thread that copies bonds and VLANs from
the master when a new slave joins the pool, and the thread that creates/updates
the PIF_metrics (the "monitor thread"). Or in fact a race between anything that
creates a bond and very soon after that a VLAN on top of it.

Since XCP1.6/XS6.1, VLANs no longer have their own PIF_metrics. Rather, they point
to the PIF_metrics of the underlying PIF, which can be either a physical PIF
(e.g. for eth0), or a bond PIF. Therefore, the carrier of a VLAN PIF is the same
as the carrier of the underlying PIF.

When a physical PIF or bond PIF is created, it does not immediately have
PIF_metrics (PIF.metrics = "OpaqueRef:NULL"). The PIF_metrics object is created
and assigned to the PIF by the monitor thread a short while after that. If you
create a VLAN before the PIF_metrics have been created, the PIF.metrics field from
the underlying PIF will still be copied, which is "OpaqueRef:NULL". This will never
get updated, because the monitor thread ignores VLANs.

The fix is simply to immediately create an (empty) metrics object when creating a
bond.

Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
9688583
@jonludlam
Owner

Looks good to me!

@jonludlam jonludlam merged commit 5ef268b into xapi-project:master
@robhoes robhoes deleted the robhoes:ca96541 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 15, 2013
  1. @robhoes

    CA-96541/SCTX-1188: Fix race condition when creating metrics for VLAN…

    robhoes authored
    … PIFs
    
    This fixes a race condition between the thread that copies bonds and VLANs from
    the master when a new slave joins the pool, and the thread that creates/updates
    the PIF_metrics (the "monitor thread"). Or in fact a race between anything that
    creates a bond and very soon after that a VLAN on top of it.
    
    Since XCP1.6/XS6.1, VLANs no longer have their own PIF_metrics. Rather, they point
    to the PIF_metrics of the underlying PIF, which can be either a physical PIF
    (e.g. for eth0), or a bond PIF. Therefore, the carrier of a VLAN PIF is the same
    as the carrier of the underlying PIF.
    
    When a physical PIF or bond PIF is created, it does not immediately have
    PIF_metrics (PIF.metrics = "OpaqueRef:NULL"). The PIF_metrics object is created
    and assigned to the PIF by the monitor thread a short while after that. If you
    create a VLAN before the PIF_metrics have been created, the PIF.metrics field from
    the underlying PIF will still be copied, which is "OpaqueRef:NULL". This will never
    get updated, because the monitor thread ignores VLANs.
    
    The fix is simply to immediately create an (empty) metrics object when creating a
    bond.
    
    Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 1 deletion.
  1. +2 −1  ocaml/xapi/xapi_bond.ml
View
3  ocaml/xapi/xapi_bond.ml
@@ -366,8 +366,9 @@ let create ~__context ~network ~members ~mAC ~mode ~properties =
(* Create master PIF and Bond objects *)
let device = choose_bond_device_name ~__context ~host in
let device_name = device in
+ let metrics = Xapi_pif.make_pif_metrics ~__context in
Db.PIF.create ~__context ~ref:master ~uuid:(Uuid.to_string (Uuid.make_uuid ()))
- ~device ~device_name ~network ~host ~mAC ~mTU:(-1L) ~vLAN:(-1L) ~metrics:Ref.null
+ ~device ~device_name ~network ~host ~mAC ~mTU:(-1L) ~vLAN:(-1L) ~metrics
~physical:false ~currently_attached:false
~ip_configuration_mode:`None ~iP:"" ~netmask:"" ~gateway:"" ~dNS:"" ~bond_slave_of:Ref.null
~vLAN_master_of:Ref.null ~management:false ~other_config:[] ~disallow_unplug:false
Something went wrong with that request. Please try again.