Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

CLEARWATER: CA-96541/SCTX-1188: Fix race condition when creating metrics for VLAN PI... #1084

Merged
merged 1 commit into from

2 participants

Rob Hoes Malcolm Crossley
Rob Hoes
Owner

...Fs

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.

Duplicate of #1083

Rob Hoes 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>
140635d
Malcolm Crossley malcolmcrossley merged commit baf28d0 into from
Rob Hoes robhoes deleted the 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. Rob Hoes

    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
3  ocaml/xapi/xapi_bond.ml
View
@@ -352,8 +352,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.