CP-4498: Added relaxed check mode to allow iSCSI SRs during XSM #1116

Merged
merged 1 commit into from Apr 23, 2013

Projects

None yet

4 participants

Contributor
BobBall commented Mar 25, 2013

Signed-off-by: Bob Ball bob.ball@citrix.com

Owner
xen-git commented Mar 25, 2013

Can one of the admins verify this patch?

Collaborator
djs55 commented Mar 26, 2013

Ok to test

On Monday, March 25, 2013, xen-git wrote:

Can one of the admins verify this patch?


Reply to this email directly or view it on GitHubhttps://github.com/xen-org/xen-api/pull/1116#issuecomment-15408344
.

Dave Scott

@jonludlam jonludlam and 1 other commented on an outdated diff Mar 26, 2013
ocaml/xapi/xapi_vm_migrate.ml
@@ -274,11 +274,30 @@ let migrate_send' ~__context ~vm ~dest ~live ~vdi_map ~vif_map ~options =
let vdi_uuid = Db.VDI.get_uuid ~__context ~self:vdi in
failwith ("No SR specified in VDI map for VDI " ^ vdi_uuid)
in
- let mirror = (not is_intra_pool) || (dest_sr <> sr) in
+
+ (* Check if the VDI uuid already exists in the target SR *)
+ let vdi_uuid = Db.VDI.get_uuid ~__context ~self:vdi in
+ let dest_vdi_exists =
+ try
+ let dest_vdi_ref = XenAPI.VDI.get_by_uuid remote_rpc session_id vdi_uuid in
+ let dest_vdi_sr_ref = XenAPI.VDI.get_SR remote_rpc session_id dest_vdi_ref in
+ if dest_vdi_sr_ref = dest_sr_ref then
jonludlam
jonludlam Mar 26, 2013 Owner

Do you want to be testing to see whether the SR references are the same or the uuids? I would have thought uuid would be better in that I can see a way to ensuring this is the same between pools, whereas I can't see how the references would ever be the same.

BobBall
BobBall Mar 26, 2013 Contributor

It's checking that the VDI requested exists on the SR that we've told XSM we want to migrate to.

dest_sr_ref is provided to the migrate_send function and must be the remote ref (personally I think this really should have been the UUID to remove uncertainties) - dest_vdi_sr_ref is therefore the SR which the VDI already exists on, and the check is that the target VDI is on the correct SR on the remote end.

BobBall
BobBall Mar 26, 2013 Contributor

Note that the sr uuid is also checked at line 292

jonludlam
jonludlam Apr 2, 2013 Owner

Gotcha, thanks.

@jonludlam jonludlam was assigned Mar 26, 2013
Owner
xen-git commented Mar 27, 2013

Can one of the admins verify this patch?

Owner

ok to test

Owner

Can we put in a strong warning or even error if we believe that the SR uuids match but the VDI doesn't exist on the other side? That's a situation we ought to be really worried about :-)

Collaborator
djs55 commented Apr 2, 2013

If the VDI doesn't exist we could do an SR.scan, since xapi's view might
just be out of date. If its still not there then something very bad is
happening (eg old mirror?)

On Tuesday, April 2, 2013, Jon Ludlam wrote:

Can we put in a strong warning or even error if we believe that the SR
uuids match but the VDI doesn't exist on the other side? That's a situation
we ought to be really worried about :-)


Reply to this email directly or view it on GitHubhttps://github.com/xen-org/xen-api/pull/1116#issuecomment-15768456
.

Dave Scott

Contributor
BobBall commented Apr 2, 2013

Added an error and an SR scan. Because these are both exceptional cases I'm not sure how to test them - so I'm hoping a visual inspection will suffice.

Owner

OK, looks good now!

Contributor
BobBall commented Apr 23, 2013

So what needs to happen next for it to be merged?

Owner

Me to click the button :-) we had a minor freeze last week for a Linux 3.x change to go in, but we've thawed now. I'll merge now.

@jonludlam jonludlam merged commit 92b1d56 into xapi-project:master Apr 23, 2013

1 check passed

default Merged build finished.
Details
Contributor
BobBall commented Apr 23, 2013

Thanks Jon!

@BobBall BobBall deleted the BobBall:cp-4498 branch Apr 23, 2013
@n0ano n0ano pushed a commit to n0ano/gantt that referenced this pull request Dec 11, 2013
@BobBall @openstack-gerrit BobBall + openstack-gerrit Enable live block migration when using iSCSI volumes
Fix for bug 1160323.

DocImpact
Depends on a version of XAPI supporting relax-xsm-sr-check.
Currently no release versions support this, so the change at
xapi-project/xen-api#1116 would need to be applied
to the source to enable this.  XenServer 6.2 is expected to
support this mode, and possibly some future hotfixes for XenServer 6.1.

It also depends on a configuration change which must be documented and added to devstack:
* Set "relax-xsm-sr-check = true" in /etc/xapi.conf

This commit makes the following changes:
* Attach the SR on the destination host prior to migrate
* Returns the SR-ref from the pre_migrate call (API change)
* Populates the XS VDI map with the sr-ref on the destination host
* Removes the connection to the SR from the source host post-migrate
* Added plugin to determine if iSCSI block migration is enabled

Associated tempest test at https://review.openstack.org/#/c/26478/

Change-Id: I917d9cf094190d636f4b9e14f6c8e728ff85af0e
Fixes: bug 1160323
cc7d980
@n0ano n0ano pushed a commit to n0ano/ganttclient that referenced this pull request Dec 13, 2013
@BobBall @openstack-gerrit BobBall + openstack-gerrit Enable live block migration when using iSCSI volumes
Fix for bug 1160323.

DocImpact
Depends on a version of XAPI supporting relax-xsm-sr-check.
Currently no release versions support this, so the change at
xapi-project/xen-api#1116 would need to be applied
to the source to enable this.  XenServer 6.2 is expected to
support this mode, and possibly some future hotfixes for XenServer 6.1.

It also depends on a configuration change which must be documented and added to devstack:
* Set "relax-xsm-sr-check = true" in /etc/xapi.conf

This commit makes the following changes:
* Attach the SR on the destination host prior to migrate
* Returns the SR-ref from the pre_migrate call (API change)
* Populates the XS VDI map with the sr-ref on the destination host
* Removes the connection to the SR from the source host post-migrate
* Added plugin to determine if iSCSI block migration is enabled

Associated tempest test at https://review.openstack.org/#/c/26478/

Change-Id: I917d9cf094190d636f4b9e14f6c8e728ff85af0e
Fixes: bug 1160323
ee05c6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment