Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hdd: Add option to use partitions other than +OPL #423

Merged
merged 1 commit into from
Jun 19, 2021
Merged

hdd: Add option to use partitions other than +OPL #423

merged 1 commit into from
Jun 19, 2021

Conversation

KrahJohlito
Copy link
Member

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK
  • Requires update of the gsKit
  • Others (please specify below)

Pull Request description

This is simpler than last time,

Upon hdd init, __common will be mounted and check “conf_hdd.cfg” for eg “hdd_partition=PP.OPL” and mount that partition instead.. if config isn’t found one will be created with +OPL as the default since that’s what it has been for years.. imo it’s better for “advanced users” to just change the cfg entry rather than lots of people not understanding why their hdd suddenly doesn’t work after updating OPL.

If partition in cfg doesn’t exist it will be made, only partitions beginning with + will work on root all others will use an OPL subfolder.

@KrahJohlito KrahJohlito requested a review from AKuHAK May 27, 2021 10:08
src/hddsupport.c Outdated Show resolved Hide resolved
@AKuHAK
Copy link
Member

AKuHAK commented May 27, 2021

Also I prefer __common:OPL/conf_hdd.cfg, not __common:conf_hdd.cfg. Cause __common is special partition there are a lot of stuff, users can easily be confused by conf_hdd.cfg. Or at least rename it to contain OPL in the name.
Also, I would like to keep system partitions intact, so OPL will not destroy them by accident (and also to not fill them with trashy opl folders). The simplest way - just to ban any partition name that starts with __. Or you can go in a hard way, and if the partition is started with __ users are forced to use a hardcoded subfolder (like __net/OPL/). However, the second way needs to implement the new method for it (cause as for now opl uses only root of the device).

@KrahJohlito
Copy link
Member Author

KrahJohlito commented May 27, 2021

Ok I will change the cfg location to hdd0:__common/OPL/conf_hdd.cfg when I get some time on pc.

Currently in this pr any partition that does not begin with ‘+’ will automatically create an OPL folder on the root of the partition and all files/folders ART CFG etc will be in that one OPL folder so they won’t be filled with trash on root :) .. the reason I would allow +partitions to work on root is only for backwards compatibility.

@AKuHAK
Copy link
Member

AKuHAK commented May 27, 2021

Currently in this pr any partition that does not begin with ‘+’ will automatically create an OPL folder on the root of the partition and all files/folders ART CFG etc will be in that one OPL folder so they won’t be filled with trash on root :) .. the reason I would allow +partitions to work on root is only for backwards compatibility.

Oh didn't realize that this was in code, then great - it is fine! Great work!

src/hddsupport.c Outdated Show resolved Hide resolved
@AKuHAK
Copy link
Member

AKuHAK commented May 27, 2021

Also, +OPL is hardcoded in apps, probably should be changed as well:

_strcpy(_argv, "hdd0:+OPL:");

Apps will try to boot always from +OPL partition, which is definitely not good.

Also, could you add a note into README.md ? Something about HDD conf file location, conf file content, and logic.

OPL will automatically create the above directory structure the first time
you launch it and enable your favourite device. For HDD users, a 128Mb +OPL
partition will be created (you can enlarge it using uLaunchELF if you need to).

@KrahJohlito
Copy link
Member Author

Also, +OPL is hardcoded in apps, probably should be changed as well:

_strcpy(_argv, "hdd0:+OPL:");

Apps will try to boot always from +OPL partition, which is definitely not good.
Also, could you add a note into README.md ? Something about HDD conf file location, conf file content, and logic.

OPL will automatically create the above directory structure the first time
you launch it and enable your favourite device. For HDD users, a 128Mb +OPL
partition will be created (you can enlarge it using uLaunchELF if you need to).

Bah, I didn’t see this.. isn’t this happening at compile time? I’m not sure how to proceed with the elfldr

@AKuHAK
Copy link
Member

AKuHAK commented May 28, 2021

Also, +OPL is hardcoded in apps, probably should be changed as well:

_strcpy(_argv, "hdd0:+OPL:");

Bah, I didn’t see this.. isn’t this happening at compile time? I’m not sure how to proceed with the elfldr

