feat: detect storage spaces

[mokurin000]

close #9 and close #10

PS C:\Windows\system32>  F:\dyskinfo.exe
[C:\Users\kavin\Repos\lfs-core\src\windows\volume.rs:342:9] &kind = Simple {
    disk_number: 4,
}
[C:\Users\kavin\Repos\lfs-core\src\windows\volume.rs:342:9] &kind = StorageSpace
[C:\Users\kavin\Repos\lfs-core\src\windows\volume.rs:342:9] &kind = DynamicDisk
[C:\Users\kavin\Repos\lfs-core\src\windows\volume.rs:342:9] &kind = Unknown
[C:\Users\kavin\Repos\lfs-core\src\windows\volume.rs:342:9] &kind = Unknown

In a word, due to Micro$hit windows designs, we have to detect different type of volume management in totally different methods, The Win32_* WMI classes are too legacy, and in theMSFT_* Storage classes, microsoft discontinued support for Dynamic Disk.

It's not too bad though, because we have storage spaces since Windows 8, therefore no reason to use Dynamic Disk until the space is really insufficient. The extend volume does not provide speed improvement, only useful to concatenate different physical space, neither widely supported nor providing RAID-like robustness

[Canop]

@acieslewicz Can I get your opinion or review ?

[mokurin000]

I found a new implementation bug, that I cannot detect a volume that was a Dynamic Disk created on Storage Space Disks, it will be detected as DynamicDisk. But this usage is useless, and inefficient than StorageSpace sparse NTFS volume.

[acieslewicz]

Thanks for bringing this up mokurin, here are my thoughts. Please correct me if I am wrong.

My initial thinking with the simple/logical distinction was that simple volumes are anything with 1 backing disk where as logical volumes have n disks in some configuration. Storage spaces clearly break this as they combine multiple disks what appears to be a single disk from the perspective of the volume. I definitely agree that we should account for this.

My thoughts on the implementation:

• I'm not sure distinguishing between DyamicDisks and Storage Space is binary. A dynamic disk could be made up of a mix of physical disks and storage space disks. I think its better to leave a general "Logical" classification that covers both cases. We could maybe rename "Logical" to something else, but I think its important to not make something appear distinct when they are not.
• Along those lines, I think we should first check if a volume sits on a dynamic disk (ie multiple extents). Then if and only if its a simple volume should we check if the underlying disk is a storage space.
• I'm not sure we need to bring in WMI at this point since we can detect storage spaces by querying StorageDeviceProperty and checking if the bus type is spaces.

Some extra thoughts regarding wmi: It might be worth bringing in WMI rs, and potentially rewrite the property fetch using it. It would probably simplify the queries, at the cost of potentially being slower (would have to double check). As far as I'm aware the only property that can't be queried with the win32 api is bitlocker encryption though if that's the case it may be ok to just implement that single wmi query manually. Would be interested in thoughts.

[mokurin000]

• I'm not sure distinguishing between DyamicDisks and Storage Space is binary. A dynamic disk could be made up of a mix of physical disks and storage space disks. I think its better to leave a general "Logical" classification that covers both cases. We could maybe rename "Logical" to something else, but I think its important to not make something appear distinct when they are not.

Yeah, and even that's a private method. though dynamicdisks are considered the legacy way

• Along those lines, I think we should first check if a volume sits on a dynamic disk (ie multiple extents). Then if and only if its a simple volume should we check if the underlying disk is a storage space.

That makes sense, possibly reduce a WMI query overhead, while still able to detect "single disk" on storage space.

• I'm not sure we need to bring in WMI at this point since we can detect storage spaces by querying StorageDeviceProperty and checking if the bus type is spaces.

Sorry, it's because I had WMI implementation and too lazy to write a DOM free one..

You're right, WMI quries for more properties can be much eaiser.

[mokurin000]

• I'm not sure we need to bring in WMI at this point since we can detect storage spaces by querying StorageDeviceProperty and checking if the bus type is spaces.

BTW for binary bloat issue, we aren't need to use deserialization, I used them because too lazy Writing a single query statement in WMI (and disable the chrono feature), we could drop the serde dependency. (~300KiB)

Nevermind, I just found WMI hard depends on serde

[acieslewicz]

In this case we can avoid using wmi at all. We can do a DeviceIOControl property query for the storage device and then check the returned descriptors bus type.

[mokurin000]

In this case we can avoid using wmi at all. We can do a DeviceIOControl property query for the storage device and then check the returned descriptors bus type.

Do you mean check if STORAGE_DEVICE_DESCRIPTOR starts with Msft?

[acieslewicz]

I mean something along these lines:

let query = STORAGE_PROPERTY_QUERY {
    PropertyId: StorageDeviceProperty,
    QueryType: PropertyStandardQuery,
    AdditionalParameters: [0; 1],
};

let mut descriptor = STORAGE_DEVICE_DESCRIPTOR::default();

let result = unsafe {
    DeviceIoControl(
        handle,
        IOCTL_STORAGE_QUERY_PROPERTY,
        Some(ptr::addr_of!(query).cast()),
        std::mem::size_of::<STORAGE_PROPERTY_QUERY>() as u32,
        Some(ptr::addr_of_mut!(descriptor).cast()),
        std::mem::size_of::<STORAGE_DEVICE_DESCRIPTOR>() as u32,
        None,
        None,
    )
};

let is_storage_space = descriptor.BusType == BusTypeSpaces;

[mokurin000]

I mean something along these lines:

let query = STORAGE_PROPERTY_QUERY {
    PropertyId: StorageDeviceProperty,
    QueryType: PropertyStandardQuery,
    AdditionalParameters: [0; 1],
};

let mut descriptor = STORAGE_DEVICE_DESCRIPTOR::default();

let result = unsafe {
    DeviceIoControl(
        handle,
        IOCTL_STORAGE_QUERY_PROPERTY,
        Some(ptr::addr_of!(query).cast()),
        std::mem::size_of::<STORAGE_PROPERTY_QUERY>() as u32,
        Some(ptr::addr_of_mut!(descriptor).cast()),
        std::mem::size_of::<STORAGE_DEVICE_DESCRIPTOR>() as u32,
        None,
        None,
    )
};

let is_storage_space = descriptor.BusType == BusTypeSpaces;

Thanks for your detailed example, will add it as soon as I get time

[mokurin000]

@acieslewicz Hi, I have removed WMI dependency

[acieslewicz]

Thanks a bunch! Left two comments but, otherwise seems good to me.

[mokurin000]

@acieslewicz Hi, every requested changes were resolved

[Canop]

@mokurin000 @acieslewicz Do you both think I should merge this PR ?

BTW, never hesitate to ping me when you suspect I might have forgotten an issue or a PR

[acieslewicz]

I do think we should account for storage spaces, and unless you have any concerns it does look good to me.

[mokurin000]

@mokurin000 @acieslewicz Do you both think I should merge this PR ?

So do I. This will not only make us able to distinguish between dynamic disks and storage spaces, but also make single "disk partition" storage spaces detectable as LVM.

I have tested it in a virtual machine, and everything worked fine

BTW, never hesitate to ping me when you suspect I might have forgotten an issue or a PR

Sorry, I just forgot commits won't give notifications