Skip to content

Commit

Permalink
storage: use exceptions for open/write instead of booleans
Browse files Browse the repository at this point in the history 
Raise and propage exceptions when the shared storage cannot be opened
or when a write error occurs instead of returning False. Exceptions
raised over ZeroRPC become RemoteError exceptions and the msg attribute
holds a human-readable error message.

Signed-off-by: Cedric Hombourger <[email protected]>
  • Loading branch information
@chombourger
chombourger committed Nov 8, 2023
1 parent 0ccb5c6 commit 07cdc58
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 69 deletions.
25 changes: 14 additions & 11 deletions mtda-cli
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,17 +352,20 @@ class Application:
return 0

def storage_write(self, args=None):
self.start_time = time.monotonic()
status = self.agent.storage_write_image(
args.image, self._storage_write_cb
)
sys.stdout.write("\n")
sys.stdout.flush()

if status is False:
print("'storage write' failed!", file=sys.stderr)
return 1
return 0
result = 0
try:
self.start_time = time.monotonic()
self.agent.storage_write_image(
args.image, self._storage_write_cb
)
sys.stdout.write("\n")
sys.stdout.flush()
except Exception as e:
msg = e.msg if hasattr(e, 'msg') else str(e)
print("\n'storage write' failed! ({})".format(msg),
file=sys.stderr)
result = 1
return result

def target_uptime(self):
result = ""
Expand Down
28 changes: 15 additions & 13 deletions mtda/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,15 @@ def storage_open(self):
tries = 60
while tries > 0:
tries = tries - 1
status = self._impl.storage_open(self._session)
if status is True:
try:
self._impl.storage_open(self._session)
return
time.sleep(1)
raise IOError('shared storage could not be opened')
except Exception:
if tries > 0:
time.sleep(1)
pass
else:
raise

def storage_status(self):
return self._impl.storage_status(self._session)
Expand Down Expand Up @@ -204,6 +208,11 @@ def storage_write_image(self, path, callback=None):
else:
file = ImageLocal(path, impl, session, blksz, callback)

# Open the shared storage device so we own it
# It also prevents us from loading a new bmap file while
# another transfer may be on-going
self.storage_open()

# Automatically discover the bmap file
bmap = None
image_path = file.path()
Expand All @@ -216,7 +225,7 @@ def storage_write_image(self, path, callback=None):
import xml.etree.ElementTree as ET

bmap = ET.fromstring(bmap)
print("discovered bmap file '{}'".format(bmap_path))
print("Discovered bmap file '{}'".format(bmap_path))
bmapDict = self.parseBmap(bmap, bmap_path)
self._impl.storage_bmap_dict(bmapDict, self._session)
image_size = bmapDict['ImageSize']
Expand All @@ -228,11 +237,6 @@ def storage_write_image(self, path, callback=None):
print("No bmap file found at location of image")
break

# Open the shared storage device
status = self.storage_open()
if status is False:
return False

try:
# Prepare for download/copy
file.prepare(image_size)
Expand All @@ -244,13 +248,11 @@ def storage_write_image(self, path, callback=None):
file.flush()

except Exception:
status = False
raise
finally:
# Storage may be closed now
self.storage_close()

return status

def parseBmap(self, bmap, bmap_path):
try:
bmapDict = {}
Expand Down
65 changes: 28 additions & 37 deletions mtda/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def storage_locked(self, session=None):
# Lastly, the shared storage device shall not be opened
elif self._storage_opened is True:
self.mtda.debug(4, "storage_locked(): "
"shared storage is in used (opened)")
"shared storage is in use (opened)")
result = True
# We may otherwise swap our shared storage device
else:
Expand Down Expand Up @@ -668,31 +668,25 @@ def storage_update(self, dst, offset, session=None):
return result

def storage_open(self, session=None):
self.mtda.debug(3, "main.storage_open()")
self.mtda.debug(3, 'main.storage_open()')

self._session_check(session)
owner = self._storage_owner
status, _, _ = self.storage_status()

if self.storage_controller is None:
self.mtda.debug(1, "storage_open(): no shared storage device")
result = False
raise RuntimeError('no shared storage device')
elif status != CONSTS.STORAGE.ON_HOST:
self.mtda.debug(1, "storage_open(): not attached to host")
result = False
raise RuntimeError('shared storage not attached to host')
elif owner is not None and owner != session:
self.mtda.debug(1, "storage_open(): in use")
result = False
else:
self.storage_close()
result = self.storage_controller.open()
if result is True:
self._storage_owner = session
self._writer.start()
self._storage_opened = result
raise RuntimeError('shared storage in use')
elif self._storage_opened is False:
self.storage_controller.open()
self._storage_opened = True
self._storage_owner = session
self._writer.start()

self.mtda.debug(3, "main.storage_open(): %s" % str(result))
return result
self.mtda.debug(3, 'main.storage_open(): success')

def storage_status(self, session=None):
self.mtda.debug(3, "main.storage_status()")
Expand Down Expand Up @@ -764,30 +758,27 @@ def storage_write(self, data, session=None):

self._session_check(session)
if self.storage_controller is None:
self.mtda.debug(1, "main.storage_write(): no storage")
result = -1
raise RuntimeError('no shared storage')
elif self._storage_opened is False:
self.mtda.debug(1, "main.storage_write(): not opened")
result = -1
raise RuntimeError('shared storage was not opened')
elif self._writer.failed is True:
self.mtda.debug(1, "main.storage_write(): write error")
result = -1
raise RuntimeError('write or decompression error '
'from shared storage')
elif session != self._storage_owner:
self.mtda.debug(1, "main.storage_write(): in use")
result = -1
else:
try:
if len(data) == 0:
self.mtda.debug(2, "main.storage_write(): "
"using queued data")
data = self._writer_data
self._writer_data = data
self._writer.put(data, timeout=10)
result = self.blksz
except queue.Full:
raise RuntimeError('shared storage in use')

try:
if len(data) == 0:
self.mtda.debug(2, "main.storage_write(): "
"queue is full")
result = 0
"using queued data")
data = self._writer_data
self._writer_data = data
self._writer.put(data, timeout=10)
result = self.blksz
except queue.Full:
self.mtda.debug(2, "main.storage_write(): "
"queue is full")
result = 0

if self._writer.failed is True:
self.mtda.debug(1, "main.storage_write(): "
Expand Down
11 changes: 3 additions & 8 deletions mtda/storage/helpers/image.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,16 +264,11 @@ def open(self):
return result

def _open_impl(self):
result = True
if self._status() == CONSTS.STORAGE.ON_HOST:
if self.handle is None:
try:
self.handle = open(self.file, "r+b")
self.handle.seek(0, 0)
except FileNotFoundError:
result = False

return result
self.handle = open(self.file, "r+b")
self.handle.seek(0, 0)
return True

def status(self):
self.mtda.debug(3, "storage.helpers.image.status()")
Expand Down

0 comments on commit 07cdc58

Please sign in to comment.