Merge lp:~use/nova/shared-storage into lp:~hudson-openstack/nova/trunk

Proposed by USE Team
Status: Needs review
Proposed branch: lp:~use/nova/shared-storage
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 176 lines (+88/-7)
3 files modified
Authors (+1/-0)
nova/virt/xenapi/vm_utils.py (+41/-0)
plugins/xenserver/xenapi/etc/xapi.d/plugins/glance (+46/-7)
To merge this branch: bzr merge lp:~use/nova/shared-storage
Reviewer Review Type Date Requested Status
Matt Dietz (community) Needs Fixing
Thierry Carrez (community) ffe Abstain
Jason Kölker (community) Approve
Christopher MacGown Pending
Review via email: mp+73466@code.launchpad.net

This proposal supersedes a proposal from 2011-08-29.

Description of the change

The current openstack+glance+xenserver environment is not (HBA) shared storage friendly. We can get around this for Linux by (mis-)labelling the HBA LUN local-storage. Unfortunately this does not work for Windows images as they are uploaded in VHD format and XenServer expects them to be VDIs.

This adds a parameter 'staging_path', to the upload_vhd xenapi call which causes the VHD to be loaded into an NFS share that has been labelled 'other-config:i18n-key=scratchnfs_storage', and uses the internal VDI.copy to transfer it to the HBA LUN labelled as local-storage.

This functionality leaves everything else the same, unless there is a share with the scratchnfs_storage label.

Update: Added "Jeff Williams <email address hidden>"
Update: Remove Copyright Headers for Infospace

To post a comment you must log in.
Revision history for this message
Christopher MacGown (0x44) wrote : Posted in a previous version of this proposal

LGTM, but you need to add yourself to AUTHORS.

review: Needs Fixing
Revision history for this message
Jason Kölker (jason-koelker) wrote : Posted in a previous version of this proposal

Is Bueno code wise.

IANAL, but I think your company needs to sign the Corporate CLA if it wishes to claim copyright on the contribution before it can hit trunk.

review: Needs Fixing
Revision history for this message
Thierry Carrez (ttx) wrote :

Also this can't land post-D4 -- please wait for Essex opening (September 8) before landing this.

review: Disapprove (ffe)
Revision history for this message
Jason Kölker (jason-koelker) :
review: Approve
Revision history for this message
Thierry Carrez (ttx) wrote :

Essex is open !

review: Abstain (ffe)
Revision history for this message
Matt Dietz (cerberus) wrote :

Implementation seems sensible

Few things:

103 + vdi_uuid = os.path.splitext(vdi_file)[0]
104 + new_path = os.path.join(staging_sr_path, vdi_file)

Weird indentation on 104

163 + if staging_sr_path == sr_path:
164 + staging_path = _make_staging_area(sr_path)
165 + else:
166 + staging_path = _make_staging_area(staging_sr_path)

Ditto

Looks like you're using tabs in place of spaces?

review: Needs Fixing

Unmerged revisions

1494. By USE Team

Removed Infospace Copyright

1493. By USE Team

Added myself to Authors file

1492. By USE Team

Cleaned up variable names

1491. By USE Team

