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

Change zvol type from "Image/file" to "internal/external" #86

Closed
wants to merge 2 commits into from

Conversation

JMoVS
Copy link
Contributor

@JMoVS JMoVS commented Jul 7, 2021

Previously, zvols were connected "files" according to the apple specification in https://opensource.apple.com/source/IOStorageFamily/IOStorageFamily-260.100.1/IOStorageProtocolCharacteristics.h.auto.html.

Changing it from kIOPropertyInterconnectFileKey to kIOPropertyInternalExternalKey makes most sense as Apple itself writes about

kIOPropertyInternalExternalKey

This key defines the value of Internal/External for the key
kIOPropertyPhysicalInterconnectLocationKey. If the device is connected
to a bus and it is indeterminate whether it is internal or external,
this key should be set.

This would likely solve #85

Motivation and Context

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Previously, zvols were connected as "RAM based files" according to the apple specification in https://opensource.apple.com/source/IOStorageFamily/IOStorageFamily-260.100.1/IOStorageProtocolCharacteristics.h.auto.html.

Changing it from kIOPropertyInterconnectFileKey to kIOPropertyExternalKey makes most sense as Apple itself writes about

kIOPropertyInternalExternalKey

```
This key defines the value of Internal/External for the key
kIOPropertyPhysicalInterconnectLocationKey. If the device is connected
to a bus and it is indeterminate whether it is internal or external,
this key should be set.
```
@rottegift
Copy link
Collaborator

What does this really do? I don't see a difference on Catalina. Is there a difference on Big Sur? Could you show it in this conversation (include a diff -u on the "before" and "after" of diskutil info or if you feel intrepid diskutil info -plist or the relevant bits of ioreg, mostly ioreg -n pool/volname -r -l -w0).

If you can actually show a difference, I'd be happy to hoist this Catalina test VM into Big Sur to see if various things like exporting (do all the filesystems still get unmounted & ejected?) still work, the volmode property is obeyed, and so forth.

with 209da17

% diskutil info disk4
   Device Identifier:         disk4
   Device Node:               /dev/disk4
   Whole:                     Yes
   Part of Whole:             disk4
   Device / Media Name:       ZVOL vmdkpool/zvtst

   Volume Name:               Not applicable (no file system)
   Mounted:                   Not applicable (no file system)
   File System:               None

   Content (IOContent):       GUID_partition_scheme
   OS Can Be Installed:       No
   Media Type:                Generic
   Protocol:                  Disk Image
   SMART Status:              Not Supported

   Disk Size:                 85.9 GB (85899345920 Bytes) (exactly 167772160 512-Byte-Units)
   Device Block Size:         512 Bytes

   Read-Only Media:           No
   Read-Only Volume:          Not applicable (no file system)

   Device Location:           External
   Removable Media:           Fixed

   Solid State:               Yes
   Virtual:                   Yes

and without

   Device Identifier:         disk4
Device Node:               /dev/disk4
Whole:                     Yes
Part of Whole:             disk4
Device / Media Name:       ZVOL vmdkpool/zvtst

Volume Name:               Not applicable (no file system)
Mounted:                   Not applicable (no file system)
File System:               None

Content (IOContent):       GUID_partition_scheme
OS Can Be Installed:       No
Media Type:                Generic
Protocol:                  Disk Image
SMART Status:              Not Supported

Disk Size:                 85.9 GB (85899345920 Bytes) (exactly 167772160 512-Byte-Units)
Device Block Size:         512 Bytes

Read-Only Media:           No
Read-Only Volume:          Not applicable (no file system)

Device Location:           External
Removable Media:           Fixed

Solid State:               Yes
Virtual:                   Yes

(they are identical per diff and eyeball, 10.15.7)

Also, this very VM's files are stored in a zvol on the Big Sur host hardware, with code lagging about a month behind the tip of the development branch. The zvol's device looks very similar:

   Device Identifier:         disk7
   Device Node:               /dev/disk7
   Whole:                     Yes
   Part of Whole:             disk7
   Device / Media Name:       ZVOL Vpool/vvol

   Volume Name:               Not applicable (no file system)
   Mounted:                   Not applicable (no file system)
   File System:               None

   Content (IOContent):       GUID_partition_scheme
   OS Can Be Installed:       No
   Media Type:                Generic
   Protocol:                  Disk Image
   SMART Status:              Not Supported

   Disk Size:                 4.4 TB (4398046511104 Bytes) (exactly 8589934592 512-Byte-Units)
   Device Block Size:         512 Bytes

   Media OS Use Only:         No
   Media Read-Only:           No
   Volume Read-Only:          Not applicable (no file system)

   Device Location:           External
   Removable Media:           Fixed

   Solid State:               Yes
   Virtual:                   Yes

So this does not seem to affect what you were hoping for. On older Big Sur, Catalina-without-patch, Catalina-with-your-patch, it's all

<key>VirtualOrPhysical</key>
        <string>Virtual</string>

I also looked a bit for ioreg differences, and did not spot any, but gave up fairly quickly. The most obvious thing that hasn't changed:

  |   "Protocol Characteristics" = {"Physical Interconnect"="Virtual Interface","Physical Interconnect Location"="External"}

@rottegift
Copy link
Collaborator

Whle you're poking around in org_openzfsonosx_zfs_zvol_device::attach(), I think it would be useful to check whether the setProperty() calls work, and IOlog if they don't (or use an ASSERT macro).

https://developer.apple.com/documentation/kernel/ioregistryentry/1811442-setproperty says they return a true-on-success bool.

…as from disk image

It seems as if the PhysicalInterconnectTypeKey should in all likelyhood be USB (or something else physical) but not kIOPropertyInterconnectFileKey. The reason is that MacOS will think that a zvol is based of a disk image - which is not what we want.

Furthermore, the key for kIOPropertyPhysicalInterconnectLocationKey could be set to "internal/external" - but I can separate that out if needed.
@JMoVS
Copy link
Contributor Author

JMoVS commented Jul 7, 2021

@rottegift I am currently unsure why gthe package that lundman provided doesn't work but if it's easy for you to test the latest commit, I would greatly appreciate your help - thanks for looking into my PR!

@JMoVS JMoVS marked this pull request as draft July 7, 2021 21:14
@JMoVS
Copy link
Contributor Author

JMoVS commented Jul 7, 2021

Thanks @rottegift - this is indeed a draft and needs thorough testing and potentially more work - now marked as draft!

@JMoVS
Copy link
Contributor Author

JMoVS commented Aug 3, 2023

PR is abonduned

@JMoVS JMoVS closed this Aug 3, 2023
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.

2 participants