On Thu, Feb 02, 2023 at 02:56:34PM -0500, Mike Snitzer wrote: > On Thu, Feb 02 2023 at 1:41P -0500, > Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx> wrote: > > > On Thu, Feb 02, 2023 at 11:50:37AM -0500, Mike Snitzer wrote: > > > On Wed, Jan 25 2023 at 10:33P -0500, > > > Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > This work aims to allow userspace to create and destroy block devices > > > > in a race-free and leak-free way, > > > > > > "race-free and leak-free way" implies there both races and leaks in > > > existing code. You're making claims that are likely very specific to > > > your Xen use-case. Please explain more carefully. > > > > Will do in v2. > > > > > > and to allow them to be exposed to > > > > other Xen VMs via blkback without leaks or races. It’s marked as RFC > > > > for a few reasons: > > > > > > > > - The code has been only lightly tested. It might be unstable or > > > > insecure. > > > > > > > > - The DM_DEV_CREATE ioctl gains a new flag. Unknown flags were > > > > previously ignored, so this could theoretically break buggy userspace > > > > tools. > > > > > > Not seeing a reason that type of DM change is needed. If you feel > > > strongly about it send a separate patch and we can discuss it. > > > > Patch 2/7 is the diskseq change. v2 will contain a revised and tested > > version with a greatly expanded commit message. > > I'm aware that 2/7 is where you make the DM change to disallow unknown > flags, what I'm saying is I don't see a reason for that change. Thanks for the clarification. > Certainly doesn't look to be a requirement for everything else in that > patch. Indeed it is not. I will make it a separate patch. > So send a separate patch, but I'm inclined to _not_ accept it because > it does potentially break some userspace. Is it okay to add DM_FILE_DESCRIPTOR_FLAG (with the same meaning as in 2/7) _without_ rejecting unknown flags? The same patch would bump the minor version number, so userspace would still be able to tell if the kernel supported DM_FILE_DESCRIPTOR_FLAG. If you wanted, I could ignore DM_FILE_DESCRIPTOR_FLAG unless the minor number passed by userspace is sufficiently recent. Another option would be to make userspace opt-in to strict parameter checking by passing 5 as the major version instead of 4. Userspace programs that passed 4 would get the old behavior, while userspace programs that passed 5 would get strict parameter checking and be able to use new features such as DM_FILE_DESCRIPTOR_FLAG. > > > > - I have no idea if I got the block device reference counting and > > > > locking correct. > > > > > > Your headers and justifcation for this line of work are really way too > > > terse. Please take the time to clearly make the case for your changes > > > in both the patch headers and code. > > > > I will expand the commit message in v2, but I am not sure what you want > > me to add to the code comments. Would you mind explaining? > > Nothing specific about code, was just a general reminder (based on how > terse the 2/7 header was). > > Mike Thanks for the feedback! -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab
Attachment:
signature.asc
Description: PGP signature
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
