dsccommunity / StorageDsc

DSC resource module is used to manage storage on Windows Servers.
https://dsccommunity.org
MIT License
71 stars 52 forks source link

xDisk requires a Disk Letter #84

Open matthitchcock opened 7 years ago

matthitchcock commented 7 years ago

Hi

This may be something I need advice on rather than being an issue. I wonder why xDisk needs to have a Drive Letter. A scenario I am dealing with is that I need to make sure a Disk has a volume and is formatted so I can use it later as a CSV in a Cluster. For this reason I wouldn't want to assign a Drive Letter or Access Path.

Is there a technical reason why this scenario is not handled in xStorage? Or is it that it hasn't been done yet? Are there problems that can be encountered when relying solely on the Disk number? If so, could Serial Number not be provided as the Unique key instead?

Thanks

PlagueHO commented 7 years ago

Hi @matthitchcock - this is a completely valid use case. But looking at the resource it doesn't look like using a different key would work.

However, another approach could be to create a new resource called something like xDiskVolume that contains a multiple keys: DiskNumber/DiskUniqueId, PartitionNumber. This could be used to create an unmounted volume (inside a partition of course).

Would this potentially solve your issue?

johlju commented 7 years ago

@PlagueHO If we are going for idea 2 in issue #81 wouldn't that able so that DiskId is the Key and DriveLetter is optional which might solve this use case. This might make Idea 2 even better, and if we already thinking of doing a breaking change... 😄

PlagueHO commented 7 years ago

Hi @johlju - unfortunately the key for this resource would still need to be DriveLetter - it can't be the DiskId (either number or unique Id) because this resource supports putting more than one volume per drive. So you may have two resources with the same DiskNumber/DiskUniqueId in the same config (e.g. D: and E:).

E.g: With idea 2 implemented:

xDisk DiskDriveD {
  DriveLetter = 'D'
  DiskId = '5000023492003903940343'
  DiskIdType = 'UniqueId'
  Size = 50GB
  FSLabel = 'Data'
}
xDisk DiskDriveD {
  DriveLetter = 'E'
  DiskId = '5000023492003903940343'
  DiskIdType = 'UniqueId'
  # Use remaining disk space
  FSLabel = 'Log'
}

Does that make sense? But I like your thinking - ideally if we're making breaking changes it would be good to solve this problem too.

I think though another resource is required. Something like this:

xVolume Disk2Volume1 {
  PartitionNumber = 1
  DiskId = '5000023492003903940343'
  DiskIdType = 'UniqueId'
  Size = 50GB
  FSLabel = 'Data'
}
xVolume Disk2Volume {
  PartitionNumber = 2
  DiskId = '5000023492003903940343'
  DiskIdType = 'UniqueId'
  # Use remaining disk space
  FSLabel = 'Data'
}

The keys in this resource would be the PartitionNumber and DiskId parameters. The above would only assume that one volume per partition was being created. I'll provide some more detail on this later on.

johlju commented 7 years ago

Ah, I understand, make sense, didn't think of more than one partition, it was ages since I split a disk like that. I usually just get LUN's in the size I asked for. 😄

Regarding xVolume. In the example with xVolume wouldn't we just need to throw in two optional parameters DriveLetter and AccessPath, or just Path for short (for both drive letters and mount points), and that would replace xDisk and xDiskAccessPath? If the are not set the logical disk/volume is then just formatted without setting it to a "path".

Maybe if we do xVolume, maybe we don't have to make a breaking change to xDisk and xAccessPath. But this xVolume would replace those to down the road.

Or I'm missing something here again? 😆

PlagueHO commented 7 years ago

@johlju - You're right - I think xVolume would need to have both DriveLetter and AccessPath.

You might be onto something here:

One of the big problems with these resources (xDisk/xDiskAccessPath) is that they only work for really simple configs because they try to obfuscate too much of the disk/partition/volume structure. They just end up not working in many situations and can't easily be fixed. For example: Resizing volumes/partitions isn't possible.

So deprecating xDisk/xDiskAccessPath/xWaitForDisk might be a good way to go (definitely in the long term).

I would suggest superseding them with: xWaitForDiskEx xDiskEx - only manipulates Disks (not partitions and volumes) xPartition - only manipulates Partitions (not disks and volumes) xVolume - only manipulates Volumes (not partitions and disks).

xDisk/xDiskAccessPath/xWaitForDisk would still remain but would no longer be updated.

I'm happy to make the changes to implement the new resources (they'll be much more straight forward than the existing xDisk/xDiskAccessPath), but it will still take time and I know @Zuldan is waiting on the changes to xDisk.

So I'd suggest that https://github.com/PowerShell/xStorage/issues/81 is completed first (because xDisk really is broken and shouldn't be left as is - even if it is deprecated) and then look at implementing the new xDiskEx/xPartition/xVolume model.

What do you think?

johlju commented 7 years ago

I agree with all of this! 😀 That sounds really great. And yes, let's not leave xDisk broken. Fix that first.

Maybe the new xDiskEx and xWaitForDiskEx could become xDisk and xWaitForDisk when this resource becomes HQRM as StorageDsc.

PlagueHO commented 7 years ago

@johlju - cool. I might try and get all these resources together this weekend. It would be really good to get these out because it would solve a fair number of problems for everyone I think.

Zuldan commented 7 years ago

@PlagueHO thank you!

johlju commented 7 years ago

@PlagueHO Awesome!