[lxc-devel] [lxd/master] Performance improvements
stgraber on Github
lxc-bot at linuxcontainers.org
Mon Apr 4 19:51:34 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160404/f48cc017/attachment.bin>
-------------- next part --------------
From 7e496bbc1fdcbe023b69f930002c325b5b853713 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 4 Apr 2016 00:56:19 -0400
Subject: [PATCH 1/5] Throttle the event listeners
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To avoid ever growing event queues, cap both queues to 10 events,
anything getting in after that will just be dropped.
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/devices.go | 76 ++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 26 deletions(-)
diff --git a/lxd/devices.go b/lxd/devices.go
index db018ca..4ea8a35 100644
--- a/lxd/devices.go
+++ b/lxd/devices.go
@@ -24,7 +24,7 @@ import (
log "gopkg.in/inconshreveable/log15.v2"
)
-var deviceSchedRebalance = make(chan []string, 0)
+var deviceSchedRebalance = make(chan []string, 2)
type deviceBlockLimit struct {
readBps int64
@@ -44,7 +44,7 @@ func (c deviceTaskCPUs) Len() int { return len(c) }
func (c deviceTaskCPUs) Less(i, j int) bool { return *c[i].count < *c[j].count }
func (c deviceTaskCPUs) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
-func deviceNetlinkListener() (chan []string, error) {
+func deviceNetlinkListener() (chan []string, chan []string, error) {
NETLINK_KOBJECT_UEVENT := 15
UEVENT_BUFFER_SIZE := 2048
@@ -54,7 +54,7 @@ func deviceNetlinkListener() (chan []string, error) {
)
if err != nil {
- return nil, err
+ return nil, nil, err
}
nl := syscall.SockaddrNetlink{
@@ -65,12 +65,13 @@ func deviceNetlinkListener() (chan []string, error) {
err = syscall.Bind(fd, &nl)
if err != nil {
- return nil, err
+ return nil, nil, err
}
- ch := make(chan []string, 0)
+ chCPU := make(chan []string, 1)
+ chNetwork := make(chan []string, 0)
- go func(ch chan []string) {
+ go func(chCPU chan []string, chNetwork chan []string) {
b := make([]byte, UEVENT_BUFFER_SIZE*2)
for {
_, err := syscall.Read(fd, b)
@@ -106,7 +107,12 @@ func deviceNetlinkListener() (chan []string, error) {
continue
}
- ch <- []string{"cpu", path.Base(props["DEVPATH"]), props["ACTION"]}
+ // As CPU re-balancing affects all containers, no need to queue them
+ select {
+ case chCPU <- []string{path.Base(props["DEVPATH"]), props["ACTION"]}:
+ default:
+ // Channel is full, drop the event
+ }
}
if props["SUBSYSTEM"] == "net" {
@@ -114,12 +120,17 @@ func deviceNetlinkListener() (chan []string, error) {
continue
}
- ch <- []string{"net", props["INTERFACE"], props["ACTION"]}
+ if !shared.PathExists(fmt.Sprintf("/sys/class/net/%s", props["INTERFACE"])) {
+ continue
+ }
+
+ // Network balancing is interface specific, so queue everything
+ chNetwork <- []string{props["INTERFACE"], props["ACTION"]}
}
}
- }(ch)
+ }(chCPU, chNetwork)
- return ch, nil
+ return chCPU, chNetwork, nil
}
func deviceTaskBalance(d *Daemon) {
@@ -387,7 +398,7 @@ func deviceNetworkPriority(d *Daemon, netif string) {
}
func deviceEventListener(d *Daemon) {
- chNetlink, err := deviceNetlinkListener()
+ chNetlinkCPU, chNetlinkNetwork, err := deviceNetlinkListener()
if err != nil {
shared.Log.Error("scheduler: couldn't setup netlink listener")
return
@@ -395,40 +406,53 @@ func deviceEventListener(d *Daemon) {
for {
select {
- case e := <-chNetlink:
- if len(e) != 3 {
- shared.Log.Error("Scheduler: received an invalid hotplug event")
+ case e := <-chNetlinkCPU:
+ if len(e) != 2 {
+ shared.Log.Error("Scheduler: received an invalid cpu hotplug event")
+ continue
+ }
+
+ if !cgCpusetController {
continue
}
- if e[0] == "cpu" && cgCpusetController {
- shared.Debugf("Scheduler: %s: %s is now %s: re-balancing", e[0], e[1], e[2])
- deviceTaskBalance(d)
+ shared.Debugf("Scheduler: cpu: %s is now %s: re-balancing", e[0], e[1])
+ deviceTaskBalance(d)
+ case e := <-chNetlinkNetwork:
+ if len(e) != 2 {
+ shared.Log.Error("Scheduler: received an invalid network hotplug event")
+ continue
}
- if e[0] == "net" && e[2] == "add" && cgNetPrioController && shared.PathExists(fmt.Sprintf("/sys/class/net/%s", e[1])) {
- shared.Debugf("Scheduler: %s: %s has been added: updating network priorities", e[0], e[1])
- deviceNetworkPriority(d, e[1])
+ if !cgNetPrioController {
+ continue
}
+
+ shared.Debugf("Scheduler: network: %s has been added: updating network priorities", e[0])
+ deviceNetworkPriority(d, e[0])
case e := <-deviceSchedRebalance:
if len(e) != 3 {
shared.Log.Error("Scheduler: received an invalid rebalance event")
continue
}
- if cgCpusetController {
- shared.Debugf("Scheduler: %s %s %s: re-balancing", e[0], e[1], e[2])
- deviceTaskBalance(d)
+ if !cgCpusetController {
+ continue
}
+
+ shared.Debugf("Scheduler: %s %s %s: re-balancing", e[0], e[1], e[2])
+ deviceTaskBalance(d)
}
}
}
func deviceTaskSchedulerTrigger(srcType string, srcName string, srcStatus string) {
// Spawn a go routine which then triggers the scheduler
- go func() {
- deviceSchedRebalance <- []string{srcType, srcName, srcStatus}
- }()
+ select {
+ case deviceSchedRebalance <- []string{srcType, srcName, srcStatus}:
+ default:
+ // Channel is full, drop the event
+ }
}
func deviceIsBlockdev(path string) bool {
From b72b0f5f6f0a6e280c766db5fbc1744fb7cb094e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 4 Apr 2016 15:33:24 -0400
Subject: [PATCH 2/5] lxd-benchmark: Allow specifying number of threads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
test/lxd-benchmark/main.go | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/test/lxd-benchmark/main.go b/test/lxd-benchmark/main.go
index b3ce881..2f69dcd 100644
--- a/test/lxd-benchmark/main.go
+++ b/test/lxd-benchmark/main.go
@@ -14,6 +14,7 @@ import (
)
var argCount = gnuflag.Int("count", 100, "Number of containers to create")
+var argParallel = gnuflag.Int("parallel", -1, "Number of threads to use")
var argImage = gnuflag.String("image", "ubuntu:", "Image to use for the test")
var argPrivileged = gnuflag.Bool("privileged", false, "Use privileged containers")
@@ -32,8 +33,8 @@ func run(args []string) error {
gnuflag.Parse(true)
if len(os.Args) == 1 || !shared.StringInSlice(os.Args[1], []string{"spawn", "delete"}) {
- fmt.Printf("Usage: %s spawn [--count=COUNT] [--image=IMAGE] [--privileged=BOOL]\n", os.Args[0])
- fmt.Printf(" %s delete\n\n", os.Args[0])
+ fmt.Printf("Usage: %s spawn [--count=COUNT] [--image=IMAGE] [--privileged=BOOL] [--parallel=COUNT]\n", os.Args[0])
+ fmt.Printf(" %s delete [--parallel=COUNT]\n\n", os.Args[0])
gnuflag.Usage()
fmt.Printf("\n")
return fmt.Errorf("An action (spawn or delete) must be passed.")
@@ -60,13 +61,17 @@ func logf(format string, args ...interface{}) {
}
func spawnContainers(c *lxd.Client, count int, image string, privileged bool) error {
- // Detect the number of parallel actions
- cpus, err := ioutil.ReadDir("/sys/bus/cpu/devices")
- if err != nil {
- return err
+ batch := *argParallel
+ if batch < 1 {
+ // Detect the number of parallel actions
+ cpus, err := ioutil.ReadDir("/sys/bus/cpu/devices")
+ if err != nil {
+ return err
+ }
+
+ batch = len(cpus)
}
- batch := len(cpus)
batches := count / batch
remainder := count % batch
@@ -214,10 +219,15 @@ func spawnContainers(c *lxd.Client, count int, image string, privileged bool) er
}
func deleteContainers(c *lxd.Client) error {
- // Detect the number of parallel actions
- cpus, err := ioutil.ReadDir("/sys/bus/cpu/devices")
- if err != nil {
- return err
+ batch := *argParallel
+ if batch < 1 {
+ // Detect the number of parallel actions
+ cpus, err := ioutil.ReadDir("/sys/bus/cpu/devices")
+ if err != nil {
+ return err
+ }
+
+ batch = len(cpus)
}
// List all the containers
@@ -239,7 +249,6 @@ func deleteContainers(c *lxd.Client) error {
count := len(containers)
logf("%d containers to delete", count)
- batch := len(cpus)
batches := count / batch
deletedCount := 0
From 02c5dfa4120c3a49f1498b681dc37e73f6b92afd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 4 Apr 2016 15:37:22 -0400
Subject: [PATCH 3/5] tests: Don't rely on the filesystem so much
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
test/suites/filemanip.sh | 2 +-
test/suites/snapshots.sh | 29 +++++++++++------------------
2 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/test/suites/filemanip.sh b/test/suites/filemanip.sh
index dd2317c..f4b7b05 100644
--- a/test/suites/filemanip.sh
+++ b/test/suites/filemanip.sh
@@ -8,7 +8,7 @@ test_filemanip() {
lxc file push main.sh filemanip/tmp/outside/
[ ! -f /tmp/main.sh ]
- [ -f "${LXD_DIR}/containers/filemanip/rootfs/tmp/main.sh" ]
+ lxc exec filemanip -- ls /tmp/main.sh
lxc delete filemanip -f
}
diff --git a/test/suites/snapshots.sh b/test/suites/snapshots.sh
index 0d93620..b1bb5eb 100644
--- a/test/suites/snapshots.sh
+++ b/test/suites/snapshots.sh
@@ -85,11 +85,8 @@ test_snap_restore() {
echo snap0 > state
lxc file push state bar/root/state
lxc file push state bar/root/file_only_in_snap0
-
- mkdir "${LXD_DIR}/containers/bar/rootfs/root/dir_only_in_snap0"
- cd "${LXD_DIR}/containers/bar/rootfs/root/"
- ln -s ./file_only_in_snap0 statelink
- cd -
+ lxc exec bar -- mkdir /root/dir_only_in_snap0
+ lxc exec bar -- ln -s file_only_in_snap0 /root/statelink
lxc stop bar --force
lxc snapshot bar snap0
@@ -100,13 +97,11 @@ test_snap_restore() {
lxc file push state bar/root/state
lxc file push state bar/root/file_only_in_snap1
- cd "${LXD_DIR}/containers/bar/rootfs/root/"
- rmdir dir_only_in_snap0
- rm file_only_in_snap0
- rm statelink
- ln -s ./file_only_in_snap1 statelink
- mkdir dir_only_in_snap1
- cd -
+ lxc exec bar -- rmdir /root/dir_only_in_snap0
+ lxc exec bar -- rm /root/file_only_in_snap0
+ lxc exec bar -- rm /root/statelink
+ lxc exec bar -- ln -s file_only_in_snap1 /root/statelink
+ lxc exec bar -- mkdir /root/dir_only_in_snap1
lxc stop bar --force
# Delete the state file we created to prevent leaking.
@@ -118,8 +113,7 @@ test_snap_restore() {
##########################################################
- # FIXME: make this backend agnostic
- if [ "${LXD_BACKEND}" = "dir" ]; then
+ if [ "${LXD_BACKEND}" != "zfs" ]; then
# The problem here is that you can't `zfs rollback` to a snapshot with a
# parent, which snap0 has (snap1).
restore_and_compare_fs snap0
@@ -127,8 +121,8 @@ test_snap_restore() {
# Check container config has been restored (limits.cpu is unset)
cpus=$(lxc config get bar limits.cpu)
if [ -n "${cpus}" ]; then
- echo "==> config didn't match expected value after restore (${cpus})"
- false
+ echo "==> config didn't match expected value after restore (${cpus})"
+ false
fi
fi
@@ -149,8 +143,7 @@ test_snap_restore() {
# Start container and then restore snapshot to verify the running state after restore.
lxc start bar
- # FIXME: make this backend agnostic
- if [ "${LXD_BACKEND}" = "dir" ]; then
+ if [ "${LXD_BACKEND}" != "zfs" ]; then
# see comment above about snap0
restore_and_compare_fs snap0
From ca989d9392ddec1ac41ce372a86ffab20768a465 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 4 Apr 2016 15:39:28 -0400
Subject: [PATCH 4/5] storage: Fixups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Share the LVM tryExec, tryMount and tryUmount functions
- Fix logging
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/storage.go | 60 +++++++++++++++++++++++++++++++++--
lxd/storage_lvm.go | 93 +++++++++++-------------------------------------------
2 files changed, 77 insertions(+), 76 deletions(-)
diff --git a/lxd/storage.go b/lxd/storage.go
index 915fa05..68f30be 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -8,6 +8,7 @@ import (
"path/filepath"
"reflect"
"syscall"
+ "time"
"github.com/gorilla/websocket"
@@ -511,12 +512,12 @@ func (lw *storageLogWrapper) ContainerSnapshotRename(
}
func (lw *storageLogWrapper) ContainerSnapshotStart(container container) error {
- lw.log.Debug("ContainerStart", log.Ctx{"container": container.Name()})
+ lw.log.Debug("ContainerSnapshotStart", log.Ctx{"container": container.Name()})
return lw.w.ContainerSnapshotStart(container)
}
func (lw *storageLogWrapper) ContainerSnapshotStop(container container) error {
- lw.log.Debug("ContainerStop", log.Ctx{"container": container.Name()})
+ lw.log.Debug("ContainerSnapshotStop", log.Ctx{"container": container.Name()})
return lw.w.ContainerSnapshotStop(container)
}
@@ -652,3 +653,58 @@ func rsyncMigrationSink(live bool, container container, snapshots []container, c
return nil
}
+
+// Useful functions for unreliable backends
+func tryExec(name string, arg ...string) ([]byte, error) {
+ var err error
+ var output []byte
+
+ for i := 0; i < 20; i++ {
+ output, err = exec.Command(name, arg...).CombinedOutput()
+ if err == nil {
+ break
+ }
+
+ time.Sleep(500 * time.Millisecond)
+ }
+
+ return output, err
+}
+
+func tryMount(src string, dst string, fs string, flags uintptr, options string) error {
+ var err error
+
+ for i := 0; i < 20; i++ {
+ err = syscall.Mount(src, dst, fs, flags, options)
+ if err == nil {
+ break
+ }
+
+ time.Sleep(500 * time.Millisecond)
+ }
+
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func tryUnmount(path string, flags int) error {
+ var err error
+
+ for i := 0; i < 20; i++ {
+ err = syscall.Unmount(path, flags)
+ if err == nil {
+ break
+ }
+
+ time.Sleep(500 * time.Millisecond)
+ }
+
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
diff --git a/lxd/storage_lvm.go b/lxd/storage_lvm.go
index 58eb02e..2cf3e77 100644
--- a/lxd/storage_lvm.go
+++ b/lxd/storage_lvm.go
@@ -9,7 +9,6 @@ import (
"strconv"
"strings"
"syscall"
- "time"
"github.com/gorilla/websocket"
@@ -365,7 +364,7 @@ func (s *storageLvm) ContainerCreateFromImage(
}
}
- err = s.tryMount(lvpath, destPath, fstype, 0, "discard")
+ err = tryMount(lvpath, destPath, fstype, 0, "discard")
if err != nil {
s.ContainerDelete(container)
return fmt.Errorf("Error mounting snapshot LV: %v", err)
@@ -373,7 +372,7 @@ func (s *storageLvm) ContainerCreateFromImage(
if !container.IsPrivileged() {
if err = s.shiftRootfs(container); err != nil {
- err2 := s.tryUnmount(destPath, 0)
+ err2 := tryUnmount(destPath, 0)
if err2 != nil {
return fmt.Errorf("Error in umount: '%s' while cleaning up after error in shiftRootfs: '%s'", err2, err)
}
@@ -388,7 +387,7 @@ func (s *storageLvm) ContainerCreateFromImage(
log.Ctx{"err": err})
}
- umounterr := s.tryUnmount(destPath, 0)
+ umounterr := tryUnmount(destPath, 0)
if umounterr != nil {
return fmt.Errorf("Error unmounting '%s' after shiftRootfs: %v", destPath, umounterr)
}
@@ -468,7 +467,7 @@ func (s *storageLvm) ContainerStart(container container) error {
fstype = "ext4"
}
- err = s.tryMount(lvpath, container.Path(), fstype, 0, "discard")
+ err = tryMount(lvpath, container.Path(), fstype, 0, "discard")
if err != nil {
return fmt.Errorf(
"Error mounting snapshot LV path='%s': %v",
@@ -480,7 +479,7 @@ func (s *storageLvm) ContainerStart(container container) error {
}
func (s *storageLvm) ContainerStop(container container) error {
- err := s.tryUnmount(container.Path(), 0)
+ err := tryUnmount(container.Path(), 0)
if err != nil {
return fmt.Errorf(
"failed to unmount container path '%s'.\nError: %v",
@@ -491,60 +490,6 @@ func (s *storageLvm) ContainerStop(container container) error {
return nil
}
-func (s *storageLvm) tryExec(name string, arg ...string) ([]byte, error) {
- var err error
- var output []byte
-
- for i := 0; i < 20; i++ {
- output, err = exec.Command(name, arg...).CombinedOutput()
- if err == nil {
- break
- }
-
- time.Sleep(500 * time.Millisecond)
- }
-
- return output, err
-}
-
-func (s *storageLvm) tryMount(src string, dst string, fs string, flags uintptr, options string) error {
- var err error
-
- for i := 0; i < 20; i++ {
- err = syscall.Mount(src, dst, fs, flags, options)
- if err == nil {
- break
- }
-
- time.Sleep(500 * time.Millisecond)
- }
-
- if err != nil {
- return err
- }
-
- return nil
-}
-
-func (s *storageLvm) tryUnmount(path string, flags int) error {
- var err error
-
- for i := 0; i < 20; i++ {
- err = syscall.Unmount(path, flags)
- if err == nil {
- break
- }
-
- time.Sleep(500 * time.Millisecond)
- }
-
- if err != nil {
- return err
- }
-
- return nil
-}
-
func (s *storageLvm) ContainerRename(
container container, newContainerName string) error {
@@ -776,7 +721,7 @@ func (s *storageLvm) ContainerSnapshotStart(container container) error {
}
}
- err = s.tryMount(lvpath, container.Path(), fstype, 0, "discard")
+ err = tryMount(lvpath, container.Path(), fstype, 0, "discard")
if err != nil {
return fmt.Errorf(
"Error mounting snapshot LV path='%s': %v",
@@ -840,7 +785,7 @@ func (s *storageLvm) ImageCreate(fingerprint string) error {
fstype = "ext4"
}
- err = s.tryMount(lvpath, tempLVMountPoint, fstype, 0, "discard")
+ err = tryMount(lvpath, tempLVMountPoint, fstype, 0, "discard")
if err != nil {
shared.Logf("Error mounting image LV for untarring: %v", err)
return fmt.Errorf("Error mounting image LV: %v", err)
@@ -849,7 +794,7 @@ func (s *storageLvm) ImageCreate(fingerprint string) error {
untarErr := untarImage(finalName, tempLVMountPoint)
- err = s.tryUnmount(tempLVMountPoint, 0)
+ err = tryUnmount(tempLVMountPoint, 0)
if err != nil {
s.log.Warn("could not unmount LV. Will not remove",
log.Ctx{"lvpath": lvpath, "mountpoint": tempLVMountPoint, "err": err})
@@ -884,7 +829,7 @@ func (s *storageLvm) ImageDelete(fingerprint string) error {
func (s *storageLvm) createDefaultThinPool() (string, error) {
// Create a tiny 1G thinpool
- output, err := s.tryExec(
+ output, err := tryExec(
"lvcreate",
"--poolmetadatasize", "1G",
"-L", "1G",
@@ -904,7 +849,7 @@ func (s *storageLvm) createDefaultThinPool() (string, error) {
}
// Grow it to the maximum VG size (two step process required by old LVM)
- output, err = s.tryExec(
+ output, err = tryExec(
"lvextend",
"--alloc", "anywhere",
"-l", "100%FREE",
@@ -952,7 +897,7 @@ func (s *storageLvm) createThinLV(lvname string) (string, error) {
lvSize = storageLvmDefaultThinLVSize
}
- output, err := s.tryExec(
+ output, err := tryExec(
"lvcreate",
"--thin",
"-n", lvname,
@@ -970,12 +915,12 @@ func (s *storageLvm) createThinLV(lvname string) (string, error) {
switch fstype {
case "xfs":
- output, err = s.tryExec(
+ output, err = tryExec(
"mkfs.xfs",
lvpath)
default:
// default = ext4
- output, err = s.tryExec(
+ output, err = tryExec(
"mkfs.ext4",
"-E", "nodiscard,lazy_itable_init=0,lazy_journal_init=0",
lvpath)
@@ -993,7 +938,7 @@ func (s *storageLvm) removeLV(lvname string) error {
var err error
var output []byte
- output, err = s.tryExec(
+ output, err = tryExec(
"lvremove", "-f", fmt.Sprintf("%s/%s", s.vgName, lvname))
if err != nil {
@@ -1012,13 +957,13 @@ func (s *storageLvm) createSnapshotLV(lvname string, origlvname string, readonly
}
var output []byte
if isRecent {
- output, err = s.tryExec(
+ output, err = tryExec(
"lvcreate",
"-kn",
"-n", lvname,
"-s", fmt.Sprintf("/dev/%s/%s", s.vgName, origlvname))
} else {
- output, err = s.tryExec(
+ output, err = tryExec(
"lvcreate",
"-n", lvname,
"-s", fmt.Sprintf("/dev/%s/%s", s.vgName, origlvname))
@@ -1031,9 +976,9 @@ func (s *storageLvm) createSnapshotLV(lvname string, origlvname string, readonly
snapshotFullName := fmt.Sprintf("/dev/%s/%s", s.vgName, lvname)
if readonly {
- output, err = s.tryExec("lvchange", "-ay", "-pr", snapshotFullName)
+ output, err = tryExec("lvchange", "-ay", "-pr", snapshotFullName)
} else {
- output, err = s.tryExec("lvchange", "-ay", snapshotFullName)
+ output, err = tryExec("lvchange", "-ay", snapshotFullName)
}
if err != nil {
@@ -1048,7 +993,7 @@ func (s *storageLvm) isLVMContainer(container container) bool {
}
func (s *storageLvm) renameLV(oldName string, newName string) (string, error) {
- output, err := s.tryExec("lvrename", s.vgName, oldName, newName)
+ output, err := tryExec("lvrename", s.vgName, oldName, newName)
return string(output), err
}
From d75a3ecc4fe7f081a2ebf0385485e3242e7355df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sun, 3 Apr 2016 17:14:51 -0400
Subject: [PATCH 5/5] zfs: Improve reliability and performance
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Don't use "zfs mount" and "zfs umount", they're just expensive
wrapper around the mount and umount syscalls.
- Use the try* functions from storage.go
Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
lxd/storage_zfs.go | 154 +++++++++++++++++++++++------------------------------
1 file changed, 66 insertions(+), 88 deletions(-)
diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go
index 438d2a7..1a8b9ec 100644
--- a/lxd/storage_zfs.go
+++ b/lxd/storage_zfs.go
@@ -8,7 +8,6 @@ import (
"strconv"
"strings"
"syscall"
- "time"
"github.com/gorilla/websocket"
@@ -320,23 +319,6 @@ func (s *storageZfs) ContainerCopy(container container, sourceContainer containe
return container.TemplateApply("copy")
}
-func (s *storageZfs) zfsMounted(path string) bool {
- output, err := exec.Command("zfs", "mount").CombinedOutput()
- if err != nil {
- shared.Log.Error("error listing zfs mounts", "err", output)
- return false
- }
-
- for _, line := range strings.Split(string(output), "\n") {
- zfsName := strings.Split(line, " ")[0]
- if zfsName == fmt.Sprintf("%s/%s", s.zfsPool, path) {
- return true
- }
- }
-
- return false
-}
-
func (s *storageZfs) ContainerRename(container container, newName string) error {
oldName := container.Name()
@@ -359,17 +341,9 @@ func (s *storageZfs) ContainerRename(container container, newName string) error
}
// In case ZFS didn't mount the filesystem, do it ourselves
- if !shared.PathExists(shared.VarPath(fmt.Sprintf("containers/%s.zfs", newName))) {
- for i := 0; i < 20; i++ {
- err = s.zfsMount(fmt.Sprintf("containers/%s", newName))
- if err == nil {
- break
- }
- time.Sleep(500 * time.Millisecond)
- }
- if err != nil {
- return err
- }
+ err = s.zfsMount(fmt.Sprintf("containers/%s", newName))
+ if err != nil {
+ return err
}
// In case the change of mountpoint didn't remove the old path, do it ourselves
@@ -560,6 +534,7 @@ func (s *storageZfs) ContainerSnapshotStart(container container) error {
if len(fields) < 2 {
return fmt.Errorf("Invalid snapshot name: %s", container.Name())
}
+
cName := fields[0]
sName := fields[1]
sourceFs := fmt.Sprintf("containers/%s", cName)
@@ -761,33 +736,17 @@ func (s *storageZfs) zfsCreate(path string) error {
}
func (s *storageZfs) zfsDestroy(path string) error {
- mountpoint, err := s.zfsGet(path, "mountpoint")
+ err := s.zfsUnmount(path)
if err != nil {
return err
}
- if mountpoint != "none" && shared.IsMountPoint(mountpoint) {
- err := syscall.Unmount(mountpoint, syscall.MNT_DETACH)
- if err != nil {
- s.log.Error("umount failed", log.Ctx{"err": err})
- return err
- }
- }
-
// Due to open fds or kernel refs, this may fail for a bit, give it 10s
- var output []byte
- for i := 0; i < 20; i++ {
- output, err = exec.Command(
- "zfs",
- "destroy",
- "-r",
- fmt.Sprintf("%s/%s", s.zfsPool, path)).CombinedOutput()
-
- if err == nil {
- break
- }
- time.Sleep(500 * time.Millisecond)
- }
+ output, err := tryExec(
+ "zfs",
+ "destroy",
+ "-r",
+ fmt.Sprintf("%s/%s", s.zfsPool, path))
if err != nil {
s.log.Error("zfs destroy failed", log.Ctx{"output": string(output)})
@@ -853,19 +812,6 @@ func (s *storageZfs) zfsGet(path string, key string) (string, error) {
return strings.TrimRight(string(output), "\n"), nil
}
-func (s *storageZfs) zfsMount(path string) error {
- output, err := exec.Command(
- "zfs",
- "mount",
- fmt.Sprintf("%s/%s", s.zfsPool, path)).CombinedOutput()
- if err != nil {
- s.log.Error("zfs mount failed", log.Ctx{"output": string(output)})
- return fmt.Errorf("Failed to mount ZFS filesystem: %s", output)
- }
-
- return nil
-}
-
func (s *storageZfs) zfsRename(source string, dest string) error {
output, err := exec.Command(
"zfs",
@@ -978,14 +924,60 @@ func (s *storageZfs) zfsSnapshotRename(path string, oldName string, newName stri
return nil
}
+func (s *storageZfs) zfsMount(path string) error {
+ var err error
+
+ fsPath := shared.VarPath(path) + ".zfs"
+ if !shared.IsDir(fsPath) {
+ fsPath, err = s.zfsGet(path, "mountpoint")
+ if err != nil {
+ return err
+ }
+
+ if fsPath == "none" {
+ return nil
+ }
+ }
+
+ // "zfs mount" is just a wrapper for the "mount" command
+ // so save ourselves some forks
+
+ fs := fmt.Sprintf("%s/%s", s.zfsPool, path)
+ err = tryMount(fs, fsPath, "zfs", 0, "rw,zfsutil")
+ if err != nil && !shared.IsMountPoint(fsPath) {
+ s.log.Error("zfs mount failed", log.Ctx{"err": err, "source": fs, "target": fsPath})
+ return fmt.Errorf("Failed to mount ZFS filesystem: %s", err)
+ }
+
+ return nil
+}
+
func (s *storageZfs) zfsUnmount(path string) error {
- output, err := exec.Command(
- "zfs",
- "unmount",
- fmt.Sprintf("%s/%s", s.zfsPool, path)).CombinedOutput()
- if err != nil {
- s.log.Error("zfs unmount failed", log.Ctx{"output": string(output)})
- return fmt.Errorf("Failed to unmount ZFS filesystem: %s", output)
+ var err error
+
+ fsPath := shared.VarPath(path) + ".zfs"
+ if !shared.IsDir(fsPath) {
+ fsPath, err = s.zfsGet(path, "mountpoint")
+ if err != nil {
+ return err
+ }
+
+ if fsPath == "none" {
+ return nil
+ }
+ }
+
+ // "zfs unmount" is just a wrapper for the "umount" command
+ // so save ourselves some forks
+
+ err = syscall.Unmount(fsPath, 0)
+ if err != nil && shared.IsMountPoint(fsPath) {
+ // Do a lazy unmount, we only care about it fully gone at
+ // destroy and "zfs destroy" will complain appropriately
+ err = tryUnmount(fsPath, syscall.MNT_DETACH)
+ if err != nil {
+ return err
+ }
}
return nil
@@ -1218,7 +1210,6 @@ func (s *zfsMigrationSourceDriver) SendWhileRunning(conn *websocket.Conn) error
lastSnap := ""
for i, snap := range s.zfsSnapshotNames {
-
prev := ""
if i > 0 {
prev = s.zfsSnapshotNames[i-1]
@@ -1356,24 +1347,11 @@ func (s *storageZfs) MigrationSink(live bool, container container, snapshots []c
* this fs (it's empty anyway) before we zfs recv. N.B. that `zfs recv`
* of a snapshot also needs tha actual fs that it has snapshotted
* unmounted, so we do this before receiving anything.
- *
- * Further, `zfs unmount` doesn't actually unmount things right away,
- * so we ask /proc/self/mountinfo whether or not this path is mounted
- * before continuing so that we're sure the fs is actually unmounted
- * before doing a recv.
*/
zfsName := fmt.Sprintf("containers/%s", container.Name())
- fsPath := shared.VarPath(fmt.Sprintf("containers/%s.zfs", container.Name()))
- for i := 0; i < 20; i++ {
- if shared.IsMountPoint(fsPath) || s.zfsMounted(zfsName) {
- if err := s.zfsUnmount(zfsName); err != nil {
- shared.Log.Error("zfs umount error for", "path", zfsName, "err", err)
- }
- } else {
- break
- }
-
- time.Sleep(500 * time.Millisecond)
+ err := s.zfsUnmount(zfsName)
+ if err != nil {
+ return err
}
for _, snap := range snapshots {
More information about the lxc-devel
mailing list