Re: [ros-dev] [ros-diffs] [cfinck] 75586: - Add 0x96 as ISO9660 partition type to ntdddisk.h (from https://en.wikipedia.org/wiki/Partition_type) and detect it using IsRecognizedPartition. - Make our isohybrid create an MBR ...

[Alex Ionescu]

This is cool work but you do realize that adding non-Windows partition
types and changing Windows headers (and Windows macros) to work with them,
as well as forking our CDFS from the Windows CDFS... doesn't really help
compatibility move forward...

It would be nice to discuss about how these changes can be done to keep our
'special' USB livecd boot and CDFS changes, without twisting the system
away from its NT roots. My suggestion would be a filter driver on top of
CDFS, and a re-definition of this macro to be used by the filter (or
whatever code relies on it). This sounds like it would give you the best of
both worlds...

Best regards,
Alex Ionescu

On Thu, Aug 17, 2017 at 1:03 PM, [e-mail] wrote:
> Author: cfinck
> Date: Thu Aug 17 11:03:40 2017
> New Revision: 75586
>
> URL: http://svn.reactos.org/svn/reactos?rev=75586&view=rev
> Log:
> Add 0x96 as ISO9660 partition type to ntdddisk.h (from
>
> https://en.wikipedia.org/wiki/Partition_type) and detect it using
> IsRecognizedPartition.
> Make our isohybrid create an MBR with partition type 0x96.
>
> This properly assigns a drive letter to the Live-CD and lets me boot into
> desktop using "qemu -hda livecd.iso".
> Fixes CORE-13184
>
> Modified:
>     trunk/reactos/boot/CMakeLists.txt
>     trunk/reactos/sdk/include/psdk/ntdddisk.h
>
> Modified: trunk/reactos/boot/CMakeLists.txt
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/boot/
> CMakeLists.txt?rev=75586&r1=75585&r2=75586&view=diff
> ============================================================
> ==================
> --- trunk/reactos/boot/CMakeLists.txt   [iso-8859-1] (original)
> +++ trunk/reactos/boot/CMakeLists.txt   [iso-8859-1] Thu Aug 17 11:03:40
> 2017
> @@ -68,7 +68,7 @@
>          -eltorito-boot loader/isoboot.bin -no-emul-boot -boot-load-size 4
> -eltorito-alt-boot -eltorito-platform efi -eltorito-boot loader/efisys.bin
> -no-emul-boot -hide boot.catalog
>          -sort ${CMAKE_CURRENT_BINARY_DIR}/bootfiles.sort
>          -no-cache-inodes -graft-points -path-list
> ${CMAKE_CURRENT_BINARY_DIR}/bootcd.lst
> COMMAND native-isohybrid -b ${_isombr_file}
>
> ${REACTOS_BINARY_DIR}/bootcd.iso
> COMMAND native-isohybrid -b ${_isombr_file} -t 0x96
>
> ${REACTOS_BINARY_DIR}/bootcd.iso
>      DEPENDS isombr native-isohybrid native-mkisofs
>      VERBATIM)
>
> @@ -83,7 +83,7 @@
>          -eltorito-boot loader/isobtrt.bin -no-emul-boot -boot-load-size 4
> -eltorito-alt-boot -eltorito-platform efi -eltorito-boot loader/efisys.bin
> -no-emul-boot -hide boot.catalog
>          -sort ${CMAKE_CURRENT_BINARY_DIR}/bootfiles.sort
>          -no-cache-inodes -graft-points -path-list
> ${CMAKE_CURRENT_BINARY_DIR}/bootcdregtest.lst
> COMMAND native-isohybrid -b ${_isombr_file} ${REACTOS_BINARY_DIR}/
>
> bootcdregtest.iso
> COMMAND native-isohybrid -b ${_isombr_file} -t 0x96
>
> ${REACTOS_BINARY_DIR}/bootcdregtest.iso
>      DEPENDS isombr native-isohybrid native-mkisofs
>      VERBATIM)
>
> @@ -126,7 +126,7 @@
>          -eltorito-boot loader/isoboot.bin -no-emul-boot -boot-load-size 4
> -eltorito-alt-boot -eltorito-platform efi -eltorito-boot loader/efisys.bin
> -no-emul-boot -hide boot.catalog
>          -sort ${CMAKE_CURRENT_BINARY_DIR}/bootfiles.sort
>          -no-cache-inodes -graft-points -path-list
> ${CMAKE_CURRENT_BINARY_DIR}/livecd.lst
> COMMAND native-isohybrid -b ${_isombr_file}
>
> ${REACTOS_BINARY_DIR}/livecd.iso
> COMMAND native-isohybrid -b ${_isombr_file} -t 0x96
>
> ${REACTOS_BINARY_DIR}/livecd.iso
>      DEPENDS isombr native-isohybrid native-mkisofs
>      VERBATIM)
>
> @@ -169,7 +169,7 @@
>          -eltorito-boot loader/isoboot.bin -no-emul-boot -boot-load-size 4
> -eltorito-alt-boot -eltorito-platform efi -eltorito-boot loader/efisys.bin
> -no-emul-boot -hide boot.catalog
>          -sort ${CMAKE_CURRENT_BINARY_DIR}/bootfiles.sort
>          -duplicates-once -no-cache-inodes -graft-points -path-list
> ${CMAKE_CURRENT_BINARY_DIR}/hybridcd.lst
> COMMAND native-isohybrid -b ${_isombr_file} ${REACTOS_BINARY_DIR}/
>
> hybridcd.iso
> COMMAND native-isohybrid -b ${_isombr_file} -t 0x96
>
> ${REACTOS_BINARY_DIR}/hybridcd.iso
>      DEPENDS bootcd livecd
>      VERBATIM)
>
> Modified: trunk/reactos/sdk/include/psdk/ntdddisk.h
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/sdk/
> include/psdk/ntdddisk.h?rev=75586&r1=75585&r2=75586&view=diff
> ============================================================
> ==================
> --- trunk/reactos/sdk/include/psdk/ntdddisk.h   [iso-8859-1] (original)
> +++ trunk/reactos/sdk/include/psdk/ntdddisk.h   [iso-8859-1] Thu Aug 17
> 11:03:40 2017
> @@ -223,6 +223,7 @@
>  #ifdef __REACTOS__
>  #define PARTITION_OLD_LINUX               0x43
>  #define PARTITION_LINUX                   0x83
> +#define PARTITION_ISO9660                 0x96
>  #define PARTITION_FREEBSD                 0xA5
>  #define PARTITION_OPENBSD                 0xA6
>  #define PARTITION_NETBSD                  0xA9
> @@ -253,6 +254,7 @@
>         ((PartitionType) == PARTITION_XINT13) || \
>         ((PartitionType) == PARTITION_LINUX) || \
>         ((PartitionType) == PARTITION_OLD_LINUX) || \
> ((PartitionType) == PARTITION_ISO9660) || \
>    ((PartitionType) == PARTITION_FREEBSD) || \
>    ((PartitionType) == PARTITION_OPENBSD) || \
>    ((PartitionType) == PARTITION_NETBSD))

