Zfs: zstd on zfs (patch inside)

Created on 15 Oct 2018  路  9Comments  路  Source: openzfs/zfs

since one asshole of the developers locked the original thread. here i post again the working patches for supporting zstd on zfsonlinux. its basicly a port of the freebsd patches and is tested in a limited way. please keep it open or make something out of it. closing a thread containing 3rd party contributions without any reason is more than just ignorant

here the required patches again

https://svn.dd-wrt.com/changeset/37376

and this post fix
https://svn.dd-wrt.com/changeset/37385
https://svn.dd-wrt.com/changeset/37399
https://svn.dd-wrt.com/changeset/37401

Most helpful comment

The normal workflow for Github is

  • fork project
  • make changes in branch, test as desired
  • open PR to see how tests fail on various targets, receive various suggestions and corrections, iterate until done or dropped

You appear to have opted for

  • make changes in another project not hosted on Git
  • assert that you're too busy to learn to use Git and open a PR
  • assert most PRs are ignored anyway so it's not worth your time
  • assert that you want people to test it for you instead of running the test suite that's triggered on opening a PR
  • when one of the project maintainers locked the feature request thread, decide opening a new thread calling the developers assholes is worth your time

This isn't really a good way to convince people to do things, or even look at your changes, versus deciding you're trying to be incendiary.

All 9 comments

The normal workflow for Github is

  • fork project
  • make changes in branch, test as desired
  • open PR to see how tests fail on various targets, receive various suggestions and corrections, iterate until done or dropped

You appear to have opted for

  • make changes in another project not hosted on Git
  • assert that you're too busy to learn to use Git and open a PR
  • assert most PRs are ignored anyway so it's not worth your time
  • assert that you want people to test it for you instead of running the test suite that's triggered on opening a PR
  • when one of the project maintainers locked the feature request thread, decide opening a new thread calling the developers assholes is worth your time

This isn't really a good way to convince people to do things, or even look at your changes, versus deciding you're trying to be incendiary.

its also not a good way to convince people to spend more time in future development by just locking threads and preventing further discussions. if i do a PR (and this what i wrote) i will do it once i ensured that the code is bug free (or lets say i cant find any more bugs in it). as you may have seen in the thread i posted further post fixes in it and did tests and posted the results in it. since i do not work with git, but on a own sourcetree which integrated zfs its very time consuming to create a secondary source tree just to maintain a git only copy of the same source until its done

to make it clear again. locking a thread which contains a on going development progress or a discussion about the current tests of this development is shutting down everything. i'm not interested to contribute anything if i just get shut down.

For reference, https://github.com/zfsonlinux/zfs/wiki/Git-and-GitHub-for-beginners
This process is much simpler than complaining in a bug tracker.

It is also my understanding that it hasn't yet been included in its origin platform, freebsd. I would also like to mention that since this feature originates on another platform it should be upstreamed to openzfs before we include it.

@kpanda: if i just would have done a PR you would get a crashing patch since the original BSD code was not bugfree at some points. it required some further work until the crashes had been resolved which wasnt that complicated and again you missed to read what i wrote. i will do a PR once it makes sense. which means that no data corruption or any other bad shit would happen. from the origin of the patch itself i also see no progress or changes on that patch over a decent amount of time and at the end it was a weekend fun to port it to linux and to try it out. so why wasting it. it may also push the whole project a little bit which is running now for more than 1,5 years on freebsd which is a very slow progress for that little piece of code (last change was in july by the way). and i also have not seen any comment regarding the finalizing in december. must be somewhere hidden from the public. only statement i found was from march 2018 with "soon". but of course i will follow up his code and integrate new changes once they are happen

@BrainSlayer Thank you for your efforts to port this work to ZoL. I am sorry the project appears dormant at this time, it is stalled on finding a solution to the case where 'compressed ARC is disabled, and an L2ARC is in use'.

I would be very interested in hearing about any bugs you found, so I could integrate the fixes back into FreeBSD.

@allanjude hello allen. just take a look on this patch https://svn.dd-wrt.com/changeset/37401 (not the first line in it, but the changes in zfs_ioctl.c if you just create a filesystem with zpool which has compression disabled. you will run into the problem that the attribute "compression" is not defined in any case. so this code will original simply fail and return so zpool destroy on destroying such a pool aborts with invalid argument, so you're deadlocked. and you can also not mount the pool in any way of course (at least on linux). this small ugly patch i made simply ignores the attribute if it isnt defined and handles it, if its defined. another patch i had to made for linux is https://svn.dd-wrt.com/changeset/37385. zfeature_register requires deps to be set if flags are ZFEATURE_FLAG_PER_DATASET. this is linux only specific. in freebsd its silently ignored, but in linux zfeature_register will raise a assert here. i'm not even sure if ZFEATURE_FLAG_PER_DATASET is correct at all. maybe ZFEATURE_FLAG_ACTIVATE_ON_ENABLE should be used like for "LZ4". it looks a little bit like a copy & paste misstake.

@BrainSlayer pull request is a great way to post work-in-progress patches too, there are many of them now https://github.com/zfsonlinux/zfs/pulls?q=is%3Apr+is%3Aopen+label%3A%22Status%3A+Work+in+Progress%22 . So, you can easily push any updates into PR, the code will be tested by our buildbot, and the main pros is a great review process. Feel free to do it when you want.

@gmelikov : yes but isnt it better if i do first some tests? i also switched now to latest git revision and remerged the patch for testing to make sure that such a pull requests succeeds the compile test at least. so be sure, i will make it. so far its looking good and i found no errors in my tests

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Hubbitus picture Hubbitus  路  4Comments

pcd1193182 picture pcd1193182  路  4Comments

marker5a picture marker5a  路  4Comments

FransUrbo picture FransUrbo  路  4Comments

jakeogh picture jakeogh  路  3Comments