[lxc-devel] [lxd/master] Add function to consolidate Do's error handling
sean-jc on Github
lxc-bot at linuxcontainers.org
Fri Jun 24 17:12:07 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 810 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160624/92498852/attachment.bin>
-------------- next part --------------
From b08c8faca061b084b45bb185fc66448f012d6d0a Mon Sep 17 00:00:00 2001
From: Sean Christopherson <sean.j.christopherson at intel.com>
Date: Fri, 24 Jun 2016 09:43:54 -0700
Subject: [PATCH] Add function to consolidate Do's error handling
Add a function that can be called inline with return to handle all
cleanup if an error occurs after migration has been started. This
fixes a missing call to driver.Cleanup() and hopefully reduces the
probability of a similar bug occuring in the future.
Signed-off-by: Sean Christopherson <sean.j.christopherson at intel.com>
---
lxd/migrate.go | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/lxd/migrate.go b/lxd/migrate.go
index 651b257..3ea1a76 100644
--- a/lxd/migrate.go
+++ b/lxd/migrate.go
@@ -302,37 +302,39 @@ func (s *migrationSourceWs) Do(op *operation) error {
driver, _ = rsyncMigrationSource(s.container)
}
- if err := driver.SendWhileRunning(s.fsConn); err != nil {
+ // All failure paths need to do a few things to correctly handle errors before returning.
+ // Unfortunately, handling errors is not well-suited to defer as the code depends on the
+ // status of driver and the error value. The error value is especially tricky due to the
+ // common case of creating a new err variable (intentional or not) due to scoping and use
+ // of ":=". Capturing err in a closure for use in defer would be fragile, which defeats
+ // the purpose of using defer. An abort function reduces the odds of mishandling errors
+ // without introducing the fragility of closing on err.
+ abort := func(err error) error {
driver.Cleanup()
s.sendControl(err)
return err
}
+ if err := driver.SendWhileRunning(s.fsConn); err != nil {
+ return abort(err)
+ }
+
if s.live {
if header.Criu == nil {
- driver.Cleanup()
- err := fmt.Errorf("Got no CRIU socket type for live migration")
- s.sendControl(err)
- return err
+ return abort(fmt.Errorf("Got no CRIU socket type for live migration"))
} else if *header.Criu != CRIUType_CRIU_RSYNC {
- driver.Cleanup()
- err := fmt.Errorf("Formats other than criu rsync not understood")
- s.sendControl(err)
- return err
+ return abort(fmt.Errorf("Formats other than criu rsync not understood"))
}
checkpointDir, err := ioutil.TempDir("", "lxd_checkpoint_")
if err != nil {
- driver.Cleanup()
- s.sendControl(err)
- return err
+ return abort(err)
}
defer os.RemoveAll(checkpointDir)
err = s.container.Migrate(lxc.MIGRATE_DUMP, checkpointDir, "migration", true)
if err != nil {
- s.sendControl(err)
- return err
+ return abort(err)
}
/*
@@ -343,15 +345,11 @@ func (s *migrationSourceWs) Do(op *operation) error {
* p.haul's protocol, it will make sense to do these in parallel.
*/
if err := RsyncSend(shared.AddSlash(checkpointDir), s.criuConn); err != nil {
- driver.Cleanup()
- s.sendControl(err)
- return err
+ return abort(err)
}
if err := driver.SendAfterCheckpoint(s.fsConn); err != nil {
- driver.Cleanup()
- s.sendControl(err)
- return err
+ return abort(err)
}
}
More information about the lxc-devel
mailing list