[Colin Finck]

Am 20.08.2017 um 18:52 schrieb Alex Ionescu:
> This is cool work but you do realize that adding non-Windows partition 
> types and changing Windows headers (and Windows macros) to work with 
> them, as well as forking our CDFS from the Windows CDFS... doesn't 
> really help compatibility move forward...

Well, does it break compatibility in any way?
Our version of the IsRecognizedPartition macro is already enclosed in an 
#ifdef __REACTOS__ and has been supporting more partition types than 
Windows even before my changes.
If you want to use our ntdddisk.h for building Windows-compatible 
drivers, you can still do so by not defining __REACTOS__.

I'm not seeing why our CDFS driver shouldn't be able to do more than the 
Windows counterpart does. As long as it doesn't break compatibility with 
the features also supported by the Windows version.
After all, ReactOS isn't meant to be an exact reference implementation 
of Windows.

Even Microsoft has changed the abilities of its CDFS driver drastically 
over time. Just look at how the first Windows 95 version of "cdfs.vxd" 
actually treated Audio CDs: 
http://forest.watch.impress.co.jp/article/1999/03/16/cdfs.gif
I could actually imagine such a useful feature for our cdfs.sys too :)
> My suggestion would be a filter driver on 
> top of CDFS, and a re-definition of this macro to be used by the filter 
> (or whatever code relies on it).

Among other places, this macro is used in ntoskrnl/fstub/disksup.c when 
assigning drive letters. Any partition that isn't recognized doesn't get 
a drive letter, which is really bad if that particular partition is your 
boot partition.

Best regards,

Colin