OpenMediaVault-Plugin-Developers / openmediavault-zfs

OpenMediaVault plugin for zfs
74 stars 15 forks source link

Can't enter ZFS plugin interface after add L2ARC device #59

Closed alphaarea closed 4 years ago

alphaarea commented 5 years ago

openmediavault 5.0.9-1 openmediavault-zfs 5.0.1

I add a SM961 as a l2arc device, after that OMV show error when I try to enter zfs plugin interface. The l2arc and zpool work right, only plugin interface can't show them.

error massage

ERROR #0:
Exception: raidz2: cache can only be plain in /usr/share/omvzfs/Zpool.php:630
Stack trace:
#0 /usr/share/omvzfs/Zpool.php(529): OMVModuleZFSZpool->output('cache', 'raidz2', 'nvme-SAMSUNG_MZ...')
#1 /usr/share/omvzfs/Zpool.php(91): OMVModuleZFSZpool->assemblePool('Pool12Disks')
#2 /usr/share/omvzfs/Utils.php(275): OMVModuleZFSZpool->__construct('Pool12Disks')
#3 /usr/share/openmediavault/engined/rpc/zfs.inc(180): OMVModuleZFSUtil::getZFSFlatArray()
#4 [internal function]: OMVRpcServiceZFS->getObjectTree(Array, Array)
#5 /usr/share/php/openmediavault/rpc/serviceabstract.inc(123): call_user_func_array(Array, Array)
#6 /usr/share/php/openmediavault/rpc/rpc.inc(86): OMV\Rpc\ServiceAbstract->callMethod('getObjectTree', Array, Array)
#7 /usr/sbin/omv-engined(537): OMV\Rpc\Rpc::call('ZFS', 'getObjectTree', Array, Array, 1)
#8 {main}
subzero79 commented 5 years ago

This is the same as

https://forum.openmediavault.org/index.php/Thread/29036-OMV5-ZFS-Unable-to-add-mirrored-vdev-to-pool/

