Merge pull request #1893 from phillxnet/1890_allow_import_of_ro,degra…

…ded_cli_mounted_pool

allow import of ro,degraded cli mounted pool. Fixes #1890

src/rockstor/fs/btrfs.py

@@ -50,7 +50,7 @@
 def add_pool(pool, disks):
     """
     Makes a btrfs pool (filesystem) of name 'pool' using the by-id disk names
-    provided, then enables quotas for this pool.
+    provided, then attempts to enables quotas for this pool.
     :param pool: name of pool to create.
     :param disks: list of by-id disk names without paths to make the pool from.
     :return o, err, rc from last command executed.
@@ -70,8 +70,6 @@ def add_pool(pool, disks):
     # additional level of isolation.
     # Only execute enable_quota on above btrfs command having an rc=0
     if rc == 0:
-        # N.B. enable_quota wraps switch_quota() which doesn't enable logging
-        # so log what we have on rc != 0.
         out2, err2, rc2 = enable_quota(pool)
         if rc2 != 0:
             e_msg = (
@@ -675,7 +673,7 @@ def rollback_snap(snap_name, sname, subvol_name, pool):
     mount_root(pool)
     if (is_share_mounted(sname)):
         umount_root(mnt_pt)
-    remove_share(pool, subvol_name, '-1/-1')
+    remove_share(pool, subvol_name, PQGROUP_DEFAULT)
     shutil.move(snap_fp, '%s/%s/%s' % (DEFAULT_MNT_DIR, pool.name, sname))
     create_tmp_dir(mnt_pt)
     subvol_str = 'subvol=%s' % sname
@@ -687,7 +685,20 @@ def rollback_snap(snap_name, sname, subvol_name, pool):
 def switch_quota(pool, flag='enable'):
     root_mnt_pt = mount_root(pool)
     cmd = [BTRFS, 'quota', flag, root_mnt_pt]
-    return run_command(cmd)
+    try:
+        o, e, rc = run_command(cmd, log=True)
+    except CommandException as e:
+        # Avoid failure when attempting an enable/disable quota change if
+        # our pool (vol) is ro: by catching this specific CommandException:
+        emsg = "ERROR: quota command failed: Read-only file system"
+        if e.err[0] == emsg:
+            logger.error('Failed to {} quotas on pool ({}). To resolve '
+                         'run "btrfs quota {} {}".'.format(flag, pool.name,
+                                                           flag, root_mnt_pt))
+            return e.out, e.err, e.rc
+        # otherwise we raise an exception as normal.
+        raise e
+    return o, e, rc
 
 
 def enable_quota(pool):
@@ -755,7 +766,7 @@ def qgroup_max(mnt_pt):
         # disabled quotas will result in o = [''], rc = 1 and e[0] =
         emsg = "ERROR: can't list qgroups: quotas not enabled"
         # this is non fatal so we catch this specific error and info log it.
-        if e.rc == 1 and e.err[0] == emsg:
+        if e.err[0] == emsg:
             logger.info('Mount Point: {} has Quotas disabled, skipping qgroup '
                         'show.'.format(mnt_pt))
             # and return our default res
@@ -772,20 +783,20 @@ def qgroup_max(mnt_pt):
     return res
 
 
-def qgroup_create(pool, qgroup='-1/-1'):
+def qgroup_create(pool, qgroup=PQGROUP_DEFAULT):
     """
     When passed only a pool an attempt will be made to ascertain if quotas is
     enabled, if not '-1/-1' is returned as a flag to indicate this state.
-    If quotas are not enabled then the highest available quota of the form
-    2015/n is selected and created, if possible.
+    If quotas are enabled then the highest available quota of the form
+    2015/n is selected and created, if possible (Read-only caveat).
     If passed both a pool and a specific qgroup an attempt is made, given the
     same behaviour as above, to create this specific group: this scenario is
     primarily used to re-establish prior existing qgroups post quota disable,
     share manipulation, quota enable cycling.
     :param pool: A pool object.
     :param qgroup: native qgroup of the form 2015/n