yes, it is happening at compile time. Of course, you can add HDD managing into elfldr, but then it will become really big and slow (users who don't use internal HDD may notice this). I advise mounting additional PFS point for __common partition (e.g. pfs1) and keep this mount point alive when OPL is running and just read pfs1:/OPL/cfg_hdd.txt with the elfldr. In fact, the reason why we use this config file is that ps2 forgot about the partition name after mounting.

The only thing, I don't know if it is allowed to mount the same partition in different mount points. I mean if both: pfs0 and pfs1 are mount points for __common.

I think this is the best solution, however, you can find another. For example, you can create a copy of __common:/OPL/cfg_hdd.txt inside the custom partition, however, you then need to keep it in sync with common, there is a chance that the user will alter it and so and so on.

@AKuHAK
Copy link
Member

AKuHAK commented May 28, 2021

Also, +OPL is hardcoded in apps, probably should be changed as well:

_strcpy(_argv, "hdd0:+OPL:");

Apps will try to boot always from +OPL partition, which is definitely not good.

I am mistaken - APPS will boot from pfs0, so common users or even devs didn't notice changes. argv[0] will be wrong, but most of the apps didnt use it. When we launch an app from pfs, we need to add partition name into argv[0] or argv[1] so APP will know where is it from in case if pfs mount point is destroyed during boot.

@KrahJohlito KrahJohlito marked this pull request as draft May 29, 2021 06:13
@KrahJohlito KrahJohlito marked this pull request as ready for review June 16, 2021 12:07
@KrahJohlito KrahJohlito requested review from AKuHAK and uyjulian June 16, 2021 12:08
@KrahJohlito
Copy link
Member Author

KrahJohlito commented Jun 16, 2021

Ok I think is this finally ready for re-review.

As we determined we can't have __common mounted on separate mount points simultaneously so, OPL partition will always be mounted to pfs0 and __common (containing the cfg) will be pfs1 UNLESS it is being used as OPL part in which case it will of course be pfs0, that is why we must check both mount points in elfldr..

However I can't really test that portion of code, so I'm hoping it just logically makes sense to you guys.. but I'm open to suggested changes :)

Edit: If merge ready, I think squash would be best as I'm not sure how rebasing will go with all the recent formatting changes?

Copy link
Member

@uyjulian uyjulian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK

@AKuHAK
Copy link
Member

AKuHAK commented Jun 16, 2021

Ok I think is this finally ready for re-review.

As we determined we can't have __common mounted on separate mount points simultaneously so, OPL partition will always be mounted to pfs0 and __common (containing the cfg) will be pfs1 UNLESS it is being used as OPL part in which case it will of course be pfs0, that is why we must check both mount points in elfldr..

However I can't really test that portion of code, so I'm hoping it just logically makes sense to you guys.. but I'm open to suggested changes :)

Edit: If merge ready, I think squash would be best as I'm not sure how rebasing will go with all the recent formatting changes?

for testing you can launch wLe from apps page and look into MISX/Debug - it will show you arguments that were parsed to elf

@AKuHAK
Copy link
Member

AKuHAK commented Jun 16, 2021

just one moment: if we point in conf_hdd.txt into an existing non-pfs partition (for example __mbr or HDL game) it will not be formatted, correct?

@AKuHAK
Copy link
Member

AKuHAK commented Jun 16, 2021

@KrahJohlito everything else looks OK

@KrahJohlito
Copy link
Member Author

just one moment: if we point in conf_hdd.txt into an existing non-pfs partition (for example __mbr or HDL game) it will not be formatted, correct?

I wouldn’t think so but haven’t tested, I will try test and confirm.. currently if partition doesn’t exist it creates it and formats so I’m not sure what would happen with a non-pfs partition.. could add a clause to not proceed with creation if __ partition if required.. I’ll try test first

@AKuHAK
Copy link
Member

AKuHAK commented Jun 17, 2021

could add a clause to not proceed with creation if __ partition if required.. I’ll try test first

It will not cover all the cases. HDL game can be installed as any partition, not only starting with __. I think that OPL code already covers that case, just to be sure that this is definitely not doing bad things.

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Jun 18, 2021

Ok I tested and non-pfs partitions will not be formatted, so that’s fine.. elfldr code didn’t work so I had to pass OPL partition as argv[1] which works fine now.. this means more than one mount point isn’t necessary so I’ve removed the pfsarg..

elfldr seems a bit quicker now also so that’s nice :).

thumbnail_image0

@AKuHAK
Copy link
Member

AKuHAK commented Jun 18, 2021

Ok I tested and non-pfs partitions will not be formatted, so that’s fine.. elfldr code didn’t work so I had to pass OPL partition as argv[1] which works fine now.. this means more than one mount point isn’t necessary so I’ve removed the pfsarg..

elfldr seems a bit quicker now also so that’s nice :).

thumbnail_image0

Nice solution, more lightweight, and no need for using strings inside elfldr. But could you modify argv[1] then? :
argv[1] == hdd0:__common:pfs0:OPL/APPS/uLE/BOOT.ELF Just to keep things standart.

@KrahJohlito
Copy link
Member Author

Ok I tested and non-pfs partitions will not be formatted, so that’s fine.. elfldr code didn’t work so I had to pass OPL partition as argv[1] which works fine now.. this means more than one mount point isn’t necessary so I’ve removed the pfsarg..
elfldr seems a bit quicker now also so that’s nice :).
thumbnail_image0

Nice solution, more lightweight, and no need for using strings inside elfldr. But could you modify argv[1] then? :
argv[1] == hdd0:__common:pfs0:OPL/APPS/uLE/BOOT.ELF Just to keep things standart.

no worries, done.

src/hddsupport.c Show resolved Hide resolved
@AKuHAK AKuHAK merged commit 68797a5 into ps2homebrew:master Jun 19, 2021
@KrahJohlito KrahJohlito deleted the hdd-nogui branch June 24, 2021 12:20
AKuHAK added a commit that referenced this pull request Sep 30, 2021
hdd: Add option to use partitions other than +OPL
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
hdd: Add option to use partitions other than +OPL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants