Skip to content

Commit

Permalink
storage: Be multipath safe.
Browse files Browse the repository at this point in the history
Show the whole picture about multipath devices and don't let people
mess with improperly configured ones.

Fixes cockpit-project#2554
  • Loading branch information
mvollmer committed Aug 19, 2015
1 parent 10820a5 commit 80c1176
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 14 deletions.
37 changes: 33 additions & 4 deletions pkg/storaged/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,42 @@ define([
*/

function update_indices() {
var path, block, dev, mdraid, vgroup, pvol, lvol, part;
var path, block, dev, mdraid, vgroup, pvol, lvol, part, i;

client.drives_multipath_blocks = { };
client.drives_block = { };
for (path in client.drives) {
client.drives_multipath_blocks[path] = [ ];
}
for (path in client.blocks) {
block = client.blocks[path];
if (block.Drive != "/")
client.drives_block[block.Drive] = block;
if (!client.blocks_part[path] && client.drives_multipath_blocks[block.Drive] !== undefined)
client.drives_multipath_blocks[block.Drive].push(block);
}
for (path in client.drives_multipath_blocks) {
var all_blocks = client.drives_multipath_blocks[path];
var mpath_members = [ ];
var non_mpath_members = [ ];

all_blocks.sort(function (a, b) { return a.DeviceNumber - b.DeviceNumber; });
for (i = 0; i < all_blocks.length; i++) {
if (all_blocks[i].IdType == "mpath_member")
mpath_members.push(all_blocks[i]);
else
non_mpath_members.push(all_blocks[i]);
}

/* A valid multipath drive has exactly one block device
* that is not a "mpath_member".
*/

if (non_mpath_members.length === 1) {
client.drives_block[path] = non_mpath_members[0];
client.drives_multipath_blocks[path] = mpath_members;
} else {
client.drives_block[path] = null;
client.drives_multipath_blocks[path] = all_blocks;
}
}

client.mdraids_block = { };
Expand Down Expand Up @@ -190,7 +219,7 @@ define([
block = client.blocks[path];
enter_slashdev(block, block.Device);
enter_slashdev(block, block.PreferredDevice);
for (var i = 0; i < block.Symlinks.length; i++)
for (i = 0; i < block.Symlinks.length; i++)
enter_slashdev(block, block.Symlinks[i]);
}

Expand Down
27 changes: 22 additions & 5 deletions pkg/storaged/details.js
Original file line number Diff line number Diff line change
Expand Up @@ -1302,21 +1302,38 @@ define([
}

var drive_model = null;
var content_block = block;
if (drive) {
var drive_block = client.drives_block[drive.path];
var multipath_blocks = client.drives_multipath_blocks[drive.path];

var multipath_model = null;
if (multipath_blocks.length > 0) {
multipath_model = {
Devices: multipath_blocks.map(utils.block_name)
};
}

drive_model = {
dbus: drive,
block_dbus: drive_block,
Size: drive.Size > 0 && utils.fmt_size_long(drive.Size),
Assessment: assessment
Assessment: assessment,
Device: drive_block && utils.block_name(drive_block),
Multipath: multipath_model
};

content_block = drive_block;
}

return mustache.render(block_detail_tmpl,
{ Block: block_model,
Drive: drive_model,
Content: mustache.render(content_tmpl,
{ Title: _("Content"),
Entries: block_content_entries(block)
})
Content: (content_block &&
mustache.render(content_tmpl,
{ Title: _("Content"),
Entries: block_content_entries(content_block)
}))
});
}

Expand Down
26 changes: 23 additions & 3 deletions pkg/storaged/devices.html
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,22 @@ <h1 translatable="yes">The "storaged" API is not available on this system.</h1>
<script id="block-detail-tmpl" type="x-template/mustache">
<div class="panel panel-default">
<div class="panel-heading">
{{#Drive}}<span translatable="yes">Drive</span>{{/Drive}}
{{^Drive}}<span translatable="yes">Block Device</span>{{/Drive}}
{{#Drive}}
<span translatable="yes">Drive</span>
{{#block_dbus}}
<button class="btn btn-default storage-privileged" style="float:right" translatable="yes"
data-action="format_disk" data-arg="{{path}}">
Format
</button>
{{/block_dbus}}
{{/Drive}}
{{^Drive}}
<span translatable="yes">Block Device</span>
<button class="btn btn-default storage-privileged" style="float:right" translatable="yes"
data-action="format_disk" data-arg="{{Block.dbus.path}}">
Format
</button>
{{/Drive}}
</div>
<div class="panel-body">
<table class="cockpit-info-table">
Expand Down Expand Up @@ -398,12 +408,22 @@ <h1 translatable="yes">The "storaged" API is not available on this system.</h1>
</td>
</tr>
{{/Assessment}}
<tr>
<td translatable="yes" context="storage">Device File</td>
<td>{{#Device}}{{.}}{{/Device}}{{^Device}}-{{/Device}}</td>
</tr>
{{#Multipath}}
<tr>
<td translatable="yes" context="storage">Multipath Devices</td>
<td>{{#Devices}}{{.}} {{/Devices}}</td>
</tr>
{{/Multipath}}
{{/Drive}}
{{^Drive}}
<tr>
<td translatable="yes" context="storage">Device File</td>
<td>{{Block.Name}}</td>
</tr>
{{^Drive}}
<tr>
<td translatable="yes" context="storage">Capacity</td>
<td>{{Block.Size}}</td>
Expand Down
12 changes: 10 additions & 2 deletions pkg/storaged/overview.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,18 @@ define([
var drive = client.drives[path];
var block = client.drives_block[path];

if (!block) {
// A drive without a primary block device might be
// a unconfigured multipath device. Try to hobble
// along here by arbitrarily picking one of the
// multipath devices.
block = client.drives_multipath_blocks[path][0];
}

if (!block)
return;

var dev = utils.decode_filename(block.Device).replace(/^\/dev\//, "");
var dev = utils.decode_filename(block.PreferredDevice).replace(/^\/dev\//, "");
var io = client.blockdev_io.data[dev];

var name = utils.drive_name(drive);
Expand Down Expand Up @@ -227,7 +235,7 @@ define([
function is_mount(path) {
var block = client.blocks[path];
var fsys = client.blocks_fsys[path];
return fsys && block.IdUsage == "filesystem" && !block.HintIgnore;
return fsys && block.IdUsage == "filesystem" && block.IdType != "mpath_member" && !block.HintIgnore;
}

function cmp_mount(path_a, path_b) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/storaged/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,20 @@ define([
return true;
}

function is_mpath_member() {
// A configured multipath member has
// IdType="mpath_member", but unconfigured ones look
// like normal block devices. We can check their
// drive to find out whether it has been properly set
// up.
return (block.IdType == "mpath_member" ||
client.drives[block.Drive] && !client.drives_block[block.Drive]);
}

return (!block.HintIgnore &&
block.Size > 0 &&
!has_fs_label() &&
!is_mpath_member() &&
!block_ptable &&
!(block_part && block_part.IsContainer));
}
Expand Down
103 changes: 103 additions & 0 deletions test/check-storage-multipath
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/usr/bin/python
# -*- coding: utf-8 -*-

# This file is part of Cockpit.
#
# Copyright (C) 2015 Red Hat, Inc.
#
# Cockpit is free software; you can redistribute it and/or modify it
# under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation; either version 2.1 of the License, or
# (at your option) any later version.
#
# Cockpit is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with Cockpit; If not, see <http://www.gnu.org/licenses/>.

from testlib import *
from storagelib import *

class TestStorage(StorageCase):
def testBasic(self):
m = self.machine
b = self.browser

def detail_row(index):
return '#detail tr:nth-child(%s)' % index

def detail_row_name(index):
return '#detail tr:nth-child(%s) td:nth-child(1)' % index

def detail_row_value(index):
return '#detail tr:nth-child(%s) td:nth-child(2)' % index

def wait_detail_row(index, name):
b.wait_present(detail_row(index))
b.wait_text(detail_row_name(index), name)

def check_free_block_devices(expected):
blocks = b.eval_js ("return ph_texts('#dialog [data-field=\"disks\"] .checkbox')")
if len(blocks) != len(expected):
return False
for i in range(len(expected)):
if not expected[i] in blocks[i]:
return False
return True

self.login_and_go("/storage/devices")
b.wait_in_text("#drives", "VirtIO")

b.eval_js("""
ph_texts = function (sel) {
return $(sel).map(function (i, e) { return $(e).text(); }).get();
}""")

# Add a disk
disk = m.add_disk("10M", serial="MYSERIAL")

b.wait_in_text("#drives", "MYSERIAL")
b.click('tr:contains("MYSERIAL")')
b.wait_visible('#storage-detail')
wait_detail_row(5, "Device File")
b.wait_text(detail_row_value(5), "/dev/sda")

# Add another path to the same disk. The primary device file
# should disappear and multipath devices should be listed.
m.add_disk_path(disk)
b.wait_text(detail_row_value(5), "-")
wait_detail_row(6, "Multipath Devices")
b.wait_in_text(detail_row_value(6), "/dev/sda")
b.wait_in_text(detail_row_value(6), "/dev/sdb")

# Check that neither is offered as a free block device
b.go("/")
b.wait_visible('#create-volume-group')
b.click('#create-volume-group')
self.dialog_wait_open()
check_free_block_devices([])
self.dialog_cancel()
self.dialog_wait_close()

b.go("/sda")
b.wait_visible('#storage-detail')

# Switch on multipathd. A primary device should appear.
m.execute("mpathconf --enable && systemctl restart multipathd")
# HACK - https://bugzilla.redhat.com/show_bug.cgi?id=1254583
m.execute("udevadm trigger")
b.wait_text(detail_row_value(5), "/dev/mapper/mpatha")

# Check that (exactly) the primary device is offered as free
b.go("/")
b.wait_visible('#create-volume-group')
b.click('#create-volume-group')
self.dialog_wait_open()
check_free_block_devices([ "/dev/mapper/mpatha" ])
self.dialog_cancel()
self.dialog_wait_close()

test_main()
1 change: 1 addition & 0 deletions test/check-verify
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ $RUNNER <<EOF
./check-storage-unused
./check-storage-resize
./check-storage-ignored
./check-storage-multipath
./check-journal
./check-terminal
./check-accounts
Expand Down

0 comments on commit 80c1176

Please sign in to comment.