-    :return: -1/-1 on quotas disabled, otherwise it will return the native
-    quota whose creation was attempt.
+    :return: -1/-1 on quotas disabled or Read-only fs encountered, otherwise
+    it will return the successfully created native quota, ie 2015/n.
     """
     # mount pool
     mnt_pt = mount_root(pool)
@@ -806,10 +817,15 @@ def qgroup_create(pool, qgroup='-1/-1'):
         # ro mount options will result in o= [''], rc = 1 and e[0] =
         emsg = 'ERROR: unable to create quota group: Read-only file system'
         # this is non fatal so we catch this specific error and info log it.
-        if e.rc == 1 and e.err[0] == emsg:
+        if e.err[0] == emsg:
             logger.info('Pool: {} is Read-only, skipping qgroup '
                         'create.'.format(pool.name))
-            return qid
+            # We now return PQGROUP_DEFAULT because our proposed next
+            # available pqgroup can't be assigned anyway (Read-only file
+            # system). This in turn avoids populating share db pqgroup with
+            # non existent pqgroups and further flags for retires via the
+            # existing quota disabled management system.
+            return PQGROUP_DEFAULT
         # raise an exception as usual otherwise
         raise
     return qid
@@ -822,7 +838,7 @@ def qgroup_destroy(qid, mnt_pt):
     except CommandException as e:
         # we may have quotas disabled so catch and deal.
         emsg = "ERROR: can't list qgroups: quotas not enabled"
-        if e.rc == 1 and e.err[0] == emsg:
+        if e.err[0] == emsg:
             # we have quotas disabled so can't destroy any anyway so skip
             # and deal by returning False so our caller moves on.
             return False
@@ -844,7 +860,7 @@ def qgroup_is_assigned(qid, pqid, mnt_pt):
     except CommandException as e:
         # we may have quotas disabled so catch and deal.
         emsg = "ERROR: can't list qgroups: quotas not enabled"
-        if e.rc == 1 and e.err[0] == emsg:
+        if e.err[0] == emsg:
             # No deed to scan output as nothing to see with quotas disabled.
             # And since no quota capability can be enacted we return True
             # to avoid our caller trying any further with quotas.
@@ -889,8 +905,14 @@ def qgroup_assign(qid, pqid, mnt_pt):
     try:
         run_command([BTRFS, 'qgroup', 'assign', qid, pqid, mnt_pt], log=True)
     except CommandException as e:
+        emsg = 'ERROR: unable to assign quota group: Read-only file system'
+        # this is non fatal so we catch this specific error and info log it.
+        if e.err[0] == emsg:
+            logger.info('Read-only fs ({}), skipping qgroup assign: '
+                        'child ({}), parent ({}).'.format(mnt_pt, qid, pqid))
+            return e.out, e.err, e.rc
         wmsg = 'WARNING: quotas may be inconsistent, rescan needed'
-        if (e.rc == 1 and e.err[0] == wmsg):
+        if e.err[0] == wmsg:
             # schedule a rescan if one is not currently running.
             dmsg = ('Quota inconsistency while assigning %s. Rescan scheduled.'
                     % qid)
@@ -899,7 +921,7 @@ def qgroup_assign(qid, pqid, mnt_pt):
                 return logger.debug(dmsg)
             except CommandException as e2:
                 emsg = 'ERROR: quota rescan failed: Operation now in progress'
-                if (e2.rc == 1 and e2.err[0] == emsg):
+                if e2.err[0] == emsg:
                     return logger.debug('%s.. Another rescan already in '
                                         'progress.' % dmsg)
                 logger.exception(e2)
@@ -919,8 +941,9 @@ def update_quota(pool, qgroup, size_bytes):
     # Set defaults in case our run_command fails to assign them.
     out = err = ['']
     rc = 0
-    if qgroup == '-1/-1':
-        # We have a 'quotas disabled' qgroup value flag, log and return blank.
+    if qgroup == PQGROUP_DEFAULT:
+        # We have a 'quotas disabled' or 'Read-only' qgroup value flag,
+        # log and return blank.
         logger.info('Pool: {} ignoring '
                     'update_quota on {}'.format(pool.name, qgroup))
         return out, err, rc
@@ -931,7 +954,7 @@ def update_quota(pool, qgroup, size_bytes):
         emsg = 'ERROR: unable to limit requested quota group: ' \
                'Read-only file system'
         # this is non fatal so we catch this specific error and info log it.
-        if e.rc == 1 and e.err[0] == emsg:
+        if e.err[0] == emsg:
             logger.info('Pool: {} is Read-only, skipping qgroup '
                         'limit.'.format(pool.name))
             return out, err, rc
@@ -942,14 +965,14 @@ def update_quota(pool, qgroup, size_bytes):
         # is a non specific error: 'Invalid argument'.
         # TODO: improve this clause as currently too broad.
         # TODO: we could for example use if qgroup_max(mnt) == -1
-        if e.rc == 1 and e.err[0] == emsg2:
+        if e.err[0] == emsg2:
             logger.info('Pool: {} has encountered a qgroup limit issue, '
                         'skipping qgroup limit. Disabled quotas can cause '
                         'this error'.format(pool.name))
             return out, err, rc
         emsg3 = 'ERROR: unable to limit requested quota group: ' \
                 'No such file or directory'
-        if e.rc == 1 and e.err[0] == emsg3:
+        if e.err[0] == emsg3:
             logger.info('Pool: {} is missing expected '
                         'qgroup {}'.format(pool.name, qgroup))
             logger.info('Previously disabled quotas can cause this issue')
@@ -970,7 +993,7 @@ def volume_usage(pool, volume_id, pvolume_id=None):
     :param pool: Pool object
     :param volume_id: qgroupid eg '0/261'
     :param pvolume_id: qgroupid eg '2015/4'
-    :return: list of len 2 (when pvolul_id=None) or 4 elements. The first 2
+    :return: list of len 2 (when pvolume_id=None) or 4 elements. The first 2
     pertain to the qgroupid=volume_id the second 2, if present, are for the
     qgroupid=pvolume_id. I.e [rfer, excl, rfer, excl]
     """