The issue seems when constructing the zfs panel tree object the function calls assemblePool from ZPool this tries to validate each vdev(don't know why because is not a creation) of the pool.

When the pool has more vdevs or elements (like log, cache etc) fails to parse outputting the error you see there. Just commenting out the output() functions suppresses the error.

@mir07 @mdziekon What do you think about just commenting out the function?

subzero79 commented 5 years ago

This is the particular section of the code

https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L518-L546

mir07 commented 5 years ago

On Sun, 13 Oct 2019 17:22:29 -0700 subzero79 notifications@github.com wrote:

This is the particular section of the code

https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L518-L546

The part handling logs fails if logs is a mirror so the code should be identical to the code for parsing the disks in the pool like:

case 'logs': if (preg_match("/^\s($types)/", $line, $match)) { / new vdev / if ($type) { $this->output(null, $type, $dev); $dev = null; } $type = $match[1]; } else if (preg_match("/^\s([\w\d-]+)\s+/", $line, $match)) { if ($dev) $dev .= " $match[1]"; else $dev = "$match[1]"; } break;

-- Hilsen/Regards Michael Rasmussen

Get my public GnuPG keys: michael rasmussen cc https://pgp.key-server.io/pks/lookup?search=0xD3C9A00E mir datanom net https://pgp.key-server.io/pks/lookup?search=0xE501F51C mir miras org https://pgp.key-server.io/pks/lookup?search=0xE3E80917

/usr/games/fortune -es says: Sank heaven for leetle curls.

mir07 commented 5 years ago

Begin forwarded message:

Date: Mon, 14 Oct 2019 04:02:25 +0200 From: Michael Rasmussen mir@datanom.net To: subzero79 notifications@github.com Cc: OpenMediaVault-Plugin-Developers/openmediavault-zfs openmediavault-zfs@noreply.github.com Subject: Re: [OpenMediaVault-Plugin-Developers/openmediavault-zfs] Can't enter ZFS plugin interface after add L2ARC device (#59)

On Sun, 13 Oct 2019 17:22:29 -0700 subzero79 notifications@github.com wrote:

This is the particular section of the code

https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L518-L546

The part handling logs fails if logs is a mirror so the code should be identical to the code for parsing the disks in the pool like:

case 'logs': if (preg_match("/^\s($types)/", $line, $match)) { / new vdev / if ($type) { $this->output(null, $type, $dev); $dev = null; } $type = $match[1]; } else if (preg_match("/^\s([\w\d-]+)\s+/", $line, $match)) { if ($dev) $dev .= " $match[1]"; else $dev = "$match[1]"; } break;

-- Hilsen/Regards Michael Rasmussen

Get my public GnuPG keys: michael rasmussen cc https://pgp.key-server.io/pks/lookup?search=0xD3C9A00E mir datanom net https://pgp.key-server.io/pks/lookup?search=0xE501F51C mir miras org https://pgp.key-server.io/pks/lookup?search=0xE3E80917

/usr/games/fortune -es says: Q: Why haven't you graduated yet? A: Well, Dad, I could have finished years ago, but I wanted my dissertation to rhyme.

-- Hilsen/Regards Michael Rasmussen

Get my public GnuPG keys: michael rasmussen cc https://pgp.key-server.io/pks/lookup?search=0xD3C9A00E mir datanom net https://pgp.key-server.io/pks/lookup?search=0xE501F51C mir miras org https://pgp.key-server.io/pks/lookup?search=0xE3E80917

/usr/games/fortune -es says: Q: Why haven't you graduated yet? A: Well, Dad, I could have finished years ago, but I wanted my dissertation to rhyme.

subzero79 commented 5 years ago

that is just case for the log, but i have seen it failing also in a pool with dual mirror or a combination of mirror and raidz. If you look at the forum post. When testing this yesterday i noticed is failing to send the array of devices to the parser when more vdevs are present

Error #0:
OMVModuleZFSException: A mirror must contain at least 2 disks in /usr/share/omvzfs/Vdev.php:54
Stack trace:
#0 /usr/share/omvzfs/Zpool.php(654): OMVModuleZFSVdev->__construct('t1', 1, Array)
#1 /usr/share/omvzfs/Zpool.php(550): OMVModuleZFSZpool->output(NULL, 'mirror', NULL)
#2 /usr/share/omvzfs/Zpool.php(91): OMVModuleZFSZpool->assemblePool('t1')
#3 /usr/share/omvzfs/Utils.php(275): OMVModuleZFSZpool->__construct('t1')
#4 /usr/share/openmediavault/engined/rpc/zfs.inc(180): OMVModuleZFSUtil::getZFSFlatArray()
#5 [internal function]: OMVRpcServiceZFS->getObjectTree(Array, Array)
#6 /usr/share/php/openmediavault/rpc/serviceabstract.inc(123): call_user_func_array(Array, Array)
#7 /usr/share/php/openmediavault/rpc/rpc.inc(86): OMV\Rpc\ServiceAbstract->callMethod('getObjectTree', Array, Array)
#8 /usr/sbin/omv-engined(537): OMV\Rpc\Rpc::call('ZFS', 'getObjectTree', Array, Array, 1)
#9 {main}
root@omv5-dev:/srv/temp# zpool status
  pool: t1
 state: ONLINE
  scan: none requested
config:

    NAME        STATE     READ WRITE CKSUM
    t1          ONLINE       0     0     0
      mirror-0  ONLINE       0     0     0
        vdc     ONLINE       0     0     0
        vdd     ONLINE       0     0     0
      raidz1-1  ONLINE       0     0     0
        vdg     ONLINE       0     0     0
        vde     ONLINE       0     0     0
        vdh     ONLINE       0     0     0

errors: No known data errors
mir07 commented 5 years ago

On Sun, 13 Oct 2019 19:11:34 -0700 subzero79 notifications@github.com wrote:

that is just case for the log, but i have seen it failing also in a pool with dual mirror or a combination of mirror and raidz. If you look at the forum post. When testing this yesterday i noticed is failing to send the array of devices to the parser when more vdevs are present

See: https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L549

Should be (Replace \d- with \d: preg_match("/^\s*([\w\da-z0-9\:.-]+)\s+/", $line, $match)) {

Same here: https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L557 https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-zfs/blob/8a885313aa162b75aaf8b6bbd194b27b154ebf67/usr/share/omvzfs/Zpool.php#L566 -- Hilsen/Regards Michael Rasmussen

Get my public GnuPG keys: michael rasmussen cc https://pgp.key-server.io/pks/lookup?search=0xD3C9A00E mir datanom net https://pgp.key-server.io/pks/lookup?search=0xE501F51C mir miras org https://pgp.key-server.io/pks/lookup?search=0xE3E80917

/usr/games/fortune -es says: As long as the answer is right, who cares if the question is wrong?

subzero79 commented 5 years ago

Thanks that seems to work. However point apart the code validating the vdevs disk members doesn't make sense to me when is trying to generate the tree since is already there.

mdziekon commented 5 years ago

This is the same as

https://forum.openmediavault.org/index.php/Thread/29036-OMV5-ZFS-Unable-to-add-mirrored-vdev-to-pool/

The issue seems when constructing the zfs panel tree object the function calls assemblePool from ZPool this tries to validate each vdev(don't know why because is not a creation) of the pool.

When the pool has more vdevs or elements (like log, cache etc) fails to parse outputting the error you see there. Just commenting out the output() functions suppresses the error.

@mir07 @mdziekon What do you think about just commenting out the function?

If I understand that correctly, then yes, I would agree that building the tree (for display only) itself should most likely not validate the pool's shape. It should either be a dedicated funcitonality (like a button "Validate zpool / vdevs"), or an infobox which would be a separate code piece, allowing the tree builder to work on its own.

However, it seems that the issue might also be solved by reusing the zpool status parser I've added quite some time ago. Since I've added some test cases for it, you should be able to quickly verify if the parser is able to handle the zpool's shape that causes problems here. Admittedly, this will require more work, but on the other hand it will most likely get rid of this very wobbly piece of code that you are trying to patch.

subzero79 commented 5 years ago

The PR is already there to patch the current code, just to not fail in the zfs tree rendering. I did take a quick look at your test units they have plenty of test examples, but to be honest i didn't get it on how to run them. Do you have to load them with an interactive php shell or in the omv backend?

mdziekon commented 5 years ago

Do you have to load them with an interactive php shell or in the omv backend?

IIRC, they do not require OMV backend (means you should be able to run them from your terminal), because they test only the parser, which is abstract enought to run on its own once you provide it with required data. The command to run the test suite is in the README.md file.

subzero79 commented 5 years ago

The command to run the test suite is in the README.md file

Ok, should've started there. Thanks

Skaronator commented 4 years ago

I run in the same issue but with a vdev with 2 mirrors:

root@omv-nas:~# zpool status
  pool: StorageAmy
 state: ONLINE
  scan: none requested
config:

        NAME                                    STATE     READ WRITE CKSUM
        StorageAmy                              ONLINE       0     0     0
          mirror-0                              ONLINE       0     0     0
            ata-WDC_WD120EMAZ-11BLFA0_2AHNW     ONLINE       0     0     0
            ata-WDC_WD120EMAZ-11BLFA0_8CKAT     ONLINE       0     0     0
          mirror-1                              ONLINE       0     0     0
            ata-WDC_WD120EMAZ-11BLFA0_8CK7K     ONLINE       0     0     0
            ata-WDC_WD120EMAZ-11BLFA0_8CK9V     ONLINE       0     0     0