Provide option to load Windows VHD into a shared storage LUN for a XenServer cluster.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Authors'
2--- Authors 2011-08-26 02:18:46 +0000
3+++ Authors 2011-08-30 23:06:24 +0000
4@@ -59,6 +59,7 @@
5 Joshua McKenty <jmckenty@gmail.com>
6 Justin Santa Barbara <justin@fathomdb.com>
7 Justin Shepherd <jshepher@rackspace.com>
8+Jeff Williams <use@infospace.com>
9 Kei Masumoto <masumotok@nttdata.co.jp>
10 masumoto<masumotok@nttdata.co.jp>
11 Ken Pepple <ken.pepple@gmail.com>
12
13=== modified file 'nova/virt/xenapi/vm_utils.py'
14--- nova/virt/xenapi/vm_utils.py 2011-08-23 05:17:51 +0000
15+++ nova/virt/xenapi/vm_utils.py 2011-08-30 23:06:24 +0000
16@@ -372,6 +372,18 @@
17 return os.path.join(FLAGS.xenapi_sr_base_path, sr_uuid)
18
19 @classmethod
20+ def get_staging_path(cls, session):
21+ """Return the path to our storage repository
22+
23+ This is used when we're dealing with VHDs directly, either by taking
24+ snapshots or by restoring an image in the DISK_VHD format.
25+ """
26+ sr_ref = safe_find_scratchnfs_sr(session)
27+ sr_rec = session.get_xenapi().SR.get_record(sr_ref)
28+ sr_uuid = sr_rec["uuid"]
29+ return os.path.join(FLAGS.xenapi_sr_base_path, sr_uuid)
30+
31+ @classmethod
32 def upload_image(cls, context, session, instance, vdi_uuids, image_id):
33 """ Requests that the Glance plugin bundle the specified VDIs and
34 push them into Glance using the specified human-friendly name.
35@@ -454,6 +466,7 @@
36 'glance_port': glance_port,
37 'uuid_stack': uuid_stack,
38 'sr_path': cls.get_sr_path(session),
39+ 'staging_path': cls.get_staging_path(session),
40 'auth_token': getattr(context, 'auth_token', None)}
41
42 kwargs = {'params': pickle.dumps(params)}
43@@ -950,6 +963,34 @@
44 return None
45
46
47+def safe_find_scratchnfs_sr(session):
48+ """Same as find_sr except raises a NotFound exception if SR cannot be
49+ determined
50+ """
51+ sr_ref = find_scratchnfs_sr(session)
52+ if sr_ref is None:
53+ sr_ref = find_sr(session)
54+ if sr_ref is None:
55+ raise exception.StorageRepositoryNotFound()
56+ return sr_ref
57+
58+
59+def find_scratchnfs_sr(session):
60+ """Return the storage repository to hold VM images"""
61+ host = session.get_xenapi_host()
62+ sr_refs = session.get_xenapi().SR.get_all()
63+ for sr_ref in sr_refs:
64+ sr_rec = session.get_xenapi().SR.get_record(sr_ref)
65+ if not ('i18n-key' in sr_rec['other_config'] and
66+ sr_rec['other_config']['i18n-key'] == 'scratchnfs-storage'):
67+ continue
68+ for pbd_ref in sr_rec['PBDs']:
69+ pbd_rec = session.get_xenapi().PBD.get_record(pbd_ref)
70+ if pbd_rec['host'] == host:
71+ return sr_ref
72+ return None
73+
74+
75 def safe_find_iso_sr(session):
76 """Same as find_iso_sr except raises a NotFound exception if SR cannot be
77 determined
78
79=== modified file 'plugins/xenserver/xenapi/etc/xapi.d/plugins/glance'
80--- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance 2011-08-05 00:12:19 +0000
81+++ plugins/xenserver/xenapi/etc/xapi.d/plugins/glance 2011-08-30 23:06:24 +0000
82@@ -96,7 +96,7 @@
83 conn.close()
84
85
86-def _import_vhds(sr_path, staging_path, uuid_stack):
87+def _import_vhds(session, sr_path, staging_sr_path, staging_path, uuid_stack):
88 """Import the VHDs found in the staging path.
89
90 We cannot extract VHDs directly into the SR since they don't yet have
91@@ -158,6 +158,35 @@
92 os.rename(orig_path, new_path)
93 return new_path
94
95+ def move_into_lvmsr(orig_path):
96+ """
97+ This will move a VHD from a local or NFS SR labeled with scratchnfs-storage to a shared
98+ HBA LUN, using the built in VDI.copy functionality.
99+ """
100+
101+ """ Move from our tar extraction tmp directory, into the SR root directory """
102+ vdi_file = os.path.basename(orig_path)
103+ vdi_uuid = os.path.splitext(vdi_file)[0]
104+ new_path = os.path.join(staging_sr_path, vdi_file)
105+ os.rename(orig_path, new_path)
106+
107+ """ Force rescan to find new VHD """
108+ vhd_sr_uuid = os.path.basename(staging_sr_path)
109+ sr_ref = session.xenapi.SR.get_by_uuid(vhd_sr_uuid)
110+ session.xenapi.SR.scan(sr_ref)
111+
112+ """ Convert VHD on scratchnfs to VDI on shared LUN """
113+ dest_sr_uuid = os.path.basename(sr_path)
114+ dest_sr_ref = session.xenapi.SR.get_by_uuid(dest_sr_uuid)
115+ src_vdi_ref = session.xenapi.VDI.get_by_uuid(vdi_uuid)
116+ dest_vdi_ref = session.xenapi.VDI.copy(src_vdi_ref, dest_sr_ref)
117+
118+ """ Cleanup """
119+ os.remove(new_path);
120+ session.xenapi.SR.scan(sr_ref)
121+
122+ return session.xenapi.VDI.get_uuid(dest_vdi_ref)
123+
124 def assert_vhd_not_hidden(path):
125 """
126 This is a sanity check on the image; if a snap.vhd isn't
127@@ -214,10 +243,12 @@
128 paths_to_move.append(snap_info[0])
129 # We return this snap as the VDI instead of image.vhd
130 vdi_return_list.append(dict(vdi_type="os", vdi_uuid=snap_info[1]))
131+
132 else:
133- assert_vhd_not_hidden(image_info[0])
134- # If there's no snap, we return the image.vhd UUID
135- vdi_return_list.append(dict(vdi_type="os", vdi_uuid=image_info[1]))
136+ if sr_path == staging_sr_path:
137+ assert_vhd_not_hidden(image_info[0])
138+ # If there's no snap, we return the image.vhd UUID
139+ vdi_return_list.append(dict(vdi_type="os", vdi_uuid=image_info[1]))
140
141 swap_info = prepare_if_exists(staging_path, 'swap.vhd')
142 if swap_info:
143@@ -226,7 +257,10 @@
144 vdi_return_list.append(dict(vdi_type="swap", vdi_uuid=swap_info[1]))
145
146 for path in paths_to_move:
147- move_into_sr(path)
148+ if sr_path != staging_sr_path:
149+ vdi_return_list.append(dict(vdi_type="os", vdi_uuid=move_into_lvmsr(path)))
150+ else:
151+ move_into_sr(path)
152
153 return vdi_return_list
154
155@@ -392,15 +426,20 @@
156 glance_port = params["glance_port"]
157 uuid_stack = params["uuid_stack"]
158 sr_path = params["sr_path"]
159+ staging_sr_path = params["staging_path"]
160 auth_token = params["auth_token"]
161
162- staging_path = _make_staging_area(sr_path)
163+ if staging_sr_path == sr_path:
164+ staging_path = _make_staging_area(sr_path)
165+ else:
166+ staging_path = _make_staging_area(staging_sr_path)
167+
168 try:
169 _download_tarball(sr_path, staging_path, image_id, glance_host,
170 glance_port, auth_token)
171 # Right now, it's easier to return a single string via XenAPI,
172 # so we'll json encode the list of VHDs.
173- return json.dumps(_import_vhds(sr_path, staging_path, uuid_stack))
174+ return json.dumps(_import_vhds(session, sr_path, staging_sr_path, staging_path, uuid_stack))
175 finally:
176 _cleanup_staging_area(staging_path)
177