src/rockstor/storageadmin/views/clone_helpers.py

@@ -19,7 +19,8 @@
 from storageadmin.models import (Share, Snapshot)
 from storageadmin.util import handle_exception
 from fs.btrfs import (add_clone, share_id, update_quota, mount_share,
-                      qgroup_create, set_property, remove_share)
+                      qgroup_create, set_property, remove_share,
+                      share_pqgroup_assign)
 from rest_framework.response import Response
 from storageadmin.serializers import ShareSerializer
 import re
@@ -122,10 +123,12 @@ def create_clone(share, new_name, request, logger, snapshot=None):
         snap_id = share_id(share.pool, new_name)
         qgroup_id = ('0/%s' % snap_id)
         pqid = qgroup_create(share.pool)
-        update_quota(share.pool, pqid, share.size * 1024)
         new_share = Share(pool=share.pool, qgroup=qgroup_id, pqgroup=pqid,
                           name=new_name, size=share.size, subvol_name=new_name)
         new_share.save()
+        if pqid is not settings.MODEL_DEFS['pqgroup']:
+            update_quota(new_share.pool, pqid, new_share.size * 1024)
+            share_pqgroup_assign(pqid, new_share)
         # Mount our new clone share.
         mnt_pt = '{}{}'.format(settings.MNT_PT, new_name)
         mount_share(new_share, mnt_pt)

src/rockstor/storageadmin/views/disk.py

@@ -709,7 +709,6 @@ def _btrfs_disk_import(self, did, request):
             po.raid = pool_raid('%s%s' % (settings.MNT_PT, po.name))['data']
             po.size = po.usage_bound()
             po.save()
-            # TODO: enable_quota could well break an import from a ro pool.
             enable_quota(po)
             import_shares(po, request)
             for share in Share.objects.filter(pool=po):

src/rockstor/storageadmin/views/share.py

@@ -38,7 +38,8 @@
 
 logger = logging.getLogger(__name__)
 
-# The following model/db default setting is also used when quotas are disabled.
+# The following model/db default setting is also used when quotas are disabled
+# or when a Read-only state prevents creation of a new pqgroup.
 PQGROUP_DEFAULT = settings.MODEL_DEFS['pqgroup']
 
 
@@ -177,11 +178,13 @@ def post(self, request):
             pqid = qgroup_create(pool)
             add_share(pool, sname, pqid)
             qid = qgroup_id(pool, sname)
-            update_quota(pool, pqid, size * 1024)
             s = Share(pool=pool, qgroup=qid, pqgroup=pqid, name=sname,
                       size=size, subvol_name=sname, replica=replica,
                       compression_algo=compression)
             s.save()
+            if pqid is not PQGROUP_DEFAULT:
+                update_quota(pool, pqid, size * 1024)
+                share_pqgroup_assign(pqid, s)
             mnt_pt = '%s%s' % (settings.MNT_PT, sname)
             if not s.is_mounted:
                 mount_share(s, mnt_pt)
@@ -233,8 +236,12 @@ def put(self, request, sid):
                         # if quotas were disabled or pqgroup non-existent.
                         share.pqgroup = qgroup_create(share.pool)
                         share.save()
-                    update_quota(share.pool, share.pqgroup, new_size * 1024)
-                    share_pqgroup_assign(share.pqgroup, share)
+                    if share.pqgroup is not PQGROUP_DEFAULT:
+                        # Only update quota and assign if now non default as
+                        # default can also indicate Read-only fs at this point.
+                        update_quota(share.pool, share.pqgroup,
+                                     new_size * 1024)
+                        share_pqgroup_assign(share.pqgroup, share)
                 else:
                     # Our pool's quotas are disabled so reset pqgroup to -1/-1.
                     if share.pqgroup != PQGROUP_DEFAULT:

src/rockstor/storageadmin/views/share_helpers.py

@@ -23,7 +23,8 @@
 from smart_manager.models import ShareUsage
 from fs.btrfs import (mount_share, mount_snap, is_mounted,
                       umount_root, shares_info, volume_usage, snaps_info,
-                      qgroup_create, update_quota, share_pqgroup_assign)
+                      qgroup_create, update_quota, share_pqgroup_assign,
+                      qgroup_assign)
 from storageadmin.util import handle_exception
 from copy import deepcopy
 
@@ -33,6 +34,9 @@
 
 NEW_ENTRY = True
 UPDATE_TS = False
+# The following model/db default setting is also used when quotas are disabled
+# or when a Read-only state prevents creation of a new pqgroup.
+PQGROUP_DEFAULT = settings.MODEL_DEFS['pqgroup']
 
 
 def helper_mount_share(share, mnt_pt=None):
@@ -100,7 +104,7 @@ def import_shares(pool, request):
             # Initially default our pqgroup value to db default of '-1/-1'
             # This way, unless quotas are enabled, all pqgroups will be
             # returned to db default.
-            pqgroup = settings.MODEL_DEFS['pqgroup']
+            pqgroup = PQGROUP_DEFAULT
             if share.pool.quotas_enabled:
                 # Quotas are enabled on our pool so we can validate pqgroup.
                 if share.pqgroup == pqgroup or not share.pqgroup_exist \
@@ -110,8 +114,9 @@ def import_shares(pool, request):
                     logger.debug('#### replacing void, non-existent, or '
                                  'duplicate pqgroup')
                     pqgroup = qgroup_create(pool)
-                    update_quota(pool, pqgroup, share.size * 1024)
-                    share_pqgroup_assign(pqgroup, share)
+                    if pqgroup is not PQGROUP_DEFAULT:
+                        update_quota(pool, pqgroup, share.size * 1024)
+                        share_pqgroup_assign(pqgroup, share)
                 else:
                     # Our share's pqgroup looks OK so use it.
                     pqgroup = share.pqgroup
@@ -180,14 +185,17 @@ def import_shares(pool, request):
                 replica = True
                 logger.debug('Initial receive quirk-subvol found: Importing '
                              'as share and setting replica flag.')
+            qid = shares_in_pool[s_in_pool]
             pqid = qgroup_create(pool)
-            update_quota(pool, pqid, pool.size * 1024)
+            if pqid is not PQGROUP_DEFAULT:
+                update_quota(pool, pqid, pool.size * 1024)
+                pool_mnt_pt = '{}{}'.format(settings.MNT_PT, pool.name)
+                qgroup_assign(qid, pqid, pool_mnt_pt)
             rusage, eusage, pqgroup_rusage, pqgroup_eusage = \
-                volume_usage(pool, shares_in_pool[s_in_pool], pqid)
-            nso = Share(pool=pool, qgroup=shares_in_pool[s_in_pool],
-                        pqgroup=pqid, name=share_name, size=pool.size,
-                        subvol_name=s_in_pool, rusage=rusage, eusage=eusage,
-                        pqgroup_rusage=pqgroup_rusage,
+                volume_usage(pool, qid, pqid)
+            nso = Share(pool=pool, qgroup=qid, pqgroup=pqid, name=share_name,
+                        size=pool.size, subvol_name=s_in_pool, rusage=rusage,
+                        eusage=eusage, pqgroup_rusage=pqgroup_rusage,
                         pqgroup_eusage=pqgroup_eusage,
                         replica=replica)
             nso.save()

src/rockstor/storageadmin/views/snapshot.py

@@ -119,8 +119,9 @@ def _create(self, share, snap_name, request, uvisible,
         add_snap(share.pool, share.subvol_name, snap_name, writable)
         snap_id = share_id(share.pool, snap_name)
         qgroup_id = ('0/%s' % snap_id)
-        qgroup_assign(qgroup_id, share.pqgroup, ('%s/%s' % (settings.MNT_PT,
-                                                            share.pool.name)))
+        if share.pqgroup is not settings.MODEL_DEFS['pqgroup']:
+            pool_mnt_pt = '{}{}'.format(settings.MNT_PT, share.pool.name)
+            qgroup_assign(qgroup_id, share.pqgroup, pool_mnt_pt)
         snap_size, eusage = volume_usage(share.pool, qgroup_id)
         s = Snapshot(share=share, name=snap_name, real_name=snap_name,
                      size=snap_size, qgroup=qgroup_id,